Skip to content

Commit d3f6c3c

Browse files
kaizenccgithub-actions
and
github-actions
authored
fix(cli): diff always prints stack name (#304)
Fixes #302 In #264, we capture the output of the Formatter into a stream, and that is then converted to a string. This diff only gets printed if the diff isn't empty, but by some quirk `formatStackDiff` used to print the stack name _outside_ of the stream, thus creating the (correct) behavior. #264 made sure that `format` doesn't print, so we need to make sure that the consumer of `formatStackDiff` prints the relevant info. This PR makes sure that the stack name is returned as the formatted diff even if the actual diff is empty. We will print the right information when we print the formatted diff now. I also made similar modifications to `formatSecurityDiff`, as we should move away from `format` methods printing anything at all. BEFORE (cdk 2.1006.0): <img width="326" alt="Screenshot 2025-04-02 at 9 04 04 AM" src="https://github.com/user-attachments/assets/03b92517-e475-4c27-a4c1-52217be824c8" /> AFTER (my local cdk): <img width="322" alt="Screenshot 2025-04-02 at 9 03 11 AM" src="https://github.com/user-attachments/assets/53a7d3e0-2e62-45ca-898b-93c923028346" /> --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license --------- Signed-off-by: github-actions <[email protected]> Co-authored-by: github-actions <[email protected]>
1 parent 20337de commit d3f6c3c

File tree

3 files changed

+33
-22
lines changed

3 files changed

+33
-22
lines changed

packages/@aws-cdk/tmp-toolkit-helpers/src/api/diff/diff-formatter.ts

+14-15
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ export class DiffFormatter {
208208
try {
209209
// must output the stack name if there are differences, even if quiet
210210
if (stackName && (!options.quiet || !diff.isEmpty)) {
211-
stream.write(format('Stack %s\n', chalk.bold(stackName)));
211+
stream.write(format(`Stack ${chalk.bold(stackName)}\n`));
212212
}
213213

214214
if (!options.quiet && options.isImport) {
@@ -239,20 +239,19 @@ export class DiffFormatter {
239239
...logicalIdMapFromTemplate(this.oldTemplate),
240240
...buildLogicalToPathMap(this.newTemplate),
241241
}, options.context);
242-
243-
// store the stream containing a formatted stack diff
244-
formattedDiff = stream.toString();
245242
} else if (!options.quiet) {
246-
options.ioDefaultHelper.info(chalk.green('There were no differences'));
243+
stream.write(chalk.green('There were no differences\n'));
244+
}
245+
246+
if (filteredChangesCount > 0) {
247+
stream.write(chalk.yellow(`Omitted ${filteredChangesCount} changes because they are likely mangled non-ASCII characters. Use --strict to print them.\n`));
247248
}
248249
} finally {
250+
// store the stream containing a formatted stack diff
251+
formattedDiff = stream.toString();
249252
stream.end();
250253
}
251254

252-
if (filteredChangesCount > 0) {
253-
options.ioDefaultHelper.info(chalk.yellow(`Omitted ${filteredChangesCount} changes because they are likely mangled non-ASCII characters. Use --strict to print them.`));
254-
}
255-
256255
for (const nestedStackLogicalId of Object.keys(nestedStackTemplates ?? {})) {
257256
if (!nestedStackTemplates) {
258257
break;
@@ -285,18 +284,18 @@ export class DiffFormatter {
285284
const diff = fullDiff(this.oldTemplate, this.newTemplate.template, options.changeSet);
286285

287286
if (diffRequiresApproval(diff, options.requireApproval)) {
288-
ioDefaultHelper.info(format('Stack %s\n', chalk.bold(options.stackName)));
289-
290-
// eslint-disable-next-line max-len
291-
ioDefaultHelper.warning(`This deployment will make potentially sensitive changes according to your current security approval level (--require-approval ${options.requireApproval}).`);
292-
ioDefaultHelper.warning('Please confirm you intend to make the following modifications:\n');
293-
294287
// The security diff is formatted via `Formatter`, which takes in a stream
295288
// and sends its output directly to that stream. To faciliate use of the
296289
// global CliIoHost, we create our own stream to capture the output of
297290
// `Formatter` and return the output as a string for the consumer of
298291
// `formatSecurityDiff` to decide what to do with it.
299292
const stream = new StringWriteStream();
293+
294+
stream.write(format(`Stack ${chalk.bold(options.stackName)}\n`));
295+
296+
// eslint-disable-next-line max-len
297+
ioDefaultHelper.warning(`This deployment will make potentially sensitive changes according to your current security approval level (--require-approval ${options.requireApproval}).`);
298+
ioDefaultHelper.warning('Please confirm you intend to make the following modifications:\n');
300299
try {
301300
// formatSecurityChanges updates the stream with the formatted security diff
302301
formatSecurityChanges(stream, diff, buildLogicalToPathMap(this.newTemplate));

packages/@aws-cdk/tmp-toolkit-helpers/test/api/diff/diff.test.ts

+9-3
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ describe('formatStackDiff', () => {
5555
} as any;
5656
});
5757

58-
test('returns no changes when templates are identical', () => {
58+
test('returns no differences when templates are identical', () => {
5959
// WHEN
6060
const formatter = new DiffFormatter({
6161
ioHelper: mockIoHelper,
@@ -76,8 +76,12 @@ describe('formatStackDiff', () => {
7676

7777
// THEN
7878
expect(result.numStacksWithChanges).toBe(0);
79-
expect(result.formattedDiff).toBe('');
80-
expect(mockIoDefaultMessages.info).toHaveBeenCalledWith(expect.stringContaining('no differences'));
79+
expect(result.formattedDiff).toBeDefined();
80+
const sanitizedDiff = result.formattedDiff!.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').trim();
81+
expect(sanitizedDiff).toBe(
82+
'Stack test-stack\n' +
83+
'There were no differences',
84+
);
8185
});
8286

8387
test('formats differences when changes exist', () => {
@@ -256,6 +260,7 @@ describe('formatSecurityDiff', () => {
256260
expect(result.formattedDiff).toBeDefined();
257261
const sanitizedDiff = result.formattedDiff!.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').trim();
258262
expect(sanitizedDiff).toBe(
263+
'Stack test-stack\n' +
259264
'IAM Statement Changes\n' +
260265
'┌───┬─────────────┬────────┬────────────────┬──────────────────────────────┬───────────┐\n' +
261266
'│ │ Resource │ Effect │ Action │ Principal │ Condition │\n' +
@@ -291,6 +296,7 @@ describe('formatSecurityDiff', () => {
291296
);
292297
const sanitizedDiff = result.formattedDiff!.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').trim();
293298
expect(sanitizedDiff).toBe(
299+
'Stack test-stack\n' +
294300
'IAM Statement Changes\n' +
295301
'┌───┬─────────────┬────────┬────────────────┬──────────────────────────────┬───────────┐\n' +
296302
'│ │ Resource │ Effect │ Action │ Principal │ Condition │\n' +

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

+10-4
Original file line numberDiff line numberDiff line change
@@ -865,7 +865,7 @@ describe('nested stacks', () => {
865865
});
866866

867867
// THEN
868-
const plainTextOutput = notifySpy.mock.calls[1][0].message.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').replace(/[ \t]+$/gm, '');
868+
const plainTextOutput = notifySpy.mock.calls[0][0].message.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').replace(/[ \t]+$/gm, '');
869869
expect(plainTextOutput.trim()).toEqual(`Stack Parent
870870
Resources
871871
[~] AWS::CloudFormation::Stack AdditionChild
@@ -906,7 +906,10 @@ Resources
906906
907907
Stack newGrandChild
908908
Resources
909-
[+] AWS::Something SomeResource`);
909+
[+] AWS::Something SomeResource
910+
911+
Stack UnChangedChild
912+
There were no differences`);
910913

911914
expect(notifySpy).toHaveBeenCalledWith(expect.objectContaining({
912915
message: expect.stringContaining('✨ Number of stacks with differences: 6'),
@@ -926,7 +929,7 @@ Resources
926929
});
927930

928931
// THEN
929-
const plainTextOutput = notifySpy.mock.calls[2][0].message.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').replace(/[ \t]+$/gm, '');
932+
const plainTextOutput = notifySpy.mock.calls[1][0].message.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '').replace(/[ \t]+$/gm, '');
930933
expect(plainTextOutput.trim()).toEqual(`Stack Parent
931934
Resources
932935
[~] AWS::CloudFormation::Stack AdditionChild
@@ -967,7 +970,10 @@ Resources
967970
968971
Stack newGrandChild
969972
Resources
970-
[+] AWS::Something SomeResource`);
973+
[+] AWS::Something SomeResource
974+
975+
Stack UnChangedChild
976+
There were no differences`);
971977

972978
expect(notifySpy).toHaveBeenCalledWith(expect.objectContaining({
973979
message: expect.stringContaining('✨ Number of stacks with differences: 6'),

0 commit comments

Comments
 (0)