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.

Sign up for GitHub

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

Jump to bottom

cfn-parse.ts: parseDeletionPolicy is missing CreateExceptOnDelete #26863

Closed
njsouthwood opened this issue Aug 23, 2023 · 3 comments · Fixed by #26880
Closed

cfn-parse.ts: parseDeletionPolicy is missing CreateExceptOnDelete #26863

njsouthwood opened this issue Aug 23, 2023 · 3 comments · Fixed by #26880
Labels
@aws-cdk/cloudformation-include Issues related to the "CFN include v.20" package @aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1

Comments

@njsouthwood
Copy link

njsouthwood commented Aug 23, 2023 •
edited
Loading

Describe the bug

CFN recently added support for a new deletion policy called RetainExceptOnCreate. This policy was added to the removal-policy enum in e0d5ca, however it is missing from the switch/case block in parseDeletionPolicy, which causes errors when this removal policy is used.

EDIT: It looks like this code may only be executed when using CfnInclude to create a stack from an existing CloudFormation template. Updating the rest of this issue to be more clear about that.

Expected Behavior

You should be able to use CfnInclude to import an existing cloudformation template with valid usage of DeletionPolicy: RetainExceptOnCreate without any errors

Current Behavior

If you define a stack resource with DeletionPolicy: RetainExceptOnCreate, then use CfnInclude to import that stack you'll get this error:

Error: Unrecognized DeletionPolicy 'RetainExceptOnCreate'

Reproduction Steps

  • Create Cloudformation template with a stack resource with DeletionPolicy: RetainExceptOnCreate
  • Import that template as a cdk stack using CfnInclude

Possible Solution

  • Add a case for RetainExceptOnCreate
  • It looks like cfn-parse uses this same parsing function for both deletionPolicy and updateReplacePolicy. It looks like the new RetainExceptOnCreate policy is not a valid updateReplacePolicy. There will need to be two separate parsing functions.

Additional Information/Context

No response

CDK CLI Version

2.91.0

Framework Version

No response

Node.js Version

v14.21.3

OS

AL2

Language

Typescript

Language Version

No response

Other information

No response
@njsouthwood njsouthwood added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 23, 2023
@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Aug 23, 2023
@peterwoodworth peterwoodworth added @aws-cdk/core Related to core CDK functionality and removed @aws-cdk/aws-iam Related to AWS Identity and Access Management labels Aug 23, 2023
@peterwoodworth
Copy link
Contributor

This would only come into play if you're using the CfnInclude module, right?

@peterwoodworth peterwoodworth added @aws-cdk/cloudformation-include Issues related to the "CFN include v.20" package good first issue Related to contributions. See CONTRIBUTING.md p1 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Aug 23, 2023
@njsouthwood
Copy link
Author

Not sure if that's the only case where it would happen, but it looks like that is what we are doing in this case.

@dineshkatamneni dineshkatamneni mentioned this issue Aug 25, 2023
Merged
@mergify mergify bot closed this as completed in #26880 Aug 25, 2023
mergify bot pushed a commit that referenced this issue Aug 25, 2023
@dineshkatamneni
fix(core): add RetainExceptOnCreate to ParseDeletionPolicy method (#2…
702d9d5 
…6880)

> Describe the reason for this change, what the solution is, and any

CFN recently added support for a new [deletion policy called RetainExceptOnCreate](https://aws.amazon.com/about-aws/whats-new/2023/07/aws-cloudformation-deletion-policies-dev-test-cycle/). This policy was added to the removal-policy enum in [e0d5ca](e0d5cad), however it is missing from the [switch/case block in parseDeletionPolicy](https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts#L468), which causes errors when this removal policy is used.


Closes #26863.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@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.

@github-actions github-actions bot mentioned this issue Sep 1, 2023
Closed
@tim-finnigan tim-finnigan mentioned this issue Dec 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@aws-cdk/cloudformation-include Issues related to the "CFN include v.20" package @aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(core): add RetainExceptOnCreate to ParseDeletionPolicy method dineshkatamneni/aws-cdk
2 participants
@njsouthwood @peterwoodworth