From a96f4583e2ea0dafbbda10a71ffded4c32ca55ec Mon Sep 17 00:00:00 2001 From: Jet Chiang Date: Wed, 22 Apr 2026 11:13:41 -0400 Subject: [PATCH 1/4] update rbac namespace chart Signed-off-by: Jet Chiang --- .github/workflows/ci.yaml | 1 - helm/kagent/templates/_helpers.tpl | 25 +++++-- helm/kagent/templates/rbac/getter-role.yaml | 24 +++--- .../templates/rbac/getter-rolebinding.yaml | 36 ++++----- helm/kagent/templates/rbac/writer-role.yaml | 24 +++--- .../templates/rbac/writer-rolebinding.yaml | 36 ++++----- .../tests/controller-deployment_test.yaml | 26 +++++++ helm/kagent/tests/rbac_test.yaml | 75 ++++++++++++++++--- helm/kagent/values.yaml | 11 +-- 9 files changed, 176 insertions(+), 82 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 80a1bb944..7a130859d 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -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 diff --git a/helm/kagent/templates/_helpers.tpl b/helm/kagent/templates/_helpers.tpl index eca5912f6..c7d613be2 100644 --- a/helm/kagent/templates/_helpers.tpl +++ b/helm/kagent/templates/_helpers.tpl @@ -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=[, ...] for namespaced RBAC." -}} +{{- end -}} +{{- if and .Values.rbac .Values.rbac.namespaces -}} +{{- $installNs := include "kagent.namespace" . -}} +{{- if not (has $installNs .Values.rbac.namespaces) -}} +{{- fail "rbac.namespaces is set but does not include the install namespace" -}} +{{- end -}} {{- end -}} {{- end -}} diff --git a/helm/kagent/templates/rbac/getter-role.yaml b/helm/kagent/templates/rbac/getter-role.yaml index b5e317017..039c5737e 100644 --- a/helm/kagent/templates/rbac/getter-role.yaml +++ b/helm/kagent/templates/rbac/getter-role.yaml @@ -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 }} --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -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 }} diff --git a/helm/kagent/templates/rbac/getter-rolebinding.yaml b/helm/kagent/templates/rbac/getter-rolebinding.yaml index dfc3547a0..e190b9278 100644 --- a/helm/kagent/templates/rbac/getter-rolebinding.yaml +++ b/helm/kagent/templates/rbac/getter-rolebinding.yaml @@ -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 }} --- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding @@ -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 }} diff --git a/helm/kagent/templates/rbac/writer-role.yaml b/helm/kagent/templates/rbac/writer-role.yaml index 941334e31..6ba18864d 100644 --- a/helm/kagent/templates/rbac/writer-role.yaml +++ b/helm/kagent/templates/rbac/writer-role.yaml @@ -75,18 +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" .)) }} -{{- range $namespace := $namespaces }} +{{- include "kagent.rbac.validate" . -}} +{{- if .Values.rbac.namespaces }} +{{- range $namespace := .Values.rbac.namespaces }} --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -98,4 +89,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 }} \ No newline at end of file diff --git a/helm/kagent/templates/rbac/writer-rolebinding.yaml b/helm/kagent/templates/rbac/writer-rolebinding.yaml index d395dc543..5285c2204 100644 --- a/helm/kagent/templates/rbac/writer-rolebinding.yaml +++ b/helm/kagent/templates/rbac/writer-rolebinding.yaml @@ -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 }} --- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding @@ -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 }} diff --git a/helm/kagent/tests/controller-deployment_test.yaml b/helm/kagent/tests/controller-deployment_test.yaml index 649035d5d..a35a9c227 100644 --- a/helm/kagent/tests/controller-deployment_test.yaml +++ b/helm/kagent/tests/controller-deployment_test.yaml @@ -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: diff --git a/helm/kagent/tests/rbac_test.yaml b/helm/kagent/tests/rbac_test.yaml index 0a02fa145..305eb1a31 100644 --- a/helm/kagent/tests/rbac_test.yaml +++ b/helm/kagent/tests/rbac_test.yaml @@ -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 @@ -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 @@ -162,11 +164,30 @@ 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: @@ -174,7 +195,7 @@ tests: template: rbac/getter-role.yaml - equal: path: metadata.namespace - value: ns1 + value: NAMESPACE template: rbac/getter-role.yaml documentIndex: 0 - equal: @@ -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=[, ...] 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\". Add \"NAMESPACE\" to rbac.namespaces explicitly (kagent's controller, bundled DB, and tools live in the install namespace and require RBAC there). If you intentionally want to exclude it, please open an issue so we can relax this validation." + + - 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 \ No newline at end of file diff --git a/helm/kagent/values.yaml b/helm/kagent/values.yaml index 9a89944e9..5563d1909 100644 --- a/helm/kagent/values.yaml +++ b/helm/kagent/values.yaml @@ -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: [] # ============================================================================== From 20dbb8e07b01a5f40cc942f27de9a577b6732a01 Mon Sep 17 00:00:00 2001 From: Jet Chiang Date: Wed, 22 Apr 2026 11:16:30 -0400 Subject: [PATCH 2/4] better warnings for install ns Signed-off-by: Jet Chiang --- helm/kagent/templates/_helpers.tpl | 2 +- helm/kagent/tests/rbac_test.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/helm/kagent/templates/_helpers.tpl b/helm/kagent/templates/_helpers.tpl index c7d613be2..2c517ba21 100644 --- a/helm/kagent/templates/_helpers.tpl +++ b/helm/kagent/templates/_helpers.tpl @@ -72,7 +72,7 @@ Guards on the rbac block {{- if and .Values.rbac .Values.rbac.namespaces -}} {{- $installNs := include "kagent.namespace" . -}} {{- if not (has $installNs .Values.rbac.namespaces) -}} -{{- fail "rbac.namespaces is set but does not include the install namespace" -}} +{{- fail (printf "rbac.namespaces is set but does not include the install namespace %q" $installNs) -}} {{- end -}} {{- end -}} {{- end -}} diff --git a/helm/kagent/tests/rbac_test.yaml b/helm/kagent/tests/rbac_test.yaml index 305eb1a31..719eb6c10 100644 --- a/helm/kagent/tests/rbac_test.yaml +++ b/helm/kagent/tests/rbac_test.yaml @@ -232,7 +232,7 @@ tests: template: rbac/writer-rolebinding.yaml asserts: - failedTemplate: - errorMessage: "rbac.namespaces is set but does not include the install namespace \"NAMESPACE\". Add \"NAMESPACE\" to rbac.namespaces explicitly (kagent's controller, bundled DB, and tools live in the install namespace and require RBAC there). If you intentionally want to exclude it, please open an issue so we can relax this validation." + 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: From b08e3a272730c2c7a12b6b03bb9cba0547f72c76 Mon Sep 17 00:00:00 2001 From: Jet Chiang Date: Wed, 22 Apr 2026 12:26:38 -0400 Subject: [PATCH 3/4] Update helm/kagent/templates/rbac/getter-role.yaml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Jet Chiang --- helm/kagent/templates/rbac/getter-role.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/kagent/templates/rbac/getter-role.yaml b/helm/kagent/templates/rbac/getter-role.yaml index 039c5737e..9e97f5122 100644 --- a/helm/kagent/templates/rbac/getter-role.yaml +++ b/helm/kagent/templates/rbac/getter-role.yaml @@ -94,7 +94,7 @@ {{- include "kagent.rbac.validate" . -}} {{- if .Values.rbac.namespaces }} -{{- range $namespace := .Values.rbac.namespaces }} +{{- range $namespace := (.Values.rbac.namespaces | uniq | sortAlpha) }} --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role From 39e29a55c9e9fadf53c28a43b2c96b6b4915bac4 Mon Sep 17 00:00:00 2001 From: Jet Chiang Date: Wed, 22 Apr 2026 12:27:36 -0400 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Jet Chiang --- helm/kagent/templates/rbac/getter-rolebinding.yaml | 2 +- helm/kagent/templates/rbac/writer-role.yaml | 3 ++- helm/kagent/templates/rbac/writer-rolebinding.yaml | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/helm/kagent/templates/rbac/getter-rolebinding.yaml b/helm/kagent/templates/rbac/getter-rolebinding.yaml index e190b9278..9d143f72d 100644 --- a/helm/kagent/templates/rbac/getter-rolebinding.yaml +++ b/helm/kagent/templates/rbac/getter-rolebinding.yaml @@ -1,6 +1,6 @@ {{- include "kagent.rbac.validate" . -}} {{- if .Values.rbac.namespaces }} -{{- range $namespace := .Values.rbac.namespaces }} +{{- range $namespace := .Values.rbac.namespaces | uniq | sortAlpha }} --- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding diff --git a/helm/kagent/templates/rbac/writer-role.yaml b/helm/kagent/templates/rbac/writer-role.yaml index 6ba18864d..3e020b56a 100644 --- a/helm/kagent/templates/rbac/writer-role.yaml +++ b/helm/kagent/templates/rbac/writer-role.yaml @@ -77,7 +77,8 @@ {{- include "kagent.rbac.validate" . -}} {{- if .Values.rbac.namespaces }} -{{- range $namespace := .Values.rbac.namespaces }} +{{- $namespaces := .Values.rbac.namespaces | uniq | sortAlpha }} +{{- range $namespace := $namespaces }} --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role diff --git a/helm/kagent/templates/rbac/writer-rolebinding.yaml b/helm/kagent/templates/rbac/writer-rolebinding.yaml index 5285c2204..d2120b35e 100644 --- a/helm/kagent/templates/rbac/writer-rolebinding.yaml +++ b/helm/kagent/templates/rbac/writer-rolebinding.yaml @@ -1,6 +1,6 @@ {{- include "kagent.rbac.validate" . -}} {{- if .Values.rbac.namespaces }} -{{- range $namespace := .Values.rbac.namespaces }} +{{- range $namespace := (.Values.rbac.namespaces | uniq) }} --- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding