terraform-aws-modules / terraform-aws-rds-aurora

Terraform module to create AWS RDS Aurora resources 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/rds-aurora/aws
Apache License 2.0
382 stars 551 forks source link

Creating a final snapshot is skipped by default #406

Closed kwn closed 7 months ago

kwn commented 9 months ago

Description

Hi guys. After upgrading the terraform-aws-rds-aurora module version from 6 to 8 I discovered quite significant issue that might cause that people will be accidentally deleting their databases without any option to restore them. A similar scenario happened to us some time ago, due to an AWS API bug in ap-south-1 region. The bug forced cluster recreation, distracted developer didn't read the terraform plan, and we didn't have the deletion protection enabled. Luckily the terraform-aws-rds-aurora module ver 6/7 was configured to take the final snapshot, so we were able to restore the database.

The problem:

Combination of default values for: skip_final_snapshot, snapshot_identifier and deletion_protection parameters causes that the final snapshots won't be taken by default. Even if the skip_final_snapshot = false the snapshot_identifier = null by default. The TF AWS RDS provider says:

snapshot_identifier - Name of your final DB snapshot when this DB cluster is deleted. If omitted, no final snapshot will be made.

Unlike in ver 6/7, terraform-aws-rds-aurora ver 8 man needs to explicitly set the value for snapshot_identifier, otherwise the snapshot won't be taken. This might lead to a situation where someone might be under impression that the snapshots will be created automatically, because skip_final_snapshot = false.

This inconsistency/issue comes from the TF AWS RDS provider, but I think it would be wise, if the terraform-aws-modules team would be smarter than Hashicorp, and set some default value for the snapshot_identifier, as it was done in ver 6/7.

I'm aware of the snapshot_identifier and global_cluster_identifier issue, but why not providing a default value for the snapshot if the global_cluster_identifier = null? I assume there are fairly less cases of global clusters than people not reading all of the module configuration parameters.

Versions

Reproduction Code [Required]

Steps to reproduce the behavior:

Expected behavior

I expect, that due to the skip_final_snapshot = false the final snapshot will be taken.

Actual behavior

The snapshot is not taken because I wasn't aware that I need to provide a value for another parameter snapshot_identifier to take the final snapshot. I lost my database along with the backups.

github-actions[bot] commented 8 months ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

kwn commented 8 months ago

Hey @antonbabenko

Do you mind removing the "stale" label from the issue? I can't control the labels unfortunately.

github-actions[bot] commented 7 months ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

github-actions[bot] commented 7 months ago

This issue was automatically closed because of stale in 10 days

github-actions[bot] commented 6 months ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.