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

Replicate of https://github.com/kagent-dev/kagent/pull/1728/changes to
kmcp charts

Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>

Author: supreme-gg-gg

helm/kmcp/templates/_helpers.tpl

@@ -69,6 +69,21 @@ Create the image reference
 {{- printf "%s:%s" .Values.image.repository $tag }}
 {{- end }}
 
+{{/*
+Guards on the rbac block
+*/}}
+{{- define "kmcp.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 "kmcp.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 -}}
+
 {{/*
 Create controller manager container args
 */}}
@@ -83,8 +98,8 @@ Create controller manager container args
 {{- if .Values.controller.metrics.enabled }}
 {{- $args = append $args (printf "--metrics-bind-address=%s" .Values.controller.metrics.bindAddress) }}
 {{- end }}
-{{- if not .Values.rbac.clusterScoped }}
-{{- $namespaces := .Values.rbac.namespaces | default (list (include "kmcp.namespace" .)) }}
+{{- if and .Values.rbac .Values.rbac.namespaces }}
+{{- $namespaces := .Values.rbac.namespaces | uniq }}
 {{- $args = append $args (printf "--watch-namespaces=%s" (join "," $namespaces)) }}
 {{- end }}
 {{- toYaml $args }}

helm/kmcp/templates/rbac/clusterrole.yaml

@@ -54,19 +54,9 @@
 {{- end -}}
 
 {{- if .Values.rbac.create }}
-
-{{- if .Values.rbac.clusterScoped }}
-apiVersion: rbac.authorization.k8s.io/v1
-kind: ClusterRole
-metadata:
-  name: {{ include "kmcp.fullname" . }}-manager-role
-  labels:
-    {{- include "kmcp.labels" . | nindent 4 }}
-rules:
-  {{- include "kmcp.manager.rules" . | nindent 2 }}
-{{- else }}
-{{- $namespaces := .Values.rbac.namespaces | default (list (include "kmcp.namespace" .)) }}
-{{- range $namespace := $namespaces }}
+{{- include "kmcp.rbac.validate" . -}}
+{{- if .Values.rbac.namespaces }}
+{{- range $namespace := (.Values.rbac.namespaces | uniq | sortAlpha) }}
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
@@ -78,5 +68,14 @@ metadata:
 rules:
   {{- include "kmcp.manager.rules" $ | nindent 2 }}
 {{- end }}
+{{- else }}
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: {{ include "kmcp.fullname" . }}-manager-role
+  labels:
+    {{- include "kmcp.labels" . | nindent 4 }}
+rules:
+  {{- include "kmcp.manager.rules" . | nindent 2 }}
 {{- end }}
 {{- end }} 

helm/kmcp/templates/rbac/clusterrolebinding.yaml

@@ -1,22 +1,7 @@
 {{- if .Values.rbac.create }}
-{{- if .Values.rbac.clusterScoped }}
-apiVersion: rbac.authorization.k8s.io/v1
-kind: ClusterRoleBinding
-metadata:
-  name: {{ include "kmcp.fullname" . }}-manager-rolebinding
-  labels:
-    {{- include "kmcp.labels" . | nindent 4 }}
-roleRef:
-  apiGroup: rbac.authorization.k8s.io
-  kind: ClusterRole
-  name: {{ include "kmcp.fullname" . }}-manager-role
-subjects:
-- kind: ServiceAccount
-  name: {{ include "kmcp.serviceAccountName" . }}
-  namespace: {{ include "kmcp.namespace" . }}
-{{- else }}
-{{- $namespaces := .Values.rbac.namespaces | default (list (include "kmcp.namespace" .)) }}
-{{- range $namespace := $namespaces }}
+{{- include "kmcp.rbac.validate" . -}}
+{{- if .Values.rbac.namespaces }}
+{{- range $namespace := (.Values.rbac.namespaces | uniq | sortAlpha) }}
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: RoleBinding
@@ -34,5 +19,20 @@ subjects:
   name: {{ include "kmcp.serviceAccountName" $ }}
   namespace: {{ include "kmcp.namespace" $ }}
 {{- end }}
+{{- else }}
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: {{ include "kmcp.fullname" . }}-manager-rolebinding
+  labels:
+    {{- include "kmcp.labels" . | nindent 4 }}
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: {{ include "kmcp.fullname" . }}-manager-role
+subjects:
+- kind: ServiceAccount
+  name: {{ include "kmcp.serviceAccountName" . }}
+  namespace: {{ include "kmcp.namespace" . }}
 {{- end }}
 {{- end }} 

helm/kmcp/tests/__snapshot__/rbac_test.yaml.snap

@@ -1,4 +1,4 @@
-should create Role when clusterScoped is false:
+should create Role when rbac.namespaces is set:
   1: |
     apiVersion: rbac.authorization.k8s.io/v1
     kind: Role
@@ -64,7 +64,7 @@ should create Role when clusterScoped is false:
           - get
           - patch
           - update
-should create RoleBinding when clusterScoped is false:
+should create RoleBinding when rbac.namespaces is set:
   1: |
     apiVersion: rbac.authorization.k8s.io/v1
     kind: RoleBinding
@@ -310,7 +310,7 @@ should create multiple Roles when multiple namespaces are provided:
         control-plane: controller-manager
         helm.sh/chart: kmcp-1.0.0
       name: RELEASE-NAME-manager-role
