Skip to content

Commit fac4a9c

Browse files
authored
revert: prevent changeset diff for non-deployed stacks (#29485)
reverts #29394, which prevented changeset creation during `cdk diff` if a stack did not exist. The lookup of the stack to check its existence is failing for customers that have CI/CD that won't assume the deploy role when running CDK diff. Long-term fix: delete the stack if it didn't exist before we created the changeset, but wait for its state to reach `DELETE_COMPLETE` to avoid problems with subsequent commands. Preserves changes from #29172 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 6fef789 commit fac4a9c

File tree

3 files changed

+12
-48
lines changed

3 files changed

+12
-48
lines changed

packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts

+6-23
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ const DIFF_HANDLERS: HandlerRegistry = {
3737
* @param currentTemplate the current state of the stack.
3838
* @param newTemplate the target state of the stack.
3939
* @param changeSet the change set for this stack.
40-
* @param isImport if the stack is importing resources (a migrate stack).
4140
*
4241
* @returns a +types.TemplateDiff+ object that represents the changes that will happen if
4342
* a stack which current state is described by +currentTemplate+ is updated with
@@ -47,7 +46,6 @@ export function fullDiff(
4746
currentTemplate: { [key: string]: any },
4847
newTemplate: { [key: string]: any },
4948
changeSet?: CloudFormation.DescribeChangeSetOutput,
50-
isImport?: boolean,
5149
): types.TemplateDiff {
5250

5351
normalize(currentTemplate);
@@ -57,9 +55,6 @@ export function fullDiff(
5755
filterFalsePositives(theDiff, changeSet);
5856
addImportInformation(theDiff, changeSet);
5957
}
60-
if (isImport) {
61-
addImportInformation(theDiff);
62-
}
6358

6459
return theDiff;
6560
}
@@ -214,25 +209,13 @@ function deepCopy(x: any): any {
214209
return x;
215210
}
216211

217-
/**
218-
* Sets import flag to true for resource imports.
219-
* When the changeset parameter is not set, the stack is a new migrate stack,
220-
* so all resource changes are imports.
221-
*/
222-
function addImportInformation(diff: types.TemplateDiff, changeSet?: CloudFormation.DescribeChangeSetOutput) {
223-
if (changeSet) {
224-
const imports = findResourceImports(changeSet);
225-
diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
226-
if (imports.includes(logicalId)) {
227-
change.isImport = true;
228-
}
229-
});
230-
} else {
231-
diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
232-
logicalId; // dont know how to get past warning that this variable is not used.
212+
function addImportInformation(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {
213+
const imports = findResourceImports(changeSet);
214+
diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
215+
if (imports.includes(logicalId)) {
233216
change.isImport = true;
234-
});
235-
}
217+
}
218+
});
236219
}
237220

238221
function filterFalsePositives(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {

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

+4-22
Original file line numberDiff line numberDiff line change
@@ -136,14 +136,7 @@ export class CdkToolkit {
136136
throw new Error(`There is no file at ${options.templatePath}`);
137137
}
138138

139-
const stackExistsOptions = {
140-
stack: stacks.firstStack,
141-
deployName: stacks.firstStack.stackName,
142-
};
143-
144-
const stackExists = await this.props.deployments.stackExists(stackExistsOptions);
145-
146-
const changeSet = (stackExists && options.changeSet) ? await createDiffChangeSet({
139+
const changeSet = options.changeSet ? await createDiffChangeSet({
147140
stack: stacks.firstStack,
148141
uuid: uuid.v4(),
149142
willExecute: false,
@@ -175,23 +168,13 @@ export class CdkToolkit {
175168
removeNonImportResources(stack);
176169
}
177170

178-
const stackExistsOptions = {
179-
stack,
180-
deployName: stack.stackName,
181-
};
182-
183-
const stackExists = await this.props.deployments.stackExists(stackExistsOptions);
184-
185-
// if the stack does not already exist, do not do a changeset
186-
// this prevents race conditions between deleting the dummy changeset stack and deploying the real changeset stack
187-
// migrate stacks that import resources will not previously exist and default to old diff logic
188-
const changeSet = (stackExists && options.changeSet) ? await createDiffChangeSet({
171+
const changeSet = options.changeSet ? await createDiffChangeSet({
189172
stack,
190173
uuid: uuid.v4(),
191174
deployments: this.props.deployments,
192175
willExecute: false,
193176
sdkProvider: this.props.sdkProvider,
194-
parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), // should this be stack?
177+
parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]),
195178
resourcesToImport,
196179
stream,
197180
}) : undefined;
@@ -200,11 +183,10 @@ export class CdkToolkit {
200183
stream.write('Parameters and rules created during migration do not affect resource configuration.\n');
201184
}
202185

203-
// pass a boolean to print if the stack is a migrate stack in order to set all resource diffs to import
204186
const stackCount =
205187
options.securityOnly
206188
? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, changeSet)))
207-
: (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, stream, nestedStacks, !!resourcesToImport));
189+
: (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, stream, nestedStacks));
208190

209191
diffs += stackCount;
210192
}

packages/aws-cdk/lib/diff.ts

+2-3
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,9 @@ export function printStackDiff(
2626
quiet: boolean,
2727
changeSet?: CloudFormation.DescribeChangeSetOutput,
2828
stream: cfnDiff.FormatStream = process.stderr,
29-
nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates },
30-
isImport?: boolean): number {
29+
nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates }): number {
3130

32-
let diff = cfnDiff.fullDiff(oldTemplate, newTemplate.template, changeSet, isImport);
31+
let diff = cfnDiff.fullDiff(oldTemplate, newTemplate.template, changeSet);
3332

3433
// detect and filter out mangled characters from the diff
3534
let filteredChangesCount = 0;

0 commit comments

Comments
 (0)