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

…30186) ### Issue # (if applicable) Closes #27128 ### Reason for this change The `--quiet` flag on the `cdk diff` command prevents the stack name and default message from being printed when no diff exists. If diffs exist, the stack name and diffs are expected to be printed, but currently, the stack name is not printed, and it is difficult to determine which stack the diff is for. for example: ```bash $ cdk diff --quiet Resources [~] AWS::S3::Bucket MyFirstBucket MyFirstBucketB8884501 ├─ [~] DeletionPolicy │ ├─ [-] Delete │ └─ [+] Retain └─ [~] UpdateReplacePolicy ├─ [-] Delete └─ [+] Retain ✨ Number of stacks with differences: 1 ``` This PR will fix to print the stack name when the `--quiet` flag is specified and diffs exist. ### Description of changes Changed the position of the `fullDiff` function call. It is possible to output the stack name in the `printSecurityDiff` or `printStackDiff` functions, but since the message has already been output before these functions are called, the stack name must be output first. I think it would be more user-friendly to have all messages after the output of the stack name, but if this is not the case, please point this out. ### Description of how you validated changes I added a unit test to confirm to print the stack name when diff exists. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Sep 25, 2024 · bcf9209 · bcf9209
1 parent 2813eb2
commit bcf9209
Show file tree

Hide file tree

Showing 3 changed files with 219 additions and 24 deletions.
diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts
@@ -138,15 +138,11 @@ export class CdkToolkit {
 
       const template = deserializeStructure(await fs.readFile(options.templatePath, { encoding: 'UTF-8' }));
       diffs = options.securityOnly
-        ? numberFromBool(printSecurityDiff(template, stacks.firstStack, RequireApproval.Broadening, undefined))
-        : printStackDiff(template, stacks.firstStack, strict, contextLines, quiet, undefined, false, stream);
+        ? numberFromBool(printSecurityDiff(template, stacks.firstStack, RequireApproval.Broadening, quiet))
+        : printStackDiff(template, stacks.firstStack, strict, contextLines, quiet, undefined, undefined, false, stream);
     } else {
       // Compare N stacks against deployed templates
       for (const stack of stacks.stackArtifacts) {
-        if (!quiet) {
-          stream.write(format('Stack %s\n', chalk.bold(stack.displayName)));
-        }
-
         const templateWithNestedStacks = await this.props.deployments.readCurrentTemplateWithNestedStacks(
           stack, options.compareAgainstProcessedTemplate,
         );
@@ -170,7 +166,9 @@ export class CdkToolkit {
             });
           } catch (e: any) {
             debug(e.message);
-            stream.write('Checking if the stack exists before creating the changeset has failed, will base the diff on template differences (run again with -v to see the reason)\n');
+            if (!quiet) {
+              stream.write(`Checking if the stack ${stack.stackName} exists before creating the changeset has failed, will base the diff on template differences (run again with -v to see the reason)\n`);
+            }
             stackExists = false;
           }
 
@@ -190,14 +188,12 @@ export class CdkToolkit {
           }
         }
 
-        if (resourcesToImport) {
-          stream.write('Parameters and rules created during migration do not affect resource configuration.\n');
-        }
-
         const stackCount =
         options.securityOnly
-          ? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, changeSet)))
-          : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, !!resourcesToImport, stream, nestedStacks));
+          ? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, quiet, stack.displayName, changeSet)))
+          : (printStackDiff(
+            currentTemplate, stack, strict, contextLines, quiet, stack.displayName, changeSet, !!resourcesToImport, stream, nestedStacks,
+          ));
 
         diffs += stackCount;
       }

