From efd618c94e54eb44db9fbb64806f766e0fc3e642 Mon Sep 17 00:00:00 2001 From: Stavros Kois <47820033+stavros-k@users.noreply.github.com> Date: Sun, 25 Sep 2022 13:50:41 +0300 Subject: [PATCH] fix(rbac): fix SA name in CRB subjects (#232) * fix(rbac): fix SA name in CRB subjects * whops * rbac and sa naming source update * remove sa naming from name.tpl * for pod SA name, use "default"? * manually build name for sa and rbac * fix rbac - sa linking * if there is an rbac with a name not matching any defined SA, fallback to default * first test * hmm * hmm * add some tests * one more case * fix pod saName, split tests * whops * add more tests * fix test --- charts/common/Chart.yaml | 2 +- charts/common/templates/class/_rbac.tpl | 12 +++- .../templates/class/_serviceaccount.tpl | 3 +- charts/common/templates/lib/chart/_names.tpl | 9 --- .../common/templates/lib/controller/_pod.tpl | 7 ++- .../tests/sa-rbac/multiple_sa_rbac_test.yaml | 56 +++++++++++++++++++ .../common-test/tests/sa-rbac/no sa-rbac.yaml | 13 +++++ .../sa-rbac/sa_rbac_different_names_test.yaml | 56 +++++++++++++++++++ .../tests/sa-rbac/single_sa_rbac_test.yaml | 35 ++++++++++++ 9 files changed, 179 insertions(+), 14 deletions(-) create mode 100644 helper-charts/common-test/tests/sa-rbac/multiple_sa_rbac_test.yaml create mode 100644 helper-charts/common-test/tests/sa-rbac/no sa-rbac.yaml create mode 100644 helper-charts/common-test/tests/sa-rbac/sa_rbac_different_names_test.yaml create mode 100644 helper-charts/common-test/tests/sa-rbac/single_sa_rbac_test.yaml diff --git a/charts/common/Chart.yaml b/charts/common/Chart.yaml index 8ae4b694..0266264c 100644 --- a/charts/common/Chart.yaml +++ b/charts/common/Chart.yaml @@ -15,4 +15,4 @@ maintainers: name: common sources: null type: library -version: 10.6.0 +version: 10.6.1 diff --git a/charts/common/templates/class/_rbac.tpl b/charts/common/templates/class/_rbac.tpl index 6a9c784a..f09a27b9 100644 --- a/charts/common/templates/class/_rbac.tpl +++ b/charts/common/templates/class/_rbac.tpl @@ -4,15 +4,23 @@ using the common library. */}} {{- define "tc.common.class.rbac" -}} {{- $fullName := include "tc.common.names.fullname" . -}} + {{- $saName := $fullName -}} {{- $rbacName := $fullName -}} {{- $values := .Values.rbac -}} - + {{- $saValues := .Values.serviceAccount -}} {{- if hasKey . "ObjectValues" -}} {{- with .ObjectValues.rbac -}} {{- $values = . -}} {{- end -}} {{ end -}} + {{- if and (hasKey $values "nameOverride") $values.nameOverride -}} + {{- $saName = printf "%v-%v" $saName $values.nameOverride -}} + {{- if not (hasKey $saValues $values.nameOverride) -}} + {{- $saName = "default" -}} + {{- end }} + {{- end }} + {{- if and (hasKey $values "nameOverride") $values.nameOverride -}} {{- $rbacName = printf "%v-%v" $rbacName $values.nameOverride -}} {{- end }} @@ -55,7 +63,7 @@ roleRef: name: {{ $rbacName }} subjects: - kind: ServiceAccount - name: {{ default (include "tc.common.names.serviceAccountName" .) $values.serviceAccountName }} + name: {{ $saName }} namespace: {{ .Release.Namespace }} {{- with $values.subjects }} {{- toYaml . | nindent 2 }} diff --git a/charts/common/templates/class/_serviceaccount.tpl b/charts/common/templates/class/_serviceaccount.tpl index 5abf3e84..2634ff48 100644 --- a/charts/common/templates/class/_serviceaccount.tpl +++ b/charts/common/templates/class/_serviceaccount.tpl @@ -11,11 +11,12 @@ using the common library. {{- with .ObjectValues.serviceAccount -}} {{- $values = . -}} {{- end -}} - {{ end -}} + {{- end -}} {{- if and (hasKey $values "nameOverride") $values.nameOverride -}} {{- $saName = printf "%v-%v" $saName $values.nameOverride -}} {{- end }} + --- apiVersion: v1 kind: ServiceAccount diff --git a/charts/common/templates/lib/chart/_names.tpl b/charts/common/templates/lib/chart/_names.tpl index 39baaa0a..ebfdf589 100644 --- a/charts/common/templates/lib/chart/_names.tpl +++ b/charts/common/templates/lib/chart/_names.tpl @@ -35,15 +35,6 @@ If release name contains chart name it will be used as a full name. {{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" -}} {{- end -}} -{{/* Create the name of the ServiceAccount to use */}} -{{- define "tc.common.names.serviceAccountName" -}} - {{- if .Values.serviceAccount.create -}} - {{- default (include "tc.common.names.fullname" .) .Values.serviceAccount.name -}} - {{- else -}} - {{- default "default" .Values.serviceAccount.name -}} - {{- end -}} -{{- end -}} - {{/* Return the properly cased version of the controller type */}} {{- define "tc.common.names.controllerType" -}} {{- if eq .Values.controller.type "deployment" -}} diff --git a/charts/common/templates/lib/controller/_pod.tpl b/charts/common/templates/lib/controller/_pod.tpl index 3e91d748..20f26017 100644 --- a/charts/common/templates/lib/controller/_pod.tpl +++ b/charts/common/templates/lib/controller/_pod.tpl @@ -6,7 +6,12 @@ The pod definition included in the controller. imagePullSecrets: {{ tpl ( toYaml . ) $ | nindent 2 }} {{- end }} -serviceAccountName: {{ include "tc.common.names.serviceAccountName" . }} + +{{- $saName := include "tc.common.names.fullname" . -}} +{{- if not .Values.serviceAccount.main.enabled }} + {{ $saName = "default" }} +{{- end }} +serviceAccountName: {{ $saName }} {{- with .Values.podSecurityContext }} securityContext: {{ tpl ( toYaml . ) $ | nindent 2 }} diff --git a/helper-charts/common-test/tests/sa-rbac/multiple_sa_rbac_test.yaml b/helper-charts/common-test/tests/sa-rbac/multiple_sa_rbac_test.yaml new file mode 100644 index 00000000..d70c3e81 --- /dev/null +++ b/helper-charts/common-test/tests/sa-rbac/multiple_sa_rbac_test.yaml @@ -0,0 +1,56 @@ +suite: sa-rbac multiple +templates: + - common.yaml +tests: + - it: multiple sa and rbac should be named correctly + set: + serviceAccount: + main: + enabled: true + secondary: + enabled: true + rbac: + main: + enabled: true + secondary: + enabled: true + asserts: + - documentIndex: &DeploymentDoc 2 + isKind: + of: Deployment + - documentIndex: *DeploymentDoc + equal: + path: spec.template.spec.serviceAccountName + value: RELEASE-NAME-common-test + + - documentIndex: &ServiceAccountDoc 0 + isKind: + of: ServiceAccount + - documentIndex: *ServiceAccountDoc + equal: + path: metadata.name + value: RELEASE-NAME-common-test + + - documentIndex: &ServiceAccountSecondaryDoc 1 + isKind: + of: ServiceAccount + - documentIndex: *ServiceAccountSecondaryDoc + equal: + path: metadata.name + value: RELEASE-NAME-common-test-secondary + + - documentIndex: &ClusterRoleBindingDoc 4 + isKind: + of: ClusterRoleBinding + - documentIndex: *ClusterRoleBindingDoc + equal: + path: subjects[0].name + value: RELEASE-NAME-common-test + + - documentIndex: &ClusterRoleBindingSecondaryDoc 6 + isKind: + of: ClusterRoleBinding + - documentIndex: *ClusterRoleBindingSecondaryDoc + equal: + path: subjects[0].name + value: RELEASE-NAME-common-test-secondary diff --git a/helper-charts/common-test/tests/sa-rbac/no sa-rbac.yaml b/helper-charts/common-test/tests/sa-rbac/no sa-rbac.yaml new file mode 100644 index 00000000..db9f08d2 --- /dev/null +++ b/helper-charts/common-test/tests/sa-rbac/no sa-rbac.yaml @@ -0,0 +1,13 @@ +suite: no sa-rbac +templates: + - common.yaml +tests: + - it: single sa and rbac should be named correctly + asserts: + - documentIndex: &DeploymentDoc 1 + isKind: + of: Deployment + - documentIndex: *DeploymentDoc + equal: + path: spec.template.spec.serviceAccountName + value: default diff --git a/helper-charts/common-test/tests/sa-rbac/sa_rbac_different_names_test.yaml b/helper-charts/common-test/tests/sa-rbac/sa_rbac_different_names_test.yaml new file mode 100644 index 00000000..e0e60012 --- /dev/null +++ b/helper-charts/common-test/tests/sa-rbac/sa_rbac_different_names_test.yaml @@ -0,0 +1,56 @@ +suite: sa-rbac different names +templates: + - common.yaml +tests: + - it: rbac with different name than sa are linked with the default sa + set: + serviceAccount: + main: + enabled: true + secondary: + enabled: true + rbac: + main: + enabled: true + third: + enabled: true + asserts: + - documentIndex: &DeploymentDoc 2 + isKind: + of: Deployment + - documentIndex: *DeploymentDoc + equal: + path: spec.template.spec.serviceAccountName + value: RELEASE-NAME-common-test + + - documentIndex: &ServiceAccountDoc 0 + isKind: + of: ServiceAccount + - documentIndex: *ServiceAccountDoc + equal: + path: metadata.name + value: RELEASE-NAME-common-test + + - documentIndex: &ServiceAccountSecondaryDoc 1 + isKind: + of: ServiceAccount + - documentIndex: *ServiceAccountSecondaryDoc + equal: + path: metadata.name + value: RELEASE-NAME-common-test-secondary + + - documentIndex: &ClusterRoleBindingDoc 4 + isKind: + of: ClusterRoleBinding + - documentIndex: *ClusterRoleBindingDoc + equal: + path: subjects[0].name + value: RELEASE-NAME-common-test + + - documentIndex: &ClusterRoleBindingThirdDoc 6 + isKind: + of: ClusterRoleBinding + - documentIndex: *ClusterRoleBindingThirdDoc + equal: + path: subjects[0].name + value: default diff --git a/helper-charts/common-test/tests/sa-rbac/single_sa_rbac_test.yaml b/helper-charts/common-test/tests/sa-rbac/single_sa_rbac_test.yaml new file mode 100644 index 00000000..37bb9403 --- /dev/null +++ b/helper-charts/common-test/tests/sa-rbac/single_sa_rbac_test.yaml @@ -0,0 +1,35 @@ +suite: sa-rbac single +templates: + - common.yaml +tests: + - it: single sa and rbac should be named correctly + set: + serviceAccount: + main: + enabled: true + rbac: + main: + enabled: true + asserts: + - documentIndex: &DeploymentDoc 1 + isKind: + of: Deployment + - documentIndex: *DeploymentDoc + equal: + path: spec.template.spec.serviceAccountName + value: RELEASE-NAME-common-test + + - documentIndex: &ServiceAccountDoc 0 + isKind: + of: ServiceAccount + - documentIndex: *ServiceAccountDoc + equal: + path: metadata.name + value: RELEASE-NAME-common-test + - documentIndex: &ClusterRoleBindingDoc 3 + isKind: + of: ClusterRoleBinding + - documentIndex: *ClusterRoleBindingDoc + equal: + path: subjects[0].name + value: RELEASE-NAME-common-test