Skip to content
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.

Sign up for GitHub

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

fix: use aws eks get-token for kubectl auth #1590

Closed
wants to merge 5 commits into from
Closed

fix: use aws eks get-token for kubectl auth #1590

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7785562
use aws eks get-token instead of aws authenticator
olivers-xaxis Sep 17, 2021
7a27249
default to null kubectl auth command
olivers-xaxis Oct 12, 2021
5c1411b
clarify variable description and re-order auth command default args f…
olivers-xaxis Oct 12, 2021
438eacd
Merge branch 'master' into use_aws_eks_token_instead_of_authenticator
schollii Oct 12, 2021
0a4a966
Merge branch 'master' into use_aws_eks_token_instead_of_authenticator
antonbabenko Oct 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion locals.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
data "aws_region" "current" {}

locals {

# EKS Cluster
Expand Down Expand Up @@ -160,12 +162,21 @@ locals {
"t2.xlarge"
]

default_kubeconfig_aws_auth_args = [
"--region",
data.aws_region.current.name,
"eks",
"get-token",
"--cluster-name",
var.cluster_name
]

kubeconfig = var.create_eks ? templatefile("${path.module}/templates/kubeconfig.tpl", {
kubeconfig_name = coalesce(var.kubeconfig_name, "eks_${var.cluster_name}")
endpoint = local.cluster_endpoint
cluster_auth_base64 = local.cluster_auth_base64
aws_authenticator_command = var.kubeconfig_aws_authenticator_command
aws_authenticator_command_args = coalescelist(var.kubeconfig_aws_authenticator_command_args, ["token", "-i", local.cluster_name])
aws_authenticator_command_args = coalescelist(var.kubeconfig_aws_authenticator_command_args, local.default_kubeconfig_aws_auth_args)
aws_authenticator_additional_args = var.kubeconfig_aws_authenticator_additional_args
aws_authenticator_env_variables = var.kubeconfig_aws_authenticator_env_variables
}) : ""
Expand Down
2 changes: 1 addition & 1 deletion variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ variable "kubeconfig_aws_authenticator_env_variables" {
variable "kubeconfig_name" {
Copy link
Contributor

Choose a reason for hiding this comment

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

would you update kubeconfig_aws_authenticator_command default value as well?

Copy link
Author

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you clarify, please, why is it required?

Copy link
Contributor

Choose a reason for hiding this comment

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

probably because:

> coalescelist([""], ["c", "d"])
[
  "",
]
>

from line

terraform-aws-eks/locals.tf

Line 179 in 7785562

aws_authenticator_command_args = coalescelist(var.kubeconfig_aws_authenticator_command_args, local.default_kubeconfig_aws_auth_args)

Copy link
Author

@schollii schollii Oct 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

The 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".

Copy link
Contributor

Choose a reason for hiding this comment

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

actually coalesce works with both "" and null

yes but it will give different results (from terraform console):

> coalescelist([""], ["c", "d"])
[
  "",
]
> coalescelist([], ["c", "d"])
[
  "c",
  "d",
]

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh, but here is just normal string

Copy link
Author

Choose a reason for hiding this comment

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

yes the function being discussed is coalesce not coalescelist

}

variable "cluster_create_timeout" {
Expand Down