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

prevent_destroy should allow plan to succeed #16392

Open
beanaroo opened this issue Oct 18, 2017 · 31 comments
Open

prevent_destroy should allow plan to succeed #16392

beanaroo opened this issue Oct 18, 2017 · 31 comments

Comments

@beanaroo
Copy link

beanaroo commented Oct 18, 2017

Terraform Version

0.10.7

Output

Error running plan: 1 error(s) occurred:

* aws_db_instance.db: 1 error(s) occurred:

* aws_db_instance.ics_db: aws_db_instance.db: the plan would destroy this resource, but it currently has lifecycle.prevent_destroy set to true. To avoid this error and continue with the plan, either disable lifecycle.prevent_destroy or adjust the scope of the plan using the -target flag.

Expected Behavior

I would like to know why it wants to destroy it. This is provided when prevent_destroy is false by means of (forces new resource)

Actual Behavior

Plan is interrupted before any useful information is displayed.

Steps to Reproduce

  1. terraform init
  2. terraform plan

Important Factoids

Current workaround is to turn prevent_destroy off (dangerous!!!) and to run plan again to see cause. In this case it is due to this open issue

@whereisaaron
Copy link

You're not wrong @beanaroo! prevent_destroy is I think broken and arguably unusable in it's current form. The problem is that prevent_destroy doesn't just keep the flagged resource from being destroyed (i.e. exclude its destruction from plans), instead it blocks planning. Often I want to destroy everything except stateful resources, like a DB or EIPs or pet EBS or EFS or S3 store, terraform can't do that. You have to leave those stateful resources outside of terraform. If you include them you can't use prevent_destroy and plan, so instead you have to manually check plans.

Quite a few people have the same issue, but unfortunately the mod's don't seem to agree it is actually a problem 😢 and keep flagging this issue and an 'enhancement'. See #3874 for a two-year long sad tale of frustrated terraform users 😝

@dguisinger
Copy link

Prevent destroy only works if the resource is still included in the script, also dangerous.

I've been using it for CodeCommit repositories to have consistent Git repos, but there is no way to prevent it from destructively wiping out an entire Git repo or all of the repos with a mistake.
there needs to be a flag you can set on the state that prevents Terraform from ever deleting a resource, unless you specifically tell it you want to release that lock.

Otherwise resources that have user content that is hard to restore should never be put in Terraform, which is a shame.

@tdmalone
Copy link

tdmalone commented Jul 2, 2018

Potential duplicate of #3874 (and possibly #2159 as well)

@caruccio
Copy link

This flag should be renamed to prevent_plan_or_apply_to_run_on_destroy.

@caruccio
Copy link

It would be useful to have -target to accept reverse match like -target aws_s3_bucket!=my-bucket-i-want-to-keep.

@whereisaaron
Copy link

@dguisinger yes that is what we are doing, only using terraform for resources that are not calamitous if destroyed. Or in one strategy we have two terraform deployments, with one small one that just creates the stateful resources. The issue is quite a thorn.

@SamWhited
Copy link

I would like to throw my support behind this being a bug and not merely a desirable enhancement. In my case I have a stateful machine (postgres) that suddenly decided that it needed to be destroyed. I have no idea why since there are no changes in the commit history to anything that affects postgres that I can see. Unfortunately, it is very difficult to debug since I don't know what property it thinks changed. Please mark this as a bug, prevent_destroy is worse than useless as is, it makes terraform actively difficult to use and debug (but is a safety measure that I can't live without, unfortunately).

@fantomc0der
Copy link

fantomc0der commented Jul 24, 2018

Agreed with the consensus in here. If it's not a bug then it's a poorly designed feature that could use some improvement. My team at work tried using it in the past to protect particular resources (like RDS) but the planning issue mentioned above made it useless. It definitely will act as a last line of defense, but it seems implied that you should never try destroying any root module that can reference a prevent_destroy resource in the first place so it's useless. We ended up preventing it with safety checks in our build script and ensuring that we had no production deployments configured to destroy those resources.

The other issue with prevent_destroy that we encountered is that you can't selectively apply it to a resource. In one of our applications, we had a Terraform module for an RDS cluster which we wanted to prevent_destroy in production; however, for non-production (testing/staging/etc) accounts, we wanted to reference the same module but allow being destroyed since testing environments can come and go.

@oceanlewis
Copy link

oceanlewis commented Jul 24, 2018

I think my biggest grievance with this bug is that I had to come to Github Issues to find enough information on this. In the documentation on the Terraform website, there are no warnings that I have seen of "Hey, you can include pre-existing S3 buckets and other stateful resources in your Terraform plan. If you do that, Terraform can destroy and recreate those resources if it wants to, or - if you are using prevent_destroy - refuse to apply changes to a plan."

If this isn't a bug (which it sounds like it is?), there should be a section in the docs for how to deal with this use-case (immutable stateful storage) and a breakdown of your options:

  1. Is it appropriate to keep this resource outside of Terraform? If so, how can I refer to it within my plan as an immutable global?

  2. If not, how I can I use prevent_destroy while being fully informed of the implications of the Terraform's behavior around that feature.

@ketzacoatl
Copy link
Contributor

ketzacoatl commented Jul 26, 2018

There is also #3874 tracking this discussion as well, one should probably be closed in favor of the other. Ping @jen20 / @phinze / @paddycarver / @radeksimko.

@whereisaaron
Copy link

@ketzacoatl #3874 is the earlier request (2015!) and has the most participants (~40) and +1's (~100), so I would suggest keeping that one.

@tdmalone
Copy link

^ Also #2159

@ckyoog
Copy link

ckyoog commented Jan 23, 2019

First

To the OP, @beanaroo. I agree with you, whether use prevent_destroy or not, the full plan information should be output. However, I don't think it's right that plan should succeed. It still should return error on plan, rather than postpone the error on apply.

Second

To @whereisaaron (and other guys, I guess?). I think the name of prevent_destroy is indeed a little confusing. You may be misled by its name.

If rename it to what @caruccio said, like prevent_plan_or_apply_to_run_on_destroy or report_error_on_destroy, will you feel better? Because if so, I believe you will learn this keyword is not meant to serve such a purpose as yours -- keep something while performing terraform destroy.

So I think what you ask maybe is a new keyword, like keep_it_on_destroy. So that both the resource you specify to keep and its dependencies can remain after destroying.

So Hi @apparentlymart, what do you think?

