Skip to content

Commit bcf9209

Browse files
sakurai-ryosakurai-ryo
authored
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*
1 parent 2813eb2 commit bcf9209

File tree

3 files changed

+219
-24
lines changed

3 files changed

+219
-24
lines changed

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

+9-13
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,11 @@ export class CdkToolkit {
138138

139139
const template = deserializeStructure(await fs.readFile(options.templatePath, { encoding: 'UTF-8' }));
140140
diffs = options.securityOnly
141-
? numberFromBool(printSecurityDiff(template, stacks.firstStack, RequireApproval.Broadening, undefined))
142-
: printStackDiff(template, stacks.firstStack, strict, contextLines, quiet, undefined, false, stream);
141+
? numberFromBool(printSecurityDiff(template, stacks.firstStack, RequireApproval.Broadening, quiet))
142+
: printStackDiff(template, stacks.firstStack, strict, contextLines, quiet, undefined, undefined, false, stream);
143143
} else {
144144
// Compare N stacks against deployed templates
145145
for (const stack of stacks.stackArtifacts) {
146-
if (!quiet) {
147-
stream.write(format('Stack %s\n', chalk.bold(stack.displayName)));
148-
}
149-
150146
const templateWithNestedStacks = await this.props.deployments.readCurrentTemplateWithNestedStacks(
151147
stack, options.compareAgainstProcessedTemplate,
152148
);
@@ -170,7 +166,9 @@ export class CdkToolkit {
170166
});
171167
} catch (e: any) {
172168
debug(e.message);
173-
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');
169+
if (!quiet) {
170+
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`);
171+
}
174172
stackExists = false;
175173
}
176174

@@ -190,14 +188,12 @@ export class CdkToolkit {
190188
}
191189
}
192190

193-
if (resourcesToImport) {
194-
stream.write('Parameters and rules created during migration do not affect resource configuration.\n');
195-
}
196-
197191
const stackCount =
198192
options.securityOnly
199-
? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, changeSet)))
200-
: (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, !!resourcesToImport, stream, nestedStacks));
193+
? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, quiet, stack.displayName, changeSet)))
194+
: (printStackDiff(
195+
currentTemplate, stack, strict, contextLines, quiet, stack.displayName, changeSet, !!resourcesToImport, stream, nestedStacks,
196+
));
201197

202198
diffs += stackCount;
203199
}

‎packages/aws-cdk/lib/diff.ts

+19-4
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,22 @@ export function printStackDiff(
3131
strict: boolean,
3232
context: number,
3333
quiet: boolean,
34+
stackName?: string,
3435
changeSet?: DescribeChangeSetOutput,
3536
isImport?: boolean,
3637
stream: FormatStream = process.stderr,
3738
nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates }): number {
38-
3939
let diff = fullDiff(oldTemplate, newTemplate.template, changeSet, isImport);
4040

41+
// must output the stack name if there are differences, even if quiet
42+
if (stackName && (!quiet || !diff.isEmpty)) {
43+
stream.write(format('Stack %s\n', chalk.bold(stackName)));
44+
}
45+
46+
if (!quiet && isImport) {
47+
stream.write('Parameters and rules created during migration do not affect resource configuration.\n');
48+
}
49+
4150
// detect and filter out mangled characters from the diff
4251
let filteredChangesCount = 0;
4352
if (diff.differenceCount && !strict) {
@@ -74,9 +83,6 @@ export function printStackDiff(
7483
break;
7584
}
7685
const nestedStack = nestedStackTemplates[nestedStackLogicalId];
77-
if (!quiet) {
78-
stream.write(format('Stack %s\n', chalk.bold(nestedStack.physicalName ?? nestedStackLogicalId)));
79-
}
8086

8187
(newTemplate as any)._template = nestedStack.generatedTemplate;
8288
stackDiffCount += printStackDiff(
@@ -85,6 +91,7 @@ export function printStackDiff(
8591
strict,
8692
context,
8793
quiet,
94+
nestedStack.physicalName ?? nestedStackLogicalId,
8895
undefined,
8996
isImport,
9097
stream,
@@ -112,10 +119,18 @@ export function printSecurityDiff(
112119
oldTemplate: any,
113120
newTemplate: cxapi.CloudFormationStackArtifact,
114121
requireApproval: RequireApproval,
122+
quiet?: boolean,
123+
stackName?: string,
115124
changeSet?: DescribeChangeSetOutput,
125+
stream: FormatStream = process.stderr,
116126
): boolean {
117127
const diff = fullDiff(oldTemplate, newTemplate.template, changeSet);
118128

129+
// must output the stack name if there are differences, even if quiet
130+
if (!quiet || !diff.isEmpty) {
131+
stream.write(format('Stack %s\n', chalk.bold(stackName)));
132+
}
133+
119134
if (difRequiresApproval(diff, requireApproval)) {
120135
// eslint-disable-next-line max-len
121136
warning(`This deployment will make potentially sensitive changes according to your current security approval level (--require-approval ${requireApproval}).`);

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

+191-7
Original file line numberDiff line numberDiff line change
@@ -393,15 +393,37 @@ describe('non-nested stacks', () => {
393393

394394
// WHEN
395395
const exitCode = await toolkit.diff({
396-
stackNames: ['A', 'A'],
396+
stackNames: ['D'],
397+
stream: buffer,
398+
fail: false,
399+
quiet: true,
400+
});
401+
402+
// THEN
403+
const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '');
404+
expect(plainTextOutput).not.toContain('Stack D');
405+
expect(plainTextOutput).not.toContain('There were no differences');
406+
expect(buffer.data.trim()).toContain('✨ Number of stacks with differences: 0');
407+
expect(exitCode).toBe(0);
408+
});
409+
410+
test('when quiet mode is enabled, stacks with diffs should print stack name to stdout', async () => {
411+
// GIVEN
412+
const buffer = new StringWritable();
413+
414+
// WHEN
415+
const exitCode = await toolkit.diff({
416+
stackNames: ['A'],
397417
stream: buffer,
398418
fail: false,
399419
quiet: true,
400420
});
401421

402422
// THEN
403-
expect(buffer.data.trim()).not.toContain('Stack A');
404-
expect(buffer.data.trim()).not.toContain('There were no differences');
423+
const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '');
424+
expect(plainTextOutput).toContain('Stack A');
425+
expect(plainTextOutput).not.toContain('There were no differences');
426+
expect(buffer.data.trim()).toContain('✨ Number of stacks with differences: 1');
405427
expect(exitCode).toBe(0);
406428
});
407429
});
@@ -548,10 +570,16 @@ describe('stack exists checks', () => {
548570
describe('nested stacks', () => {
549571
beforeEach(() => {
550572
cloudExecutable = new MockCloudExecutable({
551-
stacks: [{
552-
stackName: 'Parent',
553-
template: {},
554-
}],
573+
stacks: [
574+
{
575+
stackName: 'Parent',
576+
template: {},
577+
},
578+
{
579+
stackName: 'UnchangedParent',
580+
template: {},
581+
},
582+
],
555583
});
556584

557585
cloudFormation = instanceMockFrom(Deployments);
@@ -718,6 +746,78 @@ describe('nested stacks', () => {
718746
},
719747
physicalName: undefined,
720748
},
749+
UnChangedChild: {
750+
deployedTemplate: {
751+
Resources: {
752+
SomeResource: {
753+
Type: 'AWS::Something',
754+
Properties: {
755+
Prop: 'unchanged',
756+
},
757+
},
758+
},
759+
},
760+
generatedTemplate: {
761+
Resources: {
762+
SomeResource: {
763+
Type: 'AWS::Something',
764+
Properties: {
765+
Prop: 'unchanged',
766+
},
767+
},
768+
},
769+
},
770+
nestedStackTemplates: {},
771+
physicalName: 'UnChangedChild',
772+
},
773+
},
774+
});
775+
}
776+
if (stackArtifact.stackName === 'UnchangedParent') {
777+
stackArtifact.template.Resources = {
778+
UnchangedChild: {
779+
Type: 'AWS::CloudFormation::Stack',
780+
Properties: {
781+
TemplateURL: 'child-url',
782+
},
783+
},
784+
};
785+
return Promise.resolve({
786+
deployedRootTemplate: {
787+
Resources: {
788+
UnchangedChild: {
789+
Type: 'AWS::CloudFormation::Stack',
790+
Properties: {
791+
TemplateURL: 'child-url',
792+
},
793+
},
794+
},
795+
},
796+
nestedStacks: {
797+
UnchangedChild: {
798+
deployedTemplate: {
799+
Resources: {
800+
SomeResource: {
801+
Type: 'AWS::Something',
802+
Properties: {
803+
Prop: 'unchanged',
804+
},
805+
},
806+
},
807+
},
808+
generatedTemplate: {
809+
Resources: {
810+
SomeResource: {
811+
Type: 'AWS::Something',
812+
Properties: {
813+
Prop: 'unchanged',
814+
},
815+
},
816+
},
817+
},
818+
nestedStackTemplates: {},
819+
physicalName: 'UnchangedChild',
820+
},
721821
},
722822
});
723823
}
@@ -784,6 +884,7 @@ Stack newGrandChild
784884
Resources
785885
[+] AWS::Something SomeResource
786886
887+
Stack UnChangedChild
787888
788889
✨ Number of stacks with differences: 6`);
789890

@@ -847,12 +948,95 @@ Stack newGrandChild
847948
Resources
848949
[+] AWS::Something SomeResource
849950
951+
Stack UnChangedChild
850952
851953
✨ Number of stacks with differences: 6`);
852954

853955
expect(exitCode).toBe(0);
854956
expect(changeSetSpy).not.toHaveBeenCalled();
855957
});
958+
959+
test('when quiet mode is enabled, nested stacks with no diffs should not print stack name & no differences to stdout', async () => {
960+
// GIVEN
961+
const buffer = new StringWritable();
962+
963+
// WHEN
964+
const exitCode = await toolkit.diff({
965+
stackNames: ['UnchangedParent'],
966+
stream: buffer,
967+
fail: false,
968+
quiet: true,
969+
});
970+
971+
// THEN
972+
const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').replace(/[ \t]+$/mg, '');
973+
expect(plainTextOutput).not.toContain('Stack UnchangedParent');
974+
expect(plainTextOutput).not.toContain('There were no differences');
975+
expect(buffer.data.trim()).toContain('✨ Number of stacks with differences: 0');
976+
expect(exitCode).toBe(0);
977+
});
978+
979+
test('when quiet mode is enabled, nested stacks with diffs should print stack name to stdout', async () => {
980+
// GIVEN
981+
const buffer = new StringWritable();
982+
983+
// WHEN
984+
const exitCode = await toolkit.diff({
985+
stackNames: ['Parent'],
986+
stream: buffer,
987+
fail: false,
988+
quiet: true,
989+
});
990+
991+
// THEN
992+
const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').replace(/[ \t]+$/mg, '');
993+
expect(plainTextOutput).toContain(`Stack Parent
994+
Resources
995+
[~] AWS::CloudFormation::Stack AdditionChild
996+
└─ [~] TemplateURL
997+
├─ [-] addition-child-url-new
998+
└─ [+] addition-child-url-old
999+
[~] AWS::CloudFormation::Stack DeletionChild
1000+
└─ [~] TemplateURL
1001+
├─ [-] deletion-child-url-new
1002+
└─ [+] deletion-child-url-old
1003+
[~] AWS::CloudFormation::Stack ChangedChild
1004+
└─ [~] TemplateURL
1005+
├─ [-] changed-child-url-new
1006+
└─ [+] changed-child-url-old
1007+
1008+
Stack AdditionChild
1009+
Resources
1010+
[~] AWS::Something SomeResource
1011+
└─ [+] Prop
1012+
└─ added-value
1013+
1014+
Stack DeletionChild
1015+
Resources
1016+
[~] AWS::Something SomeResource
1017+
└─ [-] Prop
1018+
└─ value-to-be-removed
1019+
1020+
Stack ChangedChild
1021+
Resources
1022+
[~] AWS::Something SomeResource
1023+
└─ [~] Prop
1024+
├─ [-] old-value
1025+
└─ [+] new-value
1026+
1027+
Stack newChild
1028+
Resources
1029+
[+] AWS::Something SomeResource
1030+
1031+
Stack newGrandChild
1032+
Resources
1033+
[+] AWS::Something SomeResource
1034+
1035+
1036+
✨ Number of stacks with differences: 6`);
1037+
expect(plainTextOutput).not.toContain('Stack UnChangedChild');
1038+
expect(exitCode).toBe(0);
1039+
});
8561040
});
8571041

8581042
describe('--strict', () => {

0 commit comments

Comments
 (0)