Skip to content

refactor(helm): remove rbac.clusterScoped, derive RBAC scope from rbac.namespaces #1728

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.

Sign up for GitHub

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

Merged
EItanya merged 4 commits into from
Apr 22, 2026
Merged

refactor(helm): remove rbac.clusterScoped, derive RBAC scope from rbac.namespaces #1728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a96f458
update rbac namespace chart
supreme-gg-gg Apr 22, 2026
20dbb8e
better warnings for install ns
supreme-gg-gg Apr 22, 2026
b08e3a2
Update helm/kagent/templates/rbac/getter-role.yaml
supreme-gg-gg Apr 22, 2026
39e29a5
Apply suggestions from code review
supreme-gg-gg Apr 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ jobs:
env:
OPENAI_API_KEY: fake
KAGENT_HELM_EXTRA_ARGS: >-
--set rbac.clusterScoped=false
--set 'rbac.namespaces={kagent}'
run: |
# Upgrade helm to use namespace-scoped RBAC
Expand Down
25 changes: 18 additions & 7 deletions helm/kagent/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,28 @@ Allows overriding it for multi-namespace deployments in combined charts.

{{/*
Watch namespaces - transforms list of namespaces cached by the controller into comma-separated string.
Precedence: controller.watchNamespaces > rbac.namespaces (when clusterScoped=false) > empty (watch all).
Precedence: controller.watchNamespaces (explicit override) > rbac.namespaces > empty (watch all).
*/}}
{{- define "kagent.watchNamespaces" -}}
{{- if .Values.controller.watchNamespaces -}}
{{- .Values.controller.watchNamespaces | uniq | join "," -}}
{{- else if and .Values.rbac (not .Values.rbac.clusterScoped) -}}
{{- if .Values.rbac.namespaces -}}
{{- .Values.rbac.namespaces | uniq | join "," -}}
{{- else -}}
{{- .Release.Namespace -}}
{{- end -}}
{{- else if and .Values.rbac .Values.rbac.namespaces -}}
{{- .Values.rbac.namespaces | uniq | join "," -}}
{{- end -}}
{{- end -}}

{{/*
Guards on the rbac block
*/}}
{{- define "kagent.rbac.validate" -}}
{{- if and .Values.rbac (hasKey .Values.rbac "clusterScoped") -}}
{{- fail "rbac.clusterScoped has been removed. Leave rbac.namespaces empty for cluster-scoped RBAC, or set rbac.namespaces=[<ns>, ...] for namespaced RBAC." -}}
{{- end -}}
{{- if and .Values.rbac .Values.rbac.namespaces -}}
{{- $installNs := include "kagent.namespace" . -}}
{{- if not (has $installNs .Values.rbac.namespaces) -}}
{{- fail (printf "rbac.namespaces is set but does not include the install namespace %q" $installNs) -}}
{{- end -}}
{{- end -}}
{{- end -}}

Expand Down
24 changes: 12 additions & 12 deletions helm/kagent/templates/rbac/getter-role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,9 @@
- watch
{{- end -}}

