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

(StepFunctions): (CfnStateMachineVersion has default removal policy of delete, should be retain) #27250

Closed
1 of 2 tasks
DLundAJB opened this issue Sep 22, 2023 · 8 comments
Closed
1 of 2 tasks
Labels
@aws-cdk/aws-stepfunctions Related to AWS StepFunctions effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@DLundAJB
Copy link

DLundAJB commented Sep 22, 2023

Describe the feature

When initially deploying a stack containing a state machine and a state machine version def, a version is created. If any changes are then made to the state machine, because the default removal policy is delete the initial or previous version is then deleted and the only remaining version is the latest one deployed. I think this is counterintuitive as it pretty much defeats the point of versioning when trying to use it straight out the box as its not working the way it's intended.

Use Case

Its frustrating because you cant just define a state machine version and it doesn't initially work like versioning should do as you're not retaining any versions unless the removal policy is altered.

Proposed Solution

Make default removal policy for CfnStateMachineVersion RemovalPolicy.RETAIN

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

TypeScript 2.96.2

Environment details (OS name and version, etc.)

Cloud9 Ubuntu Instance Running Ubuntu 22.04

@DLundAJB DLundAJB added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Sep 22, 2023
@github-actions github-actions bot added the @aws-cdk/aws-stepfunctions Related to AWS StepFunctions label Sep 22, 2023
@khushail
Copy link
Contributor

Hi @DLundAJB , thanks for reaching out. Yes, I agree with your point about changing the default policy

@khushail khushail added p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Sep 22, 2023
@DLundAJB
Copy link
Author

@khushail glad you agree! When I get a spare moment I will look at making a change

@wong-a
Copy link
Contributor

wong-a commented Sep 25, 2023

Hi @DLundAJB , thanks for reaching out. Yes, I agree with your point about changing the default policy

@khushail - The L1 resource is generated by the CFN resource but the code linked above is for the StateMachine L2 construct. AFAIK, the L1 constructs are autogenerated from the CFN resource spec. We cannot modify the default retention policy for the CFN resource as that would be breaking. Changing the state machine ARN or revisionId will cause a resource replacement, as they are not mutable properties.

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-stepfunctions-statemachineversion.html#aws-resource-stepfunctions-statemachineversion-properties

The following would enable the use case @DLundAJB wants:

const stateMachine = new sfn.StateMachine(this, 'myStateMachine', {
  definition: new sfn.Pass(this, 'PassState', { result: sfn.Result.fromString("version 2") })
});

const version = new sfn.CfnStateMachineVersion(this, 'Version', {
  stateMachineArn: stateMachine.stateMachineArn,
  stateMachineRevisionId: stateMachine.stateMachineRevisionId,
});

version.applyRemovalPolicy(cdk.RemovalPolicy.RETAIN);

IMO, overriding the default behaviour of the resource would necessitate an L2 construct for StateMachineVersion.

@DLundAJB
Copy link
Author

Hi @wong-a thanks for clarifying the behaviour of L1 constructs, I wasn't aware of that. My main problem is that although the versions are replaced, having a default removal policy of destroy defeats the purpose of versioning really. So I guess it would have to be an L2 construct. Will see what I can do.

Cheers

@wong-a
Copy link
Contributor

wong-a commented Sep 26, 2023

By default CloudFormation deletes resources when they are removed from the template or replaced by updates, with a few exceptions. It's unfortunate that this doesn't match your mental model for versioning, but hopefully achieving the desired behaviour by overriding the removal policy isn't too much overhead.

Another way to model versions in CDK/CloudFormation without overriding the removal policy would be to declare a version resource in your stack for each version. If you want to keep N versions, you would have N instances of CfnStateMachineVersion in the app. This is much more explicit, but the obvious drawback is having to declare a version in the template for each version you publish instead of automatically creating a new one when the state machine revision ID changes. To delete old versions, you would remove the versions from the CDK stack.

Example with 3 versions, created in sequential stack updates:

const stateMachine = new sfn.StateMachine(this, 'myStateMachine', {
  definition: new sfn.Pass(this, 'PassState', { result: sfn.Result.fromString("version 3") })
});

// added first stack update
const version1 = new sfn.CfnStateMachineVersion(this, 'Version1', {
  stateMachineArn: stateMachine.stateMachineArn,
});

// added second stack update
const version2 = new sfn.CfnStateMachineVersion(this, 'Version2', {
  stateMachineArn: stateMachine.stateMachineArn,
});

// added in third stack update
const version3 = new sfn.CfnStateMachineVersion(this, 'Version3', {
  stateMachineArn: stateMachine.stateMachineArn,
});

@DLundAJB
Copy link
Author

DLundAJB commented Sep 27, 2023

@wong-a I am fully aware of how cloudformation treats resources, I feel like this response is a bit condescending.

I understand that my initial ask to change an L1 construct sounded stupid based on cloudformation's default behaviour. In response to the 'mental model' comment, it's not really a mental model, if you cannot maintain versions then surely that it is not versioning it is just updating? hence my initial but somewhat naive query. I assume what would suit my problem more would to be have an optional prop that triggers versioning or something of the like similar to S3, seems a bit far fetched but something along the lines of if code change -> create new version, set retention policy for versions to retain etc. I'm going to close this and come back when I have a better idea of what I want/should do. Thanks for your help.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@wong-a
Copy link
Contributor

wong-a commented Sep 27, 2023

@DLundAJB I apologize for the tone of my comment. It wasn't meant to be condescending, just provide information to you and anyone else who stumbles upon this issue. The behaviour you are looking for is a sensible one.

By "mental model", I was referring to different expectations users have of how Step Functions versions should work and how they should be represented as CloudFormation resources, not that there is a gap in understanding on your part. There are some limitations and challenges trying to meet those while working within the constraints of CloudFormation and Step Functions APIs, such as update replacement behaviour I tried to highlight earlier.

CDK L2 constructs are an opportunity to improve on the L1 CloudFormation resources, so I look forward to hearing your ideas.

an optional prop that triggers versioning or something of the like similar to S3, seems a bit far fetched but something along the lines of if code change -> create new version, set retention policy for versions to retain

This is not so far fetched, actually. In SAM, AWS::Serverless::StateMachine and AWS::Serverless::Function do something very similar with the AutoublishAlias property. I think a similar feature could be built into the StateMachine CDK construct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-stepfunctions Related to AWS StepFunctions effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

3 participants