[CORE-4181] schema registry json support validating drafts 5 6 and 7 #20817
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/dt |
andijcr
force-pushed
the
feat/core-4181/schema_registry_add_support_for_draft7
branch
from
3de0093 to
6d76551
Compare
|
skipped ducktape retry in https://buildkite.com/redpanda/redpanda/builds/51111#01907e92-321b-4476-b55a-e72a1d0bdc06: skipped ducktape retry in https://buildkite.com/redpanda/redpanda/builds/51111#01907e92-3215-4276-9644-9d82eecea467: skipped ducktape retry in https://buildkite.com/redpanda/redpanda/builds/51111#01907ec1-58de-4b86-921e-54abba990aad: skipped ducktape retry in https://buildkite.com/redpanda/redpanda/builds/51111#01907ec1-58e5-434e-b841-157f2c2dc075: |
BenPope
reviewed
Jul 5, 2024
andijcr
force-pushed
the
feat/core-4181/schema_registry_add_support_for_draft7
branch
from
6d76551 to
d63c3d1
Compare
andijcr
changed the title
|
force push: review comments, added draft5 |
BenPope
previously approved these changes
Jul 5, 2024
pgellert
previously approved these changes
Jul 5, 2024
BenPope
previously approved these changes
Jul 5, 2024
pgellert
previously approved these changes
Jul 5, 2024
eventually, with default values for properties being "true/false" or
"{}/{"not":{}}", we can form a call like
is_superset("{}", ...) or is_superset(..., "{"not":{}") that can cause
infinite recursion. this is because the type is assumed to mean every
type, and this can lead to this recursive chain is_superset ->
is_array_superset -> is_superset ...
this commit breaks the chain by recognizing these atoms and returning
early.
normalization and/or AST could offer a more robust solution, by ensuring
that implicit properties are explicit and removing extraneous fields.
The test cases in this commit would lead to infinite recursion without
the early return in is_superset.
boost test macros are not great at printing the exception. this commit adds a manual try/catch to intercept and print exceptions, for ease of development
will need to be visible to code in later commits
enum for the dialects supported by the implementation, meant to symbolize the URIs of the official dialects.
this function gets a dialect and a json::Document of the candidate schema, and will perform validation against the correct metaschema. to compile only once the Schema object, get_metaschema<json_schema_dialect>() will compile it lazily and save it in static const variable. to support validation of json without the "$schema" property, try_validate_json_schema will try to validate it against the known dialects, from the most recent to the oldest
this is an intermediate commit. split parsing and validating. the error messages and functionalities remain the same, but this way it's easier to wire in the new validating functions and to add support for new dialects
note: the diff of this commit is somewhat chaotic delegate schema validation to validate_json_schema. to do so, extract the dialect, if present, from the json document, and call validate_json_schema/try_validate_json_schema
draft5 is a cleanup of draft4 and reuses the metaschema, according to the official documentation. This is why the support here is implemented with the draft4 metaschema
adapt from_uri(uri) -> json_schema_dialect to recognize http://json-schema.org/draft-0N/schema# and http://json-schema.org/draft-0N/schema as draftN dialects. removed enum values for $schema in the metaschemas since the check for permitted values is already performed before validation
andijcr
force-pushed
the
feat/core-4181/schema_registry_add_support_for_draft7
branch
from
d4c39e0 to
161213c
Compare
|
force push merge conflict |
BenPope
approved these changes
Jul 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
break recursion for
is_superset(get_true_schema(), ...)andis_superset(..., get_false_schema())these two cases can happens for properties that don't have a default value: for example{"type": "array"}has no"items", so it gets the default value of{}. without this early break, we get in an infinite recursion where{}is passes throughis_array_supersetand that in turn will callis_supersetand so onadd support for draft5: according to the official documentation, this dialect reuses the metaschema for draft4, and so does the code
support for draft6 : the metaschema is adapted to draft4, because the current version of rapidjson implements only draft4
support for draft7 : the metaschema is adapted to draft4, same reasons
support for schemas uris without trailing #
Fixes https://redpandadata.atlassian.net/browse/CORE-4181
Backports Required
Release Notes