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

(bootstrap): Bootstrapping with CDK v1.139.0 removes IAM policy from the ECR asset repository #18473

Closed
MousaZeidBaker opened this issue Jan 17, 2022 · 5 comments · Fixed by #19446
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p1 package/tools Related to AWS CDK Tools or CLI

Comments

@MousaZeidBaker
Copy link
Contributor

MousaZeidBaker commented Jan 17, 2022

What is the problem?

Bootstrapping from 1.138.0 to v1.139.0 removes IAM policy from the cdk-hnb659fds-container-assets-1234567890-eu-central-1 ECR repository resulting in Lambdas being unable to run due to not able to access their images. The following policy was removed:

{
  "Version": "2008-10-17",
  "Statement": [
    {
      "Sid": "LambdaECRImageRetrievalPolicy",
      "Effect": "Allow",
      "Principal": {
        "Service": "lambda.amazonaws.com"
      },
      "Action": [
        "ecr:BatchGetImage",
        "ecr:DeleteRepositoryPolicy",
        "ecr:GetDownloadUrlForLayer",
        "ecr:GetRepositoryPolicy",
        "ecr:SetRepositoryPolicy"
      ],
      "Condition": {
        "StringLike": {
          "aws:sourceArn": "arn:aws:lambda:eu-central-1:1234567890:function:*"
        }
      }
    }
  ]
}

resulting in the following Lambda error:

Failed to restore the function MyLambda: The function does not have permission to access the specified image.

Reproduction Steps

  1. Bootstrap account with v1.138.0
  2. Add permission policy (or just deploy a Docker Lambda which will add a policy automatically)
  3. Bootstrap account with v1.139.0 and check the IAM policy gets removed from ECR cdk-hnb659fds-container-assets-1234567890-eu-central-1 repository

What did you expect to happen?

Bootstrap process should NOT remove IAM policy from ECR cdk-hnb659fds-container-assets-1234567890-eu-central-1 repository

What actually happened?

Bootstrap process removed IAM policy from ECR cdk-hnb659fds-container-assets-1234567890-eu-central-1 repository

CDK CLI Version

1.139.0

Framework Version

No response

Node.js Version

v14.15.0

OS

Ubuntu 20.04

Language

Python

Language Version

No response

Other information

No response

@MousaZeidBaker MousaZeidBaker added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 17, 2022
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Jan 17, 2022
@scedward
Copy link

We also saw this unexpected change after upgrading to the latest bootstrap stack. For a quick fix, making a trivial change to the Dockerfile of the affected Lambda was enough to change the asset hash and led to restoration of the lost policy statement.

@NGL321 NGL321 added p2 and removed needs-triage This issue or PR still needs to be triaged. labels Jan 26, 2022
@polothy
Copy link
Contributor

polothy commented Feb 28, 2022

@NGL321 hello - we ran into this today and found the source of the problem. See this lambda doc.

Lambda needs ECR to have a policy on it to allow it to pull images. The "funny" thing that lambda does though is automatically add the policy to ECR:

If the Amazon ECR repository does not include these permissions, Lambda adds ecr:BatchGetImage and ecr:GetDownloadUrlForLayer to the container image repository permissions. Lambda can add these permissions only if the Principal calling Lambda has ecr:getRepositoryPolicy and ecr:setRepositoryPolicy permissions.

The problem is, on CDK bootstrap upgrade, the ECR policy that lambda added can be removed, thus breaking things for lambda (it cannot pull).

The fix seems to be to add this policy to ECR in the CDK bootstrap.

@ievgeniiskliarenko
Copy link

Hi @NGL321,
We got into the same issue with CDK v2 updating the bootstrap version from 9 to 10. Unfortunately, that caused a severe outage on our side.

@RomainMuller
Copy link
Contributor

RomainMuller commented Mar 17, 2022

Seems the change is a collateral from the enablement of ECR Scan by default on the repository. This likely caused the repository policy to be re-written, and the Lambda-authored change to be removed. Definitely an awkward situation.

rix0rrr added a commit that referenced this issue Mar 17, 2022
Container Functions automatically add a policy to an ECR repository to
allow Lambda to pull from it; however, when the ECR repository is
rebootstrapped and has changed, the policy might be overwritten.

Add the policy to the bootstrap stack, so we don't have to rely on
Lambda to add it and it will survive rebootstraps.

This introduces version 11 of the bootstrap stack. You do not need
to upgrade to this version unless you are affected by this issue.

Fixes #18473.
@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort p1 and removed p2 labels Mar 18, 2022
@rix0rrr rix0rrr removed their assignment Mar 18, 2022
@mergify mergify bot closed this as completed in #19446 Mar 18, 2022
mergify bot pushed a commit that referenced this issue Mar 18, 2022
Container Functions automatically add a policy to an ECR repository to
allow Lambda to pull from it; however, when the ECR repository is
rebootstrapped and has changed, the policy might be overwritten.

Add the policy to the bootstrap stack, so we don't have to rely on
Lambda to add it and it will survive rebootstraps.

This introduces version 11 of the bootstrap stack. You do not need
to upgrade to this version unless you are affected by this issue.

Fixes #18473.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p1 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants