-
Notifications
You must be signed in to change notification settings - Fork 735
sr/context: implement /contexts/{context}/... prefixed handlers #30189
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
735b11d
f8e87d0
6cf12ea
7942863
cc96347
6f0a4e7
466789c
b079d4b
ad32b18
a921cd1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| // Copyright 2026 Redpanda Data, Inc. | ||
| // | ||
| // Use of this software is governed by the Business Source License | ||
| // included in the file licenses/BSL.md | ||
| // | ||
| // As of the Change Date specified in that file, in accordance with | ||
| // the Business Source License, use of this software will be governed | ||
| // by the Apache License, Version 2.0 | ||
|
|
||
| #pragma once | ||
|
|
||
| #include "base/seastarx.h" | ||
| #include "pandaproxy/schema_registry/errors.h" | ||
|
|
||
| #include <seastar/core/sstring.hh> | ||
| #include <seastar/http/request.hh> | ||
|
|
||
| #include <fmt/format.h> | ||
|
|
||
| #include <string_view> | ||
|
|
||
| namespace pandaproxy::schema_registry { | ||
|
|
||
| /// \brief Normalize a context name from a URL path parameter. | ||
| inline ss::sstring normalize_context(std::string_view ctx) { | ||
| if (ctx.starts_with(':')) { | ||
| ctx.remove_prefix(1); | ||
| } | ||
|
|
||
| if (ctx.ends_with(':')) { | ||
| ctx.remove_suffix(1); | ||
| } | ||
|
|
||
| if (ctx.find(':') != std::string_view::npos) { | ||
| throw as_exception(context_invalid(ctx)); | ||
| } | ||
|
|
||
| if (!ctx.starts_with('.')) { | ||
| return {fmt::format(".{}", ctx)}; | ||
| } | ||
| return ss::sstring(ctx); | ||
| } | ||
|
|
||
| /// \brief Check if a string already has a context prefix. | ||
| inline bool starts_with_context(std::string_view s) { | ||
| return s.starts_with(":.") || s.starts_with(":*:"); | ||
| } | ||
|
|
||
| /// \brief Scope the "subject" path parameter by prepending the context. | ||
| /// | ||
| /// ctx is expected to be in the form ".name" (with leading dot). The | ||
| /// resulting subject is ":.ctx:subject". | ||
| inline void | ||
| scope_subject_param(ss::http::request& req, const ss::sstring& ctx) { | ||
| auto sub = req.get_path_param("subject"); | ||
| if (!starts_with_context(sub)) { | ||
| auto nctx = normalize_context(ctx); | ||
| req.param.set( | ||
| ss::sstring("subject"), | ||
| ss::sstring(fmt::format("/:{0}:{1}", nctx, sub))); | ||
|
Comment on lines
+58
to
+60
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. Does rewriting the path parameter have any unintended consequences? I can see this being brittle or having unintended consequences to the data we log in audit logs or the logic in |
||
| } | ||
| } | ||
|
|
||
| /// \brief Inject or prepend context into the "subject" query parameter. | ||
| inline void | ||
| scope_subject_query(ss::http::request& req, const ss::sstring& ctx) { | ||
| auto nctx = normalize_context(ctx); | ||
| auto existing = req.get_query_param("subject"); | ||
| if (existing.empty()) { | ||
| req.set_query_param("subject", fmt::format(":{0}:", nctx)); | ||
| } else if (!starts_with_context(existing)) { | ||
| req.set_query_param("subject", fmt::format(":{0}:{1}", nctx, existing)); | ||
| } | ||
| } | ||
|
|
||
| /// \brief Inject or prepend context into the "subjectPrefix" query parameter. | ||
| inline void | ||
| scope_subject_prefix_query(ss::http::request& req, const ss::sstring& ctx) { | ||
| auto nctx = normalize_context(ctx); | ||
| auto existing = req.get_query_param("subjectPrefix"); | ||
| if (existing.empty()) { | ||
| req.set_query_param("subjectPrefix", fmt::format(":{0}:", nctx)); | ||
| } else if (!starts_with_context(existing)) { | ||
| req.set_query_param( | ||
| "subjectPrefix", fmt::format(":{0}:{1}", nctx, existing)); | ||
| } | ||
| } | ||
|
|
||
| /// \brief Inject the context as a context-only qualified subject path | ||
| /// parameter. | ||
| inline void | ||
| inject_context_as_subject(ss::http::request& req, const ss::sstring& ctx) { | ||
| auto nctx = normalize_context(ctx); | ||
| req.param.set( | ||
| ss::sstring("subject"), ss::sstring(fmt::format("/:{0}:", nctx))); | ||
| } | ||
|
|
||
| } // namespace pandaproxy::schema_registry | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
| #include "pandaproxy/logger.h" | ||
| #include "pandaproxy/parsing/httpd.h" | ||
| #include "pandaproxy/schema_registry/authorization.h" | ||
| #include "pandaproxy/schema_registry/context_router.h" | ||
| #include "pandaproxy/schema_registry/error.h" | ||
| #include "pandaproxy/schema_registry/errors.h" | ||
| #include "pandaproxy/schema_registry/exceptions.h" | ||
|
|
@@ -1514,7 +1515,7 @@ delete_context(server::request_t rq, server::reply_t rp) { | |
| parse_accept_header(rq, rp); | ||
|
|
||
| auto ctx_str = parse::request_param<ss::sstring>(*rq.req, "context"); | ||
| auto ctx = context{ctx_str}; | ||
| auto ctx = context{normalize_context(ctx_str)}; | ||
|
Comment on lines
1517
to
+1518
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. nit: we have a couple of places where we do |
||
|
|
||
| if (ctx == default_context) { | ||
| throw as_exception( | ||
|
|
||
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.
What happens to the authorization of deferred handler like
handle_get_schemas_ids_id_authzhere? I think at the moment these deferred handlers have some hardcoded assumptions about what exact path they are handling, which might break going forward. E.g.handle_get_schemas_ids_id_authzuses theget_schemas_ids_idnickname but now the nickname could depend on whether it's a context-prefixed endpoint or not I think.Can you add a few tests around the context path + ACLs + audit logging integration as well please?