-
-
Notifications
You must be signed in to change notification settings - Fork 223
feat: Add connector-level permissions for Kafka Connect #1541
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?
Conversation
922d1e4 to
01a0709
Compare
Implements granular permission control at the individual connector level, allowing administrators to grant permissions for specific connectors rather than entire Kafka Connect instances. Changes: - Add CONNECTOR resource type and ConnectorAction enum for granular permissions - Implement hierarchical permission checking (connector-level takes precedence) - Update frontend to check connector permissions with connect-level fallback - Add comprehensive tests for connector permission scenarios - Upgrade Testcontainers to 2.0.2 for Docker Engine 29 compatibility Features: - Permission format: `connect-name/connector-name` for specific connectors - Wildcard patterns supported (e.g., `.*-connect/prod-.*`) - Backwards compatible with existing CONNECT permissions - Action hierarchy maintained (EDIT includes VIEW permission)
01a0709 to
4cce922
Compare
Haarolean
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.
- let's address the backward compatibility issues first before reviewing any further
- please refrain from editing the description / using force push to remove the AI tools attributions.
api/src/main/java/io/kafbat/ui/controller/KafkaConnectController.java
Outdated
Show resolved
Hide resolved
|
|
||
| public static final Set<ConnectorAction> ALTER_ACTIONS = Set.of(CREATE, EDIT, DELETE, OPERATE, RESET_OFFSETS); | ||
|
|
||
| public static final Map<String, PermissibleAction> ALIASES = Map.of( |
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.
The original idea behind Aliases was to provide backward compatibility, I'm sure we don't need it here
| .connectorActions(connectorResource, actions) | ||
| .build(); | ||
|
|
||
| return isAccessible(connectorContext) |
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.
I think this logic should be within context, let's keep service clear
| String connectorName, | ||
| ServerWebExchange exchange) { | ||
|
|
||
| String connectorResource = ConnectorAction.buildResourcePath(connectName, connectorName); |
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.
I would prefer to see it somewhere closer to the check, on API level we should just put connect and connector as a part of the conext
|
@joshuaNathaniel thanks for contribution and sponsorship, if you are interested in moving this faster, please let us know. |
What changes did you make? (Give an overview)
Fixes #614
This PR implements connector-level permissions for Kafka Connect, addressing issue #614. The implementation adds granular permission control at the individual connector level while maintaining backwards compatibility with existing CONNECT-level permissions.
Key changes:
ActionDropdownItemWithFallbackcomponent to support hierarchical permission checking (tries connector-level first, falls back to connect-level)The permission hierarchy works as follows:
CONNECTORresource with valueconnect-name/connector-namefor specific connector accessCONNECTresource with valueconnect-namefor cluster-wide accessIs there anything you'd like reviewers to focus on?
Please review the permission fallback logic in
ActionDropdownItemWithFallback.tsxto ensure it properly handles the hierarchical permission model without violating React hooks rules.How Has This Been Tested? (put an "x" (case-sensitive!) next to an item)
Checklist (put an "x" (case-sensitive!) next to all the items, otherwise the build will fail)
Check out Contributing and Code of Conduct
A picture of a cute animal (not mandatory but encouraged)
🦫