Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
spikecurtis merged 5 commits intomainfromspike/provisioner-chart-publish
Aug 16, 2023

Conversation

spikecurtis
Copy link
Contributor

@spikecurtisspikecurtis commentedAug 11, 2023
edited
Loading

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

  • Removes the warning that provisioner daemons are "alpha" features
  • Deprecates using Template Admin or Owner tokens to authenticate, in favor of the PSK. I think we should do this since Template Admin/Owner is a very powerful token that the provisioner daemon doesn't need.

Signed-off-by: Spike Curtis <spike@coder.com>
spikecurtisand others added3 commitsAugust 14, 2023 09:19
Co-authored-by: Muhammad Atif Ali <atif@coder.com>
Signed-off-by: Spike Curtis <spike@coder.com>
Co-authored-by: Cian Johnston <cian@coder.com>
Comment on lines +26 to +27
> 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.
Copy link
Member

@bpmctbpmctAug 16, 2023
edited
Loading

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 passingCODER_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 youcreate 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.

Copy link
ContributorAuthor

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.

Copy link
ContributorAuthor

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

Comment on lines +105 to +109
provisionerDaemon:
pskSecretName: "coder-provisioner-psk"
tags:
location: auh
kind: k8s
Copy link
Member

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 setCODER_PROVISIONER_DAEMON_PSK for the coder server. I see that codehere.

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 andwhen 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.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Thevalues.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 theservice block, etc.

I kept thepskSecretName 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.

@spikecurtisspikecurtis merged commitff9252c intomainAug 16, 2023
@spikecurtisspikecurtis deleted the spike/provisioner-chart-publish branchAugust 16, 2023 12:26
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsAug 16, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@matifalimatifalimatifali left review comments

@bpmctbpmctbpmct left review comments

@johnstcnjohnstcnjohnstcn approved these changes

@mtojekmtojekAwaiting requested review from mtojek

Assignees

@spikecurtisspikecurtis

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Helm: enable automatic creation of external provisioner deployment
4 participants
@spikecurtis@johnstcn@matifali@bpmct

[8]ページ先頭

©2009-2025 Movatter.jp