Skip to content

Commit

Permalink
revert: prevent changeset diff for non-deployed stacks (#29485)
Browse files Browse the repository at this point in the history
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*
  • Loading branch information
comcalvi committed Mar 14, 2024
1 parent 6fef789 commit fac4a9c
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 48 deletions.
29 changes: 6 additions & 23 deletions packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts
Expand Up @@ -37,7 +37,6 @@ const DIFF_HANDLERS: HandlerRegistry = {
* @param currentTemplate the current state of the stack.
* @param newTemplate the target state of the stack.
* @param changeSet the change set for this stack.
* @param isImport if the stack is importing resources (a migrate stack).
*
* @returns a +types.TemplateDiff+ object that represents the changes that will happen if
* a stack which current state is described by +currentTemplate+ is updated with
Expand All @@ -47,7 +46,6 @@ export function fullDiff(
currentTemplate: { [key: string]: any },
newTemplate: { [key: string]: any },
changeSet?: CloudFormation.DescribeChangeSetOutput,
isImport?: boolean,
): types.TemplateDiff {

normalize(currentTemplate);
Expand All @@ -57,9 +55,6 @@ export function fullDiff(
filterFalsePositives(theDiff, changeSet);
addImportInformation(theDiff, changeSet);
}
if (isImport) {
addImportInformation(theDiff);
}

return theDiff;
}
Expand Down Expand Up @@ -214,25 +209,13 @@ function deepCopy(x: any): any {
return x;
}

/**
* Sets import flag to true for resource imports.
* When the changeset parameter is not set, the stack is a new migrate stack,
* so all resource changes are imports.
*/
function addImportInformation(diff: types.TemplateDiff, changeSet?: CloudFormation.DescribeChangeSetOutput) {
if (changeSet) {
const imports = findResourceImports(changeSet);
diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
if (imports.includes(logicalId)) {
change.isImport = true;
}
});
} else {
diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
logicalId; // dont know how to get past warning that this variable is not used.
function addImportInformation(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {
const imports = findResourceImports(changeSet);
diff.resources.forEachDifference((logicalId: string, change: types.ResourceDifference) => {
if (imports.includes(logicalId)) {
change.isImport = true;
});
}
}
});
}

function filterFalsePositives(diff: types.TemplateDiff, changeSet: CloudFormation.DescribeChangeSetOutput) {
Expand Down
26 changes: 4 additions & 22 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Expand Up @@ -136,14 +136,7 @@ export class CdkToolkit {
throw new Error(`There is no file at ${options.templatePath}`);
}

const stackExistsOptions = {
stack: stacks.firstStack,
deployName: stacks.firstStack.stackName,
};

const stackExists = await this.props.deployments.stackExists(stackExistsOptions);

const changeSet = (stackExists && options.changeSet) ? await createDiffChangeSet({
const changeSet = options.changeSet ? await createDiffChangeSet({
stack: stacks.firstStack,
uuid: uuid.v4(),
willExecute: false,
Expand Down Expand Up @@ -175,23 +168,13 @@ export class CdkToolkit {
removeNonImportResources(stack);
}

const stackExistsOptions = {
stack,
deployName: stack.stackName,
};

const stackExists = await this.props.deployments.stackExists(stackExistsOptions);

// if the stack does not already exist, do not do a changeset
// this prevents race conditions between deleting the dummy changeset stack and deploying the real changeset stack
// migrate stacks that import resources will not previously exist and default to old diff logic
const changeSet = (stackExists && options.changeSet) ? await createDiffChangeSet({
const changeSet = options.changeSet ? await createDiffChangeSet({
stack,
uuid: uuid.v4(),
deployments: this.props.deployments,
willExecute: false,
sdkProvider: this.props.sdkProvider,
parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]), // should this be stack?
parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]),
resourcesToImport,
stream,
}) : undefined;
Expand All @@ -200,11 +183,10 @@ export class CdkToolkit {
stream.write('Parameters and rules created during migration do not affect resource configuration.\n');
}

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

diffs += stackCount;
}
Expand Down
5 changes: 2 additions & 3 deletions packages/aws-cdk/lib/diff.ts
Expand Up @@ -26,10 +26,9 @@ export function printStackDiff(
quiet: boolean,
changeSet?: CloudFormation.DescribeChangeSetOutput,
stream: cfnDiff.FormatStream = process.stderr,
nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates },
isImport?: boolean): number {
nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates }): number {

let diff = cfnDiff.fullDiff(oldTemplate, newTemplate.template, changeSet, isImport);
let diff = cfnDiff.fullDiff(oldTemplate, newTemplate.template, changeSet);

// detect and filter out mangled characters from the diff
let filteredChangesCount = 0;
Expand Down

0 comments on commit fac4a9c

Please sign in to comment.