diff --git a/packages/aws-cdk/lib/diff.ts b/packages/aws-cdk/lib/diff.ts
@@ -31,13 +31,22 @@ export function printStackDiff(
   strict: boolean,
   context: number,
   quiet: boolean,
+  stackName?: string,
   changeSet?: DescribeChangeSetOutput,
   isImport?: boolean,
   stream: FormatStream = process.stderr,
   nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates }): number {
-
   let diff = fullDiff(oldTemplate, newTemplate.template, changeSet, isImport);
 
+  // must output the stack name if there are differences, even if quiet
+  if (stackName && (!quiet || !diff.isEmpty)) {
+    stream.write(format('Stack %s\n', chalk.bold(stackName)));
+  }
+
+  if (!quiet && isImport) {
+    stream.write('Parameters and rules created during migration do not affect resource configuration.\n');
+  }
+
   // detect and filter out mangled characters from the diff
   let filteredChangesCount = 0;
   if (diff.differenceCount && !strict) {
@@ -74,9 +83,6 @@ export function printStackDiff(
       break;
     }
     const nestedStack = nestedStackTemplates[nestedStackLogicalId];
-    if (!quiet) {
-      stream.write(format('Stack %s\n', chalk.bold(nestedStack.physicalName ?? nestedStackLogicalId)));
-    }
 
     (newTemplate as any)._template = nestedStack.generatedTemplate;
     stackDiffCount += printStackDiff(
@@ -85,6 +91,7 @@ export function printStackDiff(
       strict,
       context,
       quiet,
+      nestedStack.physicalName ?? nestedStackLogicalId,
       undefined,
       isImport,
       stream,
@@ -112,10 +119,18 @@ export function printSecurityDiff(
   oldTemplate: any,
   newTemplate: cxapi.CloudFormationStackArtifact,
   requireApproval: RequireApproval,
+  quiet?: boolean,
+  stackName?: string,
   changeSet?: DescribeChangeSetOutput,
+  stream: FormatStream = process.stderr,
 ): boolean {
   const diff = fullDiff(oldTemplate, newTemplate.template, changeSet);
 
+  // must output the stack name if there are differences, even if quiet
+  if (!quiet || !diff.isEmpty) {
+    stream.write(format('Stack %s\n', chalk.bold(stackName)));
+  }
+
   if (difRequiresApproval(diff, requireApproval)) {
     // eslint-disable-next-line max-len
     warning(`This deployment will make potentially sensitive changes according to your current security approval level (--require-approval ${requireApproval}).`);

diff --git a/packages/aws-cdk/test/diff.test.ts b/packages/aws-cdk/test/diff.test.ts
@@ -393,15 +393,37 @@ describe('non-nested stacks', () => {
 
     // WHEN
     const exitCode = await toolkit.diff({
-      stackNames: ['A', 'A'],
+      stackNames: ['D'],
+      stream: buffer,
+      fail: false,
+      quiet: true,
+    });
+
+    // THEN
+    const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '');
+    expect(plainTextOutput).not.toContain('Stack D');
+    expect(plainTextOutput).not.toContain('There were no differences');
+    expect(buffer.data.trim()).toContain('✨  Number of stacks with differences: 0');
+    expect(exitCode).toBe(0);
+  });
+
+  test('when quiet mode is enabled, stacks with diffs should print stack name to stdout', async () => {
+    // GIVEN
+    const buffer = new StringWritable();
+
+    // WHEN
+    const exitCode = await toolkit.diff({
+      stackNames: ['A'],
       stream: buffer,
       fail: false,
       quiet: true,
     });
 
     // THEN
-    expect(buffer.data.trim()).not.toContain('Stack A');
-    expect(buffer.data.trim()).not.toContain('There were no differences');
+    const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '');
+    expect(plainTextOutput).toContain('Stack A');
+    expect(plainTextOutput).not.toContain('There were no differences');
+    expect(buffer.data.trim()).toContain('✨  Number of stacks with differences: 1');
     expect(exitCode).toBe(0);
   });
 });
@@ -548,10 +570,16 @@ describe('stack exists checks', () => {
 describe('nested stacks', () => {
   beforeEach(() => {
     cloudExecutable = new MockCloudExecutable({
-      stacks: [{
-        stackName: 'Parent',
-        template: {},
-      }],
+      stacks: [
+        {
+          stackName: 'Parent',
+          template: {},
+        },
+        {
+          stackName: 'UnchangedParent',
+          template: {},
+        },
+      ],
     });
 
     cloudFormation = instanceMockFrom(Deployments);
@@ -718,6 +746,78 @@ describe('nested stacks', () => {
               },
               physicalName: undefined,
             },
+            UnChangedChild: {
+              deployedTemplate: {
+                Resources: {
+                  SomeResource: {
+                    Type: 'AWS::Something',
+                    Properties: {
+                      Prop: 'unchanged',
+                    },
+                  },
+                },
+              },
+              generatedTemplate: {
+                Resources: {
+                  SomeResource: {
+                    Type: 'AWS::Something',
+                    Properties: {
+                      Prop: 'unchanged',
+                    },
+                  },
+                },
+              },
+              nestedStackTemplates: {},
+              physicalName: 'UnChangedChild',
+            },
+          },
+        });
+      }
+      if (stackArtifact.stackName === 'UnchangedParent') {
+        stackArtifact.template.Resources = {
+          UnchangedChild: {
+            Type: 'AWS::CloudFormation::Stack',
+            Properties: {
+              TemplateURL: 'child-url',
+            },
+          },
+        };
+        return Promise.resolve({
+          deployedRootTemplate: {
+            Resources: {
+              UnchangedChild: {
+                Type: 'AWS::CloudFormation::Stack',
+                Properties: {
+                  TemplateURL: 'child-url',
+                },
+              },
+            },
+          },
+          nestedStacks: {
+            UnchangedChild: {
+              deployedTemplate: {
+                Resources: {
+                  SomeResource: {
+                    Type: 'AWS::Something',
+                    Properties: {
+                      Prop: 'unchanged',
+                    },
+                  },
+                },
+              },
+              generatedTemplate: {
+                Resources: {
+                  SomeResource: {
+                    Type: 'AWS::Something',
+                    Properties: {
+                      Prop: 'unchanged',
+                    },
+                  },
+                },
+              },
+              nestedStackTemplates: {},
+              physicalName: 'UnchangedChild',
+            },
           },
         });
       }
@@ -784,6 +884,7 @@ Stack newGrandChild
 Resources
 [+] AWS::Something SomeResource
 
+Stack UnChangedChild
 
 ✨  Number of stacks with differences: 6`);
 
@@ -847,12 +948,95 @@ Stack newGrandChild
 Resources
 [+] AWS::Something SomeResource
 
+Stack UnChangedChild
 
 ✨  Number of stacks with differences: 6`);
 
     expect(exitCode).toBe(0);
     expect(changeSetSpy).not.toHaveBeenCalled();
   });
+
+  test('when quiet mode is enabled, nested stacks with no diffs should not print stack name & no differences to stdout', async () => {
+    // GIVEN
+    const buffer = new StringWritable();
+
+    // WHEN
+    const exitCode = await toolkit.diff({
+      stackNames: ['UnchangedParent'],
+      stream: buffer,
+      fail: false,
+      quiet: true,
+    });
+
+    // THEN
+    const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').replace(/[ \t]+$/mg, '');
+    expect(plainTextOutput).not.toContain('Stack UnchangedParent');
+    expect(plainTextOutput).not.toContain('There were no differences');
+    expect(buffer.data.trim()).toContain('✨  Number of stacks with differences: 0');
+    expect(exitCode).toBe(0);
+  });
+
+  test('when quiet mode is enabled, nested stacks with diffs should print stack name to stdout', async () => {
+    // GIVEN
+    const buffer = new StringWritable();
+
+    // WHEN
+    const exitCode = await toolkit.diff({
+      stackNames: ['Parent'],
+      stream: buffer,
+      fail: false,
+      quiet: true,
+    });
+
+    // THEN
+    const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').replace(/[ \t]+$/mg, '');
+    expect(plainTextOutput).toContain(`Stack Parent
+Resources
+[~] AWS::CloudFormation::Stack AdditionChild
+ └─ [~] TemplateURL
+     ├─ [-] addition-child-url-new
+     └─ [+] addition-child-url-old
+[~] AWS::CloudFormation::Stack DeletionChild
+ └─ [~] TemplateURL
+     ├─ [-] deletion-child-url-new
+     └─ [+] deletion-child-url-old
+[~] AWS::CloudFormation::Stack ChangedChild
+ └─ [~] TemplateURL
+     ├─ [-] changed-child-url-new
+     └─ [+] changed-child-url-old
+
+Stack AdditionChild
+Resources
+[~] AWS::Something SomeResource
+ └─ [+] Prop
+     └─ added-value
+
+Stack DeletionChild
+Resources
+[~] AWS::Something SomeResource
+ └─ [-] Prop
+     └─ value-to-be-removed
+
+Stack ChangedChild
+Resources
+[~] AWS::Something SomeResource
+ └─ [~] Prop
+     ├─ [-] old-value
+     └─ [+] new-value
+
+Stack newChild
+Resources
+[+] AWS::Something SomeResource
+
+Stack newGrandChild
+Resources
+[+] AWS::Something SomeResource
+
+
+✨  Number of stacks with differences: 6`);
+    expect(plainTextOutput).not.toContain('Stack UnChangedChild');
+    expect(exitCode).toBe(0);
+  });
 });
 
 describe('--strict', () => {