(Further more, in such a case that the resource becomes force new resource while performing a normal terraform plan or apply, as long as it has keep_it_on_destroy on, it should not be destroyed as well. And, of course, the user must take the consequence which is that the changes for this resource won't be applied (maybe including other resources which depend on this one). The changes will still be seen in next plan information... All of these are my rough thoughts, not mature at all, hope it can help :)

@whereisaaron
Copy link

Thanks @ckyoog keep_on_destroy sounds great.

If the current prevent_destroy behavior is by design, then I can’t personally see a use case for it?Maybe rename prevent_any_destroy_plan_that_would_also_destroy_this.

@woky
Copy link

woky commented Feb 22, 2019

To the OP, @beanaroo. I agree with you, whether use prevent_destroy or not, the full plan information should be output. However, I don't think it's right that plan should succeed. It still should return error on plan, rather than postpone the error on apply.

What's the point of prevent_destroy exactly?

I had the impression that you can freely apply and destroy Terraform described infrastructure, but if you include single prevent_destroy resource, then you can never destroy it again even if there are resources not marked with prevent_destroy. Surely it's possible to figure out what to destroy and what to keep.

@j-martin
Copy link

To the OP, @beanaroo. I agree with you, whether use prevent_destroy or not, the full plan information should be output. However, I don't think it's right that plan should succeed. It still should return error on plan, rather than postpone the error on apply.

I think it should show the plan, but still, display an error and return a non-zero exit code.

@oceanlewis
Copy link

@j-martin Agreed - exit codes are there to communicate details about how an action may have failed to other programs; but, if someone runs terraform plan, then Terraform should output what would happen should the user escalate to terraform apply.

This includes both the fact that prevent_destroy will prevent apply from running successfully, as well as any other changes to resources that Terraform would make regardless of whether or not prevent_destroy breaks apply or not. stdout and stderr are there to inform the user and so we need to put on a UX hat when thinking about what we're communicating and why a user might run terraform plan.

Maybe I intend to destroy that resource at some point, but my Terraform plan is going through a code review process first so that I can squash+merge my code later into master so that Terraform Enterprise can perform the apply for me. In this scenario, yeah, I want terraform plan to enumerate all my changes, regardless of the lifecycle of my resources.

@vlasceanuv
Copy link

I have the same issue, caused by aks scale down from 3->1 and scale up from 1->3(triggered from the azure interface).
My expectation is to get more information from terraform - why it wants to destroy the aks ( nothing was changed actually).

@sbz
Copy link

sbz commented Aug 20, 2019

We are migrating from 0.11.x to 0.12.x version and I have exactly the same problem with the following versions:

Terraform v0.12.0
+ provider.aws v2.23.0

The following aws_instance state which contains the lifecycle with prevent_destroy is like above:

resource "aws_instance" "etcd_host_cluster_x" {
    instance_type     = var.instance_type
    availability_zone = "${data.aws_region.current.name}c"
    ami               = var.ami_coreos[data.aws_region.current.name]
    subnet_id         = var.subnet_ids[2]
    vpc_security_group_ids = [
      var.security_group_id,
    ]

    key_name = "id_keypair_deploy"

    user_data_base64 = base64encode(...)

    tags = {
      Name            = "${var.cluster_name}_3"
      Cluster         = var.cluster_name
      Environment     = var.environment
      ProvisionedWith = "terraform"
    }

    lifecycle { 
        prevent_destroy = true
    }
 }

This state define EC2 instance as member of an etcd cluster with a CoreOS image.

I tried to replace the lifecycle by the disable_api_termination=true but plan is trying to destroy and to recreate the existing resource.

My current workaround is to run terraform plan with -target in order to exclude the resource but I feel it's terrible way to handle that issue.

I have the following error message after running terraform plan:

Error: Instance cannot be destroyed

  on services/xyz/etcd_cluster.tf line 128:
 128: resource "aws_instance" "etcd_host_cluster_x" {

Resource module.xyz.aws_instance.etcd_host_cluster_x has
lifecycle.prevent_destroy set, but the plan calls for this resource to be
destroyed. To avoid this error and continue with the plan, either disable
lifecycle.prevent_destroy or reduce the scope of the plan using the -target
flag.
  1. We didn't change anything on this resource, so we don't understand why it moans
  2. This looks a very common bug reported by lot of people, would be possible to add some documentation/FAQ about that?
  3. It would be awesome if maintainers will commit to fix this well known issue

@rehevkor5
Copy link

The documentation on https://www.terraform.io/docs/configuration/resources.html#prevent_destroy states:

Since this argument must be present in configuration for the protection to apply, note that this setting does not prevent the remote object from being destroyed if the resource block were removed from configuration entirely: in that case, the prevent_destroy setting is removed along with it, and so Terraform will allow the destroy operation to succeed.

It's worth noting that when using count driven by a variable to control the presence/absence of a resource with lifecycle.prevent_destroy = true, Terraform does not treat count = 0 the same as if you had removed the resource from the configuration. It will still prohibit deletion of the resource. The protected resource must be manually deleted from state in order to proceed.

@pmcanseco
Copy link

Would a maintainer mind chiming in? This is still an issue in 2020. The current workaround is to temporarily disable prevent_destroy and run terraform plan to see why the destruction is happening, but this is useless for people who only are able to run terraform from inside of a ci/cd environment.

@boris-yakimov
Copy link

Although it seems to be the intended behaviour when you remove a resource for it to also remove the prevent_destroy flag and delete protection, it is sometimes risky in the context of CI/CD pipelines. In some cases we keep a central RDS module and individual applications use it, so each application has its own main.tf file where it is called and it sometimes happens where different teams make changes to the code in their repository and run pipelines. We would like to protect the production databases to not be removed by accident even if someone removes the whole RDS resource from their application.

@mkjmdski
Copy link

mkjmdski commented Oct 8, 2021

This is 2021 and nearly 2022 terraform since 0.10 when this issue was opened made a lot of progress but the disability to see the exact reasoned why prevent destroy blocks plan is still the most unpleasant experience with TF. I hope that one day this issue gets closed

@eduardocorral
Copy link

This is a basic e.g. cloudformation feature, and it's critical for things like CMK: you want all gone, except, for instance, RDS snapshots, and the CMK that encrypted them. And you don't want that to block the tear down, you just want those resources to be left behind.

@jufemaiz
Copy link

jufemaiz commented Nov 3, 2021

Throwing my thoughts in here too – we are trying to work out wtf an RDS is claiming to need to be destroyed and recreated. Cannot see the trigger with the current implementation. It should be part of the failure – not only that a prevent_destroy lifecycled resource is needing to be destroyed, buy the why it needs to be destroyed too.

@ajbouh
Copy link

ajbouh commented Nov 3, 2021

This brings up a great question... How are people debugging these plans for "unintentional destruction"?

@jufemaiz
Copy link

jufemaiz commented Nov 3, 2021

TBH for me it's been a lot of AWS console work.

Edit: to add a little more context.

We're using remote modules (via git), so the concept of "swapping the value of prevent_destroy is not really one that is at all effective.

@j-martin
Copy link

j-martin commented Nov 3, 2021

This brings up a great question... How are people debugging these plans for "unintentional destruction"?

By swaping the value to lifecycle.prevent_destroy = false.

@pivotal-marcela-campo
Copy link

In our use case, being able to see the plan and understanding why tf is trying to recreate the resource without having to remove the prevent_destroy and risk accidental deletion, would be very useful.

@ghost
Copy link

ghost commented Jul 10, 2023

I noticed this is partially fixed (I'm using v1.4.6). Terraform will now show you what it planned up to (but not including) the prevent-destroy'd resource, which in my case was enough to figure things out without removing my prevent-destroy.

@divStar
Copy link

divStar commented Oct 13, 2024

It's October 2024 and this issue still persists.

In my case I want to not re-download the Talos ISO for my schematic-ID EVERYTIME I run Terraform:

Snippet of the image.tf file
locals {
  version = var.image.version
  schematic = var.image.schematic
  schematic_id = jsondecode(data.http.schematic_id.response_body)["id"]
  image_id = "${local.schematic_id}_${local.version}"

  update_version = coalesce(var.image.update_version, var.image.version)
  update_schematic = coalesce(var.image.update_schematic, var.image.schematic)
  update_schematic_id = jsondecode(data.http.updated_schematic_id.response_body)["id"]
  update_image_id = "${local.update_schematic_id}_${local.update_version}"
}

data "http" "schematic_id" {
  url          = "${var.image.factory_url}/schematics"
  method       = "POST"
  request_body = local.schematic
}

data "http" "updated_schematic_id" {
  url          = "${var.image.factory_url}/schematics"
  method       = "POST"
  request_body = local.update_schematic
}

resource "proxmox_virtual_environment_download_file" "this" {
  for_each = toset(distinct([for k, v in var.nodes : "${v.host_node}_${v.update == true ? local.update_image_id : local.image_id}"]))

  node_name    = split("_", each.key)[0]
  content_type = "iso"
  datastore_id = var.image.proxmox_datastore

  file_name               = "talos-${split("_",each.key)[1]}-${split("_", each.key)[2]}-${var.image.platform}-${var.image.arch}.img"
  url = "${var.image.factory_url}/image/${split("_", each.key)[1]}/${split("_", each.key)[2]}/${var.image.platform}-${var.image.arch}.raw.gz"
  decompression_algorithm = "gz"
  overwrite               = false

  lifecycle {
    prevent_destroy = true
  }
}

I wanted to use lifecycle.prevent_destroy = true to prevent the downloaded image from being destroyed (deleted from the folder it was downloaded to) and thus Terraform using the already downloaded file. Unfortunately this does not work (or maybe I just don't know how to get it to work).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests