Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions helm/provisioner/templates/_coder.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -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) }}
{{ fail "Either provisionerDaemon.pskSecretName or provisionerDaemon.keySecretName must be specified, but not both." }}
{{- end }}
{{- if .Values.provisionerDaemon.pskSecretName }}
- name: CODER_PROVISIONER_DAEMON_PSK
valueFrom:
secretKeyRef:
name: {{ .Values.provisionerDaemon.pskSecretName | quote }}
key: psk
{{- end }}
{{- if and .Values.provisionerDaemon.keySecretName .Values.provisionerDaemon.keySecretKey }}
{{- else if and .Values.provisionerDaemon.keySecretName .Values.provisionerDaemon.keySecretKey }}
{{- if and (not (empty .Values.provisionerDaemon.pskSecretName)) (ne .Values.provisionerDaemon.pskSecretName "coder-provisioner-psk") }}
{{ fail "Either provisionerDaemon.pskSecretName or provisionerDaemon.keySecretName must be specified, but not both." }}
{{- else if .Values.provisionerDaemon.tags }}
{{ fail "provisionerDaemon.tags may not be specified with provisionerDaemon.keySecretName." }}
{{- end }}
- 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
Expand Down
10 changes: 10 additions & 0 deletions helm/provisioner/tests/chart_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: "",
},
Comment on lines +61 to +64
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review: adding an explicit test for the old workaround

{
name: "provisionerd_psk_and_key",
expectedError: `Either provisionerDaemon.pskSecretName or provisionerDaemon.keySecretName must be specified, but not both.`,
Expand All @@ -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: "",
Expand Down
2 changes: 0 additions & 2 deletions helm/provisioner/tests/testdata/provisionerd_key.golden
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions helm/provisioner/tests/testdata/provisionerd_key.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,5 @@ coder:
image:
tag: latest
provisionerDaemon:
pskSecretName: ""
keySecretName: "coder-provisionerd-key"
keySecretKey: "provisionerd-key"
tags:
location: auh
clusterType: k8s
Original file line number Diff line number Diff line change
@@ -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: []
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
coder:
image:
tag: latest
provisionerDaemon:
pskSecretName: ""
keySecretName: "coder-provisionerd-key"
keySecretKey: "provisionerd-key"
9 changes: 9 additions & 0 deletions helm/provisioner/tests/testdata/provisionerd_key_tags.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
coder:
image:
tag: latest
provisionerDaemon:
keySecretName: "coder-provisionerd-key"
keySecretKey: "provisionerd-key"
tags:
location: auh
clusterType: k8s
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,3 @@ coder:
provisionerDaemon:
pskSecretName: ""
keySecretName: ""
tags:
location: auh
clusterType: k8s
2 changes: 1 addition & 1 deletion helm/provisioner/tests/testdata/provisionerd_psk.golden
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ spec:
valueFrom:
secretKeyRef:
key: psk
name: coder-provisionerd-psk
Copy link
Member Author

@johnstcn johnstcn Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review: renamed this to be clearer that it is not the default value

name: not-the-default-coder-provisioner-psk
- name: CODER_PROVISIONERD_TAGS
value: clusterType=k8s,location=auh
- name: CODER_URL
Expand Down
2 changes: 1 addition & 1 deletion helm/provisioner/tests/testdata/provisionerd_psk.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
11 changes: 10 additions & 1 deletion helm/provisioner/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading