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..2c517ba21 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 (printf "rbac.namespaces is set but does not include the install namespace %q" $installNs) -}} +{{- end -}} {{- end -}} {{- end -}} diff --git a/helm/kagent/templates/rbac/getter-role.yaml b/helm/kagent/templates/rbac/getter-role.yaml index b5e317017..9e97f5122 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 | uniq | sortAlpha) }} --- 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..9d143f72d 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 | uniq | sortAlpha }} --- 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..3e020b56a 100644 --- a/helm/kagent/templates/rbac/writer-role.yaml +++ b/helm/kagent/templates/rbac/writer-role.yaml @@ -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 @@ -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 }} \ 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..d2120b35e 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 | uniq) }} --- 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..719eb6c10 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\"" + + - 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: [] # ==============================================================================