Skip to content

feat(helm): scope ClusterRole/ClusterRoleBinding names by release namespace - #2939

Open
bsquizz wants to merge 2 commits into
NVIDIA:mainfrom
bsquizz:feat/rbac-cluster-role-toggle
Open

feat(helm): scope ClusterRole/ClusterRoleBinding names by release namespace#2939
bsquizz wants to merge 2 commits into
NVIDIA:mainfrom
bsquizz:feat/rbac-cluster-role-toggle

Conversation

@bsquizz

@bsquizz bsquizz commented Aug 26, 2026

Copy link
Copy Markdown

Summary

  • Appends .Release.Namespace to the ClusterRole and ClusterRoleBinding metadata.name fields in clusterrole.yaml and clusterrolebinding.yaml
  • Each Helm release now gets its own cluster-scoped resources (e.g. openshell-gateway-node-reader-ns-a, openshell-gateway-node-reader-ns-b), eliminating ownership annotation conflicts in multi-tenant deployments
  • No new values or toggles -- the chart unconditionally creates the resources, just with unique names

Motivation

The chart creates a ClusterRole and ClusterRoleBinding with a fixed name (openshell-gateway-node-reader). Helm tracks ownership via meta.helm.sh/release-namespace annotations. When two releases coexist on the same cluster (e.g. HyperShell deploying multiple gateways), the second install fails:

ClusterRole "openshell-gateway-node-reader" exists and cannot be imported:
key "meta.helm.sh/release-namespace" must equal "ns-a": current value is "ns-b"

Scoping the names by namespace makes each release's cluster-scoped resources independent. The rules are identical and small, so the duplication is harmless.

Test plan

  • helm template -n ns-a renders openshell-gateway-node-reader-ns-a for both ClusterRole and ClusterRoleBinding
  • helm template -n ns-b renders openshell-gateway-node-reader-ns-b
  • Two releases on the same cluster install without conflict
  • Existing single-release deployments: helm upgrade creates the new named resources; the old fixed-name resources can be cleaned up manually or left as orphans

🤖 Generated with Claude Code

@copy-pr-bot

copy-pr-bot Bot commented Aug 26, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@bsquizz
bsquizz force-pushed the feat/rbac-cluster-role-toggle branch from 38b9064 to 2bd01b8 Compare August 26, 2026 01:37
@bsquizz bsquizz changed the title feat(helm): add rbac.clusterRole.create toggle for multi-tenant deployments feat(helm): scope ClusterRole/ClusterRoleBinding names by release namespace Aug 26, 2026
@jhjaggars

Copy link
Copy Markdown
Contributor

Thanks for addressing the cluster-scoped RBAC ownership conflict. I manually rendered the chart for both namespaces and confirmed the names remain internally consistent:

  • ns-a: openshell-gateway-node-reader-ns-a
  • ns-b: openshell-gateway-node-reader-ns-b
  • helm lint passes with agentSandbox.preflight.enabled=false

A few items to address before merge:

  1. Add regression coverage. deploy/helm/openshell/tests/clusterrole_test.yaml does not assert the generated ClusterRole name, and it does not render clusterrolebinding.yaml. Please add tests covering the namespace-qualified ClusterRole, ClusterRoleBinding, and roleRef names so the multi-namespace installation failure cannot be silently reintroduced.

  2. Rewrite the commit metadata. Commit 2bd01b8 includes Co-Authored-By: Claude Opus 4.6. OpenShell's contribution policy requires commit messages not to mention or attribute AI agents.

  3. Complete the project workflow checks. This is a user-visible Helm deployment change, but the PR does not link an accepted issue. The sync-agent-infra maintenance map also calls for reviewing debug-openshell-cluster and helm-dev-environment when Helm deployment behavior changes; please update them where needed or note why no companion change is required.

The template changes themselves look correct: the binding name and roleRef were updated together, while the subject remains scoped to .Release.Namespace.

…espace

The chart creates cluster-scoped ClusterRole and ClusterRoleBinding
resources with a fixed name derived from the release name. When
multiple Helm releases coexist on the same cluster (multi-tenant),
only one release can own these resources due to Helm ownership
annotations -- the second install fails with a conflict.

Append .Release.Namespace to the ClusterRole and ClusterRoleBinding
names so each release gets its own cluster-scoped resources. The
duplication is harmless (the rules are identical and small) and
eliminates multi-tenant conflicts entirely without requiring external
RBAC management.

Signed-off-by: Brandon Squizzato <bsquizza@redhat.com>
Assert the generated ClusterRole name, ClusterRoleBinding name, and
roleRef all include the release namespace suffix so multi-namespace
installations cannot silently regress to conflicting fixed names.

Signed-off-by: Brandon Squizzato <bsquizza@redhat.com>
@bsquizz
bsquizz force-pushed the feat/rbac-cluster-role-toggle branch from 2bd01b8 to 4edb5cd Compare August 26, 2026 21:11
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