Skip to content

Commit

Permalink
feat(cli): skip deployment if template did not change
Browse files Browse the repository at this point in the history
Bail-out before executing and creating change sets when the currently deployed template is identical to the one we are about to deploy.

This resolves #6046, where a stack that contains nested stack(s) will always try to update since CloudFormation assumes the nested template might have changed. In the CDK, since nested template URLs are immutable, we can trust that the URL will be changed, and therefore invalidate the *parent* template.

This also fixes the bug described in #2553, where a stack that includes a `Transform` always fail to deploy with `No updates are to be performed` when there are no changes to it.

The switch `--force` can be used to override this behavior.

Added unit test and performed a few manual tests to verify both bugs are resolved.

Resolves #6216
  • Loading branch information
Elad Ben-Israel committed Feb 18, 2020
1 parent c505348 commit 50a585d
Show file tree
Hide file tree
Showing 6 changed files with 265 additions and 21 deletions.
5 changes: 5 additions & 0 deletions packages/aws-cdk/README.md
Expand Up @@ -128,6 +128,11 @@ bootstrapped (using `cdk bootstrap`), only stacks that are not using assets and
$ cdk deploy --app='node bin/main.js' MyStackName
```

Before creating a change set, `cdk deploy` will compare the template of the
currently deployed stack to the template that is about to be deployed and will
skip deployment if they are identical. Use `--force` to override this behavior
and always deploy the stack.

#### `cdk destroy`
Deletes a stack from it's environment. This will cause the resources in the stack to be destroyed (unless they were
configured with a `DeletionPolicy` of `Retain`). During the stack destruction, the command will output progress
Expand Down
4 changes: 3 additions & 1 deletion packages/aws-cdk/bin/cdk.ts
Expand Up @@ -68,6 +68,7 @@ async function parseCommandLineArguments() {
.option('notification-arns', {type: 'array', desc: 'ARNs of SNS topics that CloudFormation will notify with stack related events', nargs: 1, requiresArg: true})
.option('tags', { type: 'array', alias: 't', desc: 'Tags to add to the stack (KEY=VALUE)', nargs: 1, requiresArg: true })
.option('execute', {type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true})
.option('force', { type: 'boolean', desc: 'Always deploy stack even if templates are identical', default: false })
)
.command('destroy [STACKS..]', 'Destroy the stack(s) named STACKS', yargs => yargs
.option('exclusively', { type: 'boolean', alias: 'e', desc: 'Only destroy requested stacks, don\'t include dependees' })
Expand Down Expand Up @@ -224,7 +225,8 @@ async function initCommandLine() {
reuseAssets: args['build-exclude'],
tags: configuration.settings.get(['tags']),
sdk: aws,
execute: args.execute
execute: args.execute,
force: args.force,
});

case 'destroy':
Expand Down
76 changes: 72 additions & 4 deletions packages/aws-cdk/lib/api/deploy-stack.ts
Expand Up @@ -5,7 +5,7 @@ import * as uuid from 'uuid';
import { Tag } from "../api/cxapp/stacks";
import { prepareAssets } from '../assets';
import { debug, error, print } from '../logging';
import { toYAML } from '../serialize';
import { deserializeStructure, toYAML } from '../serialize';
import { Mode } from './aws-auth/credentials';
import { ToolkitInfo } from './toolkit-info';
import { changeSetHasNoChanges, describeStack, stackExists, stackFailedCreating, waitForChangeSet, waitForStack } from './util/cloudformation';
Expand Down Expand Up @@ -54,6 +54,12 @@ export interface DeployStackOptions {
* @default - no additional parameters will be passed to the template
*/
parameters?: { [name: string]: string | undefined };

/**
* Deploy even if the deployed template is identical to the one we are about to deploy.
* @default false
*/
force?: boolean;
}

const LARGE_TEMPLATE_SIZE_KB = 50;
Expand All @@ -64,6 +70,28 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
throw new Error(`The stack ${options.stack.displayName} does not have an environment`);
}

const cfn = await options.sdk.cloudFormation(options.stack.environment.account, options.stack.environment.region, Mode.ForWriting);
const deployName = options.deployName || options.stack.stackName;

if (!options.force) {
debug(`checking if we can skip this stack based on the currently deployed template (use --force to override)`);
const deployed = await getDeployedTemplate(cfn, deployName);
if (deployed && JSON.stringify(options.stack.template) === JSON.stringify(deployed.template)) {
debug(`${deployName}: no change in template, skipping (use --force to override)`);
return {
noOp: true,
outputs: await getStackOutputs(cfn, deployName),
stackArn: deployed.stackId,
stackArtifact: options.stack
};
} else {
debug(`${deployName}: template changed, deploying...`);
}
}

// bail out if the current template is exactly the same as the one we are about to deploy
// in cdk-land, this means nothing changed because assets (and therefore nested stacks) are immutable.

const params = await prepareAssets(options.stack, options.toolkitInfo, options.reuseAssets);

// add passed CloudFormation parameters
Expand All @@ -76,11 +104,8 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
}
}

const deployName = options.deployName || options.stack.stackName;

const executionId = uuid.v4();

const cfn = await options.sdk.cloudFormation(options.stack.environment.account, options.stack.environment.region, Mode.ForWriting);
const bodyParameter = await makeBodyParameter(options.stack, options.toolkitInfo);

if (await stackFailedCreating(cfn, deployName)) {
Expand Down Expand Up @@ -212,3 +237,46 @@ export async function destroyStack(options: DestroyStackOptions) {
}
return;
}

async function getDeployedTemplate(cfn: aws.CloudFormation, stackName: string): Promise<{ template: any, stackId: string } | undefined> {
const stackId = await getStackId(cfn, stackName);
if (!stackId) {
return undefined;
}

const template = await readCurrentTemplate(cfn, stackName);
return { stackId, template };
}

export async function readCurrentTemplate(cfn: aws.CloudFormation, stackName: string) {
try {
const response = await cfn.getTemplate({ StackName: stackName, TemplateStage: 'Original' }).promise();
return (response.TemplateBody && deserializeStructure(response.TemplateBody)) || {};
} catch (e) {
if (e.code === 'ValidationError' && e.message === `Stack with id ${stackName} does not exist`) {
return {};
} else {
throw e;
}
}
}

async function getStackId(cfn: aws.CloudFormation, stackName: string): Promise<string | undefined> {
try {
const stacks = await cfn.describeStacks({ StackName: stackName }).promise();
if (!stacks.Stacks) {
return undefined;
}
if (stacks.Stacks.length !== 1) {
return undefined;
}

return stacks.Stacks[0].StackId!;

} catch (e) {
if (e.message.includes('does not exist')) {
return undefined;
}
throw e;
}
}
24 changes: 10 additions & 14 deletions packages/aws-cdk/lib/api/deployment-target.ts
@@ -1,9 +1,8 @@
import { CloudFormationStackArtifact } from '@aws-cdk/cx-api';
import { Tag } from "../api/cxapp/stacks";
import { debug } from '../logging';
import { deserializeStructure } from '../serialize';
import { Mode } from './aws-auth/credentials';
import { deployStack, DeployStackResult } from './deploy-stack';
import { deployStack, DeployStackResult, readCurrentTemplate } from './deploy-stack';
import { loadToolkitInfo } from './toolkit-info';
import { ISDK } from './util/sdk';

Expand Down Expand Up @@ -31,6 +30,12 @@ export interface DeployStackOptions {
reuseAssets?: string[];
tags?: Tag[];
execute?: boolean;

/**
* Force deployment, even if the deployed template is identical to the one we are about to deploy.
* @default false deployment will be skipped if the template is identical
*/
force?: boolean;
}

export interface ProvisionerProps {
Expand All @@ -49,18 +54,8 @@ export class CloudFormationDeploymentTarget implements IDeploymentTarget {

public async readCurrentTemplate(stack: CloudFormationStackArtifact): Promise<Template> {
debug(`Reading existing template for stack ${stack.displayName}.`);

const cfn = await this.aws.cloudFormation(stack.environment.account, stack.environment.region, Mode.ForReading);
try {
const response = await cfn.getTemplate({ StackName: stack.stackName }).promise();
return (response.TemplateBody && deserializeStructure(response.TemplateBody)) || {};
} catch (e) {
if (e.code === 'ValidationError' && e.message === `Stack with id ${stack.stackName} does not exist`) {
return {};
} else {
throw e;
}
}
return readCurrentTemplate(cfn, stack.stackName);
}

public async deployStack(options: DeployStackOptions): Promise<DeployStackResult> {
Expand All @@ -75,7 +70,8 @@ export class CloudFormationDeploymentTarget implements IDeploymentTarget {
reuseAssets: options.reuseAssets,
toolkitInfo,
tags: options.tags,
execute: options.execute
execute: options.execute,
force: options.force
});
}
}
9 changes: 8 additions & 1 deletion packages/aws-cdk/lib/cdk-toolkit.ts
Expand Up @@ -142,7 +142,8 @@ export class CdkToolkit {
reuseAssets: options.reuseAssets,
notificationArns: options.notificationArns,
tags,
execute: options.execute
execute: options.execute,
force: options.force
});

const message = result.noOp
Expand Down Expand Up @@ -308,6 +309,12 @@ export interface DeployOptions {
* @default true
*/
execute?: boolean;

/**
* Always deploy, even if templates are identical.
* @default false
*/
force?: boolean;
}

export interface DestroyOptions {
Expand Down
168 changes: 167 additions & 1 deletion packages/aws-cdk/test/api/deploy-stack.test.ts
Expand Up @@ -2,9 +2,11 @@ import { deployStack } from '../../lib';
import { testStack } from '../util';
import { MockSDK } from '../util/mock-sdk';

const FAKE_TEMPLATE = { resource: 'noerrorresource' };

const FAKE_STACK = testStack({
stackName: 'withouterrors',
template: { resource: 'noerrorresource' },
template: FAKE_TEMPLATE,
});

test('do deploy executable change set with 0 changes', async () => {
Expand Down Expand Up @@ -94,3 +96,167 @@ test('correctly passes CFN parameters, ignoring ones with empty values', async (
{ ParameterKey: 'A', ParameterValue: 'A-value' },
]);
});

test('deploy is skipped if template did not change', async () => {
const sdk = new MockSDK();
let describeStacksInput: AWS.CloudFormation.DescribeStacksInput;
let getTemplateInput: AWS.CloudFormation.GetTemplateInput;
let createChangeSetCalled = false;
let executeChangeSetCalled = false;

sdk.stubCloudFormation({
getTemplate(input) {
getTemplateInput = input;
return {
TemplateBody: JSON.stringify(FAKE_TEMPLATE)
};
},
describeStacks(input) {
describeStacksInput = input;
return {
Stacks: [
{
StackName: 'mock-stack-name',
StackId: 'mock-stack-id',
CreationTime: new Date(),
StackStatus: 'CREATE_COMPLETE'
}
]
};
},
createChangeSet() {
createChangeSetCalled = true;
return { };
},
executeChangeSet() {
executeChangeSetCalled = true;
return { };
}
});

await deployStack({
stack: FAKE_STACK,
sdk
});

expect(createChangeSetCalled).toBeFalsy();
expect(executeChangeSetCalled).toBeFalsy();
expect(describeStacksInput!).toStrictEqual({ StackName: 'withouterrors' });
expect(getTemplateInput!).toStrictEqual({ StackName: 'withouterrors', TemplateStage: 'Original' });
});

test('deploy not skipped if template did not change and --force is applied', async () => {
const sdk = new MockSDK();
let describeStacksInput: AWS.CloudFormation.DescribeStacksInput;
let getTemplateInput: AWS.CloudFormation.GetTemplateInput;
let createChangeSetCalled = false;
let executeChangeSetCalled = false;
let describeChangeSetCalled = false;

sdk.stubCloudFormation({
getTemplate(input) {
getTemplateInput = input;
return {
TemplateBody: JSON.stringify(FAKE_TEMPLATE)
};
},
describeStacks(input) {
describeStacksInput = input;
return {
Stacks: [
{
StackName: 'mock-stack-name',
StackId: 'mock-stack-id',
CreationTime: new Date(),
StackStatus: 'CREATE_COMPLETE'
}
]
};
},
createChangeSet() {
createChangeSetCalled = true;
return { };
},
executeChangeSet() {
executeChangeSetCalled = true;
return { };
},
describeChangeSet() {
describeChangeSetCalled = true;
return {
Status: 'CREATE_COMPLETE',
Changes: [],
};
}
});

await deployStack({
stack: FAKE_STACK,
sdk,
force: true
});

expect(createChangeSetCalled).toBeTruthy();
expect(executeChangeSetCalled).toBeTruthy();
expect(describeChangeSetCalled).toBeTruthy();
expect(getTemplateInput!).toBeUndefined();
expect(describeStacksInput!).toStrictEqual({ StackName: "withouterrors" });
});

test('deploy not skipped if template changed', async () => {
const sdk = new MockSDK();
let describeStacksInput: AWS.CloudFormation.DescribeStacksInput;
let getTemplateInput: AWS.CloudFormation.GetTemplateInput;
let createChangeSetCalled = false;
let executeChangeSetCalled = false;
let describeChangeSetCalled = false;

sdk.stubCloudFormation({
getTemplate(input) {
getTemplateInput = input;
return {
TemplateBody: JSON.stringify({ changed: 123 })
};
},
describeStacks(input) {
describeStacksInput = input;
return {
Stacks: [
{
StackName: 'mock-stack-name',
StackId: 'mock-stack-id',
CreationTime: new Date(),
StackStatus: 'CREATE_COMPLETE'
}
]
};
},
createChangeSet() {
createChangeSetCalled = true;
return { };
},
executeChangeSet() {
executeChangeSetCalled = true;
return { };
},
describeChangeSet() {
describeChangeSetCalled = true;
return {
Status: 'CREATE_COMPLETE',
Changes: [],
};
}
});

await deployStack({
stack: FAKE_STACK,
sdk,
force: true
});

expect(createChangeSetCalled).toBeTruthy();
expect(executeChangeSetCalled).toBeTruthy();
expect(describeChangeSetCalled).toBeTruthy();
expect(getTemplateInput!).toBeUndefined();
expect(describeStacksInput!).toStrictEqual({ StackName: "withouterrors" });
});

0 comments on commit 50a585d

Please sign in to comment.