-      namespace: ns1
+      namespace: NAMESPACE
     rules:
       - apiGroups:
           - ""

helm/kmcp/tests/deployment_test.yaml

@@ -155,23 +155,24 @@ tests:
           count: 1
       - matchSnapshot: {}
 
-  - it: should not include watch-namespaces arg when clusterScoped is true
+  - it: should not include watch-namespaces arg when rbac.namespaces is empty
     template: deployment.yaml
     set:
-      rbac.clusterScoped: true
+      rbac.namespaces: []
       image.repository: test-repo
       image.tag: v1.0.0
     asserts:
       - hasDocuments:
           count: 1
       - notContains:
           path: spec.template.spec.containers[0].args
-          content: --watch-namespaces=NAMESPACE
+          content: --watch-namespaces
 
-  - it: should include watch-namespaces arg when clusterScoped is false with default namespace
+  - it: should include watch-namespaces arg when rbac.namespaces is set
     template: deployment.yaml
     set:
-      rbac.clusterScoped: false
+      rbac.namespaces:
+        - NAMESPACE
       image.repository: test-repo
       image.tag: v1.0.0
     asserts:
@@ -181,11 +182,11 @@ tests:
           path: spec.template.spec.containers[0].args
           content: --watch-namespaces=NAMESPACE
 
-  - it: should include watch-namespaces arg with explicit namespaces when clusterScoped is false
+  - it: should include watch-namespaces arg with explicit namespaces
     template: deployment.yaml
     set:
-      rbac.clusterScoped: false
       rbac.namespaces:
+        - NAMESPACE
         - ns1
         - ns2
       image.repository: test-repo
@@ -195,4 +196,4 @@ tests:
           count: 1
       - contains:
           path: spec.template.spec.containers[0].args
-          content: --watch-namespaces=ns1,ns2 
+          content: --watch-namespaces=NAMESPACE,ns1,ns2 

helm/kmcp/tests/rbac_test.yaml

@@ -144,11 +144,12 @@ tests:
           count: 1
       - matchSnapshot: {}
 
-  - it: should create Role when clusterScoped is false
+  - it: should create Role when rbac.namespaces is set
     template: rbac/clusterrole.yaml
     set:
       rbac.create: true
-      rbac.clusterScoped: false
+      rbac.namespaces:
+        - NAMESPACE
     asserts:
       - hasDocuments:
           count: 1
@@ -157,11 +158,12 @@ tests:
           apiVersion: rbac.authorization.k8s.io/v1
       - matchSnapshot: {}
 
-  - it: should create RoleBinding when clusterScoped is false
+  - it: should create RoleBinding when rbac.namespaces is set
     template: rbac/clusterrolebinding.yaml
     set:
       rbac.create: true
-      rbac.clusterScoped: false
+      rbac.namespaces:
+        - NAMESPACE
     asserts:
       - hasDocuments:
           count: 1
@@ -170,20 +172,40 @@ tests:
           apiVersion: rbac.authorization.k8s.io/v1
       - matchSnapshot: {}
 
+  - it: should render a single role/binding in the listed namespace only (no release-ns fallback)
+    set:
+      rbac.create: true
+      rbac.namespaces:
+        - NAMESPACE
+    asserts:
+      - hasDocuments:
+          count: 1
+        template: rbac/clusterrole.yaml
+      - equal:
+          path: metadata.namespace
+          value: NAMESPACE
+        template: rbac/clusterrole.yaml
+      - hasDocuments:
+          count: 1
+        template: rbac/clusterrolebinding.yaml
+      - equal:
+          path: metadata.namespace
+          value: NAMESPACE
+        template: rbac/clusterrolebinding.yaml
+
   - it: should create multiple Roles when multiple namespaces are provided
     template: rbac/clusterrole.yaml
     set:
       rbac.create: true
-      rbac.clusterScoped: false
       rbac.namespaces:
-        - ns1
+        - NAMESPACE
         - ns2
     asserts:
       - hasDocuments:
           count: 2
       - equal:
           path: metadata.namespace
-          value: ns1
+          value: NAMESPACE
         documentIndex: 0
       - equal:
           path: metadata.namespace
@@ -195,4 +217,43 @@ tests:
       - isKind:
           of: Role
         documentIndex: 1
-      - matchSnapshot: {}
+      - matchSnapshot: {}
+
+  - it: should fail rendering if the removed rbac.clusterScoped field is set
+    set:
+      rbac.create: true
+      rbac.clusterScoped: false
+    template: rbac/clusterrolebinding.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.create: true
+      rbac.namespaces:
+        - some-other-ns
+    template: rbac/clusterrolebinding.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.create: true
+      rbac.namespaces:
+        - my-ns
+        - other-ns
+    template: rbac/clusterrole.yaml
+    asserts:
+      - hasDocuments:
+          count: 2
+      - equal:
+          path: metadata.namespace
+          value: my-ns
+        documentIndex: 0
+      - equal:
+          path: metadata.namespace
+          value: other-ns
+        documentIndex: 1

helm/kmcp/values.yaml

@@ -95,13 +95,12 @@ serviceAccount:
 rbac:
   # Specifies whether RBAC resources should be created
   create: true
-  
-  # -- 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.
   namespaces: []
 
 # Service configuration for metrics