Skip to content

Commit

Permalink
fix(cli): fast "no-op" deploys do not consider tags
Browse files Browse the repository at this point in the history
Stack-level tags apply to all supported resources in a stack. If they change, a
fast "no-op" deploy should not occur (e.g. a tag with the CDK version or a tag
with the commit hash when running in CI).

Fix the `'deploy not skipped if template changed'` test that was including the
`force` option.

Fix CLI integ tests not working anymore after aws#6463 and the `0.0.0` version.
  • Loading branch information
jogold committed Feb 26, 2020
1 parent 9a92be9 commit ef9e42d
Show file tree
Hide file tree
Showing 4 changed files with 193 additions and 24 deletions.
45 changes: 34 additions & 11 deletions packages/aws-cdk/lib/api/deploy-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,25 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
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)`);
debug(`checking if we can skip this stack based on the currently deployed template and tags (use --force to override)`);
const deployed = await getDeployedStack(cfn, deployName);
let tagsIdentical = true;
if (deployed?.tags && options.tags) {
if (deployed.tags.length !== options.tags.length) {
tagsIdentical = false;
}

for (const optionsTag of options.tags) {
const deployedTag = deployed.tags.find(tag => tag.Key === optionsTag.Key);

if (!deployedTag || deployedTag.Value !== optionsTag.Value) {
tagsIdentical = false;
break;
}
}
}
if (deployed && JSON.stringify(options.stack.template) === JSON.stringify(deployed.template) && tagsIdentical) {
debug(`${deployName}: no change in template and tags, skipping (use --force to override)`);
return {
noOp: true,
outputs: await getStackOutputs(cfn, deployName),
Expand Down Expand Up @@ -239,14 +254,22 @@ 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) {
async function getDeployedStack(cfn: aws.CloudFormation, stackName: string): Promise<{ stackId: string, template: any, tags: Tag[] } | undefined> {
const stack = await getStack(cfn, stackName);
if (!stack) {
return undefined;
}

if (!stack.StackId) {
return undefined;
}

const template = await readCurrentTemplate(cfn, stackName);
return { stackId, template };
return {
stackId: stack.StackId,
tags: stack.Tags ?? [],
template
};
}

export async function readCurrentTemplate(cfn: aws.CloudFormation, stackName: string) {
Expand All @@ -262,7 +285,7 @@ export async function readCurrentTemplate(cfn: aws.CloudFormation, stackName: st
}
}

async function getStackId(cfn: aws.CloudFormation, stackName: string): Promise<string | undefined> {
async function getStack(cfn: aws.CloudFormation, stackName: string): Promise<aws.CloudFormation.Stack | undefined> {
try {
const stacks = await cfn.describeStacks({ StackName: stackName }).promise();
if (!stacks.Stacks) {
Expand All @@ -272,12 +295,12 @@ async function getStackId(cfn: aws.CloudFormation, stackName: string): Promise<s
return undefined;
}

return stacks.Stacks[0].StackId!;
return stacks.Stacks[0];

} catch (e) {
if (e.message.includes('does not exist')) {
return undefined;
}
throw e;
}
}
}
144 changes: 140 additions & 4 deletions packages/aws-cdk/test/api/deploy-stack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,143 @@ test('deploy is skipped if template did not change', async () => {
expect(getTemplateInput!).toStrictEqual({ StackName: 'withouterrors', TemplateStage: 'Original' });
});

test('deploy is skipped if template and tags 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',
Tags: [
{
Key: 'Key1',
Value: 'Value1'
},
{
Key: 'Key2',
Value: 'Value2'
}
]
}
]
};
},
createChangeSet() {
createChangeSetCalled = true;
return { };
},
executeChangeSet() {
executeChangeSetCalled = true;
return { };
}
});

await deployStack({
stack: FAKE_STACK,
tags: [
{
Key: 'Key1',
Value: 'Value1'
},
{
Key: 'Key2',
Value: 'Value2'
}
],
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 changed but tags 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(FAKE_TEMPLATE)
};
},
describeStacks(input) {
describeStacksInput = input;
return {
Stacks: [
{
StackName: 'mock-stack-name',
StackId: 'mock-stack-id',
CreationTime: new Date(),
StackStatus: 'CREATE_COMPLETE',
Tags: [
{
Key: 'Key',
Value: 'Value'
},
]
}
]
};
},
createChangeSet() {
createChangeSetCalled = true;
return { };
},
executeChangeSet() {
executeChangeSetCalled = true;
return { };
},
describeChangeSet() {
describeChangeSetCalled = true;
return {
Status: 'CREATE_COMPLETE',
Changes: [],
};
}
});

await deployStack({
stack: FAKE_STACK,
sdk,
tags: [
{
Key: 'Key',
Value: 'NewValue'
}
]
});

expect(createChangeSetCalled).toBeTruthy();
expect(executeChangeSetCalled).toBeTruthy();
expect(describeChangeSetCalled).toBeTruthy();
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;
Expand Down Expand Up @@ -250,13 +387,12 @@ test('deploy not skipped if template changed', async () => {

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

expect(createChangeSetCalled).toBeTruthy();
expect(executeChangeSetCalled).toBeTruthy();
expect(describeChangeSetCalled).toBeTruthy();
expect(getTemplateInput!).toBeUndefined();
expect(describeStacksInput!).toStrictEqual({ StackName: "withouterrors" });
});
expect(getTemplateInput!).toStrictEqual({ StackName: 'withouterrors', TemplateStage: 'Original' });
});
18 changes: 9 additions & 9 deletions packages/aws-cdk/test/integ/cli/common.bash
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,20 @@ function prepare_fixture() {
cp -R app/* $integ_test_dir
cd $integ_test_dir

# if this directory is missing, but exists in any of the
# if this directory is missing, but exists in any of the
# parent directories, npm will install these packages there. lets make sure
# we install locally.
mkdir -p node_modules

npm install \
@aws-cdk/core \
@aws-cdk/aws-sns \
@aws-cdk/aws-iam \
@aws-cdk/aws-lambda \
@aws-cdk/aws-ssm \
@aws-cdk/aws-ecr-assets \
@aws-cdk/aws-cloudformation \
@aws-cdk/aws-ec2
@aws-cdk/core@^1 \
@aws-cdk/aws-sns@^1 \
@aws-cdk/aws-iam@^1 \
@aws-cdk/aws-lambda@^1 \
@aws-cdk/aws-ssm@^1 \
@aws-cdk/aws-ecr-assets@^1 \
@aws-cdk/aws-cloudformation@^1 \
@aws-cdk/aws-ec2@^1

echo "| setup complete at: $PWD"
echo "| 'cdk' is: $(type -p cdk)"
Expand Down
10 changes: 10 additions & 0 deletions packages/aws-cdk/test/integ/cli/test-cdk-fast-deploy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ if [ "${changeset3}" == "${changeset1}" ]; then
exit 1
fi

echo "============================================================"
echo " deploying the same stack again with different tags"
echo "============================================================"
cdk deploy -v ${stack_name} --tags key=value
changeset4=$(get_last_changeset)
if [ "${changeset4}" == "${changeset1}" ]; then
echo "TEST FAILED: expected tags to create a new changeset"
exit 1
fi

# destroy
cdk destroy -f ${stack_name}

Expand Down

0 comments on commit ef9e42d

Please sign in to comment.