Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cli): hotswapping StateMachines with a name fails #17892

Merged
merged 5 commits into from
Dec 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions packages/aws-cdk/lib/api/hotswap/stepfunctions-state-machines.ts
Original file line number Diff line number Diff line change
@@ -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);

comcalvi marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
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'),
comcalvi marked this conversation as resolved.
Show resolved Hide resolved
setup.stackSummaryOf('Bucket', 'AWS::S3::Bucket', 'my-bucket'),
);
const cdkStackArtifact = setup.cdkStackArtifactOf({
Expand Down