-
Notifications
You must be signed in to change notification settings - Fork 453
Add --token-env flag to install command
#6755
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,7 +80,7 @@ func NewWorkerCmd() *cobra.Command { | |
| c.TokenArg = args[0] | ||
| } | ||
|
|
||
| getBootstrapKubeconfig, err := kubeconfigGetterFromJoinToken(c.TokenFile, c.TokenArg) | ||
| getBootstrapKubeconfig, err := kubeconfigGetterFromJoinToken(c.TokenFile, c.TokenEnv, c.TokenArg) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -150,12 +150,23 @@ func GetNodeName(opts *config.WorkerOptions) (apitypes.NodeName, stringmap.Strin | |
| return nodeName, kubeletExtraArgs, nil | ||
| } | ||
|
|
||
| func kubeconfigGetterFromJoinToken(tokenFile, tokenArg string) (clientcmd.KubeconfigGetter, error) { | ||
| func kubeconfigGetterFromJoinToken(tokenFile, tokenEnv, tokenArg string) (clientcmd.KubeconfigGetter, error) { | ||
| tokenSources := 0 | ||
| if tokenArg != "" { | ||
| if tokenFile != "" { | ||
| return nil, errors.New("you can only pass one token argument either as a CLI argument 'k0s worker [token]' or as a flag 'k0s worker --token-file [path]'") | ||
| } | ||
| tokenSources++ | ||
| } | ||
| if tokenFile != "" { | ||
| tokenSources++ | ||
| } | ||
| if tokenEnv != "" { | ||
| tokenSources++ | ||
| } | ||
|
|
||
| if tokenSources > 1 { | ||
| return nil, errors.New("you can only pass one token source: either as a CLI argument 'k0s worker [token]', via '--token-file [path]', or via '--token-env [var]'") | ||
| } | ||
|
|
||
| if tokenArg != "" { | ||
| kubeconfig, err := loadKubeconfigFromJoinToken(tokenArg) | ||
| if err != nil { | ||
| return nil, err | ||
|
|
@@ -166,12 +177,28 @@ func kubeconfigGetterFromJoinToken(tokenFile, tokenArg string) (clientcmd.Kubeco | |
| }, nil | ||
| } | ||
|
|
||
| if tokenEnv != "" { | ||
| tokenValue := os.Getenv(tokenEnv) | ||
| if tokenValue == "" { | ||
| return nil, fmt.Errorf("environment variable %q is not set or is empty", tokenEnv) | ||
| } | ||
|
|
||
| kubeconfig, err := loadKubeconfigFromJoinToken(tokenValue) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return func() (*clientcmdapi.Config, error) { | ||
| return kubeconfig, nil | ||
| }, nil | ||
| } | ||
|
|
||
| if tokenFile == "" { | ||
| return nil, nil | ||
| } | ||
|
|
||
| return func() (*clientcmdapi.Config, error) { | ||
| return loadKubeconfigFromTokenFile(tokenFile) | ||
| return loadKubeconfigFromToken(tokenFile) | ||
| }, nil | ||
| } | ||
|
|
||
|
|
@@ -193,7 +220,7 @@ func loadKubeconfigFromJoinToken(tokenData string) (*clientcmdapi.Config, error) | |
| return kubeconfig, nil | ||
| } | ||
|
|
||
| func loadKubeconfigFromTokenFile(path string) (*clientcmdapi.Config, error) { | ||
| func loadKubeconfigFromToken(path string) (*clientcmdapi.Config, error) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's still reading a token file, so we could keep the old name. |
||
| var problem string | ||
| tokenBytes, err := os.ReadFile(path) | ||
| if errors.Is(err, os.ErrNotExist) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,6 +70,7 @@ type WorkerOptions struct { | |
| Labels map[string]string | ||
| Taints []string | ||
| TokenFile string | ||
| TokenEnv string | ||
| TokenArg string | ||
| WorkerProfile string | ||
| IPTablesMode string | ||
|
|
@@ -251,6 +252,7 @@ func GetWorkerFlags() *pflag.FlagSet { | |
| flagset.StringVar(&workerOpts.WorkerProfile, "profile", defaultWorkerProfile, "worker profile to use on the node") | ||
| flagset.BoolVar(&workerOpts.CloudProvider, "enable-cloud-provider", false, "Whether or not to enable cloud provider support in kubelet") | ||
| flagset.StringVar(&workerOpts.TokenFile, "token-file", "", "Path to the file containing join-token.") | ||
| flagset.StringVar(&workerOpts.TokenEnv, "token-env", "", "Environment variable name containing the join-token.") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather not add an additional flag for this. Instead, I'd simply check for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hiding flag means, extra document section about secret environ values
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we need to document that. |
||
| flagset.VarP((*logLevelsFlag)(&workerOpts.LogLevels), "logging", "l", "Logging Levels for the different components") | ||
| flagset.Var((*cliflag.ConfigurationMap)(&workerOpts.Labels), "labels", "Node labels, list of key=value pairs") | ||
| flagset.StringSliceVarP(&workerOpts.Taints, "taints", "", []string{}, "Node taints, list of key=value:effect strings") | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Personally, I think this is a very unintuitive thing to do. By nature, an environment variable is ephemeral and belongs to the current process only. Capturing it in a file is not something I'd want to happen with it. Moreover, this creates a discrepancy between the arguments passed to the
install controller/workercommand and the arguments passed tocontroller/workercommand, which we have managed to avoid so far. If folks need to persist the token, then they can just use--token-filewithout surprises.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.
Are you fine with keeping temporary bootstrap token inside init system file instead of keeping pointing it to a file?
And no, there are use cases that do not allow just using "--token-file without surprises"
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.
@makhov pointed out that there's a straight forward solution which wouldn't require any special-handling at all here: We could simply use
k0s install -e K0S_JOIN_TOKEN controller [...]and be done with it. That would store the token somewhere in the init system config, and the behavior would be much more obvious. /cc #6763There 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.
yes, I am working on this RN, ETA PR today
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.
alternative PR #6766