Skip to content

Handle Amazon ECR registries associated with other accounts - #40

Merged
crazy-max merged 2 commits into
docker:masterfrom
crazy-max:registry-ids
Dec 18, 2020
Merged

Handle Amazon ECR registries associated with other accounts#40
crazy-max merged 2 commits into
docker:masterfrom
crazy-max:registry-ids

Conversation

@crazy-max

@crazy-max crazy-max commented Dec 16, 2020

Copy link
Copy Markdown
Member

Fixes #39
Fixes docker/build-push-action#253

cc. @michaelb990

@davidski Can be tested with:

name: ci

on:
  push:
    branches: master

jobs:
  login:
    runs-on: ubuntu-latest
    steps:
      -
        name: Login to ECR
        uses: crazy-max/docker-login-action@registry-ids
        with:
          registry: <aws-account-number>.dkr.ecr.<region>.amazonaws.com
          username: ${{ secrets.AWS_ACCESS_KEY_ID }}
          password: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
        env:
          AWS_ACCOUNT_IDS: 012345678910,023456789012

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max
crazy-max marked this pull request as draft December 16, 2020 20:59
@michaelb990

michaelb990 commented Dec 16, 2020

Copy link
Copy Markdown

It might be easier to write a function in the aws.ts similar to getRegion that just parses the registry URL for the account ID (it will always be the first token in the URL), this way users don't have to pass the registry ID twice.

https://github.com/docker/login-action/blob/master/src/aws.ts#L17

Either way, thanks for the quick turnaround! 🎉

@crazy-max

Copy link
Copy Markdown
Member Author

@michaelb990 Ok got it! I've made some changes, let me know if it lgty. Thanks again!

@crazy-max
crazy-max marked this pull request as ready for review December 17, 2020 05:51

@michaelb990 michaelb990 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Added 1 comment. Looks good other than that.

Comment thread src/aws.ts Outdated
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>

@michaelb990 michaelb990 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@crazy-max
crazy-max merged commit f336459 into docker:master Dec 18, 2020
@crazy-max
crazy-max deleted the registry-ids branch December 18, 2020 06:41
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.

Login to cross-account ECR registry not working with AWS CLI v1 / ubuntu < 20 Unable to push to cross-account AWS ECR registry

2 participants