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

Feature request: support prevent_destroy for modules #18367

Open
tdmalone opened this issue Jul 2, 2018 · 37 comments
Open

Feature request: support prevent_destroy for modules #18367

tdmalone opened this issue Jul 2, 2018 · 37 comments

Comments

@tdmalone
Copy link

tdmalone commented Jul 2, 2018

Terraform Version

Terraform v0.11.7

Terraform Configuration Files

module "test" {
  source = "../modules/module-example"

  lifecycle {
    prevent_destroy = true
  }
}

Expected Behavior

I was hoping I could prevent destroy of any resources created by the module.

Actual Behavior

Error: module "test": "lifecycle" is not a valid argument

References

@tdmalone tdmalone changed the title Support lifecycle for modules Feature request: support lifecycle for modules Jul 2, 2018
@apparentlymart apparentlymart changed the title Feature request: support lifecycle for modules Feature request: support prevent_destroy for modules Jul 2, 2018
@apparentlymart
Copy link
Contributor

Hi @tdmalone!

The available settings inside lifecycle will vary depending on the type of block it's used in, so I adjusted the summary of this issue to be specifically about supporting prevent_destroy on modules, since that's a tighter scope to think about and design for.

I assume that your intent here was this to behave as if prevent_destroy were set to true for every resource inside the module, regardless of what it's actually set to. This does seem like a nice idea since it means that the decision can be made by the caller rather than the module author, and can thus represent a situation where e.g. a particular module must be destroyable in one configuration but not another.

We won't be able to work on this immediately due to other work in progress, but I like the idea of it and would like to dig into it a little more and see what the implementation might look like some time after the next major release (which is already pretty large in scope at this point).

Thanks for this suggestion!

@Constantin07

This comment was marked as outdated.

@flickerfly

This comment was marked as outdated.

@tdmalone

This comment was marked as outdated.

@solinas-http

This comment was marked as off-topic.

@dbektas

This comment was marked as off-topic.

@eddideku
Copy link

I have specific modules that I'd like to prevent_destroy and ignore

Unfortunately there has been drift and TF is marking them to destroy (forces new resource)

@mkjmdski
Copy link

@apparentlymart is there any update on this? It is already after major release and it seems like terraform 0.12 really misses that feature to allow moving modules between various environments (qa -> prod) where in one infrastructure should terraform should be able to destroy module and on another no destroy action is welcomed ;).

@abdennour
Copy link

abdennour commented Jul 5, 2020

