Skip to content

Commit

Permalink
fix(ec2): UserData.addSignalOnExitCommand does not work in combinat…
Browse files Browse the repository at this point in the history
…ion with `userDataCausesReplacement` (#18726)

If both `addSignalOnExitCommand` _and_ `userDataCausesReplacement` are
 used it results in an invalid logicalId being used in the
`cfn-signal` call. This is due to `addSignalOnExitCommand` getting the
logicalID from `Stack.getLogicalId` which does not take into
consideration logicalId overrides which `userDataCausesReplacement`
uses.

This updates `addSignalOnExitCommand` to use the `logicalId` of the
resource which is evaluated lazily and happens after all overrides.

fixes #12749


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
corymhall authored Jan 31, 2022
1 parent 13e1c7f commit afdc550
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 6 deletions.
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-ec2/lib/user-data.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { IBucket } from '@aws-cdk/aws-s3';
import { CfnElement, Fn, Resource, Stack } from '@aws-cdk/core';
import { Fn, Resource, Stack, CfnResource } from '@aws-cdk/core';
import { OperatingSystemType } from './machine-image';

/**
Expand Down Expand Up @@ -178,7 +178,7 @@ class LinuxUserData extends UserData {

public addSignalOnExitCommand( resource: Resource ): void {
const stack = Stack.of(resource);
const resourceID = stack.getLogicalId(resource.node.defaultChild as CfnElement);
const resourceID = (resource.node.defaultChild as CfnResource).logicalId;
this.addOnExitCommands(`/opt/aws/bin/cfn-signal --stack ${stack.stackName} --resource ${resourceID} --region ${stack.region} -e $exitCode || echo 'Failed to send Cloudformation Signal'`);
}

Expand Down Expand Up @@ -235,7 +235,7 @@ class WindowsUserData extends UserData {

public addSignalOnExitCommand( resource: Resource ): void {
const stack = Stack.of(resource);
const resourceID = stack.getLogicalId(resource.node.defaultChild as CfnElement);
const resourceID = (resource.node.defaultChild as CfnResource).logicalId;

this.addOnExitCommands(`cfn-signal --stack ${stack.stackName} --resource ${resourceID} --region ${stack.region} --success ($success.ToString().ToLower())`);
}
Expand Down
86 changes: 83 additions & 3 deletions packages/@aws-cdk/aws-ec2/test/userdata.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Bucket } from '@aws-cdk/aws-s3';
import { Aws, Stack } from '@aws-cdk/core';
import { Template, Match } from '@aws-cdk/assertions';
import { Aws, Stack, CfnResource } from '@aws-cdk/core';
import * as ec2 from '../lib';

describe('user data', () => {
Expand Down Expand Up @@ -41,6 +42,7 @@ describe('user data', () => {
const stack = new Stack();
const resource = new ec2.Vpc(stack, 'RESOURCE');
const userData = ec2.UserData.forWindows();
const logicalId = (resource.node.defaultChild as CfnResource).logicalId;

// WHEN
userData.addSignalOnExitCommand( resource );
Expand All @@ -49,16 +51,55 @@ describe('user data', () => {
// THEN
const rendered = userData.render();

expect(stack.resolve(logicalId)).toEqual('RESOURCE1989552F');
expect(rendered).toEqual('<powershell>trap {\n' +
'$success=($PSItem.Exception.Message -eq "Success")\n' +
`cfn-signal --stack Default --resource RESOURCE1989552F --region ${Aws.REGION} --success ($success.ToString().ToLower())\n` +
`cfn-signal --stack Default --resource ${logicalId} --region ${Aws.REGION} --success ($success.ToString().ToLower())\n` +
'break\n' +
'}\n' +
'command1\n' +
'throw "Success"</powershell>',
);

});
test('can create Windows with Signal Command and userDataCausesReplacement', () => {
// GIVEN
const stack = new Stack();
const vpc = new ec2.Vpc(stack, 'Vpc');
const userData = ec2.UserData.forWindows();
const resource = new ec2.Instance(stack, 'RESOURCE', {
vpc,
instanceType: ec2.InstanceType.of(ec2.InstanceClass.T2, ec2.InstanceSize.LARGE),
machineImage: ec2.MachineImage.genericWindows({ ['us-east-1']: 'ami-12345678' }),
userDataCausesReplacement: true,
userData,
});

const logicalId = (resource.node.defaultChild as CfnResource).logicalId;

// WHEN
userData.addSignalOnExitCommand( resource );
userData.addCommands('command1');

// THEN
Template.fromStack(stack).templateMatches({
Resources: Match.objectLike({
RESOURCE1989552Fdfd505305f427919: {
Type: 'AWS::EC2::Instance',
},
}),
});
expect(stack.resolve(logicalId)).toEqual('RESOURCE1989552Fdfd505305f427919');
const rendered = userData.render();
expect(rendered).toEqual('<powershell>trap {\n' +
'$success=($PSItem.Exception.Message -eq "Success")\n' +
`cfn-signal --stack Default --resource ${logicalId} --region ${Aws.REGION} --success ($success.ToString().ToLower())\n` +
'break\n' +
'}\n' +
'command1\n' +
'throw "Success"</powershell>',
);
});
test('can windows userdata download S3 files', () => {
// GIVEN
const stack = new Stack();
Expand Down Expand Up @@ -174,6 +215,7 @@ describe('user data', () => {
// GIVEN
const stack = new Stack();
const resource = new ec2.Vpc(stack, 'RESOURCE');
const logicalId = (resource.node.defaultChild as CfnResource).logicalId;

// WHEN
const userData = ec2.UserData.forLinux();
Expand All @@ -182,15 +224,53 @@ describe('user data', () => {

// THEN
const rendered = userData.render();
expect(stack.resolve(logicalId)).toEqual('RESOURCE1989552F');
expect(rendered).toEqual('#!/bin/bash\n' +
'function exitTrap(){\n' +
'exitCode=$?\n' +
`/opt/aws/bin/cfn-signal --stack Default --resource RESOURCE1989552F --region ${Aws.REGION} -e $exitCode || echo \'Failed to send Cloudformation Signal\'\n` +
`/opt/aws/bin/cfn-signal --stack Default --resource ${logicalId} --region ${Aws.REGION} -e $exitCode || echo \'Failed to send Cloudformation Signal\'\n` +
'}\n' +
'trap exitTrap EXIT\n' +
'command1');

});
test('can create Linux with Signal Command and userDataCausesReplacement', () => {
// GIVEN
const stack = new Stack();
const vpc = new ec2.Vpc(stack, 'Vpc');
const userData = ec2.UserData.forLinux();
const resource = new ec2.Instance(stack, 'RESOURCE', {
vpc,
instanceType: ec2.InstanceType.of(ec2.InstanceClass.T2, ec2.InstanceSize.LARGE),
machineImage: ec2.MachineImage.genericLinux({ ['us-east-1']: 'ami-12345678' }),
userDataCausesReplacement: true,
userData,
});

const logicalId = (resource.node.defaultChild as CfnResource).logicalId;

// WHEN
userData.addSignalOnExitCommand( resource );
userData.addCommands('command1');

// THEN
Template.fromStack(stack).templateMatches({
Resources: Match.objectLike({
RESOURCE1989552F74a24ef4fbc89422: {
Type: 'AWS::EC2::Instance',
},
}),
});
expect(stack.resolve(logicalId)).toEqual('RESOURCE1989552F74a24ef4fbc89422');
const rendered = userData.render();
expect(rendered).toEqual('#!/bin/bash\n' +
'function exitTrap(){\n' +
'exitCode=$?\n' +
`/opt/aws/bin/cfn-signal --stack Default --resource ${logicalId} --region ${Aws.REGION} -e $exitCode || echo \'Failed to send Cloudformation Signal\'\n` +
'}\n' +
'trap exitTrap EXIT\n' +
'command1');
});
test('can linux userdata download S3 files', () => {
// GIVEN
const stack = new Stack();
Expand Down

0 comments on commit afdc550

Please sign in to comment.