-
Notifications
You must be signed in to change notification settings - Fork 637
🌱 Migrate sts to sdk v2 #5601
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
🌱 Migrate sts to sdk v2 #5601
Conversation
|
/test pull-cluster-api-provider-aws-e2e-eks |
302e252 to
e8f4355
Compare
|
/test pull-cluster-api-provider-aws-e2e-eks |
|
/retest Couldn't allocate a testing account |
|
Really only one change that I can see that's needed, thanks! |
|
/test pull-cluster-api-provider-aws-e2e-eks |
|
I'm running into an issue where connecting EKS clusters is failing after the change. I suspect it's related to the token that's being generated. I'm not an expert in EKS internals, but I noticed a difference in how the AWS SDK v1 vs v2 generates the token:
|
|
Hm maybe @punkwalker can help @alexander-demicev ? |
I will take a look at how CAPA does token generation but here is some reference - https://github.com/kubernetes-sigs/aws-iam-authenticator/blob/325c38ccbbd395e3bf612205809af77fe3f923ce/pkg/token/token.go#L227 With V2, STS uses regional endpoint so the generated token should target regional endpoint. Also, STS token expiration is default 15 mins with V2 as well. |
|
@alexander-demicev I suspect issue with this custom middleware - https://github.com/alexander-demicev/cluster-api-provider-aws/blob/e8f43555098a3c7d88707dd50d14b377158e619f/pkg/cloud/services/eks/config.go#L303 Instead, can we use this pattern for adding header? |
Signed-off-by: Alexandr Demicev <[email protected]>
e8f4355 to
df25a52
Compare
|
/test pull-cluster-api-provider-aws-e2e-eks |
|
Invalid token flake: /test pull-cluster-api-provider-aws-e2e-eks |
|
/test pull-cluster-api-provider-aws-e2e |
1 similar comment
|
/test pull-cluster-api-provider-aws-e2e |
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.
|
LGTM label has been added. Git tree hash: 019c0dcf6215d0f88dc7ae8ae2c17b355ceef87f
|
|
Thanks for this @alexander-demicev /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: richardcase The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
What type of PR is this?
/kind cleanup
/kind deprecation
What this PR does / why we need it:
This PR migrates sts to sdk v2. There are still some leftovers in identity package that can only be removed once all SDK migration is done.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #5413
Special notes for your reviewer:
Checklist:
Release note: