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

oci/auth: Fix aws login with aws-sdk-go-v2 #432

Merged
merged 1 commit into from
Dec 14, 2022
Merged

Conversation

darkowlzz
Copy link
Contributor

@darkowlzz darkowlzz commented Dec 13, 2022

Follow-up of 750df67 , upgrade to aws-sdk-go-v2.

Without this change, the login fails with error:

operation error ECR: GetAuthorizationToken, https response error StatusCode: 400, RequestID: 49826b0a-de68-411d-acb1-3fd1554c64b4, api error MissingAuthenticationTokenException: Missing Authentication Token

As per aws-sdk-go-v2 migration docs, new config for creating a session should use config.LoadDefaultConfig() and account ID is not needed for ECR authorization token.

In order to maintain the auth API compatibility, which is shared with other provider, return an empty client for NewClient(). The login implementation loads the default config if the client config is nil. A test can stub the client config before calling login.

Tested this against EKS using the integration tests with the EKS terraform module fix from fluxcd/test-infra#2.

@darkowlzz darkowlzz added the area/oci OCI related issues and pull requests label Dec 13, 2022
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @darkowlzz

@darkowlzz darkowlzz force-pushed the oci-auth-aws-fix branch 2 times, most recently from 8856b2f to 3bb4cd4 Compare December 14, 2022 10:49
Copy link
Member

@pjbgf pjbgf left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for the thread safety improvements @darkowlzz! 🙇

As per aws-sdk-go-v2 docs, new config for creating a session should use
config.LoadDefaultConfig() and account ID is not needed for ECR
authorization token.

In order to maintain the auth API which is shared with other provider,
return an empty client for NewClient(). The login implementation loads
the default config if the client config is nil. A test can stub the
client config before calling login.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/oci OCI related issues and pull requests
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants