fix(helm/provisioner): prefer provisioner key if both psk and key are set (#15417)

Relates to https://github.com/coder/coder/pull/15416

This PR modifies the provisioner helm chart logic:
- Previously, when both provisionerDaemon.keySecretName and provisionerDaemon.pskSecretName
  were both set, we would fail to install the chart. This required users to have an obnoxious workaround
  in place where setting provisionerDaemon.pskSecretName="" was required in order to use provisioner
  keys. We now check for pskSecretName being set to the default value when keySecretName is also specified,
  and switch to provisioner key authentication instead of PSK. The previous workaround is still supported.
- We also had omitted to check for provisionerd.Tags being set along with provisionerDaemon.keySecretName.
  This would result in a crashlooping provisioner deployment, as setting both of these configuration options is
  not allowed. We now fast-fail the Helm deployment if we detect this scenario.

helm/provisioner/templates/_coder.tpl

@@ -34,22 +34,23 @@ env:
   value: "0.0.0.0:2112"
 {{- if and (empty .Values.provisionerDaemon.pskSecretName) (empty .Values.provisionerDaemon.keySecretName) }}
 {{ fail "Either provisionerDaemon.pskSecretName or provisionerDaemon.keySecretName must be specified." }}
-{{- else if and (.Values.provisionerDaemon.pskSecretName) (.Values.provisionerDaemon.keySecretName) }}
+{{- else if and .Values.provisionerDaemon.keySecretName .Values.provisionerDaemon.keySecretKey }}
-{{ fail "Either provisionerDaemon.pskSecretName or provisionerDaemon.keySecretName must be specified, but not both." }}
+	{{- if and (not (empty .Values.provisionerDaemon.pskSecretName)) (ne .Values.provisionerDaemon.pskSecretName "coder-provisioner-psk") }}
-{{- end }}
+	{{ fail "Either provisionerDaemon.pskSecretName or provisionerDaemon.keySecretName must be specified, but not both." }}
-{{- if .Values.provisionerDaemon.pskSecretName }}
+	{{- else if .Values.provisionerDaemon.tags }}
-- name: CODER_PROVISIONER_DAEMON_PSK
+	{{ fail "provisionerDaemon.tags may not be specified with provisionerDaemon.keySecretName." }}
-  valueFrom:
+	{{- end }}
-    secretKeyRef:
-      name: {{ .Values.provisionerDaemon.pskSecretName | quote }}
-      key: psk
-{{- end }}
-{{- if and .Values.provisionerDaemon.keySecretName .Values.provisionerDaemon.keySecretKey }}
 - name: CODER_PROVISIONER_DAEMON_KEY
   valueFrom:
     secretKeyRef:
       name: {{ .Values.provisionerDaemon.keySecretName | quote }}
       key: {{ .Values.provisionerDaemon.keySecretKey | quote }}
+{{- else }}
+- name: CODER_PROVISIONER_DAEMON_PSK
+  valueFrom:
+    secretKeyRef:
+      name: {{ .Values.provisionerDaemon.pskSecretName | quote }}
+      key: psk
 {{- end }}
 {{- if include "provisioner.tags" . }}
 - name: CODER_PROVISIONERD_TAGS

helm/provisioner/tests/chart_test.go

@@ -56,6 +56,12 @@ var testCases = []testCase{
 		name:          "provisionerd_key",
 		expectedError: "",
 	},
+	// Test explicitly for the workaround where setting provisionerDaemon.pskSecretName=""
+	// was required to use provisioner keys.
+	{
+		name:          "provisionerd_key_psk_empty_workaround",
+		expectedError: "",
+	},
 	{
 		name:          "provisionerd_psk_and_key",
 		expectedError: `Either provisionerDaemon.pskSecretName or provisionerDaemon.keySecretName must be specified, but not both.`,
@@ -64,6 +70,10 @@ var testCases = []testCase{
 		name:          "provisionerd_no_psk_or_key",
 		expectedError: `Either provisionerDaemon.pskSecretName or provisionerDaemon.keySecretName must be specified.`,
 	},
+	{
+		name:          "provisionerd_key_tags",
+		expectedError: `provisionerDaemon.tags may not be specified with provisionerDaemon.keySecretName.`,
+	},
 	{
 		name:          "extra_templates",
 		expectedError: "",

helm/provisioner/tests/testdata/provisionerd_key.golden

@@ -112,8 +112,6 @@ spec:
             secretKeyRef:
               key: provisionerd-key
               name: coder-provisionerd-key
-        - name: CODER_PROVISIONERD_TAGS
-          value: clusterType=k8s,location=auh
         - name: CODER_URL
           value: http://coder.default.svc.cluster.local
         image: ghcr.io/coder/coder:latest

helm/provisioner/tests/testdata/provisionerd_key.yaml

@@ -2,9 +2,5 @@ coder:
   image:
     tag: latest
 provisionerDaemon:
-  pskSecretName: ""
   keySecretName: "coder-provisionerd-key"
   keySecretKey: "provisionerd-key"
-  tags:
-    location: auh
-    clusterType: k8s

helm/provisioner/tests/testdata/provisionerd_key_psk_empty_workaround.golden

@@ -0,0 +1,135 @@
+---
+# Source: coder-provisioner/templates/coder.yaml
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  annotations: {}
+  labels:
+    app.kubernetes.io/instance: release-name
+    app.kubernetes.io/managed-by: Helm
+    app.kubernetes.io/name: coder-provisioner
+    app.kubernetes.io/part-of: coder-provisioner
+    app.kubernetes.io/version: 0.1.0
+    helm.sh/chart: coder-provisioner-0.1.0
+  name: coder-provisioner
+---
+# Source: coder-provisioner/templates/rbac.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: coder-provisioner-workspace-perms
+rules:
+  - apiGroups: [""]
+    resources: ["pods"]
+    verbs:
+    - create
+    - delete
+    - deletecollection
+    - get
+    - list
+    - patch
+    - update
+    - watch
+  - apiGroups: [""]
+    resources: ["persistentvolumeclaims"]
+    verbs:
+    - create
+    - delete
+    - deletecollection
+    - get
+    - list
+    - patch
+    - update
+    - watch
+  - apiGroups:
+    - apps
+    resources:
+    - deployments
+    verbs:
+    - create
+    - delete
+    - deletecollection
+    - get
+    - list
+    - patch
+    - update
+    - watch
+---
+# Source: coder-provisioner/templates/rbac.yaml
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: "coder-provisioner"
+subjects:
+  - kind: ServiceAccount
+    name: "coder-provisioner"
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: coder-provisioner-workspace-perms
+---
+# Source: coder-provisioner/templates/coder.yaml
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  annotations: {}
+  labels:
+    app.kubernetes.io/instance: release-name
+    app.kubernetes.io/managed-by: Helm
+    app.kubernetes.io/name: coder-provisioner
+    app.kubernetes.io/part-of: coder-provisioner
+    app.kubernetes.io/version: 0.1.0
+    helm.sh/chart: coder-provisioner-0.1.0
+  name: coder-provisioner
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      app.kubernetes.io/instance: release-name
+      app.kubernetes.io/name: coder-provisioner
+  template:
+    metadata:
+      annotations: {}
+      labels:
+        app.kubernetes.io/instance: release-name
+        app.kubernetes.io/managed-by: Helm
+        app.kubernetes.io/name: coder-provisioner
+        app.kubernetes.io/part-of: coder-provisioner
+        app.kubernetes.io/version: 0.1.0
+        helm.sh/chart: coder-provisioner-0.1.0
+    spec:
+      containers:
+      - args:
+        - provisionerd
+        - start
+        command:
+        - /opt/coder
+        env:
+        - name: CODER_PROMETHEUS_ADDRESS
+          value: 0.0.0.0:2112
+        - name: CODER_PROVISIONER_DAEMON_KEY
+          valueFrom:
+            secretKeyRef:
+              key: provisionerd-key
+              name: coder-provisionerd-key
+        - name: CODER_URL
+          value: http://coder.default.svc.cluster.local
+        image: ghcr.io/coder/coder:latest
+        imagePullPolicy: IfNotPresent
+        lifecycle: {}
+        name: coder
+        ports: null
+        resources: {}
+        securityContext:
+          allowPrivilegeEscalation: false
+          readOnlyRootFilesystem: null
+          runAsGroup: 1000
+          runAsNonRoot: true
+          runAsUser: 1000
+          seccompProfile:
+            type: RuntimeDefault
+        volumeMounts: []
+      restartPolicy: Always
+      serviceAccountName: coder-provisioner
+      terminationGracePeriodSeconds: 600
+      volumes: []

helm/provisioner/tests/testdata/provisionerd_key_psk_empty_workaround.yaml

@@ -0,0 +1,7 @@
+coder:
+  image:
+    tag: latest
+provisionerDaemon:
+  pskSecretName: ""
+  keySecretName: "coder-provisionerd-key"
+  keySecretKey: "provisionerd-key"

helm/provisioner/tests/testdata/provisionerd_key_tags.yaml

@@ -0,0 +1,9 @@
+coder:
+  image:
+    tag: latest
+provisionerDaemon:
+  keySecretName: "coder-provisionerd-key"
+  keySecretKey: "provisionerd-key"
+  tags:
+    location: auh
+    clusterType: k8s

helm/provisioner/tests/testdata/provisionerd_no_psk_or_key.yaml

@@ -4,6 +4,3 @@ coder:
 provisionerDaemon:
   pskSecretName: ""
   keySecretName: ""
-  tags:
-    location: auh
-    clusterType: k8s

helm/provisioner/tests/testdata/provisionerd_psk.golden

@@ -111,7 +111,7 @@ spec:
           valueFrom:
             secretKeyRef:
               key: psk
-              name: coder-provisionerd-psk
+              name: not-the-default-coder-provisioner-psk
         - name: CODER_PROVISIONERD_TAGS
           value: clusterType=k8s,location=auh
         - name: CODER_URL

helm/provisioner/tests/testdata/provisionerd_psk.yaml

@@ -2,7 +2,7 @@ coder:
   image:
     tag: latest
 provisionerDaemon:
-  pskSecretName: "coder-provisionerd-psk"
+  pskSecretName: "not-the-default-coder-provisioner-psk"
   tags:
     location: auh
     clusterType: k8s

helm/provisioner/tests/testdata/provisionerd_psk_and_key.yaml

@@ -2,7 +2,7 @@ coder:
   image:
     tag: latest
 provisionerDaemon:
-  pskSecretName: "coder-provisionerd-psk"
+  pskSecretName: "not-the-default-coder-provisioner-psk"
   keySecretName: "coder-provisionerd-key"
   keySecretKey: "provisionerd-key"
   tags:

helm/provisioner/values.yaml

@@ -204,14 +204,23 @@ provisionerDaemon:
   # provisionerDaemon.keySecretName -- The name of the Kubernetes
   # secret that contains a provisioner key to use to authenticate with Coder.
   # See: https://coder.com/docs/admin/provisioners#authentication
+  # NOTE: it is not permitted to specify both provisionerDaemon.keySecretName
+  # and provisionerDaemon.pskSecretName. An exception is made for the purposes
+  # of backwards-compatibility: if provisionerDaemon.pskSecretName is unchanged
+  # from the default value and provisionerDaemon.keySecretName is set, then
+  # provisionerDaemon.keySecretName and provisionerDaemon.keySecretKey will take
+  # precedence over provisionerDaemon.pskSecretName.
   keySecretName: ""
   # provisionerDaemon.keySecretKey -- The key of the Kubernetes
   # secret specified in provisionerDaemon.keySecretName that contains
   # the provisioner key. Defaults to "key".
   keySecretKey: "key"
 
-  # provisionerDaemon.tags -- Tags to filter provisioner jobs by.
+  # provisionerDaemon.tags -- If using a PSK, specify the set of provisioner
+  # job tags for which this provisioner daemon is responsible.
   # See: https://coder.com/docs/admin/provisioners#provisioner-tags
+  # NOTE: it is not permitted to specify both provisionerDaemon.tags and
+  # provsionerDaemon.keySecretName.
   tags:
     {}
     # location: usa