-
Notifications
You must be signed in to change notification settings - Fork 2
[25.3] Security report Admin API #35
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: main
Are you sure you want to change the base?
[25.3] Security report Admin API #35
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR adds OpenAPI documentation for a new security reporting feature. A new GET endpoint Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
admin/admin.yaml (3)
6103-6132: Inconsistent authentication field naming in kafka_interface_security_report.This schema uses
authentication_method(singular) while other interface schemas useauthentication_methods(plural, as arrays). Additionally, the enum values (SASL, mTLS, None) differ from other schemas (e.g., BASIC, OIDC in admin).
- Line 6121:
authentication_methodis a single string, not an array.- Line 6129-6132:
supported_sasl_mechanismsis a separate array, but their relationship toauthentication_methodis unclear (e.g., if mTLS is selected, are SASL mechanisms still applicable?).Recommendation: Either standardize field names and types across all interface schemas, or clarify in documentation why Kafka differs (e.g., Kafka protocol vs HTTP-based interfaces).
If intentional, add an explicit note in the schema description explaining the design choice and any conditional logic (e.g., when
supported_sasl_mechanismsis populated).
6071-6102: security_report_alert: enum values lack descriptive documentation.The
issueenum (lines 6088-6096) lists codes likeNO_TLS,NO_AUTHN,SASL_PLAIN,INSECURE_MIN_TLS_VERSION,TLS_RENEGOTIATION, etc., but these lack explanatory descriptions.Add brief inline comments or update the OpenAPI schema to include descriptions for each enum value. For example:
NO_TLS: TLS is not enabled on this interface.NO_AUTHN: Authentication is not configured.SASL_PLAIN: SASL PLAIN is used (not recommended for production).This improves API documentation clarity without changing behavior.
6103-6132: Most interface security schemas lackrequiredfield declarations.Only
security_report_alert(line 6100-6102) specifies required fields (issue,description). Other schemas (kafka, rpc, admin, schema_registry, pandaproxy, client, host_port) lack explicitrequireddeclarations, making it ambiguous which fields clients should expect to always be present.Add
requiredarrays to all schemas. For example:
kafka_interface_security_report: likely requiresname,host,port,tls_enabled,authentication_methodhost_port: likely requireshost,portclient_security_report: likely requiresbrokers,tls_enabledExplicit requirements improve API contract clarity and help clients handle responses robustly.
Also applies to: 6133-6148, 6149-6171, 6172-6194, 6195-6227, 6228-6234, 6235-6254
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
admin/admin.yaml(3 hunks)
🔇 Additional comments (3)
admin/admin.yaml (3)
550-596: Inconsistent endpoint response structure: mixing arrays and single objects for different interfaces.The response schema uses arrays for some interfaces (kafka, admin, schema_registry, pandaproxy) but single objects for others (rpc, schema_registry_client, audit_log_client). This asymmetry may confuse API consumers.
- Rationale check: If this reflects intentional design (e.g., some interfaces support multiple listeners, others don't), document this in the description or add inline comments.
- Alternative: Standardize to always use arrays, even for single-instance interfaces, for API consistency.
Consider adding a note in the endpoint description explaining why some interfaces are arrays while others are single objects, or consider normalizing the response structure.
6149-6171: Redundant or unclear authentication fields in admin, schema_registry, and pandaproxy schemas.Admin and schema_registry schemas consistently use
authentication_methodsas an array of enums. However, pandaproxy (lines 6213-6227) includes both:
authentication_methods: array of supported methods (lines 6213-6219)configured_authentication_method: single enum representing the currently configured method (lines 6222-6227)This pattern is unclear: does
configured_authentication_methodrepresent the active method vs. supported ones? If so, this relationship should be documented. Admin and schema_registry lack this "configured" field—should they have it too for consistency?Clarify whether the presence of both fields in pandaproxy is intentional and document the intended use case. Consider whether admin and schema_registry should also expose a "configured" method field.
Also applies to: 6172-6194, 6195-6227
6295-6299: Security tag added successfully.The new "Security" tag is well-placed and provides clear description for security-related endpoints.
| items: | ||
| type: string | ||
| rpc_interface_security_report: | ||
| description: Security report for RPC interface |
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.
@IoannisRP might be a nitpick - is there a reason why this one uses a singular "interface" but the others refer to "interfaces"?
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.
there are potentially multiple interfaces of the other types. Notice that the other ones are arrays of this or that. This is just a singular item where it is used.
The language used was a side effect of how these types are used. Being pedantic, they should all be singular in their description of xxx_interface_security_report
IoannisRP
left a comment
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.
lgtm
| items: | ||
| type: string | ||
| rpc_interface_security_report: | ||
| description: Security report for RPC interface |
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.
there are potentially multiple interfaces of the other types. Notice that the other ones are arrays of this or that. This is just a singular item where it is used.
The language used was a side effect of how these types are used. Being pedantic, they should all be singular in their description of xxx_interface_security_report
This pull request introduces a comprehensive security reporting feature to the Redpanda Admin API specification. The main change is the addition of a new endpoint for retrieving detailed security information about all cluster interfaces, authentication methods, TLS settings, and active security alerts. Several new schema definitions have also been added to support the structure of the security report and its alerts.
Security reporting feature:
/v1/security/reportto the OpenAPI spec, providing a cluster-wide security report covering all major interfaces, their authentication and TLS settings, and any relevant security alerts.Schema additions for structured reporting:
kafka_interface_security_report,rpc_interface_security_report,admin_interface_security_report,schema_registry_interface_security_report,pandaproxy_interface_security_report, andclient_security_report) to describe their respective security configurations.security_report_alertschema to standardize the format of security alerts, including affected interface, issue type, and human-readable description.Documentation and organization:
Securitywith a description for security management and reporting endpoints, improving API documentation and discoverability.