fix(cdk): `cdk diff --quiet` to print stack name when there is diffs by sakurai-ryo · Pull Request #28576 · aws/aws-cdk

sakurai-ryo · 2024-01-04T14:15:47Z

The --quiet flag on the cdk diff command will prevent the stack name and default message from being printed when there is no diff.
If diffs exist, the stack name and diffs are expected to be printed, but currently the stack name is not displayed and it is difficult to determine which stack the diff is for.

$ cdk diff --quiet
Resources
[~] AWS::S3::Bucket MyFirstBucket MyFirstBucketB8884501
 ├─ [~] DeletionPolicy
 │   ├─ [-] Delete
 │   └─ [+] Retain
 └─ [~] UpdateReplacePolicy
     ├─ [-] Delete
     └─ [+] Retain


✨  Number of stacks with differences: 1

This PR fixed to print the stack name when the --quiet flag is specified and diffs exist.

Closes #27128

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

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-01-04T14:25:28Z

packages/aws-cdk/test/diff.test.ts


    // WHEN
    const exitCode = await toolkit.diff({
-      stackNames: ['A', 'A'],


This test was broken, so fixed this.
This test assumed that the diff does not exist, but the stackNames: ['A', 'A'] specification indicates that the diff does exist and the not.toContain assertion succeeded because of the presence or absence of ANSI escape sequences.
Therefore, I changed it to specify stackNames: ['D'], where no diff exists, and removed the ANSI escape sequence in the string used for the assertion.

aws-cdk/packages/aws-cdk/test/diff.test.ts

Line 41 in 10ed194

stackName: 'D',

aws-cdk/packages/aws-cdk/test/diff.test.ts

Line 57 in 10ed194

if (stackArtifact.stackName === 'D') {

good catch here!

sakurai-ryo · 2024-01-04T14:31:24Z

packages/aws-cdk/lib/diff.ts

  changeSet?: CloudFormation.DescribeChangeSetOutput,
-  stream?: cfnDiff.FormatStream): number {
+  stream?: cfnDiff.FormatStream,
+  stackDiff?: cfnDiff.TemplateDiff): number {


The cfnDiff.fullDiff function must be called in the CdkToolkit.diff method, since we need to get the presence or absence of diffs when determining whether to print the stack name.
I changed it so that the diff can be passed in optionally so that the relatively heavy processing of cfnDiff.fullDiff does not need to be executed multiple times.

sakurai-ryo · 2024-01-04T14:57:42Z

Exemption Request: I think the unit tests cover the test for this change.

nomike · 2024-01-24T14:15:02Z

Exemption Request: This pull request will not cause any change in the produced code, so it should be exempt from having to contain a change to the integration test file and the resulting snapshot.

Also. as it's a CLI code change, I request someone to review it please. It's a very minor change, so it will hopefully be quick to review.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

…fix-diff-quiet-stack-name

…i-ryo/aws-cdk into fix-diff-quiet-stack-name

nomike · 2024-02-08T11:59:45Z

Can someone please approve the test-pipeline?

Thanks!

…fix-diff-quiet-stack-name

nomike · 2024-02-12T12:08:06Z

Looks like this branch needs re-basing again and then needs an approval for that test pipeline.
Could someone please kick this off?

nomike · 2024-02-21T13:21:53Z

Is there any way to push this forward?
One of my changes is blocked by this.

aws-cdk-automation · 2024-03-07T19:54:36Z

AWS CodeBuild CI Report

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

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

aws-cdk-automation · 2024-03-21T16:50:37Z

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

paulhcsun · 2024-03-21T16:55:59Z

Hi @sakurai-ryo, @nomike. I'm very sorry for not getting around to this. It looks like I added the cli-integ-tested label already so it definitely passed the cli integ tests at some point. There was some miscommunication on our side in handing over this PR to another team but I will reach out for someone to have one final look to get this over the line.

In the meantime there are a couple of merge conflicts to be addressed, I'd like to run the cli integ tests again after those have been resolved to be safe. I will keep a close eye on this PR until then.

Thank you both for you patience.

comcalvi · 2024-03-21T18:17:04Z

packages/aws-cdk/lib/cdk-toolkit.ts

-          ? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, changeSet)) > 0 ? 1 : 0)
-          : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, stream) > 0 ? 1 : 0);
+          ? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, changeSet, diff)) > 0 ? 1 : 0)
+          : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, stream, diff) > 0 ? 1 : 0);


Let's not pass the diff in the same function that takes the inputs to create the diff (the old template and new template). Instead let's move the

stream.write(format('Stack %s\n', chalk.bold(stack.displayName)));

to printStackDiff (you can access the stackName from the stack object passed as the second argument to printStackDiff).

comcalvi · 2024-03-21T18:17:37Z

