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

(cli): CDK Pipeline in account bootstrapped with 1.110.0 (bootstrap version 7): S3 Access Denied #15307

Closed
cogwirrel opened this issue Jun 25, 2021 · 23 comments · Fixed by #15348
Assignees
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 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@cogwirrel
Copy link
Member

cogwirrel commented Jun 25, 2021

CDKPipeline fails with S3 Access Denied error when account has been bootstrapped with aws-cdk@1.110.0 (bootstrap version 7).

If I downgrade my CDK CLI to 1.108.0, bootstrap again (bootstrap version 6) and push a change through the pipeline it completes successfully.

Reproduction Steps

Minimal cdk pipeline repo here: https://github.com/cogwirrel/minimal-cdk-pipeline-ts

npm i -g aws-cdk@1.110.0
cdk bootstrap
cdk deploy
git remote add cc codecommit://MyRepo (requires git-remote-codecommit)
git push cc mainline

Observe error in the pipeline:

Access Denied (Service: Amazon S3; Status Code: 403; Error Code: AccessDenied; Request ID: ZE8ZB6NBMFEY4F6F; S3 Extended Request ID: t9dE+lUpj3K+CKD87YAPLOT0i2pD8CIqoTJv+KxBZQ6S84nM05bpPHzq0EhdNyjs8L00lSrR9wg=; Proxy: null)

What did you expect to happen?

Empty CDK Pipeline to deploy successfully

What actually happened?

"Prepare" step for the pipeline stage failed with an S3 Access Denied error.

Environment

  • CDK CLI Version : 1.110.0
  • Framework Version: 1.110.0
  • Node.js Version: 14.17.0
  • OS : macOS Big Sur (11.4)
  • Language (Version): Typescript 4.3.2

Other

Possibly related to #15192 ?


This is 🐛 Bug Report

@cogwirrel cogwirrel added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 25, 2021
@github-actions github-actions bot added @aws-cdk/aws-s3 Related to Amazon S3 package/tools Related to AWS CDK Tools or CLI labels Jun 25, 2021
@cogwirrel
Copy link
Member Author

Ah from a closer look at #15192 it seems like this allows for cross-account deployments but locks down same-account deployments :)

@cogwirrel
Copy link
Member Author

Just tried deploying to a different account (with both bootstrapped with 1.110.0) and that worked ok :)

ie. I updated pipeline.ts in the example repo I linked with:

pipeline.addPipelineStage(new PipelineStage(app, 'Dev', {
  env: {
    account: '<different_account_id>',
    region: 'us-west-2',
  },
}));

So it does seem to only be when the target account for a pipeline stage is the same as the pipeline account :)

@engr-lynx
Copy link

I encountered the same issue both for new applications and old ones (previously working).

@ColeSiegelTR
Copy link

ColeSiegelTR commented Jun 27, 2021

I am getting, this error S3 Access Denied on the prepare stage from the Cloud Formation execution role, despite granting that role access in the bucket policy and granting access to the encryption key. It is a cross account setup. Any suggestions how to troubleshoot further? Even when I turned off encryption for the bucket to test, the error persists.

@czubocha
Copy link

I've tested it and looks like the problem is in condition:

                Condition:
                     StringNotEquals:
                         s3:ResourceAccount:
                             Ref: 'AWS::AccountId'
               - Sid: PipelineCrossAccountArtifactsKey

In the Prepare step of pipeline input artifacts (Artifact_Build_Synth) need to be downloaded from PipelineArtifactsBucket. Without this condition in IAM policy, the permissions were granted but with it, they are not anymore (for the account where the pipeline is deployed of course).

@rix0rrr wrote in his PR

In-account bucket accesses are not directly permitted by the principal
roles; however, in-account accesses are enabled by resource policy
statements on the buckets themselves (which are sufficient to extend
the required permissions to the principal).

which is not really true in our case as there are no S3 resource policy statements allowing such access in PipelineArtifactsBucket.

So in my opinion to fix it we need to

  • remove the above condition (but then the issue which caused the condition introduction will be still unsolved as DeploymentActionRole will still have access to all buckets in the target accounts)

OR

  • add proper statements to the bucket policy of PipelineArtifactsBucket deployed in the same account as the pipeline.

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 27, 2021 via email

@czubocha
Copy link

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 28, 2021

Oh interesting. I have to say I'm confused. I have basically the same setup as that and in my case the permissions ARE added to the bucket.

Can you share the template of the case where they aren't being added?

@rix0rrr rix0rrr added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jun 28, 2021
@cogwirrel
Copy link
Member Author

Ah does it work for you with the repro steps I provided?

@alexpulver
Copy link
Contributor

I assume that the bucket policy should be created in the pipeline template. If so, I don't see it as well. To reproduce:

