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

Terraform seems to ignore "skip_final_snapshot" for rds cluster #2588

Open
hashibot opened this issue Dec 8, 2017 · 43 comments · May be fixed by #10341
Open

Terraform seems to ignore "skip_final_snapshot" for rds cluster #2588

hashibot opened this issue Dec 8, 2017 · 43 comments · May be fixed by #10341
Assignees
Labels
bug Addresses a defect in current functionality. service/rds Issues and PRs that pertain to the rds service. upstream-terraform Addresses functionality related to the Terraform core binary.

Comments

@hashibot
Copy link

hashibot commented Dec 8, 2017

This issue was originally opened by @andrei-dascalu as hashicorp/terraform#16861. It was migrated here as a result of the provider split. The original body of the issue is below.


Hello,

I'm not sure if the following is a terraform or aws provider issue but it sure seems like it's an issue as in my case the request behaves as if "skip_final_snapshot" isn't set or it's always true and it's expecting a "final_snapshot_identifier" (though even when provided the error claims it's not there). I've linked a similar issue as well.

Thanks!

Terraform Version

Terraform v0.11.1
+ provider.aws v1.5.0

Terraform Configuration Files

variable "db_password" {
  description = "Database password"
}

variable "db_name" {
  description = "Database name"
}

resource "aws_rds_cluster_instance" "example" {
  count              = 2
  engine             = "aurora"
  identifier         = "aurora-cluster-demo-${count.index}"
  cluster_identifier = "${aws_rds_cluster.example.id}"
  instance_class     = "db.t2.small"
}

resource "aws_rds_cluster" "example" {
  cluster_identifier = "aurora-cluster-demo"
  availability_zones = ["eu-west-1a", "eu-west-1b"]
  database_name      = "${var.db_name}"
  master_username    = "admin"
  master_password    = "${var.db_password}"
  skip_final_snapshot = true
  final_snapshot_identifier = "whatever"
}

Debug Output

https://gist.github.com/andrei-dascalu/9eaaa3491b5ad77f7e4f8c4f11dc960a

Expected Behavior

RDS Cluster with 2 instances created

Actual Behavior

Error:
1 error(s) occurred:

  • aws_rds_cluster.example (destroy): 1 error(s) occurred:

  • aws_rds_cluster.example: RDS Cluster FinalSnapshotIdentifier is required when a final snapshot is required

Steps to Reproduce

run: terraform apply

References

@loivis
Copy link
Contributor

loivis commented Dec 28, 2017

From debug output, skip_final_snapshot was set to false in the beginning and you wanted to change to true and add new instances to the cluster at the same time?

2017/12/06 20:29:29 [TRACE] DiffTransformer: Module: DESTROY/CREATE: aws_rds_cluster.example
  apply_immediately:               "" => "<computed>"
  availability_zones.#:            "3" => "2" (forces new resource)
  availability_zones.1924028850:   "eu-west-1b" => "eu-west-1b"
  availability_zones.3953592328:   "eu-west-1a" => "eu-west-1a"
  availability_zones.94988580:     "eu-west-1c" => "" (forces new resource)
  backup_retention_period:         "1" => "1"
  cluster_identifier:              "aurora-cluster-demo" => "aurora-cluster-demo"
  cluster_identifier_prefix:       "" => "<computed>"
  cluster_members.#:               "0" => "<computed>"
  cluster_resource_id:             "cluster-CEPPSOIGTTLSDYOFWR33MK4NAM" => "<computed>"
  database_name:                   "mydb" => "test" (forces new resource)
  db_cluster_parameter_group_name: "default.aurora5.6" => "<computed>"
  db_subnet_group_name:            "default" => "<computed>"
  endpoint:                        "aurora-cluster-demo.cluster-czvdbs8joxz1.eu-west-1.rds.amazonaws.com" => "<computed>"
  engine:                          "aurora" => "aurora"
  engine_version:                  "5.6.10a" => "<computed>"
  final_snapshot_identifier:       "" => "whatever"
  kms_key_id:                      "" => "<computed>"
  master_password:                 "<sensitive>" => "<sensitive>" (attribute changed)
  master_username:                 "admin" => "admin"
  port:                            "3306" => "<computed>"
  preferred_backup_window:         "23:03-23:33" => "<computed>"
  preferred_maintenance_window:    "mon:04:40-mon:05:10" => "<computed>"
  reader_endpoint:                 "aurora-cluster-demo.cluster-ro-czvdbs8joxz1.eu-west-1.rds.amazonaws.com" => "<computed>"
  skip_final_snapshot:             "false" => "true"
  storage_encrypted:               "false" => "false"
  vpc_security_group_ids.#:        "1" => "<computed>"

@kojiromike
Copy link

@loivis that actually seems to capture the problem. Even though the .tf code shows skip_final_snapshot = false, attempts to run (plan/apply) show that something inside terraform wants to change it to true despite the user's specification.

@caiges
Copy link

caiges commented Jan 18, 2018

I just took a quick look at the code and the only place I could find where that variable was being set to true was here.

@radeksimko radeksimko added upstream-terraform Addresses functionality related to the Terraform core binary. service/rds Issues and PRs that pertain to the rds service. labels Jan 28, 2018
@csdev
Copy link

csdev commented Apr 17, 2018

I have also encountered this bug with RDS read replicas, for which final snapshots do not even apply.

@vlad2
Copy link

vlad2 commented May 7, 2018

This bug still exists (trying to destroy a Postgres RDS instance, Terraform v0.11.7).

It seems to manifest if 'skip_final_snapshot' parameter was missing when the instance got created, and it's added later. If the 'skip_final_snapshot' was specified from the beginning, everything works correctly.

@rtizzy
Copy link

rtizzy commented Jun 11, 2018

Alright, so on a normal RDS instance I've found a way to easily replicate this bug and a proposed fix.

Replication:

  1. Spin up an RDS instance with Terraform (with skip_final_snapshot = false and no name set for snapshot)
  2. Either set or change the identifier on the database.
  3. Attempt to apply.
  4. It will complain that it cannot do this since the instance requires a final snapshot and there is no name set.
  5. Change the identifier back to the original value.
  6. Set skip_final_snapshot = true.
  7. Apply. This will set that variable in the state file.
  8. Change the identifier.
  9. Apply. It will now properly destroy and recreate the instance.

Suggested fix:

If possible, it seems like a good idea to check if skip_final_snapshot = true via the config and NOT just the state. This would prevent the song and dance seen above.

Right now, it is going soley by the state so if you end up in a state where skip_final_snapshot is already false, then it's only possible to fix this by ensuring that is set to true (Without modifying the state file manually, which is bad practice)

@yruss972
Copy link

yruss972 commented Aug 2, 2018

Hit this also, please fix :)

@rupsray
Copy link

rupsray commented Aug 3, 2018

Even though I have mentioned skip_final_snapshot = "true", I am getting the below error:

RDS Cluster FinalSnapshotIdentifier is required when a final snapshot is required

Terraform v0.11.3

  • provider.aws v1.13.0

@rtizzy
Copy link

rtizzy commented Aug 5, 2018

@rupsray See my comment here

#2588 (comment)

This should allow you to workaround the issue.

If you examine your state file, you'll likely notice that it's not set to true and that it will attempt to destroy it before first changing skip_final_snapshot = "true" in the state file.

@krestivo-kdinfotech
Copy link

This appears to still be broken over a year later.

@rpatrick00
Copy link

Not only is it still broken, there is another issue with the using skip_final_snapshot = false.

If I set the final_snapshot_identifier to a constant name (e.g., mydb-final-snapshot), terraform destroy fails if the snapshot with that name already exists. While I understand why, there doesn't seem to be a good way to get around this limitation other than to manually delete the previous final snapshot or change the name of the final snapshot every time you run terraform destroy. Sigh...

@bostrowski13
Copy link

I hate to say this, but I keep running issues that have been for for 2, 3, 4, or 5 years that aren't fixed.

They really need to do something about prioritizing issues based on how long they've been open.

@krestivo-kdinfotech
Copy link

Is it they or us? Do they accept patches? Anyone willing to take these on?

@camservo
Copy link

Being as this only affects RDS instances that were created without final snapshot policy set to enforced, I don't know if this is a bug so much as a good place to put some notifications that you can't change this option with TF. I would think if someone wanted it protected from the start, they wouldn't want some accidental change to some TF configurations to make it unprotected.

@caiges
Copy link

caiges commented Sep 26, 2019

I just tried to reproduce the original issue with the latest AWS provider and couldn't. @rpatrick00 I'm going to take a look at your issue tonight.

@caiges
Copy link

caiges commented Sep 27, 2019

Okay, I can reproduce and see what's going on here. For starters, skip_final_snapshot defaults to False which should also require final_snapshot_identifier to be set but it's not so what happens is the create/update is applied, state updated where skip_final_snapshot is False but final_snapshot_identifier is null. This causes the destroy operation to fail it's verification stage.

This can be fixed but I don't really have a great story for those who already have prexisting state. One possibility would be that a delete operation ignores skip_final_shopshot if the identifier is null. Another might be to default final_snapshot_identifier to something random if skip_final_snapshot is set to or defaulted to False. I think for data safety reasons, ignoring skip_final_snapshot if final_snapshot_identifier is null is a bad idea and it'd be better to just randomize an identifier.

@krestivo-kdinfotech
Copy link

Thanks very much for taking the time to look into this.

A randomized ID seems safest to me, if it is a UUID or base32'ed UUID or something guaranteed to be random.

@kris1610
Copy link

is this issue resolved or not ? I am using Terraform v0.12.23 and facing this issue. Tried with some workaround but its not working

@caiges
Copy link

caiges commented Mar 17, 2020

I opened a PR last year and it looks like there's now conflicting changes. I'll update the PR but I can only be hopeful this gets more notice.

@valeriozhang
Copy link

still an issue? lol

@reiz
Copy link

reiz commented Apr 16, 2020

I'm using:

Terraform v0.12.24
+ provider.aws v2.57.0

And this is still an issue! Any plans to fix this bug?

@dinvlad
Copy link

dinvlad commented May 6, 2020

Same here:

Terraform v0.12.24
+ provider.aws v2.58.0

@alex-harvey-z3q
Copy link

alex-harvey-z3q commented Jun 20, 2020

I believe I have reproduced the same issue using Terraform 0.13-beta2:

Terraform v0.13.0-beta2
+ provider registry.terraform.io/hashicorp/aws v2.67.0

After adding skip_final_snapshot = true, I still had skip_final_snapshot = false in the state file. I edited that to be also true, and I could delete the DB instance.

@tracycarley
Copy link

I ran into the same issue here with Terraform 0.12.26. Any solutions for this?

@rotemjac
Copy link

rotemjac commented Jul 20, 2020

In my case I encountered this issue when trying to destroy my RDS resources.
Below are the steps I took for solving this issue:

  1. Change skip_final_snapshot to true and remove final_snapshot_identifier if exists.

  2. Remove backup_window (Under AWS Aurora its probably called preferred_backup_window).

  3. Change backup_retention_period to 0.

  4. Make sure that apply_immediately is set to true.

(*) A more detailed answer is given here.

(**) Running under: Terraform v0.12.28, provider.aws v2.70.0

@ameyaagashe
Copy link

Terraform Version 0.13.5 and still RDS instance deletion is not as clean and graceful as it should be.

@jan-swiecki
Copy link

Still a problem with terraform v0.14.4. Can anyone point me in the right direction, I can try to fix this.

@caiges
Copy link

caiges commented Apr 1, 2021

I got around to updating this PR with the latest API and upstream changes.

@thiagolsfortunato
Copy link

thiagolsfortunato commented Jun 5, 2021

Any news?

I've reproduced the same error using Terraform 0.14:

Terraform v0.14.0
+ provider registry.terraform.io/hashicorp/aws v3.44.0

Error:

Error: RDS Cluster FinalSnapshotIdentifier is required when a final snapshot is required

@everdrone
Copy link

Experiencing this problem with

Terraform v1.1.5
+ provider registry.terraform.io/hashicorp/aws v4.0.0

gfou-al added a commit to ministryofjustice/modernisation-platform-environments that referenced this issue Mar 4, 2022
@mrocheleau
Copy link

FWIW on Terraform v1.1.6 I encountered this bug as I had previously NOT set the "skip_final_snapshot" to anything, so the state file had it as "False". Updating my code to set this to "true" failed as it's looking at the state file first.

So I updated my state file manually (terraform state pull > somefile.tfstate) then edited that switch to "true" from false, incremented the "serial" by one and terraform state push somefile.tfstate to get it back up.

Now I can apply properly without that final snapshot trying to happen and failing.

@promenadeviki
Copy link

FWIW on Terraform v1.1.6 I encountered this bug as I had previously NOT set the "skip_final_snapshot" to anything, so the state file had it as "False". Updating my code to set this to "true" failed as it's looking at the state file first.

So I updated my state file manually (terraform state pull > somefile.tfstate) then edited that switch to "true" from false, incremented the "serial" by one and terraform state push somefile.tfstate to get it back up.

Now I can apply properly without that final snapshot trying to happen and failing.

For me, I was able to add skip_final_snapshot, re-apply, and then I was able to destroy without issues.

The issue is indeed the state is holding the default value of false, because there was not one specified.

@CErikLjung
Copy link

Excuse my ignorance, but why exactly do we need skip_final_snapshot?

Shouldn't it be possible to deduce the expected value of skip_final_snapshot from final_snapshot_identifier?

Something along the lines of:
skip_final_snapshot = is_empty(final_snapshot_identifier)

@johnsonaj
Copy link
Contributor

Hi all, just providing a quick update on this issue. To solve this on the resource level we decided to utilize the Terraform Plugin Framework, which has better support for warnings during the destroy step. During the upgrade, we discovered roadblocks which could potentially cause unexpected diffs, ultimately disrupting how the currently deployed resources work.

This upgrade has been paused, for now, until we have a better understanding of the impact that these changes would have on existing resources.

@bo-yoon
Copy link

bo-yoon commented Aug 3, 2023

Has this issue been resolved? It still happens to me.

terraform -v
Terraform v1.5.4

@sjackson0109
Copy link

This is still an issue.

│ Error: RDS Cluster final_snapshot_identifier is required when skip_final_snapshot is false

Code:

# RDS Cluster
resource "aws_rds_cluster" "rds" {
  for_each                         = try(var.rds_cluster, {})
  allow_major_version_upgrade      = each.value.allow_major_version_upgrade
  apply_immediately                = each.value.apply_immediately
  backup_retention_period          = each.value.backup_retention_period
  cluster_identifier               = each.value.name
  copy_tags_to_snapshot            = each.value.copy_tags_to_snapshot
  database_name                    = each.value.database_name
  db_cluster_parameter_group_name  = aws_rds_cluster_parameter_group.rds[each.value.cluster_parameter_group_key].name
  db_instance_parameter_group_name = aws_db_parameter_group.rds[each.value.db_parameter_group_key].name
  db_subnet_group_name             = aws_db_subnet_group.rds[each.value.db_subnet_group_key].name
  deletion_protection              = try(each.value.deletion_protection, false)
  engine                           = try(each.value.engine, "aurora-mysql")
  engine_mode                      = try(each.value.engine_mode,"serverless")
  engine_version                   = try(each.value.engine_version, "aurora-mysql8.0")
  storage_encrypted                = try(each.value.storage_encrypted, false)
  preferred_backup_window          = try(each.value.preferred_backup_window,"01:00-03:00")
  preferred_maintenance_window     = try(each.value.preferred_maintenance_window,"Sat:01:00-Sat:03:00")
  master_username                  = random_password.generated[each.value.password_key].id
  master_password                  = random_password.generated[each.value.password_key].result
  skip_final_snapshot              = try(each.value.skip_final_snapshot, true)  # DEFAULTS TO TRUE
  vpc_security_group_ids           = [aws_security_group.rds.id]

  }
}

Variable:

rds_cluster = {
      0 = {
        name = "mycluster"
        database_name = "product_name"
        backup_retention_period = 31
        apply_immediately = true
        allow_major_version_upgrade = true
        copy_tags_to_snapshot = true
        deletion_protection = false
        engine = "aurora-mysql" 
        engine_mode = "serverless" #serverless or provisioned
        engine_version = "8.0"
        instance_class = "db.serverless"
        cluster_parameter_group_key = 0
        db_parameter_group_key = 0
        db_subnet_group_key = 0
        username_key = 0
        password_key = 0
        storage_encrypted = true
        skip_final_snapshot = true   # CLEARLY SET TO TRUE
        monitoring_interval = 60

        preferred_backup_window = "23:00-00:00"
        preferred_maintenance_window = "Thu:01:00-Thu:03:00"
      }
    }

Versions:

Terraform v1.6.6
on windows_amd64
+ provider registry.terraform.io/hashicorp/aws v4.67.0

@sjackson0109
Copy link

I even get the same error with these attribute combinations set:

  skip_final_snapshot                 = null
  final_snapshot_identifier           = "DELETE-ME"

or

  skip_final_snapshot                 = false
  final_snapshot_identifier           = "DELETE-ME"

or

  skip_final_snapshot                 = true
  final_snapshot_identifier           = "DELETE-ME"

Is there another attribute that forces skip_final_snapshot to be false?

@drewlearns
Copy link

This is still not resolved with aws provider 5.31.0 and tf version 1.6.5

@drewlearns
Copy link

Was able to delete my cluster and redeploy with skip_final_snapshot = true and that appears to have stuck. Weird and frustrating for sure.

@susmitha48
Copy link

We got the same issue

@JuanMongeLL
Copy link

Still happening in 2024. 7 years now.

@hamza-louis
Copy link

Still happening in Oct 2024. 7 years now. Why ?!

@AndrewTtofi
Copy link

is there an update on this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Addresses a defect in current functionality. service/rds Issues and PRs that pertain to the rds service. upstream-terraform Addresses functionality related to the Terraform core binary.
Projects
None yet