From de67aae18cfed2694e9002a10e739a56f294040f Mon Sep 17 00:00:00 2001 From: Calvin Combs <66279577+comcalvi@users.noreply.github.com> Date: Fri, 10 Dec 2021 15:29:40 -0800 Subject: [PATCH] fix(cli): hotswapping StateMachines with a name fails (#17892) Before, when the `stateMachineName` property was used, the value of `stateMachineName` was passed directly to the SDK where an ARN was required. Now, when the `stateMachineName` property is used, we construct the ARN from its value, and pass that ARN to the SDK. Closes #17716 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../hotswap/stepfunctions-state-machines.ts | 20 +++++++++++------- .../state-machine-hotswap-deployments.test.ts | 21 ++++++++++--------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts b/packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts index dbaff58ab608b..3af97aef07455 100644 --- a/packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts +++ b/packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts @@ -1,5 +1,5 @@ import { ISDK } from '../aws-auth'; -import { ChangeHotswapImpact, ChangeHotswapResult, HotswapOperation, HotswappableChangeCandidate, establishResourcePhysicalName } from './common'; +import { ChangeHotswapImpact, ChangeHotswapResult, HotswapOperation, HotswappableChangeCandidate } from './common'; import { EvaluateCloudFormationTemplate } from './evaluate-cloudformation-template'; export async function isHotswappableStateMachineChange( @@ -11,15 +11,20 @@ export async function isHotswappableStateMachineChange( return stateMachineDefinitionChange; } - const machineNameInCfnTemplate = change.newValue?.Properties?.StateMachineName; - const machineName = await establishResourcePhysicalName(logicalId, machineNameInCfnTemplate, evaluateCfnTemplate); - if (!machineName) { + const stateMachineNameInCfnTemplate = change.newValue?.Properties?.StateMachineName; + const stateMachineArn = stateMachineNameInCfnTemplate + ? await evaluateCfnTemplate.evaluateCfnExpression({ + 'Fn::Sub': 'arn:${AWS::Partition}:states:${AWS::Region}:${AWS::AccountId}:stateMachine:' + stateMachineNameInCfnTemplate, + }) + : await evaluateCfnTemplate.findPhysicalNameFor(logicalId); + + if (!stateMachineArn) { return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; } return new StateMachineHotswapOperation({ definition: stateMachineDefinitionChange, - stateMachineName: machineName, + stateMachineArn: stateMachineArn, }); } @@ -43,7 +48,7 @@ async function isStateMachineDefinitionOnlyChange( } interface StateMachineResource { - readonly stateMachineName: string; + readonly stateMachineArn: string; readonly definition: string; } @@ -56,8 +61,7 @@ class StateMachineHotswapOperation implements HotswapOperation { public async apply(sdk: ISDK): Promise { // not passing the optional properties leaves them unchanged return sdk.stepFunctions().updateStateMachine({ - // even though the name of the property is stateMachineArn, passing the name of the state machine is allowed here - stateMachineArn: this.stepFunctionResource.stateMachineName, + stateMachineArn: this.stepFunctionResource.stateMachineArn, definition: this.stepFunctionResource.definition, }).promise(); } diff --git a/packages/aws-cdk/test/api/hotswap/state-machine-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/state-machine-hotswap-deployments.test.ts index 289921fb2bd76..94467c5ac123d 100644 --- a/packages/aws-cdk/test/api/hotswap/state-machine-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/state-machine-hotswap-deployments.test.ts @@ -63,7 +63,7 @@ test('calls the updateStateMachine() API when it receives only a definitionStrin expect(deployStackResult).not.toBeUndefined(); expect(mockUpdateMachineDefinition).toHaveBeenCalledWith({ definition: '{ Prop: "new-value" }', - stateMachineArn: 'my-machine', + stateMachineArn: 'arn:aws:states:here:123456789012:stateMachine:my-machine', }); }); @@ -138,7 +138,7 @@ test('calls the updateStateMachine() API when it receives only a definitionStrin }, }, }, null, 2), - stateMachineArn: 'my-machine', + stateMachineArn: 'arn:aws:states:here:123456789012:stateMachine:my-machine', }); }); @@ -168,14 +168,14 @@ test('calls the updateStateMachine() API when it receives a change to the defini }); // WHEN - setup.pushStackResourceSummaries(setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'mock-machine-resource-id')); + setup.pushStackResourceSummaries(setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'arn:aws:states:here:123456789012:stateMachine:my-machine')); const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(cdkStackArtifact); // THEN expect(deployStackResult).not.toBeUndefined(); expect(mockUpdateMachineDefinition).toHaveBeenCalledWith({ definition: '{ "Prop" : "new-value" }', - stateMachineArn: 'mock-machine-resource-id', // the sdk will convert the ID to the arn in a production environment + stateMachineArn: 'arn:aws:states:here:123456789012:stateMachine:my-machine', }); }); @@ -256,7 +256,7 @@ test('can correctly hotswap old style synth changes', async () => { setup.setCurrentCfnStackTemplate({ Parameters: { AssetParam1: { Type: 'String' } }, Resources: { - SM: { + Machine: { Type: 'AWS::StepFunctions::StateMachine', Properties: { DefinitionString: { Ref: 'AssetParam1' }, @@ -269,7 +269,7 @@ test('can correctly hotswap old style synth changes', async () => { template: { Parameters: { AssetParam2: { Type: String } }, Resources: { - SM: { + Machine: { Type: 'AWS::StepFunctions::StateMachine', Properties: { DefinitionString: { Ref: 'AssetParam2' }, @@ -281,13 +281,14 @@ test('can correctly hotswap old style synth changes', async () => { }); // WHEN + setup.pushStackResourceSummaries(setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'arn:aws:states:here:123456789012:stateMachine:my-machine')); const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(cdkStackArtifact, { AssetParam2: 'asset-param-2' }); // THEN expect(deployStackResult).not.toBeUndefined(); expect(mockUpdateMachineDefinition).toHaveBeenCalledWith({ definition: 'asset-param-2', - stateMachineArn: 'machine-name', + stateMachineArn: 'arn:aws:states:here:123456789012:stateMachine:machine-name', }); }); @@ -348,7 +349,7 @@ test('calls the updateStateMachine() API when it receives a change to the defini // WHEN setup.pushStackResourceSummaries( - setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'mock-machine-resource-id'), + setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'arn:aws:states:here:123456789012:stateMachine:my-machine'), setup.stackSummaryOf('Func', 'AWS::Lambda::Function', 'my-func'), ); const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(cdkStackArtifact); @@ -357,7 +358,7 @@ test('calls the updateStateMachine() API when it receives a change to the defini expect(deployStackResult).not.toBeUndefined(); expect(mockUpdateMachineDefinition).toHaveBeenCalledWith({ definition: '"Resource": arn:aws:lambda:here:123456789012:function:my-func', - stateMachineArn: 'my-machine', + stateMachineArn: 'arn:aws:states:here:123456789012:stateMachine:my-machine', }); }); @@ -446,7 +447,7 @@ test("will not perform a hotswap deployment if it doesn't know how to handle a s }, }); setup.pushStackResourceSummaries( - setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'my-machine'), + setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'arn:aws:states:here:123456789012:stateMachine:my-machine'), setup.stackSummaryOf('Bucket', 'AWS::S3::Bucket', 'my-bucket'), ); const cdkStackArtifact = setup.cdkStackArtifactOf({