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

(rds): ServerlessClusterProps is missing copyTagsToSnapshot #20968

Closed
SydneyUni-Jim opened this issue Jul 3, 2022 · 2 comments · Fixed by #21056
Closed

(rds): ServerlessClusterProps is missing copyTagsToSnapshot #20968

SydneyUni-Jim opened this issue Jul 3, 2022 · 2 comments · Fixed by #21056
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@SydneyUni-Jim
Copy link
Contributor

Describe the bug

DatabaseClusterProps.copyTagsToSnapshot exists but ServerlessClusterProps does not have this property.

The CloudFormation documentation does not suggest that this doesn't apply to Aurora Serverless.

Expected Behavior

This compiles

new rds.ServerlessCluster(scope, id, {
  copyTagsToSnapshot: true,
  engine: rds.DatabaseClusterEngine.auroraMysql({ version: rds.AuroraMysqlEngineVersion.VER_2_07_1 }),
  // …
})

and generates a CFT with

  DBCluster:
    Type: AWS::RDS::DBCluster
    Properties:
       CopyTagsToSnapshot: true
       Engine: aurora-mysql
       EngineMode: serverless
       EngineVersion: 5.7.mysql_aurora.2.07.1
       #

Current Behavior

A TypeScript error that ServerlessClusterProps does not have copyTagsToSnapshot.

Reproduction Steps

Try to copyTagsToSnapshot: true to the props when creating a rds.ServerlessCluster.

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.30.0 (build 1529743)

Framework Version

No response

Node.js Version

v14.19.3

OS

macOS 12.4 arm64

Language

Typescript

Language Version

TypeScript (4.7.4)

Other information

No response

@SydneyUni-Jim SydneyUni-Jim added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 3, 2022
@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label Jul 3, 2022
@corymhall
Copy link
Contributor

I can't find anything that says that it is not available for serverless either.

This issue has been classified as p2. That means a workaround is available or it is deemed a nice-to-have feature. Given the amount of work there is to do and the relative priority of this issue, the CDK team is unlikely to address it. That does not mean the issue will never be fixed! If someone from the community submits a PR to fix this issue, and the PR is small and straightforward enough, and meets the quality bars to be reviewed and merged with little effort we will accept that PR. PRs that do not build or need complex or multiple rounds of reviews are unlikely to be merged and will be closed to keep our backlog manageable.

In the mean time, remember that you can always use the escape hatch (https://docs.aws.amazon.com/cdk/v2/guide/cfn_layer.html) mechanism to have fine control over the CloudFormation output you want. We will keep the issue open for discoverability, to collect upvotes, and to facilitate discussion around this topic.

We use +1s on this issue to help prioritize our work, and are happy to re-evaluate the prioritization of this issue based on community feedback. You can reach out to the cdk.dev community on Slack (https://cdk.dev/) to solicit support for reprioritization.

@corymhall corymhall 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 Jul 5, 2022
@corymhall corymhall removed their assignment Jul 5, 2022
@SydneyUni-Jim SydneyUni-Jim changed the title rds: ServerlessClusterProps missing copyTagsToSnapshot (rds): ServerlessClusterProps is missing copyTagsToSnapshot Jul 8, 2022
@mergify mergify bot closed this as completed in #21056 Jul 30, 2022
mergify bot pushed a commit that referenced this issue Jul 30, 2022
…ssCluster and ServerlessClusterFromSnapshot (#21056)

In f7c6289 copyTagsToSnapshot was added to DatabaseCluster and in 40a6ceb it was added to DatabaseClusterFromSnapshot as well.

This does the same for ServerlessCluster and ServerlessClusterFromSnapshot.

The implementation and tests parallels the changes mentioned above.

fixes #20968

## Behaviour change

In parallel to copyTagsToSnapshot on DatabaseCluster and DatabaseClusterFromSnapshot, the default for copyTagsToSnapshot on ServerlessCluster and ServerlessClusterFromSnapshot is true.

Before this change, ServerlessCluster and ServerlessClusterFromSnapshot did not emit the CopyTagsToSnapshot property in the CloudFormation template. That resulted in the CopyTagsToSnapshot property being false.

My rationale for this behaviour change is to minimise the differences between DatabaseCluster[FromSnapshot] and ServerlessCluster[FromSnapshot], which are both realised as the same L1 construct.

Changing the CopyTagsToSnapshot property on an existing serverless RDS cluster introduces no interruption. Ref: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-rds-dbcluster.html#cfn-rds-dbcluster-copytagstosnapshot

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `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*
@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.

josephedward pushed a commit to josephedward/aws-cdk that referenced this issue Aug 30, 2022
…ssCluster and ServerlessClusterFromSnapshot (aws#21056)

In f7c6289 copyTagsToSnapshot was added to DatabaseCluster and in 40a6ceb it was added to DatabaseClusterFromSnapshot as well.

This does the same for ServerlessCluster and ServerlessClusterFromSnapshot.

The implementation and tests parallels the changes mentioned above.

fixes aws#20968

## Behaviour change

In parallel to copyTagsToSnapshot on DatabaseCluster and DatabaseClusterFromSnapshot, the default for copyTagsToSnapshot on ServerlessCluster and ServerlessClusterFromSnapshot is true.

Before this change, ServerlessCluster and ServerlessClusterFromSnapshot did not emit the CopyTagsToSnapshot property in the CloudFormation template. That resulted in the CopyTagsToSnapshot property being false.

My rationale for this behaviour change is to minimise the differences between DatabaseCluster[FromSnapshot] and ServerlessCluster[FromSnapshot], which are both realised as the same L1 construct.

Changing the CopyTagsToSnapshot property on an existing serverless RDS cluster introduces no interruption. Ref: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-rds-dbcluster.html#cfn-rds-dbcluster-copytagstosnapshot

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `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*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
2 participants