{{- if .Values.rbac.clusterScoped }}
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: {{ include "kagent.fullname" . }}-getter-role
labels:
{{- include "kagent.labels" . | nindent 4 }}
rules:
{{- include "kagent.getter.rules" . | nindent 2 }}
{{- else }}
{{- $namespaces := .Values.rbac.namespaces | default (list (include "kagent.namespace" .)) }}
{{- range $namespace := $namespaces }}
{{- include "kagent.rbac.validate" . -}}
{{- if .Values.rbac.namespaces }}
{{- range $namespace := (.Values.rbac.namespaces | uniq | sortAlpha) }}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
Expand All @@ -115,4 +106,13 @@ metadata:
rules:
{{- include "kagent.getter.rules" $ | nindent 2 }}
{{- end }}
{{- else }}
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: {{ include "kagent.fullname" . }}-getter-role
labels:
{{- include "kagent.labels" . | nindent 4 }}
rules:
{{- include "kagent.getter.rules" . | nindent 2 }}
{{- end }}
36 changes: 18 additions & 18 deletions helm/kagent/templates/rbac/getter-rolebinding.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,6 @@
{{- if .Values.rbac.clusterScoped }}
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: {{ include "kagent.fullname" . }}-getter-rolebinding
labels:
{{- include "kagent.labels" . | nindent 4 }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: {{ include "kagent.fullname" . }}-getter-role
subjects:
- kind: ServiceAccount
name: {{ include "kagent.fullname" . }}-controller
namespace: {{ include "kagent.namespace" . }}
{{- else }}
{{- $namespaces := .Values.rbac.namespaces | default (list (include "kagent.namespace" .)) }}
{{- range $namespace := $namespaces }}
{{- include "kagent.rbac.validate" . -}}
{{- if .Values.rbac.namespaces }}
{{- range $namespace := .Values.rbac.namespaces | uniq | sortAlpha }}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
Expand All @@ -33,4 +18,19 @@ subjects:
name: {{ include "kagent.fullname" $ }}-controller
namespace: {{ include "kagent.namespace" $ }}
{{- end }}
{{- else }}
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: {{ include "kagent.fullname" . }}-getter-rolebinding
labels:
{{- include "kagent.labels" . | nindent 4 }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: {{ include "kagent.fullname" . }}-getter-role
subjects:
- kind: ServiceAccount
name: {{ include "kagent.fullname" . }}-controller
namespace: {{ include "kagent.namespace" . }}
{{- end }}
23 changes: 12 additions & 11 deletions helm/kagent/templates/rbac/writer-role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,9 @@
- delete
{{- end -}}

{{- if .Values.rbac.clusterScoped }}
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: {{ include "kagent.fullname" . }}-writer-role
labels:
{{- include "kagent.labels" . | nindent 4 }}
rules:
{{- include "kagent.writer.rules" . | nindent 2 }}
{{- else }}
{{- $namespaces := .Values.rbac.namespaces | default (list (include "kagent.namespace" .)) }}
{{- include "kagent.rbac.validate" . -}}
{{- if .Values.rbac.namespaces }}
{{- $namespaces := .Values.rbac.namespaces | uniq | sortAlpha }}
{{- range $namespace := $namespaces }}
---
apiVersion: rbac.authorization.k8s.io/v1
Expand All @@ -98,4 +90,13 @@ metadata:
rules:
{{- include "kagent.writer.rules" $ | nindent 2 }}
{{- end }}
{{- else }}
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: {{ include "kagent.fullname" . }}-writer-role
labels:
{{- include "kagent.labels" . | nindent 4 }}
rules:
{{- include "kagent.writer.rules" . | nindent 2 }}
{{- end }}
36 changes: 18 additions & 18 deletions helm/kagent/templates/rbac/writer-rolebinding.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,6 @@
{{- if .Values.rbac.clusterScoped }}
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: {{ include "kagent.fullname" . }}-writer-rolebinding
labels:
{{- include "kagent.labels" . | nindent 4 }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: {{ include "kagent.fullname" . }}-writer-role
subjects:
- kind: ServiceAccount
name: {{ include "kagent.fullname" . }}-controller
namespace: {{ include "kagent.namespace" . }}
{{- else }}
{{- $namespaces := .Values.rbac.namespaces | default (list (include "kagent.namespace" .)) }}
{{- range $namespace := $namespaces }}
{{- include "kagent.rbac.validate" . -}}
{{- if .Values.rbac.namespaces }}
{{- range $namespace := (.Values.rbac.namespaces | uniq) }}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
Expand All @@ -33,4 +18,19 @@ subjects:
name: {{ include "kagent.fullname" $ }}-controller
namespace: {{ include "kagent.namespace" $ }}
{{- end }}
{{- else }}
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: {{ include "kagent.fullname" . }}-writer-rolebinding
labels:
{{- include "kagent.labels" . | nindent 4 }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: {{ include "kagent.fullname" . }}-writer-role
subjects:
- kind: ServiceAccount
name: {{ include "kagent.fullname" . }}-controller
namespace: {{ include "kagent.namespace" . }}
{{- end }}
26 changes: 26 additions & 0 deletions helm/kagent/tests/controller-deployment_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,32 @@ tests:
path: data.WATCH_NAMESPACES
value: "namespace-1,namespace-2"

- it: should derive watch namespaces from rbac.namespaces when controller.watchNamespaces is unset
template: controller-configmap.yaml
set:
rbac:
namespaces:
- NAMESPACE
- ns2
asserts:
- equal:
path: data.WATCH_NAMESPACES
value: "NAMESPACE,ns2"

- it: controller.watchNamespaces should override rbac.namespaces for WATCH_NAMESPACES
template: controller-configmap.yaml
set:
controller:
watchNamespaces:
- explicit-ns
rbac:
namespaces:
- NAMESPACE
asserts:
- equal:
path: data.WATCH_NAMESPACES
value: "explicit-ns"

- it: should set podAnnotations
template: controller-deployment.yaml
set:
Expand Down
75 changes: 66 additions & 9 deletions helm/kagent/tests/rbac_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,10 @@ tests:
path: metadata.labels["app.kubernetes.io/managed-by"]
value: Helm

- it: should render Roles when clusterScoped is false
- it: should render Roles when rbac.namespaces is set
set:
rbac.clusterScoped: false
rbac.namespaces:
- NAMESPACE
asserts:
- isKind:
of: Role
Expand All @@ -151,9 +152,10 @@ tests:
of: Role
template: rbac/writer-role.yaml

- it: should render RoleBindings when clusterScoped is false
- it: should render RoleBindings when rbac.namespaces is set
set:
rbac.clusterScoped: false
rbac.namespaces:
- NAMESPACE
asserts:
- isKind:
of: RoleBinding
Expand All @@ -162,19 +164,38 @@ tests:
of: RoleBinding
template: rbac/writer-rolebinding.yaml

- it: should render multiple roles and bindings when namespaces are specified and clusterScoped is false
- it: should render a single role/binding in the listed namespace only (no release-ns fallback)
set:
rbac.namespaces:
- NAMESPACE
asserts:
- hasDocuments:
count: 1
template: rbac/getter-role.yaml
- equal:
path: metadata.namespace
value: NAMESPACE
template: rbac/getter-role.yaml
- hasDocuments:
count: 1
template: rbac/writer-rolebinding.yaml
- equal:
path: metadata.namespace
value: NAMESPACE
template: rbac/writer-rolebinding.yaml

- it: should render multiple roles and bindings when namespaces are specified
set:
rbac.clusterScoped: false
rbac.namespaces:
- ns1
- NAMESPACE
- ns2
asserts:
- hasDocuments:
count: 2
template: rbac/getter-role.yaml
- equal:
path: metadata.namespace
value: ns1
value: NAMESPACE
template: rbac/getter-role.yaml
documentIndex: 0
- equal:
Expand All @@ -187,11 +208,47 @@ tests:
template: rbac/getter-rolebinding.yaml
- equal:
path: metadata.namespace
value: ns1
value: NAMESPACE
template: rbac/getter-rolebinding.yaml
documentIndex: 0
- equal:
path: metadata.namespace
value: ns2
template: rbac/getter-rolebinding.yaml
documentIndex: 1

- it: should fail rendering if the removed rbac.clusterScoped field is set
set:
rbac.clusterScoped: false
template: rbac/writer-rolebinding.yaml
asserts:
- failedTemplate:
errorMessage: "rbac.clusterScoped has been removed. Leave rbac.namespaces empty for cluster-scoped RBAC, or set rbac.namespaces=[<ns>, ...] for namespaced RBAC."

- it: should fail rendering if rbac.namespaces is set but does not include the install namespace
set:
rbac.namespaces:
- some-other-ns
template: rbac/writer-rolebinding.yaml
asserts:
- failedTemplate:
errorMessage: "rbac.namespaces is set but does not include the install namespace \"NAMESPACE\""

- it: should accept a custom install namespace when listed in rbac.namespaces
set:
namespaceOverride: my-ns
rbac.namespaces:
- my-ns
- other-ns
template: rbac/getter-role.yaml
asserts:
- hasDocuments:
count: 2
- equal:
path: metadata.namespace
value: my-ns
documentIndex: 0
- equal:
path: metadata.namespace
value: other-ns
documentIndex: 1
11 changes: 6 additions & 5 deletions helm/kagent/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,12 @@ database:
# ==============================================================================

rbac:
# -- If true, creates ClusterRole and ClusterRoleBinding resources.
# If false, creates Role and RoleBinding resources instead.
clusterScoped: true
# -- When clusterScoped is false, specify additional namespaces to create Roles and RoleBindings in.
# If empty, defaults to the release namespace.
# -- Namespaces in which to create Role and RoleBinding resources.
# If empty (default), the chart creates cluster-scoped ClusterRole and ClusterRoleBinding
# resources and the controller watches all namespaces.
# If set, the chart creates a Role + RoleBinding per listed namespace and the controller's
# WATCH_NAMESPACES is derived from this list (unless controller.watchNamespaces is set
# explicitly, which always takes precedence).
namespaces: []

# ==============================================================================
Expand Down
Loading