packages/aws-cdk/test/diff.test.ts


    // WHEN
    const exitCode = await toolkit.diff({
-      stackNames: ['A', 'A'],


good catch here!

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.

aws-cdk-automation · 2024-03-22T00:08:46Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2024-03-25T00:08:52Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2024-03-25T18:34:30Z

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/28576/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.

aws-cdk-automation · 2024-03-30T00:07:44Z

This PR has been in the MERGE CONFLICTS 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.

aws-cdk-automation · 2024-04-08T00:08:36Z

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

aws-cdk-automation · 2024-04-08T00:10:15Z

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/28576/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.

TheCaffeintedSloth · 2024-04-25T15:10:48Z

This is still broken and should be fixed. The --quiet flag is broken without it.

fix: quiet flag to print stack name when having diffs

d9b6bc8

github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort p1 labels Jan 4, 2024

aws-cdk-automation requested a review from a team January 4, 2024 14:16

github-actions bot added the admired-contributor [Pilot] contributed between 13-24 PRs to the CDK label Jan 4, 2024

sakurai-ryo had a problem deploying to test-pipeline January 4, 2024 14:16 — with GitHub Actions Failure

aws-cdk-automation previously requested changes Jan 4, 2024

View reviewed changes

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

sakurai-ryo commented Jan 4, 2024

View reviewed changes

sakurai-ryo changed the title ~~feat(cdk): cdk diff --quiet to print stack name when there is no diff~~ fix(cdk): cdk diff --quiet to print stack name when there is no diff Jan 4, 2024

fix lint

fd24c9b

sakurai-ryo had a problem deploying to test-pipeline January 4, 2024 14:37 — with GitHub Actions Failure

sakurai-ryo changed the title ~~fix(cdk): cdk diff --quiet to print stack name when there is no diff~~ fix(cdk): cdk diff --quiet to print stack name when there is diffs Jan 4, 2024

aws-cdk-automation added pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. pr/needs-maintainer-review This PR needs a review from a Core Team Member labels Jan 4, 2024

nomike mentioned this pull request Jan 24, 2024

fix(cdk): Print names for changed stacks when using --quiet #28838

Closed

Merge branch 'main' into fix-diff-quiet-stack-name

7243fd2

paulhcsun temporarily deployed to test-pipeline January 25, 2024 00:08 — with GitHub Actions Inactive

paulhcsun added the pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested label Jan 25, 2024

aws-cdk-automation removed the pr/needs-cli-test-run This PR needs CLI tests run against it. label Jan 25, 2024

paulhcsun added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label Jan 25, 2024

Merge branch 'main' into fix-diff-quiet-stack-name

2e2f68c

paulhcsun temporarily deployed to test-pipeline January 25, 2024 21:38 — with GitHub Actions Inactive

Merge branch 'main' into fix-diff-quiet-stack-name

3b9c9e9

jysheng123 temporarily deployed to test-pipeline January 26, 2024 00:40 — with GitHub Actions Inactive

sakurai-ryo added 2 commits February 4, 2024 00:13

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

3120ac5

…fix-diff-quiet-stack-name

Merge branch 'fix-diff-quiet-stack-name' of https://github.com/sakura…

5192cb0

…i-ryo/aws-cdk into fix-diff-quiet-stack-name

sakurai-ryo had a problem deploying to test-pipeline February 3, 2024 15:14 — with GitHub Actions Failure

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

7e6866c

…fix-diff-quiet-stack-name

sakurai-ryo had a problem deploying to test-pipeline February 8, 2024 12:19 — with GitHub Actions Failure

Merge branch 'main' into fix-diff-quiet-stack-name

6625a4d

paulhcsun temporarily deployed to test-pipeline March 7, 2024 19:27 — with GitHub Actions Inactive

comcalvi suggested changes Mar 21, 2024

View reviewed changes

comcalvi self-assigned this Mar 21, 2024

comcalvi added pr/needs-cli-test-run This PR needs CLI tests run against it. and removed pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested pr/needs-maintainer-review This PR needs a review from a Core Team Member labels Mar 21, 2024

aws-cdk-automation requested changes Mar 21, 2024

View reviewed changes

comcalvi removed the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label Mar 25, 2024

aws-cdk-automation added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Apr 8, 2024

aws-cdk-automation closed this Apr 8, 2024

sakurai-ryo mentioned this pull request May 13, 2024

fix(cdk): cdk diff --quiet to print stack name when there is diffs #30186

Merged

1 task

Conversation

sakurai-ryo commented Jan 4, 2024

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

sakurai-ryo Jan 4, 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!

comcalvi Mar 21, 2024

Choose a reason for hiding this comment

Uh oh!

sakurai-ryo Jan 4, 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 commented Jan 4, 2024

Uh oh!

nomike commented Jan 24, 2024

Uh oh!

nomike commented Feb 8, 2024

Uh oh!

nomike commented Feb 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nomike commented Feb 21, 2024

Uh oh!

aws-cdk-automation commented Mar 7, 2024

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Mar 21, 2024

Uh oh!

paulhcsun commented Mar 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

comcalvi Mar 21, 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!

comcalvi Mar 21, 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!

aws-cdk-automation commented Mar 22, 2024

Uh oh!

aws-cdk-automation commented Mar 25, 2024

Uh oh!

aws-cdk-automation commented Mar 25, 2024

Uh oh!

aws-cdk-automation commented Mar 30, 2024

Uh oh!

aws-cdk-automation commented Apr 8, 2024

Uh oh!

aws-cdk-automation commented Apr 8, 2024

Uh oh!

TheCaffeintedSloth commented Apr 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

sakurai-ryo Jan 4, 2024 •

edited

Loading

sakurai-ryo Jan 4, 2024 •

edited

Loading

nomike commented Feb 12, 2024 •

edited

Loading

paulhcsun commented Mar 21, 2024 •

edited

Loading

comcalvi Mar 21, 2024 •

edited

Loading