-
Notifications
You must be signed in to change notification settings - Fork 4.1k
feat(rds): allow DatabaseClusterFromSnapshot
to set copyTagsToSnapshot
property
#19932
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
feat(rds): allow DatabaseClusterFromSnapshot
to set copyTagsToSnapshot
property
#19932
Conversation
…ps://github.com/AnuragMohapatra/aws-cdk into feat(aws-rds)-MoveCopyTagsToSnapshotToBaseProps
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.
Thanks for the contribution @AnuragMohapatra! A few small comments.
@@ -425,6 +431,8 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase { | |||
// Encryption | |||
kmsKeyId: props.storageEncryptionKey?.keyArn, | |||
storageEncrypted: props.storageEncryptionKey ? true : props.storageEncrypted, | |||
//Tag |
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.
//Tag | |
// Tags |
DatabaseClusterFromSnapshot
to set copyTagsToSnapshot
property
Thanks @skinny85 for reviewing, I have updated the changes as per suggestions. |
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.
Thanks for the contribution @AnuragMohapatra!
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
This change will now allow users to set
copyTagsToSnapshot
attribute for theDatabaseClusterFromSnapshot
.This will now be consistent with the way the database instance works.
Integration test may not be possible since this requires a valid snapshot to pre-exist before building the CDK stack to generate the CDK local snapshot.
It might be possible to add a snapshot first by creating a new DB Cluster taking a manual snapshot then using that in
DatabaseClusterFromSnapshot
but it will become a hassle for anyone else in future to maintain or update since they will always need to proceed in the same manner to get the exact snapshot.An integration test for
copyTagsToSnapshot
already exists forDatabaseCluster
ininteg.cluster.js
which is passing successfully on executing the tests.Closes #19884
All Submissions:
Adding new Unconventional Dependencies:
New Features
yarn integ
to deploy the infrastructure and generate the snapshot (i.e.yarn integ
without--dry-run
)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license