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

feat: Added support for client.authentication.k8s.io/v1beta1 #1550

Merged
merged 4 commits into from
Nov 2, 2021

Conversation

vsamidurai
Copy link
Contributor

PR o'clock

Description

  • Make kubeconfig_api_version configurable in templates kubeconfig.tpl

Please explain the changes you made here and link to any relevant issues.

Recently aws-cli switched to client.authentication.k8s.io/v1beta from v1alpha1 and reverted back to v1alpha1. So this PR makes kubeconfig API version configurable to support Kubernetes v1.22.

aws/aws-cli#6309

#1526

Checklist

@daroga0002
Copy link
Contributor

background around this documented here #1526

@daroga0002
Copy link
Contributor

@vsamidurai do you think this should be variable?
I have a mixed feeling here as most probably this variable will be never customised and is increasing a module inputs complexity. I would rather opt to change this permanently in template and not use variable for this.

With k8s 1.22 variable will need to be always changed to beta api so we will not avoid a impacting change which will happen sooner or later.

I think as of now this can be just parked for a while and merged when there will be other change modifying a launch configuration to release together.

@daroga0002
Copy link
Contributor

resolves #1526

@stale
Copy link

stale bot commented Oct 9, 2021

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
To track this PR (even if closed), please open a corresponding issue if one does not already exist.

@stale stale bot added the stale label Oct 9, 2021
@antonbabenko
Copy link
Member

@daroga0002 Should we merge this one? If yes, we need to fix the formatting and rebase it.

@stale stale bot removed the stale label Oct 9, 2021
…pl to support client.authentication.k8s.io/v1beta
@vsamidurai
Copy link
Contributor Author

vsamidurai commented Oct 11, 2021

@vsamidurai do you think this should be variable? I have a mixed feeling here as most probably this variable will be never customised and is increasing a module inputs complexity. I would rather opt to change this permanently in template and not use variable for this.

With k8s 1.22 variable will need to be always changed to beta api so we will not avoid a impacting change which will happen sooner or later.

I think as of now this can be just parked for a while and merged when there will be other change modifying a launch configuration to release together.

I thought this change may block, incase if anyone wants to use the earlier version of k8s with the old version of aws-cli. If you think the new version terraform-eks only supports clusters above 1.22 we can use hardcoded values than the optional parameters.

@vsamidurai
Copy link
Contributor Author

@daroga0002 Should we merge this one? If yes, we need to fix the formatting and rebase it.

rebased and formatted PR.

@daroga0002
Copy link
Contributor

I will review this PR and test soon

Some usefull link to understand also issue is kubernetes-sigs/aws-iam-authenticator@0221afb#diff-b03d5162238d36a569ac0c110484bf356f617e22967aeb1af853b02993da60b8

Copy link
Contributor

@daroga0002 daroga0002 left a comment

Choose a reason for hiding this comment

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

@vsamidurai thank you for contribution 🎉

I have tested and this is working:

$ diff kubeconfig_lt_with_mng-xunVNPIS kubeconfig_lt_with_mng-xunVNPIS_test
23c23
<       apiVersion: client.authentication.k8s.io/v1beta1
---
>       apiVersion: client.authentication.k8s.io/v1alpha1

so we can merge this.

@daroga0002
Copy link
Contributor

@antonbabenko lets merge this to master

@antonbabenko
Copy link
Member

antonbabenko commented Nov 2, 2021

@vsamidurai Please fix the failing checks before the merge. I have fixed it.

@antonbabenko antonbabenko changed the title improvement: support for client.authentication.k8s.io/v1beta1 feat: Added support for client.authentication.k8s.io/v1beta1 Nov 2, 2021
@antonbabenko antonbabenko merged commit ed048f3 into terraform-aws-modules:master Nov 2, 2021
@antonbabenko
Copy link
Member

Here we go! 🎉

v17.23.0 has been just released.

@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 12, 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.

3 participants