[CORE-15194] schema_registry: add GET /schemas/ids/{id}/schema endpoint#29539
[CORE-15194] schema_registry: add GET /schemas/ids/{id}/schema endpoint#29539nguyen-andrew wants to merge 11 commits intoredpanda-data:devfrom
Conversation
Update context_subject::from_string to handle context-only strings.
Rename is_default_context() to is_default_context_only() to better reflect its behavior: it returns true only when the context is the default context AND the subject is empty (context-only). Add a new method is_non_default_context() that checks if a subject is in a non-default context. This will be used in future changes to handle subject query parameters.
This enables retrieving subjects filtered by a specific context.
There was a problem hiding this comment.
Pull request overview
This PR implements the GET /schemas/ids/{id}/schema endpoint that returns raw schema strings, extending the existing GET /schemas/ids/{id} endpoint. The new endpoint supports context-aware lookups via an optional subject query parameter and output format control via a format parameter.
Changes:
- Added new
/schemaendpoint that returns only the schema string without metadata wrapper - Enhanced both endpoints to support optional
subjectquery parameter for context-aware schema resolution - Implemented cross-context search logic that falls back across contexts when subject is unqualified
- Added comprehensive authorization tests for context-qualified subjects
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/rptest/tests/schema_registry_test.py | Added test helpers and comprehensive tests for subject parameter handling and new /schema endpoint |
| src/v/pandaproxy/schema_registry/types.h | Renamed method and added helper for context checking |
| src/v/pandaproxy/schema_registry/types.cc | Fixed parsing logic to handle context-only qualified subjects |
| src/v/pandaproxy/schema_registry/test/context_subject.cc | Updated tests for corrected context-only parsing behavior |
| src/v/pandaproxy/schema_registry/store.h | Added method to retrieve subjects for a specific context |
| src/v/pandaproxy/schema_registry/sharded_store.h | Added sharded store interface for context-specific subject retrieval |
| src/v/pandaproxy/schema_registry/sharded_store.cc | Implemented sharded subject retrieval for specific contexts |
| src/v/pandaproxy/schema_registry/service.cc | Registered new /schema endpoint in route table |
| src/v/pandaproxy/schema_registry/seq_writer.cc | Updated calls to renamed is_default_context_only method |
| src/v/pandaproxy/schema_registry/handlers.h | Added handler declaration for new /schema endpoint |
| src/v/pandaproxy/schema_registry/handlers.cc | Implemented schema resolution logic and new endpoint handler |
| src/v/pandaproxy/api/api-doc/schema_registry.json | Added API documentation for new endpoint and subject parameter |
| ctx1_only_id = result.json()["id"] # ID 3 in ctx1, no ID 3 in default | ||
|
|
||
| # === Test: Subject portion empty (sub().empty()) === | ||
| self.logger.info("Testing: Subject portion empty") |
There was a problem hiding this comment.
Corrected spelling of 'Retrurns' to 'Returns' in comment at line 201 of src/v/pandaproxy/schema_registry/types.h
| 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); | ||
| } |
There was a problem hiding this comment.
The validation logic that prevents calling resolve_schema_id_simple with default context and non-empty subject duplicates the same check in resolve_schema_id_extended (lines 238-243). Consider extracting this validation into a helper function or document why this constraint exists in both functions.
| auto [resp, type, refs, meta] = std::move(def).destructure(); | ||
| log_response(*rq.req, resp); | ||
| rp.rep->write_body("json", ppj::as_body_writer(std::move(resp)())); |
There was a problem hiding this comment.
The destructured variables type, refs, and meta are unused after destructuring. Consider using structured bindings with [[maybe_unused]] attributes or only destructure the needed resp variable to make the intent clearer.
| class SchemaRegistryAclAuthzTestBase(SchemaRegistryEndpoints): | ||
| """ | ||
| Verify that schema registry endpoints are protected by the correct ACL resource and operation. | ||
| Base class providing shared ACL test infrastructure (setup, helpers) without test methods. |
There was a problem hiding this comment.
The docstring should clarify that this base class was extracted to enable test reuse with different configurations (specifically for context authorization tests), not just that it lacks test methods.
| Base class providing shared ACL test infrastructure (setup, helpers) without test methods. | |
| Base class extracted to share schema registry ACL / context-authorization | |
| test infrastructure (common setup, helpers) across multiple concrete | |
| test classes with different Redpanda and schema registry configurations. | |
| This class intentionally defines no test methods; subclasses provide the | |
| actual tests while reusing the common authorization setup. |
Previously, the GET /schemas/ids/{id} endpoint resolved schemas using
only the numeric ID without any context or subject scoping. This meant
clients could not target a specific context when retrieving a schema.
This change adds support for an optional `subject` query parameter,
parsed via context_subject::from_string(), that controls how the
schema ID is resolved:
- Default context with subject (e.g., "subj" or ":.:subj"): perform
an extended search: default context with subject first, then other
contexts with subject, then default context without the subject
restriction
- All other cases: resolve within a single context only
The schema definition is now retrieved using the resolved context,
ensuring the correct schema is returned in multi-context deployments.
Add test for the `subject` query parameter on GET /schemas/ids/{id},
verifying that it correctly extracts context for schema lookup.
Also update the test client to support the new parameter.
Extract shared ACL test infrastructure (setup, helpers) into a new SchemaRegistryAclAuthzTestBase class to enable reuse by other tests.
Add SchemaRegistryContextAuthzTest to verify ACL enforcement when using
context-qualified subjects and the subject query parameter with
GET /schemas/ids/{id}. The tests cover authorization scenarios including
literal and prefix ACLs on context-qualified subjects, cross-context
search behavior, and proper 403 responses to prevent information leakage.
Add OpenAPI documentation for the raw schema retrieval endpoint, including the optional subject and format query parameters.
Add handler for the raw schema retrieval endpoint, supporting optional subject query parameter for context-aware schema lookup.
Add test client method and integration test verifying the raw schema
endpoint returns the same schema as GET /schemas/ids/{id}["schema"].
f63edbe to
35d4892
Compare
|
Force push to rebase on latest base branch. |
Retry command for Build#80199please wait until all jobs are finished before running the slash command |
CI test resultstest results on build#80199
|
pgellert
left a comment
There was a problem hiding this comment.
This looks good to go. Not approving yet just to wait for the base PR to merge.
NOTE: This PR stacks on top of #29451. Please only review the last 3 commits.
Implement the GET /schemas/ids/{id}/schema endpoint that returns the raw schema string.
The endpoint supports the following parameters:
Backports Required
Release Notes