From 4b90331009ea8da3bbd7d0623fd7582262b7c2e6 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 7 Dec 2021 12:06:54 -0800 Subject: [PATCH 1/4] fixed invalid arn issue --- .../hotswap/stepfunctions-state-machines.ts | 20 +++++++++++++------ .../state-machine-hotswap-deployments.test.ts | 15 +++++++------- 2 files changed, 22 insertions(+), 13 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..2e3b4d773ccc3 100644 --- a/packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts +++ b/packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts @@ -12,14 +12,23 @@ export async function isHotswappableStateMachineChange( } const machineNameInCfnTemplate = change.newValue?.Properties?.StateMachineName; - const machineName = await establishResourcePhysicalName(logicalId, machineNameInCfnTemplate, evaluateCfnTemplate); - if (!machineName) { + let machineArn; + + if (machineNameInCfnTemplate) { + machineArn = await evaluateCfnTemplate.evaluateCfnExpression({ + 'Fn::Sub': 'arn:${AWS::Partition}:states:${AWS::Region}:${AWS::AccountId}:stateMachine:' + machineNameInCfnTemplate, + }); + } else { + machineArn = await establishResourcePhysicalName(logicalId, machineNameInCfnTemplate, evaluateCfnTemplate); + } + + if (!machineArn) { return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; } return new StateMachineHotswapOperation({ definition: stateMachineDefinitionChange, - stateMachineName: machineName, + stateMachineArn: machineArn, }); } @@ -43,7 +52,7 @@ async function isStateMachineDefinitionOnlyChange( } interface StateMachineResource { - readonly stateMachineName: string; + readonly stateMachineArn: string; readonly definition: string; } @@ -56,8 +65,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..428b45ded1c04 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', }); }); @@ -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', 'mock-machine-resource-id')); 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', }); }); @@ -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({ From f1a0d2f19f65dd36869121b3c22c1129a2521d5d Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 7 Dec 2021 12:29:09 -0800 Subject: [PATCH 2/4] changed IDs in tests and refactored fix --- .../lib/api/hotswap/stepfunctions-state-machines.ts | 12 ++++-------- .../state-machine-hotswap-deployments.test.ts | 8 ++++---- 2 files changed, 8 insertions(+), 12 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 2e3b4d773ccc3..09d7a35841747 100644 --- a/packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts +++ b/packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts @@ -12,15 +12,11 @@ export async function isHotswappableStateMachineChange( } const machineNameInCfnTemplate = change.newValue?.Properties?.StateMachineName; - let machineArn; - if (machineNameInCfnTemplate) { - machineArn = await evaluateCfnTemplate.evaluateCfnExpression({ - 'Fn::Sub': 'arn:${AWS::Partition}:states:${AWS::Region}:${AWS::AccountId}:stateMachine:' + machineNameInCfnTemplate, - }); - } else { - machineArn = await establishResourcePhysicalName(logicalId, machineNameInCfnTemplate, evaluateCfnTemplate); - } + const machineArn = machineNameInCfnTemplate ? await evaluateCfnTemplate.evaluateCfnExpression({ + 'Fn::Sub': 'arn:${AWS::Partition}:states:${AWS::Region}:${AWS::AccountId}:stateMachine:' + machineNameInCfnTemplate, + }) : await establishResourcePhysicalName(logicalId, machineNameInCfnTemplate, evaluateCfnTemplate); + if (!machineArn) { return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; 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 428b45ded1c04..8bdfcd2378c6d 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 @@ -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', 'mock-state-machine-arn')); 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: 'mock-state-machine-arn', }); }); @@ -281,7 +281,7 @@ test('can correctly hotswap old style synth changes', async () => { }); // WHEN - setup.pushStackResourceSummaries( setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'mock-machine-resource-id')); + setup.pushStackResourceSummaries( setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'mock-state-machine-arn')); const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(cdkStackArtifact, { AssetParam2: 'asset-param-2' }); // THEN @@ -349,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', 'mock-state-machine-arn'), setup.stackSummaryOf('Func', 'AWS::Lambda::Function', 'my-func'), ); const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(cdkStackArtifact); From e0376dfac7dcb16e9dfef18fa2960862feaa3efa Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Tue, 7 Dec 2021 12:30:14 -0800 Subject: [PATCH 3/4] removed blank line --- packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts | 1 - 1 file changed, 1 deletion(-) 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 09d7a35841747..2f13250b3879a 100644 --- a/packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts +++ b/packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts @@ -17,7 +17,6 @@ export async function isHotswappableStateMachineChange( 'Fn::Sub': 'arn:${AWS::Partition}:states:${AWS::Region}:${AWS::AccountId}:stateMachine:' + machineNameInCfnTemplate, }) : await establishResourcePhysicalName(logicalId, machineNameInCfnTemplate, evaluateCfnTemplate); - if (!machineArn) { return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; } From e20d4a357696a3bbbc85826ed814f565c352e1ab Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Wed, 8 Dec 2021 19:30:32 -0800 Subject: [PATCH 4/4] addressed review comments --- .../api/hotswap/stepfunctions-state-machines.ts | 17 +++++++++-------- .../state-machine-hotswap-deployments.test.ts | 8 ++++---- 2 files changed, 13 insertions(+), 12 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 2f13250b3879a..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,19 +11,20 @@ export async function isHotswappableStateMachineChange( return stateMachineDefinitionChange; } - const machineNameInCfnTemplate = change.newValue?.Properties?.StateMachineName; + 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); - const machineArn = machineNameInCfnTemplate ? await evaluateCfnTemplate.evaluateCfnExpression({ - 'Fn::Sub': 'arn:${AWS::Partition}:states:${AWS::Region}:${AWS::AccountId}:stateMachine:' + machineNameInCfnTemplate, - }) : await establishResourcePhysicalName(logicalId, machineNameInCfnTemplate, evaluateCfnTemplate); - - if (!machineArn) { + if (!stateMachineArn) { return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT; } return new StateMachineHotswapOperation({ definition: stateMachineDefinitionChange, - stateMachineArn: machineArn, + stateMachineArn: stateMachineArn, }); } 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 8bdfcd2378c6d..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 @@ -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-state-machine-arn')); + 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-state-machine-arn', + stateMachineArn: 'arn:aws:states:here:123456789012:stateMachine:my-machine', }); }); @@ -281,7 +281,7 @@ test('can correctly hotswap old style synth changes', async () => { }); // WHEN - setup.pushStackResourceSummaries( setup.stackSummaryOf('Machine', 'AWS::StepFunctions::StateMachine', 'mock-state-machine-arn')); + 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 @@ -349,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-state-machine-arn'), + 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);