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

ecr: switch implementation to use the AWS SDK #126

Merged
merged 2 commits into from Dec 20, 2021

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Dec 19, 2021

Follow-up #114 (review)
Fixes #125

@Flydiverny Now using the AWS SDK so we don't need to rely on the AWS CLI.

With this we are also able to authenticate on Windows runners.

crazy-max added 2 commits Dec 20, 2021
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max crazy-max merged commit b20b9f5 into docker:master Dec 20, 2021
2 checks passed
@crazy-max crazy-max deleted the aws-sdk branch Dec 20, 2021
Copy link
Contributor

@Flydiverny Flydiverny left a comment

Nice!
I tested docker/login-action@42ba9a1dc8ea3ab85b1c32bdcb2bff18ba03c94f (before last force-push) on our self-hosted (linux) runner without providing any input credentials and it worked as expected for that use case 👍

I was thinking maybe we could use the v3 aws-sdk lib instead of the v2 one, ie @aws-sdk/client-ecr and @aws-sdk/client-ecr-public. I tried to make these changes locally but for some reason docker buildx bake validation fails saying there's diffs to output. Seems to be CRLF vs LF issue 😅
It seemed to reduce the dist size a bit since v3 sdk is modular.

@Flydiverny
Copy link
Contributor

Flydiverny commented Dec 20, 2021

#128 if you're curious

@aramalipoor
Copy link

aramalipoor commented Dec 20, 2021

FYI from ~1 hour when this PR is merged our pipelines started failing to login to AWS ECR, it could be related to the changes here.

image

Using exact version v1.10.0 is working.

@crazy-max
Copy link
Member Author

crazy-max commented Dec 20, 2021

@aramalipoor Can you add a comment with your workflow in #129 please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants