-
Notifications
You must be signed in to change notification settings - Fork 685
Client Monitoring: require license + integrate filter #28000
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?
Conversation
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.
Pull Request Overview
This PR integrates filtering functionality and license requirements into the Kafka connections admin API endpoint. The changes enhance security by requiring an enterprise license and improve usability by allowing filtered queries.
- Adds enterprise license validation to the
list_kafka_connections
endpoint - Integrates AIP filter parser and predicate for connection filtering
- Updates tests to verify both filtering functionality and license enforcement
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/rptest/tests/list_kafka_connections_test.py | Adds tests for filtering integration and license requirement enforcement |
src/v/redpanda/admin/services/broker.h | Updates constructor to accept feature table dependency for license checking |
src/v/redpanda/admin/services/broker.cc | Implements license validation and filter integration in list_kafka_connections |
src/v/redpanda/admin/services/BUILD | Adds dependencies for features, AIP filter, and protobuf RPC |
src/v/redpanda/admin/server.cc | Updates broker service instantiation to pass feature table |
Integrate the filter parser and predicate into the ListKafkaConnections admin API endpoint.
e4b9a8c
to
6e595a8
Compare
force-push: address copilot review |
auto result = connection_gather_result{}; | ||
|
||
auto conn_ptrs = server.list_connections(); | ||
co_await ss::maybe_yield(); |
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.
nit: ss::coroutine::maybe_yield()
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.
Fixed
throw serde::pb::rpc::permission_denied_exception( | ||
fmt::format("Invalid license: {}", status())); |
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.
failed_precondition?
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.
Happy to switch to that if you think that's more appropriate, I was debating between the two. I went with permission_denied
for consistency with the existing SR ACLs endpoint:
redpanda/src/v/pandaproxy/schema_registry/handlers.cc
Lines 904 to 907 in 1ffde40
throw ss::httpd::base_exception( | |
fmt::format("Invalid license: {}", status()), | |
ss::http::reply::status_type::forbidden); | |
} |
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.
After reading https://grpc.github.io/grpc/core/md_doc_statuscodes.html, failed_precondition
seems more appropriate so I've switched to that.
Retry command for Build#73952please wait until all jobs are finished before running the slash command
|
This endpoint requires an enterprise license
Carry-over improvement from an earlier PR
Carry-over cleanup from an earlier PR
6e595a8
to
87c8fbb
Compare
force-push: change error type + minor fixups from earlier PRs |
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.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Fixes https://redpandadata.atlassian.net/browse/CORE-13361
Fixes https://redpandadata.atlassian.net/browse/CORE-13573
Backports Required
Release Notes