-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
fix: use aws eks get-token for kubectl auth #1590
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
Changes from all commits
7785562
7a27249
5c1411b
438eacd
0a4a966
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 | ||
|---|---|---|---|---|
|
|
@@ -193,13 +193,13 @@ variable "workers_additional_policies" { | |||
| } | ||||
|
|
||||
| variable "kubeconfig_aws_authenticator_command" { | ||||
| description = "Command to use to fetch AWS EKS credentials." | ||||
| description = "Command to use to fetch AWS EKS credentials. Defaults to aws CLI (will use eks command)." | ||||
| type = string | ||||
| default = "aws-iam-authenticator" | ||||
| default = null | ||||
| } | ||||
|
|
||||
| variable "kubeconfig_aws_authenticator_command_args" { | ||||
| description = "Default arguments passed to the authenticator command. Defaults to [token -i $cluster_name]." | ||||
| description = "Default arguments passed to the authenticator command. Defaults to [eks get-token ...]." | ||||
| type = list(string) | ||||
| default = [] | ||||
| } | ||||
|
|
@@ -219,7 +219,7 @@ variable "kubeconfig_aws_authenticator_env_variables" { | |||
| variable "kubeconfig_name" { | ||||
|
Contributor
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. would you update
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. Fixed |
||||
| description = "Override the default name used for items kubeconfig." | ||||
| type = string | ||||
| default = "" | ||||
| default = null | ||||
|
Contributor
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. Could you clarify, please, why is it required?
Contributor
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. probably because: from line Line 179 in 7785562
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. actually coalesce works with both "" and null, BUT it is almost always preferable to use null than empty string to indicate "no value set".
Contributor
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 but it will give different results (from
Contributor
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. ahh, but here is just normal string
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. yes the function being discussed is |
||||
| } | ||||
|
|
||||
| variable "cluster_create_timeout" { | ||||
|
|
||||
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.
And remove
coalesce()where this variable is used. Right?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.
@schollii do you will be able to change it and then I will make testing