[CORE-1507] SR: Add support for `?verbose` compatibility checks #18332

oleiman · 2024-05-09T20:16:37Z

Some side effects of this:

In general, this PR keeps existing checks in place but adds reference-compatible error codes and messages to provide human-readable detail.
On the protobuf side, this PR introduces a handful of checks that are functionally new (i.e. schemas which were previously considered compatible are no longer so):
- ONEOF_FIELD_REMOVED
- MULTIPLE_FIELDS_MOVED_TO_ONEOF
- REQUIRED_FIELD_{ADDED,REMOVED}
- FIELD_NAMED_TYPE_CHANGED
- MESSAGE_REMOVED
  - We already detect some of these (file level) but this PR adds additional checks for nested message definitions
Previously our compatibility checks would short circuit, returning a 👍 / 👎 as soon as a determination is made. Now we want to build up a collection of errors so that users can make all the required changes and avoid repeated checks. To do this, we need to rejigger the structure of the recursion a bit and introduce some new types for collecting errors.
We also want to report "path" information about errors (i.e. where in the schema the error occurred), so we need to carry that information through the recursion. I used std::filesystem::path for this as a first pass, but in the end it looks fit for purpose. We could implement something similar if the cost is a concern, but I suspect most of the cost is in storing the path elements themselves, so it may be a wash. It might be nice to just not do this if verbose error tracking is not enabled.

Fixes CORE-1507

Backports Required

Release Notes

Improvements

SchemaRegistry: Adds support for the ?verbose parameter on POST /compatibility/subjects/{subject}/versions/{version}

oleiman · 2024-05-09T20:29:37Z

force push move some compatibility code from header

oleiman · 2024-05-09T20:29:59Z

/dt

vbotbuildovich · 2024-05-09T22:58:09Z

new failures in https://buildkite.com/redpanda/redpanda/builds/48905#018f5f60-2f1f-49b5-9525-d2010401037a:

"rptest.tests.schema_registry_test.SchemaRegistryBasicAuthTest.test_post_compatibility_subject_version"

new failures in https://buildkite.com/redpanda/redpanda/builds/48905#018f5f51-1de4-47ce-b35c-e447df7d2995:

"rptest.tests.schema_registry_test.SchemaRegistryBasicAuthTest.test_post_compatibility_subject_version"

vbotbuildovich · 2024-05-09T23:00:40Z

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48905#018f5f60-2f1f-49b5-9525-d2010401037a

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48905#018f5f60-2f27-47c1-8a6a-c53f8393bb98

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48912#018f6055-325b-4a7f-883b-d9f53a360d48

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48962#018f65f4-a4d7-4fb0-94e8-9e3b0c3d643e

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/48962#018f65f4-a4d2-4971-aa11-9112ae7a151e

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/49069#018f757f-a9d4-405c-923d-0c9527e33c52

oleiman · 2024-05-09T23:06:59Z

force push typo

oleiman · 2024-05-10T00:54:57Z

/ci-repeat 1

Deflaimun · 2024-05-13T15:00:18Z

tests/rptest/tests/schema_registry_test.py

        assert result_raw.status_code == requests.codes.ok
        assert result_raw.json()["is_compatible"] == False

+        self.logger.debug(
+            "Check compatibility backward, no default, not verbose")


I know this is just a logger but I don't understand what it's supposed to mean...Writing can possibly be improved

FYI - these are integration tests (i.e. not broker logs), but I can try to make it more clear on the next push.

oleiman · 2024-05-14T03:36:28Z

force push improve commits, tests, remove some junk

oleiman · 2024-05-14T04:02:01Z

force push remove some cruft

Signed-off-by: Oren Leiman <[email protected]>

oleiman · 2024-05-17T19:32:36Z

force push rebase dev to account for breaking seastar change

Signed-off-by: Oren Leiman <[email protected]>

oleiman · 2024-05-17T20:44:52Z

force push downcase enums

Signed-off-by: Oren Leiman <[email protected]>

