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.

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

chore: change default aws authenticator command to aws eks get-token instead of aws-iam-authenticator #1699

Closed
wants to merge 3 commits into from

Conversation

lisfo4ka
Copy link
Contributor

Description

This PR changes default aws authenticator command to aws eks get-token instead of aws-iam-authenticator since it's already deprecated. Fixes #957.

The next step is to drop aws-iam-authenticator support from the terraform-aws-eks module.

Motivation and Context

Fixes #957.

Breaking Changes

No breaking changes here, just kubconfig file updates.

How Has This Been Tested?

EKS cluster was deployed from the master branch and updated with the suggested changes successfully.
Pre-commit hook was run.

@schollii
Copy link

LGTM

@bryantbiggs
Copy link
Member

this all looks good and makes sense to me, but unfortunately I think its still a breaking change because we have changed the default behavior. are we able to keep the current behavior (aws-iam-authenticator) as the default, but provide users the ability to use the aws eks get-token if they so choose? I think that would make this change very easy to merge in without disruption

@schollii
Copy link

schollii commented Nov 26, 2021

I guess it will be a breaking change for those who had customized their iam authenticator args, but for everyone else it's an implementation detail.

If the current PR Is going into a minor release, I agree we have to honor backwards compatibility.

We have to wait for next major to make it the default.

At that point, we could also adjust the code so that one flag makes tf use the iam authenticator (and its associated variables).

So whoever has overridden the args for iam authenticator and wants to continue using it after the major update, can do so with one toggle switch. Automated updates on major release, with customizations for iam auth args, will likely fail, but automated updates on major is looking for trouble.

@lisfo4ka
Copy link
Contributor Author

At the moment, it's already possible to use aws eks get-token by defining kubeconfig_aws_authenticator_command and kubeconfig_aws_authenticator_command_args input variables.
So it doesn't require any new changes to release that. Something like that:

  kubeconfig_aws_authenticator_command         = var.kubeconfig_aws_authenticator_command
  kubeconfig_aws_authenticator_command_args    = var.kubeconfig_aws_authenticator_command == "aws" ? ["eks", "get-token", "--cluster-name", element(concat(data.aws_eks_cluster_auth.cluster[*].name, [""]), 0)] : []
  kubeconfig_aws_authenticator_additional_args = ["--role-arn", "arn:aws:iam::012345678910:role/EKSDeployerRole"]
  

But I believe that everyone who keep terraform-aws-eks module up to date and use v.17 have already updated awc cli to the version 1.16.156 or later since it was introduced 3 years ago.
For that users there are two possible cases:

  • they will see updates in the kubeconfig file if they've used default aws-iam-authenticator, but still can apply the changes and authenticate to the cluster with no additional steps.
    If they intentionally want to use use aws-iam-authenticator there is still possible to re-define defaults.

  • they've already defined specified kubeconfig_aws_authenticator variables to use eks get-token and it will take precedence over our changes. As a result, terraform wouldn't suggest any changes at all.
    I believe that is our target audience for the moment.

In case, the user use aws cli version before 1.16.156 it will really be a breaking change. But I'm sure there are no many people who use it. And we shouldn't focus on this since it's too outdated.

So I agree that it can lead to some changes during terraform module minor version upgrade. But I think it's quite enough to leave some details in the CHANGELOG.md to notify everybody about possible changes.
The official AWS documentation proposes to use native aws eks get-token auth instead of aws-iam-authenticator tool, and our default values for aws-iam-authenticator is really confusing.
https://docs.aws.amazon.com/eks/latest/userguide/managing-auth.html
https://docs.aws.amazon.com/eks/latest/userguide/install-aws-iam-authenticator.html

From my side, I'd prefer to introduce this before v18 since there would be too many breaking changes and not everybody are ready to install them.

@github-actions
Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@antonbabenko
Copy link
Member

This issue has been resolved in version 18.0.0 🎉

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use "aws eks get-token" instead of "aws-iam-authenticator"
4 participants