feat(cdk): introduce the new cfn-changeset-review-role for the diff command by sakurai-ryo · Pull Request #30329 · aws/aws-cdk

sakurai-ryo · 2024-05-24T16:01:03Z

Issue # (if applicable)

Reason for this change

The cdk diff command needs to assume the deploy-role to create the changeset, but the deploy-role has strong privileges, such as deleting the cfn stack.
Granting developers the ability to be able to assume the deploy-role to use the cdk diff command is a problem that some organizations should avoid.
To avoid this, creating a new role in the Bootstrapping process with only limited privileges is necessary to avoid using the deploy-role when using the cdk diff command.

Description of changes

Introducing a new role

Introduce a new cfn-changeset-review-role and give the cdk diff command the permissions it needs to create changesets.
With this change, the required roles for each operation are as follows

Operation	CDK Bootstrap Roles to Assume
Lookup	lookup-role
Diff	lookup-role cfn-changeset-review-role
Deploy	cfn-exec-role lookup-role deploy-role file-publishing-role image-publishing-role

The cfn-changeset-review-role has the following permissions.

Action	Description
cloudformation:CreateChangeSet	Creates a changeset.
cloudformation:DeleteChangeSet	Deletes a specified changeset.
cloudformation:DescribeChangeSet	Returns the description of the specified change set.
cloudformation:DescribeStacks	Check for the existence of the specified stack.
s3:PutObject*	Upload to S3 when a template exceeding 51,200 bytes is specified.
s3:GetObject*	Download a template uploaded to S3
kms:GenerateDataKey*	Upload files to the staging bucket in SSE-KMS encryption mode.

The cloud-assembly-schema is modified to allow users to use this role.
It will be added to manifest.json in the same format as the existing lookup-role.
Retaining the role's Arn and the requiresBootstrapStackVersion will help if permissions are changed in the future.