Terraform v0.12.28 but it is still not released

 on main.tf line 53, in module "dns":
  53:   lifecycle {

The block type name "lifecycle" is reserved for use by Terraform in a future version.

I also tried to pass variable for the module

# main.tf
module "dns" {
  source = "./modules/dns"
  prevent_destroy = true
}

# modules/dns/main.tf
resource "aws_route53_zone" "publiczone" {
  lifecycle {
    prevent_destroy = var.prevent_destroy
  }
 comment = "..."
}

Getting :

   3:     prevent_destroy = var.prevent_destroy

Unsuitable value: value must be known

No pb! please let me violate DRY.

@jpbuecken
Copy link

Our Use Case: We build X instances derived from a module. During daily tasks, they should be prevented from destroy.
Now one instance should be deleted. We want to disable the protection for this specific instance, then destroy in an uninstallation process. Solution of OP (rewrite hcl) or #18367 (comment) (via variable) will work

@politician
Copy link

politician commented Oct 16, 2020

@apparentlymart 0.13 is out and this feature is still not included in the release even though it has received a lot of love over the years. Now that modules support for_each, this feature is even more pressing to finally truly use modules just like any other resource.

Any plan to integrate it soon?

By the way for other users stuck with this, there is a (dirty) workaround which is not scalable at all but might work for small modules: You can put a condition at the resource level rather than at the block level. It's not scalable because you'd have to duplicate any resource you want to protect and use conditional expressions whenever you want to reference them. And if you eventually want to destroy these resources, you will need to remove the prevent_destroy from the module code.

For example, to take @abdennour's example

# main.tf
module "dns" {
  source = "./modules/dns"
  prevent_destroy = true
}

# modules/dns/main.tf
resource "aws_route53_zone" "publiczone" {
  count = var.prevent_destroy ? 0 : 1
  comment = "..."
}

# modules/dns/main_protected.tf
resource "aws_route53_zone" "publiczone_protected" {
  count = var.prevent_destroy ? 1 : 0
  lifecycle {
    prevent_destroy = true
  }
  comment = "..."
}

@Luminoth
Copy link

Luminoth commented Feb 6, 2021

Would really love to see this feature implemented to help guard against accidentally deleting an environment worth of resources.

@rd-michel
Copy link

current workflow:

  1. rollout terraform code and find out that i complains about an existing "prevent_destroy"
  2. raise a PR to set prevent_destroy=false for e.g. all existing gcp clusters just to change one cluster
  3. waiting 30 min for review/approval of the PR
  4. rollout the code without any problems
  5. rollback the change done in step 2
  6. wait 30 min for review/approval of the PR
  7. back to normal state where every cluster is protected again

we cannot use the workaround by @politician because we cant duplicate 30 lines of code just to workaround terraform internal issues. we`re using terraform 0.15

we want to use prevent_destroy in module calls for dns, clusters, project, databases in gcp

@petewilcock

This comment was marked as off-topic.

@nitmatgeo

This comment was marked as off-topic.

@c4m4

This comment was marked as off-topic.

@sayujnath

This comment was marked as off-topic.

@aelbarkani

This comment was marked as off-topic.

@NicolasKarnick

This comment was marked as off-topic.

@jrowinski3d
Copy link

This would be an amazing feature to have giving the user control on their resources instead of depending on the module author. Obviously this can be worked around via creating your own modules, but does create overhead. +1 for seeing this come to light

@slawekzachcial
Copy link

I put together an example that shows how to, for certain use cases, protect resources created by 3rd party modules.

In a nutshell, assuming your configuration uses a 3rd party module (e.g. module.security_group) that has output(s) that changes when the resource(s) gets re-created (e.g. group_id), add the following resource:

resource "null_resource" "module_guardian" {
  triggers = {
    security_group_id = module.security_group.group_id
  }

  lifecycle {
    prevent_destroy = true
  }
}

The triggers value creates implicit dependency. When calling terraform destroy Terraform will first attempt to destroy null_resource.module_guardian and this will fail due to prevent_destroy value. Also, in the example above, when the Terraform plan indicates that security group ID changes (happens when security group is re-created), Terraform will attempt to re-create null_resource.module_guardian and this will fail as well.

Finally, null_resource.module_guardian resource can be created conditionally, by adding count expression, to protect for example "production" module resource(s) but not "development":

resource "null_resource" "module_guardian" {
  count = var.env == "pro" ? 1 : 0

  triggers = {
    security_group_id = module.security_group.group_id
  }

  lifecycle {
    prevent_destroy = true
  }
}

@joshfria
Copy link

Is this in the plans at all? Without it reusing code seems nearly impossible. The above solution works, but adds complication when you do actually need to destroy that resource later.

@asifhj

This comment was marked as off-topic.

@james-green-affinity

This comment was marked as off-topic.

@dpolivaev
Copy link

Read the docs:

"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."

https://www.terraform.io/language/meta-arguments/lifecycle

It definitely makes no sense.

@RootHopper
Copy link

People use custom terraform modules to provision entire clusters of primary-source-of-truth databases.

To place a prevent_destroy at every resource in the custom_module makes it very hard to test changes downstream, necessitating multiple pushes across both the calling context in your main repo and the modular context in your custom module repo, if your setup is like mine.

I mean, you don't even get a plan if a single resource in a custom module protected by prevent_destroy would be destroyed by your HCL changes. Instead, you have to push a change to your custom module, then do the work you actually set out to do, then hopefully remember to change the module back when you are done, which is error-prone and joy-destroying. Not to mention all the peer review cycles, etc., interspersed throughout this cumbersome process.

Modular programming shouldn't have to work this way, and it makes me cringe at my job when I should really be having fun using an efficient tool that doesn't work against me.

In my opinion, this feature is critical for the continued adoption of Terraform.

@BenB196
Copy link

BenB196 commented Jul 8, 2023

I think that this feature would be valuable, but I think something that hasn't really been brought up is that prevent_destroy doesn't actually prevent destruction on removal (#17599), which when it comes to deployments and module consumption, I think is a far more likely risk.


To provide some additional feedback/insight here, below is an example of how I currently protect deployment modules/resources from accidentally being removed and therefore destroying things.

Note: This solution is not pure Terraform, as I'm not sure if there is really a way to do this with just Terraform.

Supposes we have a structure like so:

.
├── deployments
│      └── deployment-a.tf
└── modules
        └── module-a

deployment-a.tf

module "module_a" {
  source "../modules/module-a"
}

module-a/main.tf

resource "something" "something" {
  # contents
}

Currently, if we were to remove module "module-a" { from deployment-a.tf, it will delete resource "something" "something" {.

The current way I prevent this is to change the contents of deployment-a.tf to:

module "protected_module_a" {
  source "../modules/module-a"
}

(We've now prefixed the module definition with protected_).

This is the script I use to prevent "protected" definitions from being deleted:
Note: This script is run a simplification of a CI/CD pipeline used.

#!/bin/bash
set -e

# Run Terraform plan as normal, and generate a local plan file.
terraform plan -out=plan.cache

# Take the generated plan file and create another file, but this time in JSON
terraform show -json plan.cache > plan.cache.json

# This is where we check for deleted protected resources
# 1. Use `jq` to parse our generated plan JSON file
# 2. `jq` looks for any `resource_changes` with the `action_reason ` "delete_because_no_resource_config"
# 3. `jq` then looks for any of those findings that has the address which starts with `^(module.)?protected_`
#    - Note: `module.` is optional in the event you define actual resources in deployment-a.tf as well
# 4. Store a list of all matching addresses to a temp file named `changes`.
cat plan.cache.json | jq -r '.resource_changes[] | select( .action_reason == "delete_because_no_resource_config" ) | select( .address | match( "^(module.)?protected_" ) ) | .address' > changes

# Get the number of protected_ things being removed.
change_count=$(cat changes | wc -l)

# Check if the count is greater than 0
if [ $change_count -ne 0 ];
then
  # print some helpful info
  echo "Error: Protected Resources are Removed Ungracefully."
  echo "Resources affected:"
  cat changes
  # exit 1 (error) to prevent anything further
  exit 1
fi

The way you'd need to properly remove a protected_ definition, is now:

  1. Rename module "protected_module-a" {
    • This can be done safely by renaming then module and using the moved block.

deployment-a.tf

- module "protected_module_a" {
-   source "../modules/module-a"
- }
+ module "module_a" {
+   source "../modules/module-a"
+ }
+ moved {
+  from = module.protected_module_a
+  to   = module.module_a
+ }
  1. Run terraform apply to apply the rename/moved
  2. Remove the resources

deployment-a.tf

- module "module_a" {
-   source "../modules/module-a"
- }
- moved {
-  from = module.protected_module_a
-  to     = module.module_a
- }
  1. Run terraform apply to remove the resources without the safety check stopping it.

By gating the ability to remove "protected_" definitions behind 2 applies, and some additional state manipulation, it becomes an intentional act to remove something "protected_" then an accidental one.

@zefir01

This comment was marked as off-topic.

@mloskot

This comment was marked as off-topic.

@EddyMaestroDev

This comment was marked as off-topic.

@slnw

This comment was marked as off-topic.

@bschaatsbergen
Copy link
Member

bschaatsbergen commented Jan 28, 2024

I'd be glad to take a stab at this. I can't promise quick results. Also, I see this as a nice opportunity to get to know the module part of the codebase better. @apparentlymart, does your view on this feature remain the same today?

I've went ahead and opened a draft PR, I'll link it to this PR whenever it's starting to look like something 😉.

@Syto16

This comment was marked as off-topic.

2 similar comments
@LeoAnt02

This comment was marked as off-topic.

@thundering-herd

This comment was marked as duplicate.

@tspearconquest
Copy link

tspearconquest commented Dec 19, 2024

Would be nice to have an update on this 6 year old issue. We module authors can't even specify prevent_destroy as a local variable in the module, so that module consumers could have a way to specify the value. Module consumers need a way to override this setting without having to rely on CI scripts editing the file. It appears to me that the only way to make this dynamic at present is via CI scripts (and only by doing exactly what you're not supposed to do, which is editing the cached copy under the .terraform directory). Please can someone pick this up?

@jbardin
Copy link
Member

jbardin commented Dec 19, 2024

@tspearconquest, this was actually a duplicate of #27360, but changed slightly. You can see my comments here regarding the possibilities of a feature like this: #27360 (comment)

To repeat some of the fundamental problems, policy about what actions can take place is generally better handled outside of Terraform. prevent_destroy for example doesn't work if you remove the config block, and if the config block or lifecycle option can be removed, then there is no policy enforcement in place. We also have to contend with the discrepancies that a module on its own has no lifecycle, and resources within the module may need different options.

I'll leave this open for now to better account for the 👍's it's gathered, but if there is a solution it would probably need to be designed somewhat differently than adding a lifecycle block.

@hashicorp hashicorp locked as resolved and limited conversation to collaborators Dec 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests