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

[assets] set up docker login when docker base image is in ecr #11544

Closed
nija-at opened this issue Nov 18, 2020 · 16 comments
Closed

[assets] set up docker login when docker base image is in ecr #11544

nija-at opened this issue Nov 18, 2020 · 16 comments
Assignees
Labels
@aws-cdk/assets Related to the @aws-cdk/assets package effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. p2

Comments

@nija-at
Copy link
Contributor

nija-at commented Nov 18, 2020

When the base image in the Dockerfile is an image from a private ECR repository, run the docker login command as part of asset creation during synthesis.

The command looks something like

aws ecr get-login-password | docker login --username AWS --password-stdin <ecr repo url>

This is a 🚀 Feature Request

@nija-at nija-at added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Nov 18, 2020
@github-actions github-actions bot added the @aws-cdk/assets Related to the @aws-cdk/assets package label Nov 18, 2020
@erudisch
Copy link

erudisch commented Nov 20, 2020

The way to do the login directly via a command execution before the creation of the pipeline:

import * as cp from 'child_process';

cp.execSync('aws ecr get-login-password | docker login --username AWS --password-stdin <ecr repo url>');

fails with the error message:


An error occurred (AccessDeniedException) when calling the GetAuthorizationToken operation: User: arn:aws:sts::<user-id>:assumed-role/InfrastructureStack-CdkPipelineBuildSynthCdkBuildP-1Q0CJAMRUF46N/AWSCodeBuild-a11b21c5-96f2-400b-960d-2383afedc067 is not authorized to perform: ecr:GetAuthorizationToken on resource: *
--
Error: Cannot perform an interactive login from a non TTY device

To modify the synthCommand in the SimpleSynthAction like this:

aws ecr get-login-password --region ${props?.env?.region} | docker login --username AWS --password-stdin <ecr repo url> && npx cdk synth

also doesn't solve the problem. It gets kind of ignored.

There is also something like this available:

const repo = ecr.Repository.fromRepositoryAttributes(this, 'python-repo', {
            repositoryName: <repo-name>,
            repositoryArn: <repo-arn>,
});
repo.grantPull(synthAction)

But here I don't know how to set the credentials for the repo or if this is even a valid way to handle repos in cdk.

Setting the policy to allow the pull would also not work:

const synthAction = pipelines.SimpleSynthAction.standardNpmSynth({
            sourceArtifact,
            cloudAssemblyArtifact,
            rolePolicyStatements: [
                new iam.PolicyStatement({
                    actions: [
                      'ecr:GetAuthorizationToken',
                      'ecr:GetDownloadUrlForLayer',
                      'ecr:BatchGetImage',
                    ],
                    resources: ['*'],
                    sid: 'AllowECRLoginAndPull',
                  })
            ],
            subdirectory: 'infrastructure',
            buildCommand: 'aws ecr get-login-password --region eu-west-1 | docker login --username AWS --password-stdin <ecr-repo> && npm run build',
            environment: {
                privileged: true,
            }
        });

This results in the error:

Resolution error: Cannot read property 'roleName' of undefined.

But according to the pr #9527 at least the setup of the policy should work.

At the moment our pipeline is stuck because of the docker rate limit.

@erudisch
Copy link

erudisch commented Nov 21, 2020

Could please provide an example on how to implement ecr pull via ecs in cdk. Basically we need to be able to allow the synthAction to be able to pull the the image from ecr.

See this

@eladb
Copy link
Contributor

eladb commented Nov 25, 2020

Before this is implemented, you should be able to login to ECR before running cdk synth using the above command.

@eladb eladb added effort/medium Medium work item – several days of effort p2 labels Nov 25, 2020
@erudisch
Copy link

Hello @eladb

Thank you for your answer. But we need to do the login inside the pipeline. Locally it's working fine and we are able to login before cdk synth.

But when we try to do it before the synth step inside the pipeline, we're getting this error:


Running command aws ecr get-login-password --region eu-west-1 | docker login --username AWS --password-stdin <user-id>.dkr.ecr.eu-west-1.amazonaws.com/<repo> && npx cdk synth
--

An error occurred (AccessDeniedException) when calling the GetAuthorizationToken operation: User: arn:aws:sts::548874010033:assumed-role/InfrastructureStack-CdkPipelineBuildSynthCdkBuildP-GEI946HAAHM9/AWSCodeBuild-decde1c9-60e9-44a7-b60f-7b70f1279e09 is not authorized to perform: ecr:GetAuthorizationToken on resource: *
Error: Cannot perform an interactive login from a non TTY device

So is there a way to login during the pipeline is running?

@eladb
Copy link
Contributor

eladb commented Nov 25, 2020

@rix0rrr any idea why rolePolicyStatements doesn't work in that case?

@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 25, 2020

There are some issues are being confused here.

Can't use ECR images as an asset base

Asset building happens in an Asset CodeBuild job, and you have no control over the commands that get run there. Changing the command in the Synth job will not affect the command that gets run in the asset job.

There is no way to run docker login right now. There is no way to tell CDK what the ECR repository is that you need pull access to, so there is no way for CDK to automatically update the IAM policy, and also no way for you to manually control that IAM policy.

Cannot read property 'roleName' of undefined.

You reported you get this error if you specify rolePolicyStatements as part of the synth action. Apart from that not being the right job (as mentioned in the previous paragraph), this should not be happening.

It seems we have a test for that, so I'm mystified why that wouldn't work for you. Can you paste a stack trace?

@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 25, 2020

Duplicate of #10999

Semi-related #11425

@rix0rrr rix0rrr added this to the [GA] CDK Pipelines milestone Nov 25, 2020
@erudisch
Copy link

Hello

We were finally able to pull from our own ecr. But we needed to set this policy manually in our pipeline-role via iam in management console:

                 new iam.PolicyStatement({
                    actions: [
                      'ecr:GetAuthorizationToken',
                      'ecr:GetDownloadUrlForLayer',
                      'ecr:BatchGetImage',
                    ],
                    resources: ['*'],
                    sid: 'AllowECRLoginAndPull',
                  })

and change the synthCommand in the synthAction like this:

synthCommand: `docker login -u AWS -p $(aws ecr get-login-password --region eu-west-1) <user-id>.dkr.ecr.eu-west-1.amazonaws.com && npx cdk synth`,

Now we are getting confused. If we remove the manually added ecr-policy from our role and set it programmatically and run cdk deploy it also works.

Maybe the role problem got fixed in the meantime? Or is this kind of a caching problem?

Regards
Erik

@eladb
Copy link
Contributor

eladb commented Nov 25, 2020

@erudisch let's move the conversation to the duplicate issue #10999

@eladb eladb closed this as completed Nov 25, 2020
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@erudisch
Copy link

erudisch commented Nov 25, 2020

@eladb but now both issues are closed...

@eladb
Copy link
Contributor

eladb commented Nov 25, 2020

Which issues?

@erudisch
Copy link

sorry, i mean this one #10999. It's also closed, so where to continue this discussion?

@eladb
Copy link
Contributor

eladb commented Nov 25, 2020

#10999 is open as far as I can tell.

@erudisch
Copy link

sorry, my fault ;-). checked the wrong tag

@david-fractiunate
Copy link

+1 Push, we need this.... CDK Docker Assets are not usable without it for our team.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/assets Related to the @aws-cdk/assets package effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. p2
Projects
None yet
Development

No branches or pull requests

5 participants