Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

revert: prevent changeset diff for non-deployed stacks #29485

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 6 additions & 23 deletions packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading