[CORE-16775] cluster_link: unsupported-feature policy for Schema Registry config sync#31070
Conversation
9d43d4a to
016983b
Compare
There was a problem hiding this comment.
Pull request overview
Extends the existing Schema Registry “unsupported feature” policy (FAIL/REMOVE) from schema bodies to Schema Registry config sync, so Confluent-only config governance fields are either rejected per-item (FAIL) or stripped-and-counted after successful writes (REMOVE).
Changes:
- Teach the SR rest_client config parser to surface unmodeled non-null fields as structured
unsupported_featureentries and allow missing/nullcompatibilityLevel. - Plumb config unsupported diagnostics through the source reader (
source_config_read) to the mirroring task, and apply FAIL/REMOVE consistently on the config write path. - Add unit/integration/e2e coverage for unsupported schema + config behavior under both policies.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/rptest/tests/cluster_linking_schema_registry_sync_test.py | Adds e2e cases for unsupported schema fields and unsupported config governance fields under FAIL/REMOVE. |
| src/v/pandaproxy/schema_registry/rest_client/tests/parse_test.cc | Updates parse tests to validate structured unsupported and missing/null compatibilityLevel behavior. |
| src/v/pandaproxy/schema_registry/rest_client/tests/client_test.cc | Updates rest_client tests to assert unsupported instead of unknown_fields. |
| src/v/pandaproxy/schema_registry/rest_client/tests/client_integration_test.cc | Updates integration assertions to use unsupported for config reads. |
| src/v/pandaproxy/schema_registry/rest_client/parse.h | Updates parse_config contract documentation (missing/null level allowed; unsupported surfaced). |
| src/v/pandaproxy/schema_registry/rest_client/parse.cc | Implements config unsupported-feature collection and missing/null compatibility handling. |
| src/v/pandaproxy/schema_registry/rest_client/config.h | Changes config_info to optional<level> + structured unsupported. |
| src/v/pandaproxy/schema_registry/rest_client/client.h | Updates API docs to reflect unsupported reporting for config reads. |
| src/v/cluster_link/schema_registry_sync/unavailable_source_reader.h | Updates interface return type for config reads to source_config_read. |
| src/v/cluster_link/schema_registry_sync/unavailable_source_reader.cc | Implements updated config read signature for unavailable reader. |
| src/v/cluster_link/schema_registry_sync/tests/sr_sync_test_fixtures.h | Extends fake source/destination test fixtures to inject unsupported config fields and config write failures. |
| src/v/cluster_link/schema_registry_sync/tests/mirroring_task_test.cc | Adds unit tests covering FAIL/REMOVE behavior on config sync, including “no count on failed write”. |
| src/v/cluster_link/schema_registry_sync/tests/http_source_reader_test.cc | Adds test for governance-only configs surfacing unsupported fields while having no compatibility override. |
| src/v/cluster_link/schema_registry_sync/source_reader.h | Introduces source_config_read (compatibility + unsupported diagnostics). |
| src/v/cluster_link/schema_registry_sync/mirroring_task.h | Adds helpers to apply FAIL/REMOVE policy on config sync path. |
| src/v/cluster_link/schema_registry_sync/mirroring_task.cc | Applies policy when syncing config: FAIL skips write; REMOVE counts after successful change. |
| src/v/cluster_link/schema_registry_sync/http_source_reader.h | Updates config read signature to return source_config_read. |
| src/v/cluster_link/schema_registry_sync/http_source_reader.cc | Carries unsupported config features through and allows governance-only configs to parse as “no override”. |
Replace the names-only config_info::unknown_fields with structured unsupported_feature entries (JSON pointer + type), mirroring source_schema_read::unsupported on the schema-fetch path, so the config path can carry the same diagnostics for the unsupported-feature policy. A null-valued unmodeled field is treated as absent. The config sync still replicates only compatibilityLevel; the extended fields are surfaced for policy handling rather than modeled.
read_config now returns a source_config_read carrying the compatibility override plus the unsupported config fields the source served, instead of dropping them at the reader. The mirroring task consumes the new shape; applying the policy to them is the next commit.
016983b to
bd94392
Compare
|
Force-pushed: |
CI test resultstest results on build#87004
|
| // Count only when the write applied a change, so a periodic re-sync of | ||
| // a static source config does not re-count the same dropped fields. | ||
| count_if_contains_unsupported_removed( | ||
| target, feature_policy, cfg.unsupported); |
There was a problem hiding this comment.
What are your thoughts on this bit? I'm tempted to say that we should count (potentially over-count) dropped fields, even if we did not end up changing the compatibility level, so that a response like {"compatibilityGroup": "app.major.version"} that you show in the ducktape doesn't hide that we silently ignored the compatibilityGroup field.
There was a problem hiding this comment.
I think it's reasonable. I didn't want to have very verbose logging but there is second side of that which is indeed hiding ignoring fields. Let me change it.
sync_mode_and_config applies the configured unsupported_schema_feature_policy to config-borne features (default/override metadata and rule sets, compatibility group): FAIL counts a per-item error and skips that subject's config write; REMOVE logs the fields, increments unsupported_features_removed (which the proto documents as covering schemas or schema configs), and syncs only the supported compatibilityLevel. This matches the schema-fetch path's per-item handling.
bd94392 to
70b0360
Compare
…source Add four Confluent-source e2e cases exercising both the schema-fetch and the config path under both policies: a source schema carrying metadata.tags and a subject config carrying compatibilityGroup are replicated under REMOVE (counted in unsupported_features_removed, supported projection synced) and rejected as a per-item error under FAIL (offender skipped, the rest of the sync proceeds). The source registers the unsupported fields directly (only a Confluent source accepts them), so these run in the Confluent leaf only. _register grows optional metadata/ruleSet and _create_sr_link an unsupported_schema_feature_policy.
70b0360 to
0eb842d
Compare
|
Force-pushed: |
Extends
unsupported_schema_feature_policy(FAIL default / REMOVE) from theschema-fetch path to the config path, so a migration from a
Confluent-compatible source also handles config-borne features Redpanda cannot
store:
compatibilityGroup, default/override metadata & rule sets. The tolerantconfig parser now returns structured
config_info::unsupported(a JSON pointersource_schema_read::unsupported), whichread_configcarries through as asource_config_readto the mirroring taskinstead of dropping it. The mirroring task applies the policy per config target
(subject / context / global).
REMOVE syncs only the supported projection (
compatibilityLevel), incrementsunsupported_features_removed— which the proto documents as covering "schemasor schema configs" — and logs the dropped fields to the broker log; the count is
taken only after the config write lands, so a failed write does not report
features as removed. FAIL (the default) treats an unsupported config as a
per-item failure: it logs the offending fields, counts an error, and skips that
subject's config write while the rest of the sync proceeds; the task stays
active, and only global errors (source unavailable) fail-fast. A null-valued
config field is treated as absent. This mirrors the schema-fetch path's per-item
handling exactly.
Note: follow-up to #31047 (schema-fetch path), same ticket. These config
governance fields are the config-borne half the RFC lists as unsupported. No
proto/model/converter/rpk changes: the config contract, the counter, and
rpk shadow statusrendering already exist.Tests: reconciler and mirroring-task unit tests cover config REMOVE (strip +
count; no count on a failed config write) and FAIL (count + skip the offender,
sync the rest, task stays active), plus config diagnostics parsing (structured
JSON pointer + type, null-as-absent). Four Confluent-source e2e cases (schema
and config, each under FAIL and REMOVE) exercise the policy end-to-end against a
real Confluent Schema Registry via ducktape.
Backports Required
Release Notes
*none