Specifically we reed to enforce the presence of _nested_ message types. Previously we only checked at file scope. Signed-off-by: Oren Leiman <[email protected]>

Signed-off-by: Oren Leiman <[email protected]>

Common test code for working with detailed compatibility info about protobuf and avro schemas.

Signed-off-by: Oren Leiman <[email protected]>

oleiman · 2024-05-17T21:09:29Z

/ci-repeat 1

oleiman · 2024-05-17T21:09:33Z

This project is getting deprioritized in favor of FIPS work for at least the next couple weeks. Please direct any inquiries to myself, @aanthony-rp, or @michael-redpanda

oleiman · 2024-08-23T21:35:19Z

subsumed by #22798, #22877, and #22958

oleiman self-assigned this May 9, 2024

github-actions bot added area/rpk area/redpanda labels May 9, 2024

oleiman force-pushed the ds/core-1507/verbose-compat-sr branch from 1a2da99 to 2f889eb Compare May 9, 2024 20:29

oleiman force-pushed the ds/core-1507/verbose-compat-sr branch from 2f889eb to cf73462 Compare May 9, 2024 23:06

oleiman force-pushed the ds/core-1507/verbose-compat-sr branch from cf73462 to 7a95322 Compare May 11, 2024 03:08

oleiman changed the title ~~SR: Add support for ?verbose compatibility checks~~ [CORE-1507] SR: Add support for ?verbose compatibility checks May 11, 2024

oleiman marked this pull request as ready for review May 11, 2024 03:09

oleiman requested review from twmb, r-vasquez, gene-redpanda and Deflaimun as code owners May 11, 2024 03:09

Deflaimun reviewed May 13, 2024

View reviewed changes

oleiman marked this pull request as draft May 13, 2024 20:48

oleiman force-pushed the ds/core-1507/verbose-compat-sr branch from 7a95322 to 437aecb Compare May 14, 2024 03:33

oleiman marked this pull request as ready for review May 14, 2024 03:36

oleiman force-pushed the ds/core-1507/verbose-compat-sr branch from 437aecb to ee92933 Compare May 14, 2024 04:01

oleiman requested review from a team, michael-redpanda and BenPope and removed request for a team May 14, 2024 04:30

oleiman marked this pull request as draft May 17, 2024 19:31

sr/types: Introduce verbose flag and compatibility_result

e794fe2

Signed-off-by: Oren Leiman <[email protected]>

oleiman force-pushed the ds/core-1507/verbose-compat-sr branch from ee92933 to 371727b Compare May 17, 2024 19:31

sr: Introduce compatility module

ade25a9

Signed-off-by: Oren Leiman <[email protected]>

oleiman force-pushed the ds/core-1507/verbose-compat-sr branch from 371727b to 59d102b Compare May 17, 2024 20:44

oleiman added 15 commits May 17, 2024 14:00

sr/avro: Add detailed compatibility checks

d6242bf

Signed-off-by: Oren Leiman <[email protected]>

sr/protobuf: Add detailed compatibility checks

a86a201

Signed-off-by: Oren Leiman <[email protected]>

sr/protobuf: Compatibility check: MESSAGE_REMOVED

a51d7a2

Specifically we reed to enforce the presence of _nested_ message types. Previously we only checked at file scope. Signed-off-by: Oren Leiman <[email protected]>

sr/protobuf: Compatibility check: FIELD_NAMED_TYPE_CHANGED

c4843d2

Signed-off-by: Oren Leiman <[email protected]>

sr/protobuf: Compatibility check: REQUIRED_FIELD_{ADDED,REMOVED}

cc1cf20

Signed-off-by: Oren Leiman <[email protected]>

sr/protobuf: Compatibility check: MULTIPLE_FIELDS_MOVED_TO_ONEOF

d8cf075

Signed-off-by: Oren Leiman <[email protected]>

sr/protobuf: Compatibility check: ONEOF_FIELD_REMOVED

5c68bf4

