Skip to content

Commit

Permalink
fix(cli): hotswapping StateMachines with a name fails (#17892)
Browse files Browse the repository at this point in the history
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*
  • Loading branch information
comcalvi committed Dec 10, 2021
1 parent 1df478b commit de67aae
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 18 deletions.
20 changes: 12 additions & 8 deletions 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(
Expand All @@ -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,
});
}

Expand All @@ -43,7 +48,7 @@ async function isStateMachineDefinitionOnlyChange(
}

interface StateMachineResource {
readonly stateMachineName: string;
readonly stateMachineArn: string;
readonly definition: string;
}

Expand All @@ -56,8 +61,7 @@ class StateMachineHotswapOperation implements HotswapOperation {
public async apply(sdk: ISDK): Promise<any> {
// 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();
}
Expand Down
Expand Up @@ -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',
});
});

Expand Down Expand Up @@ -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',
});
});

Expand Down Expand Up @@ -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',
});
});

Expand Down Expand Up @@ -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' },
Expand All @@ -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' },
Expand All @@ -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',
});
});

Expand Down Expand Up @@ -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);
Expand All @@ -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',
});
});

Expand Down Expand Up @@ -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({
Expand Down

0 comments on commit de67aae

Please sign in to comment.