git clone -b future https://github.com/alexpulver/aws-cdk-sam-chalice
cd aws-cdk-sam-chalice
python3.7 -m venv .venv
source .venv/bin/activate
./scripts/install-deps.sh
export AWS_ACCESS_KEY_ID="***"
export AWS_SECRET_ACCESS_KEY="***"
export AWS_SESSION_TOKEN="***"
cdk --version  # Currently set to 1.107.0
cdk synth
grep AWS::S3::BucketPolicy cdk.out/AwsCdkSamChalice-Pipeline.template.json
vi package.json  # Update to 1.110.0
./scripts/install-deps.sh
cdk --version
cdk synth
grep AWS::S3::BucketPolicy cdk.out/AwsCdkSamChalice-Pipeline.template.json

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 28, 2021

iiiiiiiiiiiiiiinteresting.

$  grep AWS::S3::BucketPolicy cdk.out/PipelineStack.template.json 
      "Type": "AWS::S3::BucketPolicy",

https://github.com/rix0rrr/test-cicd/blob/master/lib/pipeline-stack.ts

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 28, 2021

AHA!

It breaks if you DO give an account on the stage, and the account is the same as the pipeline's account! If you leave out the account, then the permissions get properly added to the bucket.

@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jun 28, 2021
@rix0rrr rix0rrr removed their assignment Jun 28, 2021
@PierreKiwi
Copy link

Faced this issue today. Is the recommended fix to remove the account from the stage itself?

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 29, 2021

For now, yes. We will fix it shortly.

rix0rrr added a commit that referenced this issue Jun 29, 2021
…ployments

A recent change to the default `deploy-role` policies required that the
required permissions statements are added to the artifacts bucket
instead.

This happened to be working for stacks which had NO `account` env property
set, but not for stacks that DID have an `account` property set which
was the same as the pipeline's account property.

Fixes #15307.
@aax
Copy link

aax commented Jun 29, 2021

I have this problem, too. And like @czubocha I don't specify any account anywhere (neither in the app nor stage nor stack). Is your fix still working in this configuration, @rix0rrr ?

@mergify mergify bot closed this as completed in #15348 Jul 1, 2021
mergify bot pushed a commit that referenced this issue Jul 1, 2021
…ployments (#15348)

A recent change to the default `deploy-role` policies required that the
required permissions statements are added to the artifacts bucket
instead.

This happened to be working for stacks which had NO `account` env property
set, but not for stacks that DID have an `account` property set which
was the same as the pipeline's account property.

Ultimately this was caused by an account comparison deep in the bowels of the
Grants system, which happened to work out because in cases of mismatched
accounts it would think there was a cross-account access and it was trying to
add permissions to both principal and resource. But in this case, it's much more
accurate to say that we can NEVER add permissions to the `deploy-role`, and
instead always want the permissions added to the bucket (rather than dropping
them).

Fixes #15307.


----

*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

github-actions bot commented Jul 1, 2021

⚠️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.

@fbdo
Copy link

fbdo commented Jul 2, 2021

Hi all, @rix0rrr I tried today to deploy a pipeline using the newly released version 1.111.0 and I can see see the same S3 Access Denied error. I already tried to clean up/bootstrap again and no success.

@peterwoodworth
Copy link
Contributor

Reopening because multiple customers are still experiencing this issue in the latest version #15406

@rix0rrr
Copy link
Contributor

rix0rrr commented Jul 9, 2021

There are multiple causes for "S3 Access Denied".

I saw someone mention in the other thread they deleted and recreated their bootstrap stack.

Doing that will permanently break Pipelines.

See the pipelines README on how to unbreak yourself in that situation

@fbdo
Copy link

fbdo commented Jul 9, 2021

That was my case, I followed the instructions in the README and it started to work. Sorry for the false alarm, and thanks!

@czubocha
Copy link

czubocha commented Jul 9, 2021

My case was that I upgraded CDK CLI but I didn't upgraded CDK version in go.mod (I'm using Go bindings). Thanks for the fix, issue can be closed.

@alexpulver
Copy link
Contributor

My case was resolved by this after upgrading the Python CDK dependencies

@rix0rrr rix0rrr closed this as completed Jul 12, 2021
@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.

@peterwoodworth peterwoodworth removed the @aws-cdk/aws-s3 Related to Amazon S3 label Jul 13, 2021
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
…ployments (aws#15348)

A recent change to the default `deploy-role` policies required that the
required permissions statements are added to the artifacts bucket
instead.

This happened to be working for stacks which had NO `account` env property
set, but not for stacks that DID have an `account` property set which
was the same as the pipeline's account property.

Ultimately this was caused by an account comparison deep in the bowels of the
Grants system, which happened to work out because in cases of mismatched
accounts it would think there was a cross-account access and it was trying to
add permissions to both principal and resource. But in this case, it's much more
accurate to say that we can NEVER add permissions to the `deploy-role`, and
instead always want the permissions added to the bucket (rather than dropping
them).

Fixes aws#15307.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
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 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

Successfully merging a pull request may close this issue.