- Notifications
You must be signed in to change notification settings - Fork1.1k
feat(helm/provisioner): add support for provisioner keys, add note re psk#15122
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
matifali left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM.
| - name: CODER_PROVISIONER_DAEMON_PSK | ||
| valueFrom: | ||
| secretKeyRef: | ||
| key: psk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nit: any reason why this is calledpsk instead ofprovisionerd-psk?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The key name is currently hard-coded in_coder.tpl:
- name: CODER_PROVISIONER_DAEMON_PSK valueFrom: secretKeyRef: name: {{ .Values.provisionerDaemon.pskSecretName | quote }} key: psk{{- if include "provisioner.tags" . }}This wasn't changed as part of this PR. I can add a separate PR to allow customizing the key name, if required.
413928b intomainUh oh!
There was an error while loading.Please reload this page.
| expectedError:"", | ||
| }, | ||
| { | ||
| name:"provisionerd_psk_and_key", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is it ever sensible to accept both? I think we can only accept one or other as the authentication credential
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yep, you're correct:
error: cannot provide both provisioner key --key and pre-shared key --pskI can remove this test and add a check in the Helm chart, but I'd worry about logic drift.
Uh oh!
There was an error while loading.Please reload this page.
Relates to#14985
provisionerDaemon.keySecretNameandprovisionerDaemon.keySecretKeyprovisionerDaemon.pskSecretNamewill now cause the PSK secret to no longer be created.NOTES.txtregarding provisioner PSKs.provisionerDaemon.keySecretNameorprovisionerDaemon.pskSecretNameis specified, and will fail the install in this case.Manual smoke-testing:
pskSecretName:pskSecretNameempty andkeySecretNamespecified:pskSecretNameempty andkeySecretNameempty: