serverless-application-model icon indicating copy to clipboard operation
serverless-application-model copied to clipboard

Update state machine generator to use Definition object instead of DefinitionString

Open jormello opened this issue 4 years ago • 2 comments

Description of changes: Step Functions recently released a new CloudFormation property called Definition, which allows users to specify their state machine definitions in object format. Since SAM already supported users providing their state machine definitions as an object, we would convert that object to a DefinitionString and use that in the generated CloudFormation template. With this update to the state machine resource schema, we no longer need to do this conversion in SAM, and can simply pass the Definition object directly to the CFN template.

A request for documentation updates has been made to note that SAM Definition property is now passed directly to CFN Definition property.

Description of how you validated changes:

  • Update to unit test suite for State Machine resource.
  • Manual testing of generated CloudFormation template

Checklist:

  • [X] Write/update tests
  • [X] make pr passes
  • [X] Update documentation
  • [X] Verify transformed template deploys and application functions as expected

Examples? No changes are needed for the state machine examples.

Please reach out in the comments, if you want to add an example. Examples will be added to sam init through https://github.com/awslabs/aws-sam-cli-app-templates/

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

jormello avatar Mar 08 '21 18:03 jormello

Will these changes cause any impact to customers who have created State Machine resources using SAM? If a customer provides the same SAM template, they will now generate a different CFN template (using Definition property instead of DefinitionString), meaning there will potentially be a CFN update even if there is no change to their template. Is there typically customer wording provided by the service team to help communicate that these template changes are expected and do not necessarily indicate a change in the underlying resource?

jormello avatar Mar 08 '21 18:03 jormello

  1. The expected CFN template change is that the DefinitionString property will no longer be defined, and instead the state machine definition will be specified in the Definition property. Given that the CFN template is changing, customers should expect to see an UpdateStateMachine call to be performed even if they have not changed the definition in their template in any way. The definitionString value in the UpdateStateMachineRequest could potentially be different (if sibling properties are listed in a different order, or if there is a different number of indentation spaces used). These definitionString values will be functionally identical, meaning although there is a call to UpdateStateMachine, the resource itself should have no changes.
  2. I have reached out to the CFN team for confirmation on who will own the communication.

jormello avatar Mar 25 '21 20:03 jormello

Not quite sure why this stalled, but the number one concern I would have is causing an unexpected redeploy for existing stacks without there being any other change to the statemachine. I think we need to understand potential impact for existing customers (e.g. if they are under load would they experience an outage /drop traffic and for how long). Secondly I am not clear if the resolution would always be identical, so there may be other unexpected impact. An alternative approach might be to add a new parameter for this so it can be backward compatible but users can use the SFN native approach.

SimonCMoore avatar Nov 01 '22 13:11 SimonCMoore

We could potentially have a separate Definition property, but I'm not sure what the benefit is. Is this an alternative to DefinitionString and DefinitionSubstitutions?

hoffa avatar Dec 14 '22 17:12 hoffa