Skip to content

Fix(helm): align ClusterRole with controller-gen and add AuthBridge SCC - #456

Merged
pdettori merged 1 commit into
rossoctl:mainfrom
Bobbins228:fix/helm-rbac-authbridge-scc
Jun 23, 2026
Merged

Fix(helm): align ClusterRole with controller-gen and add AuthBridge SCC#456
pdettori merged 1 commit into
rossoctl:mainfrom
Bobbins228:fix/helm-rbac-authbridge-scc

Conversation

@Bobbins228

Copy link
Copy Markdown
Contributor

Summary

The Helm chart's ClusterRole has drifted from the controller-gen output (config/rbac/role.yaml), granting excess permissions. The RBAC markers in the Go source (+kubebuilder:rbac) are the single source of truth — make manifests generates config/rbac/role.yaml from them, but the Helm chart's copy is manually maintained with no sync step (unlike CRDs which have sync-chart-crds). This PR realigns them.

Additionally, the AuthBridge SecurityContextConstraints — required for OpenShift deployments — exists in the kustomize path (config/security/kagenti-authbridge-scc.yaml) but is missing from the Helm chart.

Changes

ClusterRole alignment (role.yaml)

Rewrite the Helm chart ClusterRole to exactly match config/rbac/role.yaml (verified via make manifests + diff). Specific fixes:

  • Remove unused endpoints rule — no +kubebuilder:rbac marker exists for core endpoints in the codebase
  • Remove update from namespaces — controller-gen only grants create/get/list/patch/watch
  • Remove patch/watch from services — controller-gen only grants create/get/list/update/watch
  • Remove list from operator.openshift.io/networks — controller-gen only grants get
  • Remove delete from roles — only rolebindings needs delete
  • Merge configmaps + secrets into one rule (same verbs, matches controller-gen)
  • Merge serviceaccounts + services into one rule (same verbs, matches controller-gen)
  • Reorder rules to match controller-gen alphabetical output

Also regenerates config/rbac/role.yaml via make manifests to pick up controller-gen's latest normalisation (no permission changes, just grouping/ordering).

AuthBridge SCC template (authbridge-scc.yaml) — new file

Adds three resources for OpenShift, mirroring config/security/kagenti-authbridge-scc.yaml:

  1. SecurityContextConstraints (kagenti-authbridge) — least-privilege SCC allowing CSI volumes for SPIRE workload identity
  2. ClusterRole (system:openshift:scc:kagenti-authbridge) — grants use verb scoped to resourceNames: ["kagenti-authbridge"]
  3. ClusterRoleBinding — binds the operator ServiceAccount to the ClusterRole

All three are guarded by {{- if and .Values.rbac.enable (.Capabilities.APIVersions.Has "security.openshift.io/v1") }} so they only render on OpenShift.

Test Plan

  • helm lint charts/kagenti-operator/ passes
  • helm template without --api-versions security.openshift.io/v1 does not render authbridge-scc.yaml
  • helm template --api-versions security.openshift.io/v1 renders SCC, ClusterRole, and ClusterRoleBinding
  • diff <(sed -n '/^rules:/,$p' config/rbac/role.yaml) <(sed -n '/^rules:/,$p' charts/.../role.yaml) produces no output
  • Existing Helm-based deployments continue working (permissions only removed, not added)

@Bobbins228
Bobbins228 requested a review from a team as a code owner June 22, 2026 10:31
@Bobbins228 Bobbins228 changed the title fix(helm): align ClusterRole with controller-gen and add AuthBridge SCC Fix(helm): align ClusterRole with controller-gen and add AuthBridge SCC Jun 22, 2026
@Bobbins228
Bobbins228 force-pushed the fix/helm-rbac-authbridge-scc branch 2 times, most recently from f5b4d19 to 169bb93 Compare June 22, 2026 11:12

@ChristianZaccaria ChristianZaccaria left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great work Mark! - I left one requested change. After that, lgtm!

Comment thread charts/kagenti-operator/templates/rbac/role.yaml

@pdettori pdettori left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the RBAC alignment and AuthBridge SCC addition. The ClusterRole changes correctly match controller-gen output, the SCC template is well-structured (proper capability guard, minimal privileges, operator SA delegation pattern), and CI is all green.

The only blocking issue (missing endpoints rule) has already been identified by @ChristianZaccaria — the controller performs a Get on corev1.Endpoints at agentcard_networkpolicy_controller.go:346 but lacks a corresponding +kubebuilder:rbac marker, so controller-gen legitimately excludes it. The fix should add the marker and re-run make manifests to keep the Helm chart and kustomize output in sync.

Sync the Helm chart ClusterRole to exactly match controller-gen output.
Remove unused endpoints rule, excess verbs on namespaces/services/
networks/roles, and merge configmaps+secrets and serviceaccounts+
services to match kustomize grouping.

Add AuthBridge SCC template for OpenShift (guarded by Capabilities
check), mirroring config/security/kagenti-authbridge-scc.yaml.

Signed-off-by: Bobbins228 <mcampbel@redhat.com>
@Bobbins228
Bobbins228 force-pushed the fix/helm-rbac-authbridge-scc branch from 169bb93 to 8f2664c Compare June 23, 2026 08:18
@Bobbins228

Copy link
Copy Markdown
Contributor Author

@pdettori, @ChristianZaccaria updated the PR thanks for the reviews

@ChristianZaccaria ChristianZaccaria left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/lgtm /approve

@r3v5

r3v5 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Related to #460 right?

@r3v5 r3v5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@pdettori pdettori left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@pdettori
pdettori merged commit f5e4826 into rossoctl:main Jun 23, 2026
16 checks passed
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.

4 participants