Skip to content

Commit f9f6d1d

Browse files
kaizenccgithub-actions
and
github-actions
authoredMar 28, 2025··
refactor(tmp-toolkit-helpers): DiffFormatter class (#283)
the point of this PR is to refactor `formatStackDiff` and `formatSecurityDiff` into the same class. there's more to do to simplify these functions, but i'm making small refactors a priority. the achievement of this PR is to get rid of the giant laundry list of properties that go into each function in favor of a property bag, and to store the repeated properties within a class. this is tested by current unit tests -- as long as they still pass, this refactor should have not changed any functionality. --- 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 <github-actions@github.com> Co-authored-by: github-actions <github-actions@github.com>
1 parent 2a9ee4c commit f9f6d1d

File tree

6 files changed

+521
-396
lines changed

6 files changed

+521
-396
lines changed
 
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,386 @@
1+
import { format } from 'node:util';
2+
import { Writable } from 'stream';
3+
import * as cxschema from '@aws-cdk/cloud-assembly-schema';
4+
import {
5+
type DescribeChangeSetOutput,
6+
type TemplateDiff,
7+
fullDiff,
8+
formatSecurityChanges,
9+
formatDifferences,
10+
mangleLikeCloudFormation,
11+
} from '@aws-cdk/cloudformation-diff';
12+
import type * as cxapi from '@aws-cdk/cx-api';
13+
import * as chalk from 'chalk';
14+
15+
import type { NestedStackTemplates } from '../cloudformation/nested-stack-templates';
16+
import type { IoHelper } from '../io/private';
17+
import { IoDefaultMessages } from '../io/private';
18+
import { RequireApproval } from '../require-approval';
19+
import { ToolkitError } from '../toolkit-error';
20+
21+
/*
22+
* Custom writable stream that collects text into a string buffer.
23+
* Used on classes that take in and directly write to a stream, but
24+
* we intend to capture the output rather than print.
25+
*/
26+
class StringWriteStream extends Writable {
27+
private buffer: string[] = [];
28+
29+
constructor() {
30+
super();
31+
}
32+
33+
_write(chunk: any, _encoding: string, callback: (error?: Error | null) => void): void {
34+
this.buffer.push(chunk.toString());
35+
callback();
36+
}
37+
38+
toString(): string {
39+
return this.buffer.join('');
40+
}
41+
}
42+
43+
/**
44+
* Output of formatSecurityDiff
45+
*/
46+
export interface FormatSecurityDiffOutput {
47+
/**
48+
* Complete formatted security diff, if it is prompt-worthy
49+
*/
50+
readonly formattedDiff?: string;
51+
}
52+
53+
/**
54+
* Output of formatStackDiff
55+
*/
56+
export interface FormatStackDiffOutput {
57+
/**
58+
* Number of stacks with diff changes
59+
*/
60+
readonly numStacksWithChanges: number;
61+
62+
/**
63+
* Complete formatted diff
64+
*/
65+
readonly formattedDiff: string;
66+
}
67+
68+
/**
69+
* Props for the Diff Formatter
70+
*/
71+
export interface DiffFormatterProps {
72+
/**
73+
* Helper for the IoHost class
74+
*/
75+
readonly ioHelper: IoHelper;
76+
77+
/**
78+
* The old/current state of the stack.
79+
*/
80+
readonly oldTemplate: any;
81+
82+
/**
83+
* The new/target state of the stack.
84+
*/
85+
readonly newTemplate: cxapi.CloudFormationStackArtifact;
86+
}
87+
88+
/**
89+
* Properties specific to formatting the security diff
90+
*/
91+
export interface FormatSecurityDiffOptions {
92+
/**
93+
* The approval level of the security diff
94+
*/
95+
readonly requireApproval: RequireApproval;
96+
97+
/**
98+
* The name of the Stack.
99+
*/
100+
readonly stackName?: string;
101+
102+
/**
103+
* The changeSet for the Stack.
104+
*
105+
* @default undefined
106+
*/
107+
readonly changeSet?: DescribeChangeSetOutput;
108+
}
109+
110+
/**
111+
* PRoperties specific to formatting the stack diff
112+
*/
113+
export interface FormatStackDiffOptions {
114+
/**
115+
* do not filter out AWS::CDK::Metadata or Rules
116+
*
117+
* @default false
118+
*/
119+
readonly strict?: boolean;
120+
121+
/**
122+
* lines of context to use in arbitrary JSON diff
123+
*
124+
* @default 3
125+
*/
126+
readonly context?: number;
127+
128+
/**
129+
* silences \'There were no differences\' messages
130+
*
131+
* @default false
132+
*/
133+
readonly quiet?: boolean;
134+
135+
/**
136+
* The name of the stack
137+
*/
138+
readonly stackName?: string;
139+
140+
/**
141+
* @default undefined
142+
*/
143+
readonly changeSet?: DescribeChangeSetOutput;
144+
145+
/**
146+
* @default false
147+
*/
148+
readonly isImport?: boolean;
149+
150+
/**
151+
* @default undefined
152+
*/
153+
readonly nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates };
154+
}
155+
156+
interface ReusableStackDiffOptions extends Omit<FormatStackDiffOptions, 'stackName' | 'nestedStackTemplates'> {
157+
readonly ioDefaultHelper: IoDefaultMessages;
158+
}
159+
160+
/**
161+
* Class for formatting the diff output
162+
*/
163+
export class DiffFormatter {
164+
private readonly ioHelper: IoHelper;
165+
private readonly oldTemplate: any;
166+
private readonly newTemplate: cxapi.CloudFormationStackArtifact;
167+
168+
constructor(props: DiffFormatterProps) {
169+
this.ioHelper = props.ioHelper;
170+
this.oldTemplate = props.oldTemplate;
171+
this.newTemplate = props.newTemplate;
172+
}
173+
174+
/**
175+
* Format the stack diff
176+
*/
177+
public formatStackDiff(options: FormatStackDiffOptions): FormatStackDiffOutput {
178+
const ioDefaultHelper = new IoDefaultMessages(this.ioHelper);
179+
return this.formatStackDiffHelper(
180+
this.oldTemplate,
181+
options.stackName,
182+
options.nestedStackTemplates,
183+
{
184+
...options,
185+
ioDefaultHelper,
186+
},
187+
);
188+
}
189+
190+
private formatStackDiffHelper(
191+
oldTemplate: any,
192+
stackName: string | undefined,
193+
nestedStackTemplates: { [nestedStackLogicalId: string]: NestedStackTemplates } | undefined,
194+
options: ReusableStackDiffOptions,
195+
) {
196+
let diff = fullDiff(oldTemplate, this.newTemplate.template, options.changeSet, options.isImport);
197+
198+
// The stack diff is formatted via `Formatter`, which takes in a stream
199+
// and sends its output directly to that stream. To faciliate use of the
200+
// global CliIoHost, we create our own stream to capture the output of
201+
// `Formatter` and return the output as a string for the consumer of
202+
// `formatStackDiff` to decide what to do with it.
203+
const stream = new StringWriteStream();
204+
205+
let numStacksWithChanges = 0;
206+
let formattedDiff = '';
207+
let filteredChangesCount = 0;
208+
try {
209+
// must output the stack name if there are differences, even if quiet
210+
if (stackName && (!options.quiet || !diff.isEmpty)) {
211+
stream.write(format('Stack %s\n', chalk.bold(stackName)));
212+
}
213+
214+
if (!options.quiet && options.isImport) {
215+
stream.write('Parameters and rules created during migration do not affect resource configuration.\n');
216+
}
217+
218+
// detect and filter out mangled characters from the diff
219+
if (diff.differenceCount && !options.strict) {
220+
const mangledNewTemplate = JSON.parse(mangleLikeCloudFormation(JSON.stringify(this.newTemplate.template)));
221+
const mangledDiff = fullDiff(this.oldTemplate, mangledNewTemplate, options.changeSet);
222+
filteredChangesCount = Math.max(0, diff.differenceCount - mangledDiff.differenceCount);
223+
if (filteredChangesCount > 0) {
224+
diff = mangledDiff;
225+
}
226+
}
227+
228+
// filter out 'AWS::CDK::Metadata' resources from the template
229+
// filter out 'CheckBootstrapVersion' rules from the template
230+
if (!options.strict) {
231+
obscureDiff(diff);
232+
}
233+
234+
if (!diff.isEmpty) {
235+
numStacksWithChanges++;
236+
237+
// formatDifferences updates the stream with the formatted stack diff
238+
formatDifferences(stream, diff, {
239+
...logicalIdMapFromTemplate(this.oldTemplate),
240+
...buildLogicalToPathMap(this.newTemplate),
241+
}, options.context);
242+
243+
// store the stream containing a formatted stack diff
244+
formattedDiff = stream.toString();
245+
} else if (!options.quiet) {
246+
options.ioDefaultHelper.info(chalk.green('There were no differences'));
247+
}
248+
} finally {
249+
stream.end();
250+
}
251+
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+
256+
for (const nestedStackLogicalId of Object.keys(nestedStackTemplates ?? {})) {
257+
if (!nestedStackTemplates) {
258+
break;
259+
}
260+
const nestedStack = nestedStackTemplates[nestedStackLogicalId];
261+
262+
(this.newTemplate as any)._template = nestedStack.generatedTemplate;
263+
const nextDiff = this.formatStackDiffHelper(
264+
nestedStack.deployedTemplate,
265+
nestedStack.physicalName ?? nestedStackLogicalId,
266+
nestedStack.nestedStackTemplates,
267+
options,
268+
);
269+
numStacksWithChanges += nextDiff.numStacksWithChanges;
270+
formattedDiff += nextDiff.formattedDiff;
271+
}
272+
273+
return {
274+
numStacksWithChanges,
275+
formattedDiff,
276+
};
277+
}
278+
279+
/**
280+
* Format the security diff
281+
*/
282+
public formatSecurityDiff(options: FormatSecurityDiffOptions): FormatSecurityDiffOutput {
283+
const ioDefaultHelper = new IoDefaultMessages(this.ioHelper);
284+
285+
const diff = fullDiff(this.oldTemplate, this.newTemplate.template, options.changeSet);
286+
287+
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+
294+
// The security diff is formatted via `Formatter`, which takes in a stream
295+
// and sends its output directly to that stream. To faciliate use of the
296+
// global CliIoHost, we create our own stream to capture the output of
297+
// `Formatter` and return the output as a string for the consumer of
298+
// `formatSecurityDiff` to decide what to do with it.
299+
const stream = new StringWriteStream();
300+
try {
301+
// formatSecurityChanges updates the stream with the formatted security diff
302+
formatSecurityChanges(stream, diff, buildLogicalToPathMap(this.newTemplate));
303+
} finally {
304+
stream.end();
305+
}
306+
// store the stream containing a formatted stack diff
307+
const formattedDiff = stream.toString();
308+
return { formattedDiff };
309+
}
310+
return {};
311+
}
312+
}
313+
314+
/**
315+
* Return whether the diff has security-impacting changes that need confirmation
316+
*
317+
* TODO: Filter the security impact determination based off of an enum that allows
318+
* us to pick minimum "severities" to alert on.
319+
*/
320+
function diffRequiresApproval(diff: TemplateDiff, requireApproval: RequireApproval) {
321+
switch (requireApproval) {
322+
case RequireApproval.NEVER: return false;
323+
case RequireApproval.ANY_CHANGE: return diff.permissionsAnyChanges;
324+
case RequireApproval.BROADENING: return diff.permissionsBroadened;
325+
default: throw new ToolkitError(`Unrecognized approval level: ${requireApproval}`);
326+
}
327+
}
328+
329+
export function buildLogicalToPathMap(stack: cxapi.CloudFormationStackArtifact) {
330+
const map: { [id: string]: string } = {};
331+
for (const md of stack.findMetadataByType(cxschema.ArtifactMetadataEntryType.LOGICAL_ID)) {
332+
map[md.data as string] = md.path;
333+
}
334+
return map;
335+
}
336+
337+
export function logicalIdMapFromTemplate(template: any) {
338+
const ret: Record<string, string> = {};
339+
340+
for (const [logicalId, resource] of Object.entries(template.Resources ?? {})) {
341+
const path = (resource as any)?.Metadata?.['aws:cdk:path'];
342+
if (path) {
343+
ret[logicalId] = path;
344+
}
345+
}
346+
return ret;
347+
}
348+
349+
/**
350+
* Remove any template elements that we don't want to show users.
351+
* This is currently:
352+
* - AWS::CDK::Metadata resource
353+
* - CheckBootstrapVersion Rule
354+
*/
355+
export function obscureDiff(diff: TemplateDiff) {
356+
if (diff.unknown) {
357+
// see https://github.com/aws/aws-cdk/issues/17942
358+
diff.unknown = diff.unknown.filter(change => {
359+
if (!change) {
360+
return true;
361+
}
362+
if (change.newValue?.CheckBootstrapVersion) {
363+
return false;
364+
}
365+
if (change.oldValue?.CheckBootstrapVersion) {
366+
return false;
367+
}
368+
return true;
369+
});
370+
}
371+
372+
if (diff.resources) {
373+
diff.resources = diff.resources.filter(change => {
374+
if (!change) {
375+
return true;
376+
}
377+
if (change.newResourceType === 'AWS::CDK::Metadata') {
378+
return false;
379+
}
380+
if (change.oldResourceType === 'AWS::CDK::Metadata') {
381+
return false;
382+
}
383+
return true;
384+
});
385+
}
386+
}

0 commit comments

Comments
 (0)
Please sign in to comment.