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

@@ -15,4 +15,4 @@ maintainers:
 name: common
 sources: null
 type: library
-version: 10.6.0
+version: 10.6.1

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 }}

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

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" -}}

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 }}

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

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

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

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