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

Change docker_username and docker_password output variables for ECR private repositories #483

Open
pascalgulikers opened this issue Jul 28, 2023 · 4 comments
Labels
feature-request A feature should be added or improved.

Comments

@pascalgulikers
Copy link

pascalgulikers commented Jul 28, 2023

Is your feature request related to a problem? Please describe.
We're having a reusable workflow in which we're pulling a base image from ECR from multiple AWS accounts. Passing the credentials to another job is hard, since the output variable is dynamic depending on the account on which the calling repo is authenticated to. I.e.

job-1:
    environment: ${{ needs.Initialize.outputs.environment }}
    runs-on: ubuntu-latest
    needs: [Initialize, Setup]
    permissions:
      contents: write
      packages: write
      pull-requests: write
      # This is used to complete the identity challenge
      # with sigstore/fulcio when running outside of PRs.
      id-token: write
    outputs:
      registry: ${{ steps.login-ecr.outputs.registry }}
### The following 2 lines of code isn't working as it seems
      docker_username: steps.login-ecr.outputs.docker_username_${{ needs.Setup.outputs.accountId }}_dkr_ecr_eu_central_1_amazonaws_com
      docker_password: steps.login-ecr.outputs.docker_password_${{ needs.Setup.outputs.accountId }}_dkr_ecr_eu_central_1_amazonaws_com
    steps:
      - name: 'Configure AWS Credentials'
        uses: aws-actions/configure-aws-credentials@v2
        with:
          aws-region: ${{ inputs.region }}
          role-to-assume: arn:aws:iam::${{ needs.Setup.outputs.accountId }}:role/${{ inputs.ghaIamRolePrefix }}-${{ github.event.repository.name }}
          role-session-name: GitHubActions
      - name: Login to Amazon ECR
        id: login-ecr
        uses: aws-actions/amazon-ecr-login@v1
        
job-2:
    environment: ${{ needs.Initialize.outputs.environment }}
    runs-on: [self-hosted, X64, Linux, default ]
    needs: [Initialize, Setup, job-1]
    container: 
      image: ***.dkr.ecr.eu-central-1.amazonaws.com/our_custom_baseimage:latest
      credentials:
        username: ${{ needs.job-1.outputs.docker_username }}
        password: ${{ needs.job-1.outputs.docker_password }}

Describe the solution you'd like
Change the output variables for a private ECR to docker_username_private_ecr_aws and docker_password_private_ecr_aws respectively instead of a dynamic generated output variable.

@ChristopherHX
Copy link

You have to vote / create a GitHub Actions feature request (https://github.com/orgs/community/discussions/categories/actions) for sharing secrets via job outputs. If these values are masked the actions/runner skips sending the outputs and if not they are leaked at least while step debugging is on.

job-1:
    environment: ${{ needs.Initialize.outputs.environment }}
    runs-on: ubuntu-latest
    needs: [Initialize, Setup]
    permissions:
      contents: write
      packages: write
      pull-requests: write
      # This is used to complete the identity challenge
      # with sigstore/fulcio when running outside of PRs.
      id-token: write
    outputs:
      registry: ${{ steps.login-ecr.outputs.registry }}
### The following 2 lines of code are trying to leak secret data to non secret storage
      docker_username: ${{ steps.login-ecr.outputs[format('docker_username_{0}_dkr_ecr_eu_central_1_amazonaws_com', needs.Setup.outputs.accountId)] }}
      docker_password: ${{ steps.login-ecr.outputs[format('docker_password_{0}_dkr_ecr_eu_central_1_amazonaws_com', needs.Setup.outputs.accountId)] }}
    steps:
      - name: 'Configure AWS Credentials'
        uses: aws-actions/configure-aws-credentials@v2
        with:
          aws-region: ${{ inputs.region }}
          role-to-assume: arn:aws:iam::${{ needs.Setup.outputs.accountId }}:role/${{ inputs.ghaIamRolePrefix }}-${{ github.event.repository.name }}
          role-session-name: GitHubActions
      - name: Login to Amazon ECR
        id: login-ecr
        uses: aws-actions/amazon-ecr-login@v1
        
job-2:
    environment: ${{ needs.Initialize.outputs.environment }}
    runs-on: [self-hosted, X64, Linux, default ]
    needs: [Initialize, Setup, job-1]
    container: 
      image: ***.dkr.ecr.eu-central-1.amazonaws.com/our_custom_baseimage:latest
      credentials:
        username: ${{ needs.job-1.outputs.docker_username }}
        password: ${{ needs.job-1.outputs.docker_password }}

@pascalgulikers
Copy link
Author

Thanks @ChristopherHX , also for the output construct (much better than my jq statement). I've put in a FR now: https://github.com/orgs/community/discussions/62269

@ChristopherHX
Copy link

The ::add-mask:: command works as long it is used as it has been designed and all escaping rules are correctly applied. Only this action can mask it's outputs, any attempt to do it later is going to leak the output to the console.

Seems like you have created two feature requests in one discussion.

The maintainer of this repo may consider to add a core.setSecret (alias ::add-mask:: with escaping rules for ln, cr and percent) for the output value before settting the output

amazon-ecr-login/index.js

Lines 172 to 173 in c2d6bdb

core.setOutput(`${OUTPUTS.dockerUsername}_${secretSuffix}`, creds[0]);
core.setOutput(`${OUTPUTS.dockerPassword}_${secretSuffix}`, creds[1]);

As of this time your usecase will blow up as described above and others may depend on unmasked outputs due to the GitHub Actions limitation of beeing unable to share secrets via job outputs.

@marekaf
Copy link

marekaf commented Sep 6, 2024

${{ steps.login-ecr.outputs[format('docker_password_{0}_dkr_ecr_eu_central_1_amazonaws_com', needs.Setup.outputs.accountId)] }}

omg thank you! this really saved me, finally some working workaround

it would be great to add this to README
or yet better to simplify the action output to something like docker_password_private_ecr or anything static and simple. This is just unnecessary complexity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

3 participants