Signed-off-by: Oren Leiman <[email protected]>

sr/sharded_store: Wire up compatibility_result

f6b3b7e

Signed-off-by: Oren Leiman <[email protected]>

sr/test: Introduce compatibility_common

5af2b20

Common test code for working with detailed compatibility info about protobuf and avro schemas.

sr/test: Unit tests for avro compat messages

0aa8e97

Signed-off-by: Oren Leiman <[email protected]>

sr/test: Unit tests for protobuf compat messages

3af7dc6

Signed-off-by: Oren Leiman <[email protected]>

sr/swagger: Add messages and verbose flag to compatibility API spec

5a3a60e

Signed-off-by: Oren Leiman <[email protected]>

sr/handlers: Wire up compatibility_result

5a2652f

Signed-off-by: Oren Leiman <[email protected]>

rpk: check_compatibility --verbose-compat

243d042

Signed-off-by: Oren Leiman <[email protected]>

dt/sr_test: Test for post_compat messages

25c8445

Signed-off-by: Oren Leiman <[email protected]>

oleiman force-pushed the ds/core-1507/verbose-compat-sr branch from 59d102b to 25c8445 Compare May 17, 2024 21:07

This was referenced Aug 12, 2024

CORE-6824 Schema Registry/Proto: add 5 missing checks #22798

Merged

CORE-6860 SR: Wire up support for ?verbose compatibility checks #22877

Merged

pgellert mentioned this pull request Aug 20, 2024

CORE-6860 Schema Registry: add detailed verbose compatibility checks #22958

Merged

7 tasks

oleiman closed this Aug 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CORE-1507] SR: Add support for `?verbose` compatibility checks #18332

[CORE-1507] SR: Add support for `?verbose` compatibility checks #18332

oleiman commented May 9, 2024 •

edited

Loading

oleiman commented May 9, 2024

oleiman commented May 9, 2024

vbotbuildovich commented May 9, 2024 •

edited

Loading

vbotbuildovich commented May 9, 2024 •

edited

Loading

oleiman commented May 9, 2024

oleiman commented May 10, 2024

Deflaimun May 13, 2024 •

edited

Loading

oleiman May 13, 2024 •

edited

Loading

oleiman commented May 14, 2024

oleiman commented May 14, 2024

oleiman commented May 17, 2024 •

edited

Loading

oleiman commented May 17, 2024

oleiman commented May 17, 2024

oleiman commented May 17, 2024

oleiman commented Aug 23, 2024

[CORE-1507] SR: Add support for ?verbose compatibility checks #18332

[CORE-1507] SR: Add support for ?verbose compatibility checks #18332

Conversation

oleiman commented May 9, 2024 • edited Loading

Backports Required

Release Notes

Improvements

oleiman commented May 9, 2024

oleiman commented May 9, 2024

vbotbuildovich commented May 9, 2024 • edited Loading

vbotbuildovich commented May 9, 2024 • edited Loading

oleiman commented May 9, 2024

oleiman commented May 10, 2024

Deflaimun May 13, 2024 • edited Loading

Choose a reason for hiding this comment

oleiman May 13, 2024 • edited Loading

Choose a reason for hiding this comment

oleiman commented May 14, 2024

oleiman commented May 14, 2024

oleiman commented May 17, 2024 • edited Loading

oleiman commented May 17, 2024

oleiman commented May 17, 2024

oleiman commented May 17, 2024

oleiman commented Aug 23, 2024

[CORE-1507] SR: Add support for `?verbose` compatibility checks #18332

[CORE-1507] SR: Add support for `?verbose` compatibility checks #18332

oleiman commented May 9, 2024 •

edited

Loading

vbotbuildovich commented May 9, 2024 •

edited

Loading

vbotbuildovich commented May 9, 2024 •

edited

Loading

Deflaimun May 13, 2024 •

edited

Loading

oleiman May 13, 2024 •

edited

Loading

oleiman commented May 17, 2024 •

edited

Loading