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

Single registry fails #48

Closed
hgezim opened this issue May 19, 2020 · 4 comments
Closed

Single registry fails #48

hgezim opened this issue May 19, 2020 · 4 comments
Assignees

Comments

@hgezim
Copy link

hgezim commented May 19, 2020

It really does appear to fail as #27 suggested.

      - name: Login to Amazon ECR
        id: login-ecr
        uses: aws-actions/amazon-ecr-login@v1
        with:
          registries: 1111111111
        env:
          AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ECR_ACCESS_KEY_ID }}
          AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_ECR_SECRET_ACCESS_KEY }}
          AWS_REGION: 'my-hardcoded-region-here'
      - name: Build, tag, and push image to Amazon ECR
        id: build-tag-push
        env:
          ECR_REGISTRY: ${{ steps.login-ecr.outputs.registry }}
          ECR_REPOSITORY: api
          IMAGE_TAG: ${{ github.sha }}
        run: |
          docker build -t $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG .
          docker push $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG

Replaced my actual registry id with 1s above. Not space after it.

ECR_REGISTRY ends up empty.

@hgezim
Copy link
Author

hgezim commented May 19, 2020

Only the empty registries case is handled in the code, it appears: https://github.com/aws-actions/amazon-ecr-login/blob/master/index.js#L35

@allisaurus allisaurus self-assigned this May 22, 2020
@allisaurus
Copy link
Contributor

Hi @hgezim it looks like you're right, I was able to reproduce the error with both a single registry and a comma delimited list. I'll work on putting together a fix.

@allisaurus allisaurus added the bug Something isn't working label May 22, 2020
@allisaurus
Copy link
Contributor

@hgezim sorry, it looks like I spoke too soon - I was able to reproduce a workflow failure, but what's actually failing is the step after the amazon-ecr-login which is leveraging the "output" of this action.

The expectation is that outputs.registry will only be set if the default has to be inferred. If you provide an explicit registry or multiple registries, then you'll need to specify them explicitly in any subsequent build step. For example:

...
- name: Login to Amazon ECR
      id: login-ecr
      uses: aws-actions/amazon-ecr-login@v1
      with:
        registries: 111111111111    <---- registry ID

- name: Build, tag, and push image to Amazon ECR
      id: build-image
      env:
        ECR_REGISTRY: 111111111111.dkr.ecr.us-east-1.amazonaws.com   <---- registry URI
        ECR_REPOSITORY: gha-repo-ecs
        IMAGE_TAG: ${{ github.sha }}
      run: |
        # Build a docker container and
        # push it to ECR so that it can
        # be deployed to ECS.
        docker build -t $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG .
        docker push $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG
        echo "::set-output name=image::$ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG"

That said, I understand the expectation in the single registry case may be that it's also provided as an output.

Can you confirm that where you're seeing a failure is with leveraging the output of this step (vs. a failure with login itself)? If so, should we consider this a feature request to populate the output with the registry when a single Id is provided?

@allisaurus
Copy link
Contributor

Closing b/c single registry URIs are now added to action output in v1.1.0

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

No branches or pull requests

2 participants