From afdc550ee372dd25d9d2eef81a545da1e923f796 Mon Sep 17 00:00:00 2001 From: Cory Hall <43035978+corymhall@users.noreply.github.com> Date: Mon, 31 Jan 2022 08:46:28 -0500 Subject: [PATCH] fix(ec2): `UserData.addSignalOnExitCommand` does not work in combination 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* --- packages/@aws-cdk/aws-ec2/lib/user-data.ts | 6 +- .../@aws-cdk/aws-ec2/test/userdata.test.ts | 86 ++++++++++++++++++- 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk/aws-ec2/lib/user-data.ts b/packages/@aws-cdk/aws-ec2/lib/user-data.ts index 1e92eb888e6f8..82e256b9a895a 100644 --- a/packages/@aws-cdk/aws-ec2/lib/user-data.ts +++ b/packages/@aws-cdk/aws-ec2/lib/user-data.ts @@ -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'; /** @@ -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'`); } @@ -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())`); } diff --git a/packages/@aws-cdk/aws-ec2/test/userdata.test.ts b/packages/@aws-cdk/aws-ec2/test/userdata.test.ts index e2596d8699abd..c385ce83a7254 100644 --- a/packages/@aws-cdk/aws-ec2/test/userdata.test.ts +++ b/packages/@aws-cdk/aws-ec2/test/userdata.test.ts @@ -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', () => { @@ -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 ); @@ -49,9 +51,10 @@ describe('user data', () => { // THEN const rendered = userData.render(); + expect(stack.resolve(logicalId)).toEqual('RESOURCE1989552F'); expect(rendered).toEqual('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' + @@ -59,6 +62,44 @@ describe('user data', () => { ); }); + 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('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"', + ); + }); test('can windows userdata download S3 files', () => { // GIVEN const stack = new Stack(); @@ -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(); @@ -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();