feat: enable users to disable validation only on specific topics #759
Conversation
eliax1996
force-pushed
the
eliax1996/enable-users-to-disable-validation-only-on-specific-topics
branch
6 times, most recently
from
474140a
to
0932649
Compare
eliax1996
changed the title
eliax1996
force-pushed
the
eliax1996/enable-users-to-disable-validation-only-on-specific-topics
branch
3 times, most recently
from
65bf208
to
1fd6c57
Compare
|
Thank you so much, @eliax1996! We are now struggling from disability of publishing new schemes for existing topics. |
eliax1996
force-pushed
the
eliax1996/enable-users-to-disable-validation-only-on-specific-topics
branch
from
1fd6c57
to
bf1f114
Compare
|
@GSokol glad to know that this would help. I will create a new optional release in Aiven as soon as it is merged. |
eliax1996
force-pushed
the
eliax1996/enable-users-to-disable-validation-only-on-specific-topics
branch
3 times, most recently
from
a0e13c6
to
d6c1221
Compare
eliax1996
force-pushed
the
eliax1996/enable-users-to-disable-validation-only-on-specific-topics
branch
3 times, most recently
from
1f024a8
to
7603add
Compare
| topic_name: TopicName, | ||
| skip_validation: bool, | ||
| ) -> None: | ||
| key = {"topic": topic_name, "keytype": str(MessageType.schema_validation), "magic": 0} |
There was a problem hiding this comment.
The formatting of the key needs to be aligned in the key_format.py. This record needs the canonical format, topic is an unknown key in data currently.
How does the Confluent Schema Registry react to custom message like this?
There was a problem hiding this comment.
still a valid point, taking a look tomorrow
There was a problem hiding this comment.
discussed offline, probably solved. Waiting an update on that
There was a problem hiding this comment.
I just realized that I need to add the topic to the key, otherwise the compaction could delete the topic information, this doesn't change the behaviour of confluent schema registry that is simply to skip the messages he cannot understand (a behaviour that we also do while receiving messages that aren't parsable by us)
|
Also it's good to have a config setting, to disable schema validation by default. But it could be done in a separate PR to make this thing in faster. |
|
@GSokol this is already implemented (and merged and available in Aiven), if you look there is a config called In that way, users who didn't follow a validation strategy in the past can now start using it without losing or having to rewrite the existing flows but they can enforce the validation on the new ones |
|
@eliax1996, |
eliax1996
force-pushed
the
eliax1996/enable-users-to-disable-validation-only-on-specific-topics
branch
2 times, most recently
from
b1564e8
to
7cebf32
Compare
eliax1996
force-pushed
the
eliax1996/enable-users-to-disable-validation-only-on-specific-topics
branch
2 times, most recently
from
ee21be0
to
c4cffdd
Compare
eliax1996
force-pushed
the
eliax1996/enable-users-to-disable-validation-only-on-specific-topics
branch
4 times, most recently
from
2cec245
to
3305ba6
Compare
|
Hi @GSokol, you wrote that
and it made me curious about your expectations regarding this feature. This PR adds ability to disable validation per topic, but also this configuration affects only REST API logic. I hope this still matches your expectations. |
There was a problem hiding this comment.
Access control for new endpoint needs to be defined and documented.
Also new API endpoints need some documentation of semantics. For example if configuration name_strategy_validation is disabled, is it sill possible to enable validation for specific topics? With this code it seems that is not possible. Would it be useful, or not? Maybe per topic validation endpoints should reject setting if name_strategy_validation is disabled?
karapace/schema_registry_apis.py
Outdated
| schema_request=True, | ||
| with_request=False, | ||
| json_body=False, | ||
| auth=None, |
There was a problem hiding this comment.
auth=None means anyone can change this setting. I think this should be better defined what permissions are needed to change validation configuration per topic, and also documented in README.
| ) -> None: | ||
| key = {"keytype": str(MessageType.schema_validation), "magic": 0} | ||
| value = {"skip_validation": skip_validation, "topic": topic_name} | ||
| self.producer.send_message(key=key, value=value) |
There was a problem hiding this comment.
Depending on skip_validation this could also send tombstone record instead of toggling between true/false.
There was a problem hiding this comment.
I don't think so, otherwise how can you enable again the validation for a certain topic (after you disable it)?
eliax1996
force-pushed
the
eliax1996/enable-users-to-disable-validation-only-on-specific-topics
branch
2 times, most recently
from
9b1eb58
to
c8f35f3
Compare
eliax1996
force-pushed
the
eliax1996/enable-users-to-disable-validation-only-on-specific-topics
branch
from
c8f35f3
to
c5cfd46
Compare
This PR enables users to avoid validation only on specific topics. It is necessary because, prior to this commit it was possible to create messages with incorrect or missing subjects. By fixing the validation of subject flows that were previously functioning (but shouldn't have been) now no longer work due to the added validation.
With this PR, users can migrate their flows one by one while adding new validated flows, rather than opting out of validation completely if they wish to retain the functionality of old flows without refactoring them.