[CORE-3184] schema registry - json compatibility checks for objects #19893

andijcr · 2024-06-19T13:22:56Z

adds support for {"type": "number", "multipleOf": 1.1} -> {"type": "number", "multipleOf": 2.2}
- compute the reminder and ensure it's close to 0
adds minimal support for "not": this is needed to support "additionalProperties", but the implementation is overly restrictive, requiring "not" to be present in both schemas
adds support for "type": "object". this check is likely to recurse on the various subschemas

Fixes https://redpandadata.atlassian.net/browse/CORE-4218
Fixes https://redpandadata.atlassian.net/browse/CORE-3184

Backports Required

Release Notes

none

support non-integer values for "multipleOf" by checking that the reminder of newer["multipleOf"]/older["multipleOf"] is close to 0.0

force a reformat, so that next commit is only the new test case

the check is overly strict, as it requires for "not" to be present in both or neither. In principle a missing "not" can be intepreted as having a default value of false, but requiring an explicit "not" is simpler

vbotbuildovich · 2024-06-21T23:11:33Z

new failures in https://buildkite.com/redpanda/redpanda/builds/50530#01903cc8-b990-4d58-838d-4cee7a533b29:

"rptest.tests.raft_availability_test.RaftAvailabilityTest.test_leadership_transfer"

vbotbuildovich · 2024-06-21T23:12:53Z

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/50530#01903cc8-b993-4272-be00-b919743f78d9

andijcr · 2024-06-24T10:56:16Z

failure unrelated, as this pr is not yet wired in

BenPope

Very nice,

I think a lot of the loops might be a bit clearer with std::ranges::all_of, but I'll leave that up to you.

src/v/pandaproxy/schema_registry/json.cc

BenPope · 2024-06-24T11:04:11Z

src/v/pandaproxy/schema_registry/json.cc

+        }
+
+        return older_it->value;
+    }();


nitpick: The invocation can be deferred so that it's not called if it's not needed.

good idea, done

pgellert

lgtm

used in the next set of commits, to implement is_object_superset the first three define json::Value const& for a "true" and a "false" schema, these will be useful while checking "additionalProperties" for a "type": "object" the last two are getter for array and object that produce an empty value if the property does not exists. useful to streamline the implementation

implement "minProperties" "maxProperties" compat check for "type":"object"

this subcheck is for "additionalProperties", it will recurse if either older/newer is a schema and check compatibility.

sub check for "type": "object" on "properties". for each property in newer, check compatibility against the same property in older, or against the matching patternProperty in older, or against additionalProperties in older

subcheck for "type": "object" "patternProperties". the check is toensure that no new pattern are introduced, and that the schemas are compatible

final sub-check for "type": "object" "required" field. the check only look at "required" values that are defined in the "properties" map.

andijcr · 2024-06-24T15:29:24Z

force push: reviews comments

I think a lot of the loops might be a bit clearer with std::ranges::all_of, but I'll leave that up to you.

I'll take a look and convert loops in the next PR for array support

BenPope

LGTM

andijcr · 2024-06-25T08:44:51Z

only issue is in debug

Module: rptest.tests.crl_test
Class:  CertificateRevocationTest
Method: test_pp_api

