Skip to content

fix(rbac): align Helm ClusterRole with controller-gen output#249

Open
ChristianZaccaria wants to merge 1 commit intokagenti:mainfrom
ChristianZaccaria:role-alignment
Open

fix(rbac): align Helm ClusterRole with controller-gen output#249
ChristianZaccaria wants to merge 1 commit intokagenti:mainfrom
ChristianZaccaria:role-alignment

Conversation

@ChristianZaccaria
Copy link
Copy Markdown

Summary

  • Helm ClusterRole had significant drift from the controller-gen-generated config/rbac/role.yaml, granting permissions the operator code never uses
  • Removes over-provisioned rules (secrets, CRDs, webhook configs, RBAC management, ingresses, deprecated extensions API group)
  • Adds missing agentruntimes resources that exist in kubebuilder markers but were absent from the Helm chart

Key changes

charts/kagenti-operator/templates/rbac/role.yaml:

Removed (confirmed zero runtime usage — not in any // +kubebuilder:rbac marker):

  • core: endpoints, namespaces, persistentvolumeclaims, pods/log, secrets, serviceaccounts with full CRUD
  • admissionregistration.k8s.io: mutating/validating webhook configs (operator is the webhook server, not the webhook config manager)
  • apiextensions.k8s.io: CRDs
  • apps: daemonsets, replicasets, create/delete on deployments/statefulsets, finalizers subresource
  • extensions: deprecated API group (removed since k8s 1.16)
  • networking.k8s.io: ingresses
  • rbac.authorization.k8s.io: roles and bindings

Added (present in kubebuilder markers, missing from Helm):

  • agent.kagenti.dev: agentruntimes, agentruntimes/finalizers, agentruntimes/status

Runtime impact

The Helm ClusterRole now matches config/rbac/role.yaml 1:1 on all RBAC rules — both deployment methods (helm install and make deploy) grant identical permissions. No behavioural change: all removed permissions were unused by the operator code.

The Helm chart's ClusterRole had significant drift from the kubebuilder
marker-generated config/rbac/role.yaml, granting permissions the operator
code never uses (secrets, CRDs, webhook configs, RBAC management, ingresses,
deprecated extensions API group) while missing agentruntimes resources
entirely. Rules are now a 1:1 match with the controller-gen output,
following least-privilege principle.

Removed (not in kubebuilder markers):
- core: endpoints, namespaces, PVCs, pods/log, secrets, serviceaccounts
- admissionregistration.k8s.io: mutating/validating webhooks
- apiextensions.k8s.io: customresourcedefinitions
- apps: daemonsets, replicasets, create/delete on deployments/statefulsets
- extensions: deprecated API group (removed since k8s 1.16)
- networking.k8s.io: ingresses
- rbac.authorization.k8s.io: roles and bindings

Added (present in markers, missing from Helm):
- agent.kagenti.dev: agentruntimes, agentruntimes/finalizers, agentruntimes/status

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: ChristianZaccaria <christian.zaccaria.cz@gmail.com>
@ChristianZaccaria ChristianZaccaria requested a review from a team as a code owner March 27, 2026 14:40
Copy link
Copy Markdown
Collaborator

@cwiklik cwiklik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Clean RBAC cleanup aligning the Helm ClusterRole with the controller-gen output (config/rbac/role.yaml). Removes significant over-provisioning: secrets, CRDs, webhooks, RBAC management, ingresses, deprecated extensions API group, and unused verbs. Adds missing agentruntimes resources. Verified: resulting rules match config/rbac/role.yaml on main 1:1.

Merge coordination note: PR #247 (Keycloak client registration) adds secrets RBAC to config/rbac/role.yaml. If #247 merges first, this Helm chart will need to pick up the new secrets rule — rebase after #247 lands.

Areas reviewed: Helm/K8s RBAC
Commits: 1 commit, signed-off: yes, proper Assisted-By trailer
CI status: DCO passing

- pods/log
- secrets
- serviceaccounts
- services
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Excellent RBAC hygiene — removing 79 lines of over-provisioned permissions (secrets, CRDs, webhooks, RBAC management, ingresses, deprecated extensions API group) while adding the missing agentruntimes resources. Verified against config/rbac/role.yaml on main: 1:1 match.

- watch
- apiGroups:
- apps
- agent.kagenti.dev
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (merge coordination): PR #247 (Keycloak client registration) adds a secrets rule to config/rbac/role.yaml. If #247 merges before this PR, you'll need to rebase and add the secrets rule here too — otherwise the Helm chart will drift again on that one resource.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads-up on this. I guess this could be addressed as a follow-up PR since it's still inconclusive which roles will remain in PR #247 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants