-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
Force replacement on snapshot_identifier
change for DB cluster resources
#29409
Conversation
Community NoteVoting for Prioritization
For Submitters
|
We should also ensure consistency for the AWS services that are veneers on top of RDS: |
2c49b38
to
0a2bc65
Compare
r/aws_rds_cluster
: Force replacement on snapshot_identifier
changesnapshot_identifier
change for DB cluster resources
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.
LGTM 🚀
$ make testacc PKG=rds TESTS="TestAccRDSCluster_snapshotIdentifier|TestAccRDSCluster_SnapshotIdentifier_"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/rds/... -v -count 1 -parallel 20 -run='TestAccRDSCluster_snapshotIdentifier|TestAccRDSCluster_SnapshotIdentifier_' -timeout 180m
--- PASS: TestAccRDSCluster_SnapshotIdentifier_tags (363.02s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_kmsKeyID (363.02s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_preferredBackupWindow (363.34s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_encryptedRestore (363.34s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_vpcSecurityGroupIDs (377.49s)
--- PASS: TestAccRDSCluster_snapshotIdentifier (383.46s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_preferredMaintenanceWindow (402.28s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_deletionProtection (406.54s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_masterUsername (463.95s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_masterPassword (689.70s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/rds 692.918s
$ make testacc PKG=docdb TESTS=TestAccDocDBCluster_
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/docdb/... -v -count 1 -parallel 20 -run='TestAccDocDBCluster_' -timeout 180m
--- PASS: TestAccDocDBCluster_missingUserNameCausesError (11.53s)
--- PASS: TestAccDocDBCluster_generatedName (144.14s)
--- PASS: TestAccDocDBCluster_basic (146.76s)
--- PASS: TestAccDocDBCluster_GlobalClusterIdentifier_Remove (149.57s)
--- PASS: TestAccDocDBCluster_GlobalClusterIdentifier_Add (152.85s)
--- PASS: TestAccDocDBCluster_GlobalClusterIdentifier (166.68s)
--- PASS: TestAccDocDBCluster_encrypted (167.44s)
--- PASS: TestAccDocDBCluster_updateCloudWatchLogsExports (170.11s)
--- PASS: TestAccDocDBCluster_kmsKey (173.10s)
--- PASS: TestAccDocDBCluster_backupsUpdate (190.27s)
--- PASS: TestAccDocDBCluster_updateTags (190.38s)
--- PASS: TestAccDocDBCluster_namePrefix (193.25s)
--- PASS: TestAccDocDBCluster_GlobalClusterIdentifier_Update (205.53s)
--- PASS: TestAccDocDBCluster_deleteProtection (268.14s)
--- PASS: TestAccDocDBCluster_port (272.94s)
--- PASS: TestAccDocDBCluster_takeFinalSnapshot (350.44s)
--- PASS: TestAccDocDBCluster_GlobalClusterIdentifier_PrimarySecondaryClusters (2305.32s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/docdb 2308.473s
$ make testacc PKG=neptune TESTS=TestAccNeptuneCluster_
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/neptune/... -v -count 1 -parallel 20 -run='TestAccNeptuneCluster_' -timeout 180m
--- PASS: TestAccNeptuneCluster_serverlessConfiguration (166.40s)
--- PASS: TestAccNeptuneCluster_namePrefix (167.06s)
--- PASS: TestAccNeptuneCluster_iamAuth (199.83s)
--- PASS: TestAccNeptuneCluster_encrypted (199.89s)
--- PASS: TestAccNeptuneCluster_basic (199.93s)
--- PASS: TestAccNeptuneCluster_kmsKey (205.97s)
--- PASS: TestAccNeptuneCluster_updateIAMRoles (206.61s)
--- PASS: TestAccNeptuneCluster_disappears (215.52s)
--- PASS: TestAccNeptuneCluster_tags (235.34s)
--- PASS: TestAccNeptuneCluster_backupsUpdate (262.44s)
--- PASS: TestAccNeptuneCluster_deleteProtection (310.26s)
--- PASS: TestAccNeptuneCluster_copyTagsToSnapshot (311.72s)
--- PASS: TestAccNeptuneCluster_takeFinalSnapshot (332.98s)
--- PASS: TestAccNeptuneCluster_updateCloudWatchLogsExports (333.99s)
--- PASS: TestAccNeptuneCluster_restoreFromSnapshot (457.47s)
--- PASS: TestAccNeptuneCluster_updateEngineVersion (1835.02s)
--- PASS: TestAccNeptuneCluster_updateEngineMajorVersion (2018.32s)
--- PASS: TestAccNeptuneCluster_GlobalClusterIdentifier_PrimarySecondaryClusters (2937.84s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/neptune 2941.269s
I want to add caution that there are dragons lurking inside of this change that will catch users off guard.
|
Hi @TechIsCool 👋 - Thanks for your comment! I've added The automated snapshot scenario is a more difficult one to account for as the snapshot deletion occurs outside the provider workflow. To my knowledge we can't (reliably) distinguish between automated and manual snapshots from identifier alone, which rules out a validation step that could error or warn at plan time. At a minimum we should include some documentation that calls attention to this possibility in each of the impacted resources. I'll also bring this point back to discuss internally and decide if we should consider holding this change for a major release, or pursuing other options. Thanks again! |
After discussing internally, we are going to hold this until |
23e2ed1
to
e7c2818
Compare
e7c2818
to
296fa82
Compare
This functionality has been released in v5.0.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Description
Changes to the
snapshot_identifier
attribute of theaws_rds_cluster
,aws_docdb_cluster
, andaws_neptune_cluster
resources will now trigger a replacement. Previously, changing this attribute would result in a successful apply, but without the cluster actually being restored (only the resource state was changed).Relations
Closes #15563
References
The
aws_db_instance
already forces re-creation on changes to this attribute:terraform-provider-aws/internal/service/rds/instance.go
Lines 504 to 509 in 2eab277
Output from Acceptance Testing