Change behavior when executing `cdk diff

Currently, we assume to deploy-role before creating the changeset, but changing it to assume to the new changeset-role.
If for some reason it is not possible to assume to changeset-role, it will fall back to assume the deploy-role.
For example, when changeset-role does not exist.
This allows the cdk diff to work in environments without the latest Bootstrap version.

app-staging-synthesizer module

With the introduction of the new changeset-role, I modified the app-staging-synthesizer module so that it can be used in the same way.

Description of how you validated changes

Added cli-integ test to verify successful upload of cfn template and creation of changeset using the new role, plus several other unit tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

sakurai-ryo · 2024-05-24T16:03:07Z

packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts

  // Now install that `package.json` using NPM7
-  await fixture.shell(['node', require.resolve('npm'), 'install']);
+  const isRepoMode = !!process.env.REPO_ROOT;
+  const npmPath = isRepoMode ? path.join(__dirname, 'package-sources/repo-tools/npm') : require.resolve('npm');


The require.resolve function searches under node_modules, so we were not able to use fake npm to use local packages.

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

sakurai-ryo · 2024-05-24T16:05:38Z

packages/@aws-cdk/app-staging-synthesizer-alpha/test/integ.synth-default-resources.ts


 // IMAGE_COPIES env variable is used to test maximum number of ECR repositories allowed.
-const IMAGE_COPIES = Number(process.env.IMAGE_COPIES) ?? 1;
+const IMAGE_COPIES = Number(process.env.IMAGE_COPIES) || 1;


If we use ??, the right side will not be returned if Number(~) returns NaN when the IMAGE_COPIES is undefined.

sakurai-ryo · 2024-05-24T16:07:10Z

packages/aws-cdk/lib/api/util/cloudformation.ts

      bodyParameter,
      parameters: options.parameters,
      resourcesToImport: options.resourcesToImport,
-      role: executionRoleArn,


Because CloudFormation cannot assume the cfn-changeset-review-role.

sakurai-ryo · 2024-05-24T16:43:01Z

Exemption Request
Wait for the maintainer to run the test.

…ff-changeset-role

sakurai-ryo · 2024-05-24T17:34:37Z

hmm, the pr-linter/exemption-requested label is not attached.

…ff-changeset-role

…aws-cdk into diff-changeset-role

aws-cdk-automation · 2024-06-15T00:09:56Z

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

…ff-changeset-role

aws-cdk-automation · 2024-06-17T12:32:50Z

The pull request linter fails with the following errors:

❌ CLI code has changed. A maintainer must run the code through the testing pipeline (git fetch origin pull/30329/head && git push -f origin FETCH_HEAD:test-main-pipeline), then add the 'pr-linter/cli-integ-tested' label when the pipeline succeeds.

PRs must pass status checks before we can provide a meaningful review.

If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing Exemption Request and/or Clarification Request.

✅ A exemption request has been requested. Please wait for a maintainer's review.

mergify · 2024-06-17T12:33:24Z

⚠️ The sha of the head commit of this PR conflicts with #30568. Mergify cannot evaluate rules on this PR. ⚠️

aws-cdk-automation · 2024-06-17T13:01:21Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 2b28865
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2024-06-26T12:14:59Z

⚠️ The sha of the head commit of this PR conflicts with #30568. Mergify cannot evaluate rules on this PR. ⚠️

mergify · 2024-07-07T10:35:26Z

⚠️ The sha of the head commit of this PR conflicts with #30568. Mergify cannot evaluate rules on this PR. ⚠️

mergify · 2024-07-14T13:04:41Z

⚠️ The sha of the head commit of this PR conflicts with #30568. Mergify cannot evaluate rules on this PR. ⚠️

feat: introduce the cfn-changeset-role

7774e21

github-actions bot added star-contributor [Pilot] contributed between 25-49 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1 labels May 24, 2024

aws-cdk-automation requested a review from a team May 24, 2024 16:01

sakurai-ryo had a problem deploying to test-pipeline May 24, 2024 16:02 — with GitHub Actions Failure

sakurai-ryo commented May 24, 2024

View reviewed changes

aws-cdk-automation requested changes May 24, 2024

View reviewed changes

chore: add a new line

d0b32ef

sakurai-ryo commented May 24, 2024

View reviewed changes

sakurai-ryo had a problem deploying to test-pipeline May 24, 2024 16:09 — with GitHub Actions Failure

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into di…

6a6a37c

…ff-changeset-role

sakurai-ryo had a problem deploying to test-pipeline May 24, 2024 16:45 — with GitHub Actions Failure

github-actions bot added bug This issue is a bug. and removed feature-request A feature should be added or improved. labels May 24, 2024

sakurai-ryo changed the title ~~feat(cdk): introduce the new cfn-changeset-role for the diff command~~ feat(cdk): introduce the new cfn-changeset-review-role for the diff command May 24, 2024

github-actions bot mentioned this pull request May 27, 2024

Weekly PR metrics report #30348

Closed

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into di…

bd5626b

…ff-changeset-role

sakurai-ryo had a problem deploying to test-pipeline May 27, 2024 12:14 — with GitHub Actions Failure

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into di…

0b789a1

…ff-changeset-role

sakurai-ryo had a problem deploying to test-pipeline May 28, 2024 13:34 — with GitHub Actions Failure

fix: syntax miss

172c353

sakurai-ryo had a problem deploying to test-pipeline May 28, 2024 15:24 — with GitHub Actions Failure

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into di…

12678ca

…ff-changeset-role

sakurai-ryo had a problem deploying to test-pipeline May 29, 2024 01:43 — with GitHub Actions Failure

github-actions bot mentioned this pull request Jun 1, 2024

Monthly PR metrics report #30414

Closed

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into di…

d26cc16

…ff-changeset-role

sakurai-ryo had a problem deploying to test-pipeline June 7, 2024 10:34 — with GitHub Actions Failure

aws-cdk-automation added the pr/needs-cli-test-run This PR needs CLI tests run against it. label Jun 7, 2024

sakurai-ryo added 2 commits June 14, 2024 22:01

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into di…

a5cf23f

…ff-changeset-role

Merge branch 'diff-changeset-role' of https://github.com/sakurai-ryo/…

a0b199c

…aws-cdk into diff-changeset-role

sakurai-ryo had a problem deploying to test-pipeline June 14, 2024 13:04 — with GitHub Actions Failure

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into di…

2b28865

…ff-changeset-role

sakurai-ryo had a problem deploying to test-pipeline June 17, 2024 12:31 — with GitHub Actions Failure

sakurai-ryo closed this Jun 17, 2024

sakurai-ryo had a problem deploying to test-pipeline June 17, 2024 12:33 — with GitHub Actions Failure

sakurai-ryo mentioned this pull request Jun 17, 2024

feat(cli): introduce the new cfn-changeset-review-role for the diff command #30568

Closed

1 task

Conversation

sakurai-ryo commented May 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue # (if applicable)

Reason for this change

Description of changes

Introducing a new role

Change behavior when executing `cdk diff

app-staging-synthesizer module

Description of how you validated changes

Checklist

Uh oh!

sakurai-ryo May 24, 2024

Choose a reason for hiding this comment

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

sakurai-ryo May 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sakurai-ryo May 24, 2024

Choose a reason for hiding this comment

Uh oh!

sakurai-ryo commented May 24, 2024

Uh oh!

sakurai-ryo commented May 24, 2024

Uh oh!

aws-cdk-automation commented Jun 15, 2024

Uh oh!

aws-cdk-automation commented Jun 17, 2024

Uh oh!

mergify bot commented Jun 17, 2024

Uh oh!

aws-cdk-automation commented Jun 17, 2024

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Jun 26, 2024

Uh oh!

mergify bot commented Jul 7, 2024

Uh oh!

mergify bot commented Jul 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sakurai-ryo commented May 24, 2024 •

edited

Loading

sakurai-ryo May 24, 2024 •

edited

Loading