diff --git a/examples/chart/teleport-cluster/templates/_helpers.tpl b/examples/chart/teleport-cluster/templates/_helpers.tpl index 1edf2404f4368..668e51092ca2a 100644 --- a/examples/chart/teleport-cluster/templates/_helpers.tpl +++ b/examples/chart/teleport-cluster/templates/_helpers.tpl @@ -6,10 +6,48 @@ if serviceAccount is not defined or serviceAccount.name is empty, use .Release.N {{- coalesce .Values.serviceAccount.name .Release.Name -}} {{- end -}} +{{/* +Create the name of the service account to use in the auth config check hook. + +If the chart is creating service accounts, we know we can create new arbitrary service accounts. +We cannot reuse the same name as the deployment SA because the non-hook service account might +not exist yet. We tried being smart with hooks but ArgoCD doesn't differentiate between install +and upgrade, causing various issues on update and eventually forcing us to use a separate SA. + +If the chart is not creating service accounts, for backward compatibility we don't want +to force new service account names to existing chart users. We know the SA should already exist, +so we can use the same SA for deployments and hooks. +*/}} +{{- define "teleport-cluster.auth.hookServiceAccountName" -}} +{{- include "teleport-cluster.auth.serviceAccountName" . -}} +{{- if .Values.serviceAccount.create -}} +-hook +{{- end -}} +{{- end -}} + {{- define "teleport-cluster.proxy.serviceAccountName" -}} {{- coalesce .Values.serviceAccount.name .Release.Name -}}-proxy {{- end -}} +{{/* +Create the name of the service account to use in the proxy config check hook. + +If the chart is creating service accounts, we know we can create new arbitrary service accounts. +We cannot reuse the same name as the deployment SA because the non-hook service account might +not exist yet. We tried being smart with hooks but ArgoCD doesn't differentiate between install +and upgrade, causing various issues on update and eventually forcing us to use a separate SA. + +If the chart is not creating service accounts, for backward compatibility we don't want +to force new service account names to existing chart users. We know the SA should already exist, +so we can use the same SA for deployments and hooks. +*/}} +{{- define "teleport-cluster.proxy.hookServiceAccountName" -}} +{{- include "teleport-cluster.proxy.serviceAccountName" . -}} +{{- if .Values.serviceAccount.create -}} +-hook +{{- end -}} +{{- end -}} + {{- define "teleport-cluster.version" -}} {{- coalesce .Values.teleportVersionOverride .Chart.Version }} {{- end -}} diff --git a/examples/chart/teleport-cluster/templates/auth/predeploy_job.yaml b/examples/chart/teleport-cluster/templates/auth/predeploy_job.yaml index e75c0f20a55e8..d5a38e93ead74 100644 --- a/examples/chart/teleport-cluster/templates/auth/predeploy_job.yaml +++ b/examples/chart/teleport-cluster/templates/auth/predeploy_job.yaml @@ -104,5 +104,5 @@ spec: {{- if .Values.extraVolumes }} {{- toYaml .Values.extraVolumes | nindent 6 }} {{- end }} - serviceAccountName: {{ include "teleport-cluster.auth.serviceAccountName" . }} + serviceAccountName: {{ include "teleport-cluster.auth.hookServiceAccountName" . }} {{- end }} diff --git a/examples/chart/teleport-cluster/templates/auth/predeploy_serviceaccount.yaml b/examples/chart/teleport-cluster/templates/auth/predeploy_serviceaccount.yaml index 8510f30145fb0..893078f9a902d 100644 --- a/examples/chart/teleport-cluster/templates/auth/predeploy_serviceaccount.yaml +++ b/examples/chart/teleport-cluster/templates/auth/predeploy_serviceaccount.yaml @@ -2,12 +2,13 @@ # upon first install of the chart. it will be deleted by Helm after the pre-deploy hooks run, then the # regular serviceAccount is created with the same name and exists for the lifetime of the release. {{- $auth := mustMergeOverwrite (mustDeepCopy .Values) .Values.auth -}} +{{- if $auth.validateConfigOnDeploy }} {{- $projectedServiceAccountToken := semverCompare ">=1.20.0-0" .Capabilities.KubeVersion.Version }} {{- if $auth.serviceAccount.create }} apiVersion: v1 kind: ServiceAccount metadata: - name: {{ template "teleport-cluster.auth.serviceAccountName" . }} + name: {{ template "teleport-cluster.auth.hookServiceAccountName" . }} namespace: {{ .Release.Namespace }} labels: {{- include "teleport-cluster.auth.labels" . | nindent 4 }} @@ -15,9 +16,7 @@ metadata: {{- toYaml $auth.extraLabels.serviceAccount | nindent 4 }} {{- end }} annotations: - # this ServiceAccount resource MUST only be hooked on pre-install, as it would conflict - # with the existing ServiceAccount if hooked on pre-upgrade. - "helm.sh/hook": pre-install + "helm.sh/hook": pre-install,pre-upgrade "helm.sh/hook-weight": "3" "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded {{- if or $auth.annotations.serviceAccount $auth.azure.clientID }} @@ -32,3 +31,4 @@ metadata: automountServiceAccountToken: false {{- end }} {{- end }} +{{- end }} diff --git a/examples/chart/teleport-cluster/templates/proxy/predeploy_job.yaml b/examples/chart/teleport-cluster/templates/proxy/predeploy_job.yaml index 627aa4c626fcc..0f4ddb4f7fff4 100644 --- a/examples/chart/teleport-cluster/templates/proxy/predeploy_job.yaml +++ b/examples/chart/teleport-cluster/templates/proxy/predeploy_job.yaml @@ -100,5 +100,5 @@ spec: {{- if $proxy.extraVolumes }} {{- toYaml $proxy.extraVolumes | nindent 6 }} {{- end }} - serviceAccountName: {{ include "teleport-cluster.proxy.serviceAccountName" . }} + serviceAccountName: {{ include "teleport-cluster.proxy.hookServiceAccountName" . }} {{- end }} diff --git a/examples/chart/teleport-cluster/templates/proxy/predeploy_serviceaccount.yaml b/examples/chart/teleport-cluster/templates/proxy/predeploy_serviceaccount.yaml index 89a056ceed58e..6c5b9a4a1476c 100644 --- a/examples/chart/teleport-cluster/templates/proxy/predeploy_serviceaccount.yaml +++ b/examples/chart/teleport-cluster/templates/proxy/predeploy_serviceaccount.yaml @@ -3,11 +3,12 @@ # regular serviceAccount is created with the same name and exists for the lifetime of the release. {{- $proxy := mustMergeOverwrite (mustDeepCopy .Values) .Values.proxy -}} {{- $projectedServiceAccountToken := semverCompare ">=1.20.0-0" .Capabilities.KubeVersion.Version }} +{{- if $proxy.validateConfigOnDeploy }} {{- if $proxy.serviceAccount.create }} apiVersion: v1 kind: ServiceAccount metadata: - name: {{ include "teleport-cluster.proxy.serviceAccountName" . }} + name: {{ include "teleport-cluster.proxy.hookServiceAccountName" . }} namespace: {{ .Release.Namespace }} labels: {{- include "teleport-cluster.proxy.labels" . | nindent 4 }} @@ -15,9 +16,7 @@ metadata: {{- toYaml $proxy.extraLabels.serviceAccount | nindent 4 }} {{- end }} annotations: - # this ServiceAccount resource MUST only be hooked on pre-install, as it would conflict - # with the existing ServiceAccount if hooked on pre-upgrade. - "helm.sh/hook": pre-install + "helm.sh/hook": pre-install,pre-upgrade "helm.sh/hook-weight": "3" "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded {{- if $proxy.annotations.serviceAccount }} @@ -27,3 +26,4 @@ metadata: automountServiceAccountToken: false {{- end }} {{- end }} +{{- end }} diff --git a/examples/chart/teleport-cluster/tests/predeploy_test.yaml b/examples/chart/teleport-cluster/tests/predeploy_test.yaml index c9de357957901..3ab3ad799e99c 100644 --- a/examples/chart/teleport-cluster/tests/predeploy_test.yaml +++ b/examples/chart/teleport-cluster/tests/predeploy_test.yaml @@ -2,8 +2,10 @@ suite: Pre-Deploy Config Test Hooks templates: - auth/predeploy_job.yaml - auth/predeploy_config.yaml + - auth/predeploy_serviceaccount.yaml - proxy/predeploy_job.yaml - proxy/predeploy_config.yaml + - proxy/predeploy_serviceaccount.yaml tests: - it: Deploys the auth-test config template: auth/predeploy_config.yaml @@ -53,6 +55,7 @@ tests: asserts: - hasDocuments: count: 0 + - it: should set resources on auth predeploy job when set in values template: auth/predeploy_job.yaml values: @@ -189,41 +192,65 @@ tests: path: metadata.labels.baz value: overridden - - it: should use default serviceAccount name for auth predeploy job SA when not set in values + - it: should use default serviceAccount name suffixed with -hook for auth predeploy job SA when not set in values and we're creating SAs + template: auth/predeploy_job.yaml + set: + clusterName: helm-lint + asserts: + - equal: + path: spec.template.spec.serviceAccountName + value: RELEASE-NAME-hook + + - it: should use serviceAccount.name suffixed with -hook for auth predeploy job SA when set in values and we're creating SAs template: auth/predeploy_job.yaml set: clusterName: helm-lint + serviceAccount: + name: helm-test-sa asserts: - equal: path: spec.template.spec.serviceAccountName - value: RELEASE-NAME + value: helm-test-sa-hook - - it: should use serviceAccount.name for auth predeploy job SA when set in values + - it: should use serviceAccount.name for auth predeploy job SA when set in values and we're not creating SAs template: auth/predeploy_job.yaml set: clusterName: helm-lint serviceAccount: name: helm-test-sa + create: false asserts: - equal: path: spec.template.spec.serviceAccountName value: helm-test-sa - - it: should use default serviceAccount name for proxy predeploy job SA when not set in values + - it: should use default serviceAccount name suffixed with -hook for proxy predeploy job SA when not set in values and we're creating SAs template: proxy/predeploy_job.yaml set: clusterName: helm-lint asserts: - equal: path: spec.template.spec.serviceAccountName - value: RELEASE-NAME-proxy + value: RELEASE-NAME-proxy-hook + + - it: should use serviceAccount.name suffixed with -hook for proxy predeploy job SA when set in values and we're creating SAs + template: proxy/predeploy_job.yaml + set: + clusterName: helm-lint + serviceAccount: + name: helm-test-sa + asserts: + - equal: + path: spec.template.spec.serviceAccountName + value: helm-test-sa-proxy-hook - - it: should use serviceAccount.name for proxy predeploy job SA when set in values + - it: should use serviceAccount.name for proxy predeploy job SA when set in values and we're not creating SAs template: proxy/predeploy_job.yaml set: clusterName: helm-lint serviceAccount: name: helm-test-sa + create: false asserts: - equal: path: spec.template.spec.serviceAccountName