diff --git a/helm/kmcp/templates/_helpers.tpl b/helm/kmcp/templates/_helpers.tpl index edf4980..e71ec96 100644 --- a/helm/kmcp/templates/_helpers.tpl +++ b/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=[, ...] 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 }} diff --git a/helm/kmcp/templates/rbac/clusterrole.yaml b/helm/kmcp/templates/rbac/clusterrole.yaml index 602cf7b..a30d106 100644 --- a/helm/kmcp/templates/rbac/clusterrole.yaml +++ b/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 }} \ No newline at end of file diff --git a/helm/kmcp/templates/rbac/clusterrolebinding.yaml b/helm/kmcp/templates/rbac/clusterrolebinding.yaml index 6386867..4e93a75 100644 --- a/helm/kmcp/templates/rbac/clusterrolebinding.yaml +++ b/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 }} \ No newline at end of file diff --git a/helm/kmcp/tests/__snapshot__/rbac_test.yaml.snap b/helm/kmcp/tests/__snapshot__/rbac_test.yaml.snap index 4068ab0..3b7426d 100644 --- a/helm/kmcp/tests/__snapshot__/rbac_test.yaml.snap +++ b/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: - "" diff --git a/helm/kmcp/tests/deployment_test.yaml b/helm/kmcp/tests/deployment_test.yaml index 5bb622e..d17d730 100644 --- a/helm/kmcp/tests/deployment_test.yaml +++ b/helm/kmcp/tests/deployment_test.yaml @@ -155,10 +155,10 @@ 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: @@ -166,12 +166,13 @@ tests: 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 \ No newline at end of file + content: --watch-namespaces=NAMESPACE,ns1,ns2 \ No newline at end of file diff --git a/helm/kmcp/tests/rbac_test.yaml b/helm/kmcp/tests/rbac_test.yaml index 178a461..e6bd262 100644 --- a/helm/kmcp/tests/rbac_test.yaml +++ b/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: {} \ No newline at end of file + - 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=[, ...] 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 \ No newline at end of file diff --git a/helm/kmcp/values.yaml b/helm/kmcp/values.yaml index 72a16fc..efd8b7d 100644 --- a/helm/kmcp/values.yaml +++ b/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