Skip to content

Commit 8af4a02

Browse files
schema_registry/rest_client: structured unsupported-feature diagnostics
Replace the names-only parsed_schema::unknown_fields with a structured source_schema_read carrying unsupported_feature entries (JSON pointers), so a schema migration task can apply its unsupported-feature policy to GET /subjects/{subject}/versions/{version} responses. Server-assigned fields (guid, ts) are dropped silently and null-valued unmodeled fields are treated as absent, so a source that returns them does not spuriously surface a feature. The config path is unchanged. The unsupported_feature struct carries only the JSON pointer for now; its JSON type and a shallow summary can be added later without changing the source_reader signature.
1 parent a9dd590 commit 8af4a02

9 files changed

Lines changed: 248 additions & 103 deletions

File tree

src/v/pandaproxy/schema_registry/rest_client/client.cc

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,14 @@ namespace {
3535

3636
constexpr std::string_view accept_json = "application/json";
3737

38+
// Confluent returns extended schema fields (guid, ts, deleted) on a schema
39+
// response only when this request header is present. The migration client opts
40+
// in so the tolerant parser sees the full response and the ignorable-field list
41+
// explicitly drops the server-assigned ones (guid, ts) rather than relying on
42+
// the server to omit them.
43+
constexpr std::string_view accept_unknown_properties_header
44+
= "Confluent-Accept-Unknown-Properties";
45+
3846
// Schema Registry error_codes for the not-found conditions.
3947
constexpr int32_t error_code_subject_not_found = 40401;
4048
constexpr int32_t error_code_version_not_found = 40402;
@@ -564,7 +572,7 @@ client::list_subject_versions(
564572
co_return std::move(parsed.value());
565573
}
566574

567-
ss::future<std::expected<parsed_schema, domain_error>>
575+
ss::future<std::expected<source_schema_read, domain_error>>
568576
client::get_schema_by_version(
569577
const context_subject& subject,
570578
schema_version version,
@@ -580,7 +588,8 @@ client::get_schema_by_version(
580588
.path(
581589
fmt::format(
582590
"/subjects/{}/versions/{}", encode_subject(subject), version()))
583-
.header("accept", accept_json);
591+
.header("accept", accept_json)
592+
.header(accept_unknown_properties_header, "true");
584593
add_deleted_param(request, deleted);
585594
maybe_add_basic_auth(request);
586595

src/v/pandaproxy/schema_registry/rest_client/client.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,11 +191,11 @@ class client {
191191
/// be fetched. A missing subject yields subject_not_found; a missing
192192
/// version (of an existing subject) yields version_not_found.
193193
///
194-
/// The result wraps the schema together with the names of any response
195-
/// fields the client did not model (parsed_schema::unknown_fields); a
194+
/// The result wraps the schema together with any unsupported response
195+
/// fields the client could not store (source_schema_read::unsupported); a
196196
/// caller that needs fidelity can inspect them and apply its own strictness
197197
/// policy.
198-
ss::future<std::expected<parsed_schema, domain_error>>
198+
ss::future<std::expected<source_schema_read, domain_error>>
199199
get_schema_by_version(
200200
const context_subject& subject,
201201
schema_version version,

src/v/pandaproxy/schema_registry/rest_client/config.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,12 @@ registry_compatibility_level_from_wire(std::string_view sv) {
105105
/// flags, metadata, rule sets) that this client does not model; the names of
106106
/// any such top-level fields present are recorded in \ref unknown_fields, so a
107107
/// caller can tell config content was dropped without this client having to
108-
/// model it (mirroring parsed_schema::unknown_fields). Redpanda's own server
109-
/// emits only `compatibilityLevel`, so unknown_fields is empty against it; a
110-
/// third-party Confluent-compatible registry may populate it.
108+
/// model it. This serves the same intent as source_schema_read::unsupported on
109+
/// the schema-fetch path, but is a simpler representation: bare top-level field
110+
/// names only, not the structured JSON pointer + type that path records.
111+
/// Redpanda's own server emits only `compatibilityLevel`, so unknown_fields is
112+
/// empty against it; a third-party Confluent-compatible registry may populate
113+
/// it.
111114
struct config_info {
112115
registry_compatibility_level level;
113116
ss::sstring raw;

src/v/pandaproxy/schema_registry/rest_client/parse.cc

Lines changed: 91 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,12 @@
1515

1616
#include <seastar/core/coroutine.hh>
1717

18+
#include <algorithm>
19+
#include <array>
1820
#include <cstdint>
1921
#include <limits>
2022
#include <optional>
23+
#include <string_view>
2124
#include <utility>
2225

2326
namespace pandaproxy::schema_registry::rest_client {
@@ -43,6 +46,59 @@ std::optional<int32_t> checked_nonnegative_i32(int64_t v) {
4346
return static_cast<int32_t>(v);
4447
}
4548

49+
// Server-assigned response fields Redpanda does not model but which carry no
50+
// user content. The client opts into them via the
51+
// `Confluent-Accept-Unknown-Properties` header (see client.cc), so they arrive
52+
// on schema responses; this list drops them rather than surfacing them to the
53+
// unsupported-feature policy, so a source that returns them does not spuriously
54+
// trip it.
55+
//
56+
// A constexpr array scanned with ranges::contains is a deliberate choice at
57+
// this size: it keeps the list trivially extensible (just add a literal) rather
58+
// than a switch/case, and for N=2 a linear scan beats a hash set (no static
59+
// init, stays constexpr). Promote to a flat_hash_set only if this list grows
60+
// large or gains a bulk-lookup site (cf. cluster_link's
61+
// disallowed_topic_properties, materialized into a set in its validator).
62+
constexpr auto ignorable_fields = std::to_array<std::string_view>(
63+
{"guid", "ts"});
64+
65+
bool is_ignorable_field(std::string_view key) {
66+
return std::ranges::contains(ignorable_fields, key);
67+
}
68+
69+
// The field's JSON type name, for unsupported-feature diagnostics; takes the
70+
// parser's current value token.
71+
const char* json_type_name(serde::json::token t) {
72+
using token = serde::json::token;
73+
switch (t) {
74+
case token::start_object:
75+
return "object";
76+
case token::start_array:
77+
return "array";
78+
case token::value_string:
79+
return "string";
80+
case token::value_int:
81+
case token::value_double:
82+
return "number";
83+
case token::value_true:
84+
case token::value_false:
85+
return "boolean";
86+
case token::value_null:
87+
return "null";
88+
// Non-value tokens never reach here (this is called only on the parser's
89+
// current value token). They are enumerated rather than folded into a
90+
// default so the switch stays exhaustive: -Wswitch (via -Werror) then flags
91+
// a newly-added token at compile time instead of silently returning
92+
// "unknown".
93+
case token::error:
94+
case token::key:
95+
case token::end_object:
96+
case token::end_array:
97+
case token::eof:
98+
return "unknown";
99+
}
100+
}
101+
46102
} // namespace
47103

48104
ss::future<std::expected<chunked_vector<context_subject>, parse_error>>
@@ -517,21 +573,19 @@ parse_references(serde::json::parser& p, qualified_subjects_enabled qualified) {
517573
parse_error{.reason = "truncated or malformed references array"});
518574
}
519575

520-
// The result of parsing a metadata object: the modeled portion plus the names
521-
// of any sub-keys we don't model.
576+
// The result of parsing a metadata object: the modeled portion plus any
577+
// unsupported sub-fields, each already reported as a `/metadata/<key>` pointer.
522578
struct parsed_metadata {
523579
schema_metadata metadata;
524-
// Unmodeled keys found directly inside `metadata` (e.g. `tags`,
525-
// `sensitive`), unqualified; the caller qualifies and propagates them.
526-
chunked_vector<ss::sstring> unknown_fields;
580+
chunked_vector<unsupported_feature> unsupported;
527581
};
528582

529583
// Parse a metadata object of the form {"properties": {<str>: <str>}, ...}.
530584
// Entered with the current token at the object start; leaves the parser at the
531585
// end_object token. Only `properties` is modeled; its values are stored as
532586
// strings, with numbers and booleans coerced to strings to match the write
533-
// path. Any other key (e.g. `tags`, `sensitive`) is returned, unqualified, in
534-
// parsed_metadata::unknown_fields for the caller to record.
587+
// path. Any other non-null key (e.g. `tags`, `sensitive`) is reported in
588+
// parsed_metadata::unsupported as a `/metadata/<key>` pointer.
535589
ss::future<std::expected<parsed_metadata, parse_error>>
536590
parse_metadata(serde::json::parser& p) {
537591
using token = serde::json::token;
@@ -546,7 +600,14 @@ parse_metadata(serde::json::parser& p) {
546600
parse_error{.reason = "truncated JSON in schema metadata"});
547601
}
548602
if (key != "properties") {
549-
result.unknown_fields.push_back(std::move(key));
603+
// A null value means the sub-field is absent; anything else is an
604+
// unsupported feature, surfaced as a `/metadata/<key>` pointer.
605+
if (p.token() != token::value_null) {
606+
result.unsupported.push_back(
607+
unsupported_feature{
608+
.json_pointer = ssx::sformat("/metadata/{}", key),
609+
.json_type = json_type_name(p.token())});
610+
}
550611
co_await p.skip_value();
551612
continue;
552613
}
@@ -602,7 +663,7 @@ parse_metadata(serde::json::parser& p) {
602663

603664
} // namespace
604665

605-
ss::future<std::expected<parsed_schema, parse_error>>
666+
ss::future<std::expected<source_schema_read, parse_error>>
606667
parse_subject_version(iobuf body, qualified_subjects_enabled qualified) {
607668
using token = serde::json::token;
608669
// Firewall exceptions from the parser: malformed input is reported via the
@@ -623,7 +684,7 @@ parse_subject_version(iobuf body, qualified_subjects_enabled qualified) {
623684
schema_definition::references refs;
624685
is_deleted deleted{false};
625686
std::optional<schema_metadata> metadata;
626-
chunked_vector<ss::sstring> unknown_fields;
687+
chunked_vector<unsupported_feature> unsupported;
627688

628689
while (co_await p.next()) {
629690
if (p.token() == token::end_object) {
@@ -637,8 +698,8 @@ parse_subject_version(iobuf body, qualified_subjects_enabled qualified) {
637698
}
638699
// Absent fields fall back to defaults/sentinels; completeness
639700
// is a higher-layer concern. Unmodeled fields were recorded in
640-
// unknown_fields above for the caller to act on.
641-
co_return parsed_schema{
701+
// `unsupported` above for the caller to act on.
702+
co_return source_schema_read{
642703
.schema = stored_schema{
643704
.schema = subject_schema{
644705
subject.value_or(invalid_subject),
@@ -651,7 +712,7 @@ parse_subject_version(iobuf body, qualified_subjects_enabled qualified) {
651712
.version = version.value_or(invalid_schema_version),
652713
.id = id.value_or(invalid_schema_id),
653714
.deleted = deleted},
654-
.unknown_fields = std::move(unknown_fields)};
715+
.unsupported = std::move(unsupported)};
655716
}
656717
if (p.token() != token::key) {
657718
co_return std::unexpected(
@@ -730,30 +791,35 @@ parse_subject_version(iobuf body, qualified_subjects_enabled qualified) {
730791
refs = std::move(*r);
731792
} else if (key == "metadata") {
732793
// Partially modeled: parse_metadata captures `properties` and
733-
// returns any other sub-key (e.g. `tags`), which we qualify
734-
// with a `metadata.` prefix into unknown_fields. A null
735-
// metadata is treated as absent; any other non-object is
736-
// unrepresentable.
794+
// returns any other sub-key (e.g. `tags`) as a
795+
// `/metadata/<key>` unsupported feature. A null metadata is
796+
// treated as absent; any other non-object is unrepresentable.
737797
if (p.token() == token::start_object) {
738798
auto m = co_await parse_metadata(p);
739799
if (!m) {
740800
co_return std::unexpected(std::move(m.error()));
741801
}
742802
metadata = std::move(m->metadata);
743-
for (const auto& sub : m->unknown_fields) {
744-
unknown_fields.push_back(
745-
ssx::sformat("metadata.{}", sub));
803+
for (auto& f : m->unsupported) {
804+
unsupported.push_back(std::move(f));
746805
}
747806
} else if (p.token() != token::value_null) {
748807
co_return std::unexpected(
749808
parse_error{.reason = "metadata must be an object"});
750809
}
751810
} else {
752-
// Unknown / not-yet-modeled field (guid, ts, ruleSet,
753-
// schemaTags, ...): skip its value, but record the top-level
754-
// key so the caller can decide whether dropping it is
755-
// acceptable.
756-
unknown_fields.push_back(std::move(key));
811+
// Unmodeled field. Server-assigned fields that carry no user
812+
// content (`guid`, `ts`) are dropped silently; a null value
813+
// means the field is absent; anything else is an unsupported
814+
// feature, surfaced as a `/<key>` pointer for the migration
815+
// policy to act on.
816+
if (
817+
!is_ignorable_field(key) && p.token() != token::value_null) {
818+
unsupported.push_back(
819+
unsupported_feature{
820+
.json_pointer = ssx::sformat("/{}", key),
821+
.json_type = json_type_name(p.token())});
822+
}
757823
co_await p.skip_value();
758824
}
759825
}

src/v/pandaproxy/schema_registry/rest_client/parse.h

Lines changed: 24 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -130,49 +130,36 @@ ss::future<std::expected<chunked_vector<subject_version>, parse_error>>
130130
parse_schema_id_subject_versions(
131131
iobuf body, qualified_subjects_enabled qualified);
132132

133-
/// The outcome of parsing a get-schema-by-version response: the schema, plus
134-
/// the names of any top-level response fields the parser did not model.
135-
///
136-
/// parse_subject_version is deliberately lenient — it never rejects a response
137-
/// merely for carrying fields it doesn't model; it skips them and records their
138-
/// names here. This lets a caller that needs fidelity (e.g. schema migration)
139-
/// apply its own policy — reject, warn, or ignore — while a caller that doesn't
140-
/// care simply disregards the list. Recorded names are top-level keys, with one
141-
/// exception: `metadata` is only partially modeled (just `metadata.properties`
142-
/// is captured), so an unmodeled key directly under it is reported with a
143-
/// `metadata.` prefix (e.g. `metadata.tags`). An unmodeled key nested inside
144-
/// any other modeled field (e.g. within a reference) is skipped without being
145-
/// reported. It is therefore a best-effort signal that content was dropped, not
146-
/// a proof of a lossless round-trip.
147-
struct parsed_schema {
148-
stored_schema schema;
149-
chunked_vector<ss::sstring> unknown_fields;
150-
};
151-
152133
/// Parse the body of a `GET /subjects/{subject}/versions/{version}` response
153-
/// into a parsed_schema (the schema plus the names of any unmodeled top-level
154-
/// fields).
134+
/// into a source_schema_read: the schema projected into Redpanda's supported
135+
/// model, plus a sidecar list of unsupported fields it could not store.
136+
///
137+
/// This is a faithful, lenient deserialization (the lowest layer): it never
138+
/// rejects a response merely for carrying fields it doesn't model. Server-
139+
/// assigned fields that carry no user content (`guid`, `ts`) are dropped
140+
/// silently. Any other unmodeled field is surfaced in
141+
/// source_schema_read::unsupported as a JSON pointer (e.g. `/ruleSet`), for a
142+
/// caller that needs fidelity (e.g. schema migration) to apply its policy —
143+
/// fail, remove, or ignore. `metadata` is partially modeled:
144+
/// `metadata.properties` is captured into the schema's metadata (values are
145+
/// stringified, matching the write path), while any other key under `metadata`
146+
/// (e.g. `metadata.tags`) is reported as `/metadata/<key>`. A field whose value
147+
/// is null is treated as absent (not reported). An unmodeled key nested inside
148+
/// another modeled field (e.g. within a reference) is skipped without being
149+
/// reported — so `unsupported` is a best-effort signal that content was
150+
/// dropped, not a proof of a lossless round-trip.
155151
///
156-
/// This is a faithful, lenient deserialization (the lowest layer): unknown or
157-
/// not-yet-modeled fields (`guid`, `ts`, `ruleSet`, `schemaTags`, ...) are
158-
/// skipped — their names are collected in parsed_schema::unknown_fields for the
159-
/// caller to act on. `metadata` is partially modeled: `metadata.properties` is
160-
/// captured into the schema's metadata (values are stringified, matching the
161-
/// write path), while any other key under `metadata` (e.g. `metadata.tags`) is
162-
/// reported in unknown_fields under a `metadata.` prefix. Absent fields take
163-
/// their default/sentinel (absent `schemaType` -> AVRO, `deleted` -> false,
164-
/// `references` -> empty, `metadata` -> absent, and absent
165-
/// `subject`/`version`/`id`/`schema` -> the invalid sentinels). It does NOT
166-
/// enforce completeness or reject for unmodeled fields — whether an incomplete
167-
/// or lossy response is acceptable (a strict mode) is a higher-layer concern.
168-
/// It rejects only inputs it cannot represent: a non-object body, malformed
169-
/// JSON, a present modeled field with a wrong-typed or out-of-range value, or
170-
/// an unknown `schemaType`.
152+
/// Absent fields take their default/sentinel (absent `schemaType` -> AVRO,
153+
/// `deleted` -> false, `references` -> empty, `metadata` -> absent, and absent
154+
/// `subject`/`version`/`id`/`schema` -> the invalid sentinels). It rejects only
155+
/// inputs it cannot represent: a non-object body, malformed JSON, a present
156+
/// modeled field with a wrong-typed or out-of-range value, or an unknown
157+
/// `schemaType`.
171158
///
172159
/// \p qualified is the caller-supplied policy for interpreting
173160
/// context-qualified subject strings (the response `subject` and each
174161
/// reference's `subject`). The function does not throw.
175-
ss::future<std::expected<parsed_schema, parse_error>>
162+
ss::future<std::expected<source_schema_read, parse_error>>
176163
parse_subject_version(iobuf body, qualified_subjects_enabled qualified);
177164

178165
/// The structured error body Schema Registry returns on failures:

src/v/pandaproxy/schema_registry/rest_client/tests/client_integration_test.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ FIXTURE_TEST(sr_rest_client_integration, pandaproxy_test_fixture) {
294294
= sut.get_schema_by_version(multi, pps::schema_version{2}, rtc).get();
295295
BOOST_REQUIRE(res.has_value());
296296
// Redpanda's SR emits only fields we model, so nothing is dropped.
297-
BOOST_REQUIRE(res->unknown_fields.empty());
297+
BOOST_REQUIRE(res->unsupported.empty());
298298
const auto& s = res->schema;
299299
BOOST_REQUIRE_EQUAL(s.schema.sub(), multi);
300300
BOOST_REQUIRE_EQUAL(s.version, pps::schema_version{2});
@@ -310,8 +310,8 @@ FIXTURE_TEST(sr_rest_client_integration, pandaproxy_test_fixture) {
310310
.get();
311311
BOOST_REQUIRE(res.has_value());
312312
// metadata.properties is modeled, so it parses back in full and nothing
313-
// is reported as unknown.
314-
BOOST_REQUIRE(res->unknown_fields.empty());
313+
// is reported as unsupported.
314+
BOOST_REQUIRE(res->unsupported.empty());
315315
const auto& def = res->schema.schema.def();
316316
BOOST_REQUIRE(def.meta().has_value());
317317
BOOST_REQUIRE(def.meta()->properties.has_value());

0 commit comments

Comments
 (0)