RuntimeError('Expected an exception!')
Traceback (most recent call last):
  File "/opt/.ducktape-venv/lib/python3.10/site-packages/ducktape/tests/runner_client.py", line 184, in _do_run
    data = self.run_test()
  File "/opt/.ducktape-venv/lib/python3.10/site-packages/ducktape/tests/runner_client.py", line 276, in run_test
    return self.test_context.function(self.test)
  File "/root/tests/rptest/services/cluster.py", line 105, in wrapped
    r = f(self, *args, **kwargs)
  File "/root/tests/rptest/tests/crl_test.py", line 188, in test_pp_api
    with expect_exception((requests.exceptions.SSLError,
  File "/usr/lib/python3.10/contextlib.py", line 142, in __exit__
    next(self.gen)
  File "/root/tests/rptest/util.py", line 309, in expect_exception
    raise RuntimeError("Expected an exception!")
RuntimeError: Expected an exception!

it shouldn't be caused by this pr, but after a quick search i can't find this failure in the tracker on in other nightly runs. rerunning this test

andijcr · 2024-06-25T08:46:20Z

/ci-repeat 3 skip-unit dt-repeat=10 test/rptest/tests/crl_test.py::CertificateRevocationTest.test_pp_api

andijcr · 2024-06-25T10:25:49Z

/ci-repeat 3 skip-redpanda-build skip-unit dt-repeat=10 tests/rptest/tests/crl_test.py::CertificateRevocationTest.test_pp_api

andijcr · 2024-06-25T12:51:18Z

filed a ticket for the failure #20133 , seems unrelated

michael-redpanda · 2024-06-25T12:54:00Z

Confirmed this was the only CI failure. Ticket created. Not cuased by this PR.

github-actions bot added the area/redpanda label Jun 19, 2024

schema_registry/json: check compat for decimal multipleOf

18bfbd7

support non-integer values for "multipleOf" by checking that the reminder of newer["multipleOf"]/older["multipleOf"] is close to 0.0

andijcr force-pushed the feat/core-3184/schema_registry_objects branch from 481db4a to 0aa7778 Compare June 19, 2024 16:52

andijcr added 2 commits June 21, 2024 15:52

test/test_json_schema: reformat

71e512f

force a reformat, so that next commit is only the new test case

schema_registry/json: "not" combinator

a950ac6

the check is overly strict, as it requires for "not" to be present in both or neither. In principle a missing "not" can be intepreted as having a default value of false, but requiring an explicit "not" is simpler

andijcr force-pushed the feat/core-3184/schema_registry_objects branch from d1fc64b to 43daaa4 Compare June 21, 2024 20:37

andijcr marked this pull request as ready for review June 21, 2024 20:42

andijcr requested review from BenPope, pgellert and a team June 21, 2024 20:42

BenPope reviewed Jun 24, 2024

View reviewed changes

pgellert previously approved these changes Jun 24, 2024

View reviewed changes

andijcr added 7 commits June 24, 2024 17:22

schema_registry/json: is_object_superset size check

366f8dd

implement "minProperties" "maxProperties" compat check for "type":"object"

schema_registry/json: is_object_additional_properties_superset

c8dc748

this subcheck is for "additionalProperties", it will recurse if either older/newer is a schema and check compatibility.

schema_registry/json: is_object_properties_superset

2974a08

sub check for "type": "object" on "properties". for each property in newer, check compatibility against the same property in older, or against the matching patternProperty in older, or against additionalProperties in older

schema_registry/json: is_object_pattern_properties_superset:

c8b3af1

subcheck for "type": "object" "patternProperties". the check is toensure that no new pattern are introduced, and that the schemas are compatible

schema_registry/json: is_object_required_superset

2a70373

final sub-check for "type": "object" "required" field. the check only look at "required" values that are defined in the "properties" map.

schema_registry/test/json: object compat test

1eab55b

andijcr dismissed pgellert’s stale review via 1eab55b June 24, 2024 15:26

andijcr force-pushed the feat/core-3184/schema_registry_objects branch from 43daaa4 to 1eab55b Compare June 24, 2024 15:26

andijcr requested review from pgellert and BenPope June 24, 2024 15:29

BenPope approved these changes Jun 24, 2024

View reviewed changes

pgellert approved these changes Jun 24, 2024

View reviewed changes

michael-redpanda merged commit eac1a02 into redpanda-data:dev Jun 25, 2024
14 of 18 checks passed

andijcr deleted the feat/core-3184/schema_registry_objects branch June 25, 2024 16:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CORE-3184] schema registry - json compatibility checks for objects #19893

[CORE-3184] schema registry - json compatibility checks for objects #19893

andijcr commented Jun 19, 2024 •

edited

Loading

vbotbuildovich commented Jun 21, 2024

vbotbuildovich commented Jun 21, 2024

andijcr commented Jun 24, 2024

BenPope left a comment •

edited

Loading

BenPope Jun 24, 2024

andijcr Jun 24, 2024

pgellert left a comment

andijcr commented Jun 24, 2024

BenPope left a comment

andijcr commented Jun 25, 2024

andijcr commented Jun 25, 2024

andijcr commented Jun 25, 2024

andijcr commented Jun 25, 2024

michael-redpanda commented Jun 25, 2024

[CORE-3184] schema registry - json compatibility checks for objects #19893

[CORE-3184] schema registry - json compatibility checks for objects #19893

Conversation

andijcr commented Jun 19, 2024 • edited Loading

Backports Required

Release Notes

vbotbuildovich commented Jun 21, 2024

vbotbuildovich commented Jun 21, 2024

andijcr commented Jun 24, 2024

BenPope left a comment • edited Loading

Choose a reason for hiding this comment

BenPope Jun 24, 2024

Choose a reason for hiding this comment

andijcr Jun 24, 2024

Choose a reason for hiding this comment

pgellert left a comment

Choose a reason for hiding this comment

andijcr commented Jun 24, 2024

BenPope left a comment

Choose a reason for hiding this comment

andijcr commented Jun 25, 2024

andijcr commented Jun 25, 2024

andijcr commented Jun 25, 2024

andijcr commented Jun 25, 2024

michael-redpanda commented Jun 25, 2024

andijcr commented Jun 19, 2024 •

edited

Loading

BenPope left a comment •

edited

Loading