-
Notifications
You must be signed in to change notification settings - Fork 5
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
CDK RemovalPolicy maps to Pulumi retainOnDelete #223
Conversation
This PR maps CloudFormation [DeletionPolicy](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-attribute-deletionpolicy.html) to Pulumi `retainOnDelete`. **`DeletionPolicy` => `retainOnDelete`** - `Delete` => `false` - `Retain` => `true` - `RetainExceptOnCreate` => `true` - `Snapshot` => `true` Looking specifically for validation on `Snapshot` mapping. This functionality does not exist in Pulumi so I decided to err on the side of caution and retain the resource instead of deleting it. We log a warning in this case and they should see the retention in the preview. closes #188
35f621d
to
6f35433
Compare
super(app, id, options); | ||
new s3.Bucket(this, 'testbucket', { | ||
bucketName: bucketName, | ||
removalPolicy: RemovalPolicy.RETAIN, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
case cdk.CfnDeletionPolicy.RETAIN_EXCEPT_ON_CREATE: | ||
return true; | ||
case cdk.CfnDeletionPolicy.SNAPSHOT: | ||
warn(`DeletionPolicy Snapshot is not supported. Resource '${logicalId}' will be retained.`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. This is good I think.
case cdk.CfnDeletionPolicy.DELETE: | ||
return false; | ||
case cdk.CfnDeletionPolicy.RETAIN: | ||
case cdk.CfnDeletionPolicy.RETAIN_EXCEPT_ON_CREATE: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Comment from L312 can move here :) But I found it. Good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great.🚢
We can table discussing if we want further work on this but let's maybe merge this. Much better to have this handling that none. The code itself LGTM.
This PR has been shipped in release v1.0.0. |
This PR maps CloudFormation DeletionPolicy
to Pulumi
retainOnDelete
.DeletionPolicy
=>retainOnDelete
Delete
=>false
Retain
=>true
RetainExceptOnCreate
=>true
Snapshot
=>true
Looking specifically for validation on
Snapshot
mapping. Thisfunctionality does not exist in Pulumi so I decided to err on the side
of caution and retain the resource instead of deleting it. We log a
warning in this case and they should see the retention in the preview.
closes #188