-
Notifications
You must be signed in to change notification settings - Fork 888
feat: add provisioner chart to release and docs #9050
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
Conversation
Signed-off-by: Spike Curtis <spike@coder.com>
Co-authored-by: Muhammad Atif Ali <atif@coder.com>
…isioner-chart-publish
Co-authored-by: Cian Johnston <cian@coder.com>
> Template Admin or Owner role. This method is deprecated in favor of the PSK, which only has permission to access | ||
> provisioner daemon APIs. We recommend migrating to the PSK as soon as practical. |
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.
If we supported API token scopes, would you be open to us supporting both, or are there other reasons for deprecating user token auth?
Something I like about relying on user authentication is that this token could be rotated without restarting the Coder server and #7992 could be used down the road to limit specific user tokens to specific provisioners versus full API access.
It does feel a little odd passing CODER_SESSION_TOKEN
to the provisioner, especially if it is a long-lived token, but that could be said about any automation somebody is trying to do with Coder.
I know it also requires that you create a token prior to starting a provisioner and you can only create a token on behalf of another user (machine account) via the REST API which is clunky.
Perhaps, for now, we mention the drawbacks of the token admin auth and avoid "deprecation" language and then we can support it again as a first-class citizen when we add scopes to API tokens.
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.
I think we need to keep supporting user token auth anyway because of local provisioners.
I agree that we'll want to add support for tightly-scoped credentials that can be rotated without restarting Coderd in the future. At present I'm agnostic about whether they are API tokens or some other kind of credential (as a specific example: a JWT signed by an external authority). We should speak with our large customers to get their take on how they'd like to do this auth.
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.
We should speak with our large customers to get their take on how they'd like to do this auth.
Bet you a 🌮 someone is going to ask for Azure Workload Identity
provisionerDaemon: | ||
pskSecretName: "coder-provisioner-psk" | ||
tags: | ||
location: auh | ||
kind: k8s |
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.
It's a bit confusing to me how the provisioner chart and the Coder chart seemingly accept the same Helm values format despite being different charts. If I am understanding correctly, when:
provisionerDaemon:
pskSecretName: "coder-provisioner-psk"
is set for the server values, it will set CODER_PROVISIONER_DAEMON_PSK
for the coder server. I see that code here.
However, when the same values file is used set for the provisioner server, it passes the PSK to the provisioner.
Is this a normal practice for Helm charts? If so, I'm on board. I also wonder, in a follow-up PR, if we should rewrite our workspace proxies Helm to do something similar.
Our workspace proxies Helm is currently on the Coder chart and when enabled, Helm changes the entrypoint/command to start a wsproxy server instead of a full server. Our current wsproxy approach is easier for me to wrap my head around, but I can see how it may not be a best practice.
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 values.yaml
between the charts is very similar, but there are a number of differences. For example, the provisioner chart doesn't accept TLS certificates (because it doesn't serve traffic), doesn't include the service
block, etc.
I kept the pskSecretName
value the same, since from a user's perspective, it's the same thing: the name of the K8s secret with the PSK. It's true that the two charts are doing slightly different things with the same piece of information.
I think having different charts for coderd and provisionerd is conceptually easier to understand, rather than a single massive chart with lots of values that don't make sense when running provisioners. Workspace proxies are conceptually much more similar to Coderd in that they serve traffic, so maybe it's ok to keep those two use cases in a single chart. We should keep an eye on how much they diverge and split it into two charts if we're finding there is a lot of either-or config knobs.
fixes #8243
Adds the provisioner chart to our release artifacts, make targets, etc.
Adds documentation for how to use the chart.
Also makes changes to our existing docs