-
Notifications
You must be signed in to change notification settings - Fork 713
[CORE-15194] schema_registry: add subject query param to GET /schemas/ids/{id} #29451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
271efa8
1be2470
b6a3527
95b9792
1bd913b
da24b7f
6fb5eb5
31aaab3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,7 @@ | |
| #include <algorithm> | ||
| #include <iterator> | ||
| #include <limits> | ||
| #include <optional> | ||
|
|
||
| namespace ppj = pandaproxy::json; | ||
|
|
||
|
|
@@ -153,6 +154,160 @@ to_non_context_schema_ids(const chunked_vector<context_schema_id>& ids) { | |
| | std::ranges::to<chunked_vector<schema_id>>(); | ||
| } | ||
|
|
||
| /// Resolve a schema ID within a single context, optionally filtering by | ||
| /// subject. | ||
| ss::future<context_schema_id> resolve_schema_id_simple( | ||
| const server::request_t& rq, | ||
| std::optional<request_auth_result>& auth_result, | ||
| schema_id id, | ||
| context_subject ctx_sub) { | ||
| if (ctx_sub.ctx == default_context && !ctx_sub.sub().empty()) { | ||
| vlog( | ||
| srlog.error, | ||
| "resolve_schema_id_simple cannot be called with default context " | ||
| "and non-empty subject"); | ||
| throw exception(error_code::internal_server_error); | ||
| } | ||
|
|
||
| vlog( | ||
| srlog.debug, | ||
| "Resolving schema ID {} in context '{}'{}", | ||
| id, | ||
| ctx_sub.ctx, | ||
| ctx_sub.sub().empty() ? "" | ||
| : ss::sstring{", subject '"} + ctx_sub.sub() + "'"); | ||
|
|
||
| const context_schema_id ctx_id{ctx_sub.ctx, id}; | ||
| auto schema_subjects | ||
| = co_await rq.service().schema_store().get_schema_subjects( | ||
| ctx_id, include_deleted::yes); | ||
| // If a subject is provided, filter the schema_subjects to only that subject | ||
| // (if it exists) | ||
| if (!ctx_sub.sub().empty()) { | ||
| vlog( | ||
| srlog.debug, | ||
| "Filtering schema subjects for subject '{}'", | ||
| ctx_sub.sub()); | ||
| schema_subjects = std::ranges::contains(schema_subjects, ctx_sub) | ||
| ? decltype(schema_subjects){ctx_sub} | ||
| : decltype(schema_subjects){}; | ||
| } | ||
|
|
||
| // Ensure requester is authorized to access at least one of the subjects | ||
| // associated with the schema ID in the given context. | ||
| enterprise::handle_get_schemas_ids_id_authz( | ||
| rq, auth_result, schema_subjects); | ||
|
|
||
| if (schema_subjects.empty()) { | ||
| // The schema ID is not associated with any subject in this context, or | ||
| // if the requester provided a ctx_sub.sub, the schema is not associated | ||
| // with that subject. | ||
| vlog( | ||
| srlog.debug, | ||
| "Schema ID {} not found in context '{}'{}", | ||
| id, | ||
| ctx_sub.ctx, | ||
| ctx_sub.sub().empty() | ||
| ? "" | ||
| : ss::sstring{", subject '"} + ctx_sub.sub() + "'"); | ||
| throw as_exception(not_found(id)); | ||
| } | ||
|
|
||
| vlog( | ||
| srlog.debug, | ||
| "Schema ID {} resolved in context '{}'{}", | ||
| id, | ||
| ctx_sub.ctx, | ||
| ctx_sub.sub().empty() ? "" | ||
| : ss::sstring{", subject '"} + ctx_sub.sub() + "'"); | ||
|
|
||
| co_return ctx_id; | ||
| } | ||
|
|
||
| /// Resolve a schema ID by searching across contexts and subjects. This function | ||
| /// assumes that the subject is non-empty. | ||
| /// The search order is: | ||
| /// 1. Default context with provided subject | ||
| /// 2. Other contexts with provided subject | ||
| /// 3. Default context without subject restriction | ||
| ss::future<context_schema_id> resolve_schema_id_extended( | ||
| const server::request_t& rq, | ||
| std::optional<request_auth_result>& auth_result, | ||
| schema_id id, | ||
| subject subject) { | ||
| if (subject().empty()) { | ||
| vlog( | ||
| srlog.error, | ||
| "resolve_schema_id_extended should only be called with non-empty " | ||
| "subject"); | ||
| throw exception(error_code::internal_server_error); | ||
| } | ||
|
|
||
| vlog( | ||
| srlog.debug, | ||
| "Performing an extended search to resolve schema ID {} for subject '{}'.", | ||
| id, | ||
| subject()); | ||
|
|
||
| // First, try default context with the provided subject | ||
| if (context_subject ctx_sub{default_context, subject}; | ||
| co_await rq.service().schema_store().has_version( | ||
| ctx_sub, id, include_deleted::yes)) { | ||
| vlog( | ||
| srlog.debug, | ||
| "Schema ID {} found in default context with subject '{}'", | ||
| id, | ||
| subject()); | ||
| enterprise::handle_get_schemas_ids_id_authz( | ||
| rq, auth_result, {std::move(ctx_sub)}); | ||
| co_return context_schema_id{default_context, id}; | ||
| } | ||
|
|
||
| // Next, try other contexts with the provided subject | ||
| auto contexts | ||
| = co_await rq.service().schema_store().get_materialized_contexts(); | ||
| for (const auto& ctx : contexts | std::views::filter([](const auto& c) { | ||
| return c != default_context; | ||
| })) { | ||
| if (context_subject ctx_sub{ctx, subject}; | ||
| co_await rq.service().schema_store().has_version( | ||
| ctx_sub, id, include_deleted::yes)) { | ||
| vlog( | ||
| srlog.debug, | ||
| "Schema ID {} found in context '{}' with subject '{}'", | ||
| id, | ||
| ctx, | ||
| subject()); | ||
| enterprise::handle_get_schemas_ids_id_authz( | ||
| rq, auth_result, {ctx_sub}); | ||
| co_return context_schema_id{ctx, id}; | ||
| } | ||
| } | ||
|
|
||
| // Finally, try default context without subject restriction | ||
| auto default_ctx_subjects | ||
| = co_await rq.service().schema_store().get_subjects( | ||
| default_context, include_deleted::yes); | ||
| enterprise::handle_get_schemas_ids_id_authz( | ||
| rq, auth_result, default_ctx_subjects); | ||
| if (!default_ctx_subjects.empty()) { | ||
| vlog( | ||
| srlog.debug, | ||
| "Schema ID {} found in default context without subject restriction", | ||
| id); | ||
| co_return context_schema_id{default_context, id}; | ||
| } | ||
|
|
||
| vlog( | ||
| srlog.debug, | ||
| "Schema ID {} not found in any context with subject '{}' or in default " | ||
| "context without subject restriction", | ||
| id, | ||
| subject()); | ||
| enterprise::handle_get_schemas_ids_id_authz(rq, auth_result, {}); | ||
| throw as_exception(not_found(id)); | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| ss::future<server::reply_t> | ||
|
|
@@ -486,20 +641,21 @@ ss::future<server::reply_t> get_schemas_ids_id( | |
| const auto format = parse_output_format(*rq.req); | ||
|
|
||
| co_await rq.service().writer().read_sync(); | ||
pgellert marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| auto subjects = co_await rq.service().schema_store().get_schema_subjects( | ||
| id, include_deleted::yes); | ||
|
|
||
| enterprise::handle_get_schemas_ids_id_authz(rq, auth_result, subjects); | ||
| // Parse optional subject query parameter to extract context | ||
| auto subject_param = parse::query_param<std::optional<ss::sstring>>( | ||
| *rq.req, "subject") | ||
| .value_or(""); | ||
|
|
||
| // With deferred schema validation, there might be a schema that | ||
| // had invalid references. These might have already been posted, so | ||
| // we need to sync | ||
| co_await rq.service().writer().read_sync(); | ||
| auto ctx_sub = context_subject::from_string(subject_param); | ||
|
Comment on lines
+645
to
+650
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if the code would be clearer if we did not use
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was using |
||
|
|
||
| auto def = co_await get_or_load(rq, [&rq, id, format]() { | ||
| return rq.service().schema_store().get_schema_definition(id, format); | ||
| }); | ||
| auto ctx_id = co_await ( | ||
| ctx_sub.ctx == default_context && !ctx_sub.sub().empty() | ||
| ? resolve_schema_id_extended(rq, auth_result, id, ctx_sub.sub) | ||
| : resolve_schema_id_simple(rq, auth_result, id, ctx_sub)); | ||
|
|
||
| auto def = co_await rq.service().schema_store().get_schema_definition( | ||
| ctx_id, format); | ||
| auto resp = ppj::rjson_serialize_iobuf( | ||
| get_schemas_ids_id_response{.definition{std::move(def)}}); | ||
| log_response(*rq.req, resp); | ||
|
|
@@ -576,7 +732,8 @@ ss::future<server::reply_t> get_subjects( | |
| auto res = co_await rq.service().schema_store().get_subjects( | ||
| inc_del, subject_prefix); | ||
|
|
||
| // Handle AuthZ - Filters res for the subjects the user is allowed to see | ||
| // Handle AuthZ - Filters res for the subjects the user is allowed to | ||
| // see | ||
pgellert marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| enterprise::handle_get_subjects_authz(rq, auth_result, res); | ||
|
|
||
| // Convert context_subject to qualified string format for JSON response | ||
|
|
@@ -627,7 +784,8 @@ post_subject(server::request_t rq, server::reply_t rp) { | |
| const auto format = parse_output_format(*rq.req); | ||
| vlog( | ||
| srlog.debug, | ||
| "post_subject subject='{}', normalize='{}', deleted='{}', format='{}'", | ||
| "post_subject subject='{}', normalize='{}', deleted='{}', " | ||
| "format='{}'", | ||
| ctx_sub, | ||
| norm, | ||
| inc_del, | ||
|
|
@@ -790,7 +948,8 @@ post_subject_versions(server::request_t rq, server::reply_t rp) { | |
| throw exception( | ||
| error_code::schema_incompatible, | ||
| fmt::format( | ||
| "Schema being registered is incompatible with an earlier " | ||
| "Schema being registered is incompatible with an " | ||
| "earlier " | ||
| "schema for subject \"{}\", details: [{}]", | ||
| ctx_sub, | ||
| fmt::join(compat.messages, ", "))); | ||
|
|
@@ -990,7 +1149,8 @@ compatibility_subject_version(server::request_t rq, server::reply_t rp) { | |
| auto unparsed = co_await rjson_parse( | ||
| *rq.req, post_subject_versions_request_handler<>{ctx_sub}); | ||
|
|
||
| // Must read, in case we have the subject in cache with an outdated config | ||
| // Must read, in case we have the subject in cache with an outdated | ||
| // config | ||
| co_await rq.service().writer().read_sync(); | ||
|
|
||
| vlog( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I should have been clearer in the ticket, but can you please also implement this for
GET /schemas/ids/{id}/versionsandGET /schemas/ids/{id}/schematoo, in addition toGET /schemas/ids/{id}?