Refactor tenant RBAC rules #534
Conversation
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
dosubot
bot
added
the
size:L
WalkthroughThis pull request introduces version updates and significant RBAC (Role-Based Access Control) modifications for the tenant application. The changes include updating the chart version from Changes
Sequence DiagramsequenceDiagram
participant User
participant K8s as Kubernetes Cluster
participant RBAC as Role-Based Access Control
User->>RBAC: Request Access
RBAC->>RBAC: Evaluate Roles
alt View Permissions
RBAC-->>User: Grant Read-Only Access
else Use Permissions
RBAC-->>User: Grant Limited Interaction
else Admin Permissions
RBAC-->>User: Grant Modify/Delete Rights
else Super Admin Permissions
RBAC-->>User: Grant Full Access
end
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/apps/tenant/templates/tenant.yaml (1)
Line range hint
17-66: Consider restricting default role permissionsThe default role has broad permissions with
"*"resource access. Consider:
- Explicitly listing required resources instead of using wildcard
- Adding explicit deny rules for sensitive operations
rules: - - apiGroups: [""] - resources: ["*"] + - apiGroups: [""] + resources: ["pods", "services", "configmaps", "secrets"] verbs: ["get", "list", "watch", "create", "update", "patch"]
🧹 Nitpick comments (1)
packages/apps/tenant/templates/tenant.yaml (1)
Line range hint
216-310: Consider adding security controls for admin roleWhile the permissions are appropriate, consider enhancing security with:
- Rate limiting annotations to prevent abuse
- Audit logging configuration for tracking administrative actions
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/apps/tenant/Chart.yaml(1 hunks)packages/apps/tenant/templates/tenant.yaml(7 hunks)packages/apps/versions_map(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/apps/tenant/Chart.yaml
🔇 Additional comments (5)
packages/apps/versions_map (1)
94-95: LGTM: Version updates are properly structured
The version progression from 1.6.2 to 1.6.3 is correctly implemented with appropriate commit references.
packages/apps/tenant/templates/tenant.yaml (4)
Line range hint 67-139: LGTM: View role follows principle of least privilege
The view role is well-structured with appropriate read-only permissions.
Line range hint 140-215: LGTM: Use role properly extends view permissions
The role correctly adds VM console/VNC access while maintaining read-only restrictions on other resources.
381-414: LGTM: Dashboard role properly restricts access
The role correctly provides limited access to Helm resources in the cozy-public namespace with appropriate group bindings.
Line range hint 311-380: Verify super-admin role usage and monitoring
The role has extensive permissions as expected for a super-admin. Ensure:
- Access to this role is strictly controlled
- Actions are monitored and logged
✅ Verification successful
Let's search for files containing super-admin references and analyze their content to verify the role configuration and its bindings.
Based on the search results, I can see that the super-admin role is well-defined in packages/apps/tenant/templates/tenant.yaml with appropriate RBAC controls. The role bindings show that access is restricted to specific groups and follows a hierarchical tenant structure. The role is used in other critical components like cluster management and CSI drivers, but always with proper service account configurations.
Super-admin role configuration is properly secured
The implementation follows Kubernetes RBAC best practices:
- Role bindings are namespace-scoped
- Group-based access control is implemented
- Hierarchical tenant structure is enforced through conditional bindings
- Service accounts using this role (e.g., in CSI, cluster-autoscaler) have limited scope
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any existing RoleBindings to super-admin role
rg -A 5 "super-admin" | grep -B 2 -A 2 "RoleBinding"
Length of output: 50
Script:
#!/bin/bash
# Search for super-admin role definitions and bindings with more context
rg -l "super-admin"
# Then get detailed content of files containing super-admin
fd . -t f -x sh -c 'echo "=== {} ===" && cat {} | grep -A 10 -B 10 "super-admin"' \;
Length of output: 65736
Signed-off-by: Andrei Kvapil kvapss@gmail.com
Summary by CodeRabbit
New Features
Bug Fixes
Chores