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

"Variables may not be used here" for prevent_destroy #22544

Open
tomasaschan opened this issue Aug 21, 2019 · 74 comments
Open

"Variables may not be used here" for prevent_destroy #22544

tomasaschan opened this issue Aug 21, 2019 · 74 comments
Labels
config enhancement lifecycle v0.12 Issues (primarily bugs) reported against v0.12 releases

Comments

@tomasaschan
Copy link

Terraform Version

Terraform v0.12.6

Terraform Configuration Files

locals {
  test = true
}

resource "null_resource" "res" {
  lifecycle {
    prevent_destroy = locals.test
  }
}

terraform {
  required_version = "~> 0.12.6"
}

Steps to Reproduce

  1. terraform init

Description

The documentation notes that

[...] only literal values can be used because the processing happens too early for arbitrary expression evaluation.

so while I'm bummed that this doesn't work, I understand that I shouldn't expect it to.

However, we discovered this behavior because running terraform init failed where it had once worked. And indeed, if you comment out the variable reference in the snippet above, and replace it with prevent_destroy = false, it works - and if you then change it back it keeps working.

Is that intended behavior? And will it, if I do this workaround, keep working?

Debug Output

λ terraform init
2019/08/21 15:48:54 [INFO] Terraform version: 0.12.6
2019/08/21 15:48:54 [INFO] Go runtime version: go1.12.4
2019/08/21 15:48:54 [INFO] CLI args: []string{"C:\\Users\\Tomas Aschan\\scoop\\apps\\terraform\\current\\terraform.exe", "init"}
2019/08/21 15:48:54 [DEBUG] Attempting to open CLI config file: C:\Users\Tomas Aschan\AppData\Roaming\terraform.rc
2019/08/21 15:48:54 [DEBUG] File doesn't exist, but doesn't need to. Ignoring.
2019/08/21 15:48:54 [INFO] CLI command args: []string{"init"}
There are some problems with the configuration, described below.

The Terraform configuration must be valid before initialization so that
Terraform can determine which modules and providers need to be installed.

Error: Variables not allowed

  on main.tf line 7, in resource "null_resource" "res":
   7:     prevent_destroy = locals.test

Variables may not be used here.


Error: Unsuitable value type

  on main.tf line 7, in resource "null_resource" "res":
   7:     prevent_destroy = locals.test

Unsuitable value: value must be known
@teamterraform
Copy link
Contributor

Hi @tomasaschan,

prevent_destroy cannot support references like that, so if you are not seeing an error then the bug is that the error isn't being shown; the reference will still not be evaluated.

@hashibot hashibot added the v0.12 Issues (primarily bugs) reported against v0.12 releases label Aug 27, 2019
@baurmatt
Copy link
Contributor

baurmatt commented Oct 1, 2019

Just ran into this but with a "normal" variable. It would be create if we can use variables in the lifecycle block because without using variables I'm literally unable to use prevent_destroy in combination with a "Destroy-Time Provisioner" in a module.

@mleonhard
Copy link

I'm hitting this, too. Please allow variables derived from static values to be used in lifecycle blocks. This would let me effectively use modules to run dev & test environments with the same config as prod, while providing deletion protection for prod resources. AWS RDS has a deletion_protection option that is easy to set. S3 Buckets have an mfa_delete option which is difficult to enable. I found no way to prevent accidental deletion of an Elastic Beanstalk Application Environment.

module "backend" {
  source = "../backend"
  flavor = "dev"
  ...
}
resource "aws_elastic_beanstalk_environment" "api_service" {
  lifecycle {
    prevent_destroy = (var.flavor == "prod")  // <-- error
  }
  ...
}

@weldrake13
Copy link

Seen multiple threads like this. There is an ongoing issue (#3116) which is currently open but @teamterraform seem to have made that private to contributors only.
The need to set lifecycle properties as variables is required in a lot of production environments.
We are trying to give our development teams control of their infrastructure whilst maintaining standards using modules. Deployment is 100% automated for us, and if the dev teams need to make a change to a resource, or remove it then that change would have gone through appropriate testing and peer review before being checked into master and deployed.

Our modules need to be capable of having lifecycle as variables.
Can we get an answer as to why this is not supported?

@sdemura
Copy link

sdemura commented Oct 11, 2019

My use case is very much like @weldrake13's. It would be nice to understand why this can't work.

@sudhanwadindorkar
Copy link

I would also appreciate if Terraform allows variables for specifying "prevent_destroy" values. As a workaround, since we use the S3 backend for managing our Terraform workspaces, I block the access to the Terraform workspace S3 bucket for the Terraform IAM user in my shell script after Terraform has finished creating the prod resources. This effectively locks down the infrastructure in the workspace and requires a IAM policy change to re-enable it.

@throwaway8787
Copy link

I write tests for my modules. I need to be able to re-run tests over and over. There's no way for me to delete buckets in a test account and set protection in a production account. Swing and a miss on this one.

@Clete2
Copy link

Clete2 commented Jan 6, 2020

Is there a general issue open with Terraform to improve conditional support? Off the top of my head I can think of the following limitations:

  • Variable defaults / declarations cannot use conditionals
  • Lifecycle rules cannot use conditionals
  • provider = argument cannot use conditionals
  • Modules cannot have count set

All of these make writing enterprise-level Terraform code difficult and more dangerous.

@rios0rios0
Copy link

The same of: #3116
Can you close, please?

@weldrake13
Copy link

The same of: #3116
Can you close, please?

Hashicorp locked down 3116. If this gets closed then those following cant view the issue.

@hawksight
Copy link

It's over 4 years since #3116 was opened, I think we'd all appreciate some indication of where this is? Is it still waiting on the proposal mentioned in this comment, #4149 ?

Thought I'd offer up a work around I've used in some small cases. Example here is a module for gcloud sql instance, where obviously in production I want to protect it, but more ephemeral environments I want to be able to pull the environment down without editing the code temporarily.

It's not pretty but it works, and is hidden away in the module for the most part:

### variables.tf
variable "conf" {
  type = map(object({
    database_version  = string
    ...
    prevent_destroy   = string
  }))
  description = "Map of configuration per environment"
  default = {
    dev = {
      database_version  = "POSTGRES_9_6"
      ...
      prevent_destroy   = "false"
    }
  # add more env configs here
  }
}

variable "env" {
  type        = string
  description = "Custom environment used to select conf settings"
  default     = "dev"
}

### main.tf
resource "google_sql_database_instance" "protected" {
  count   = var.conf[var.env]["prevent_destroy"] == "true" ? 1 : 0
  ...
  lifecycle {
    prevent_destroy = "true"
  }
}

resource "google_sql_database_instance" "unprotected" {
  count = var.conf[var.env]["prevent_destroy"] == "false" ? 1 : 0
  ...
  lifecycle {
    prevent_destroy = "false"
  }
}

### outputs.tf
output "connection_string" {
  value = coalescelist(
    google_sql_database_instance.protected.*.connection_name,
    google_sql_database_instance.unprotected.*.connection_name,
  )
  description = "Connection string for accessing database"
}

Module originated prior to 0.12, so those conditionals could well be shortened using bool now. Also I appreciate this is one resource duplicated, and it would be much worse elsewhere for larger configurations.

@realsby
Copy link

realsby commented Mar 16, 2020

It is so funny. I am asking this question WHY? WHY?

@Penumbra69
Copy link

Penumbra69 commented Mar 26, 2020

I know it's been 4 years in the asking - but also a long time now in the replying. Commenting on #3119 was locked almost 2 years ago saying "We'll open it again when we are working on this".

Can someone with the inner knowledge of this "feature" work please step up and give us some definitive answers on simple things like:

  • If this will be done?
  • Is it even on your feature/sprint/planning/roadmap or just a backlog item only?
  • When may be expected if it IS on the roadmap?

Thanks for your work - Hashicorp - this tool is awesome! Not slanting at you, just frustrated that this feature is languishing and I NEED it ... Now....

@danieldreier
Copy link
Contributor

@Penumbra69 and all the folks on here: I hear you, and the use cases you're describing totally make sense to me. I'm recategorizing this as an enhancement request because although it doesn't work the way you want it to, this is a known limitation rather than an accidental bug.

@danieldreier danieldreier removed the bug label Jun 12, 2020
@alex-3sr
Copy link

Hi team

Maybe a duplicate of #3116 ?

@bmurtagh
Copy link

bmurtagh commented Feb 9, 2021

@danieldreier given that Hashicorp has acknowledged this issue as a "known limitation" based on your June 12, 2020 comment, is the company able to provide a standard or recommended workaround to address this?

@danpilch
Copy link

danpilch commented Feb 9, 2021

I think the recommended workaround is find-and-replace value before running terraform :(

@richtong
Copy link

I think the recommended workaround is find-and-replace value before running terraform :(

Wow this is a real problem so either we duplicate all resources with prevent_destroy, you we use m4 or something to do a search for this (like you have to do with Dockerfiles. pretty ugly :-)

@seed-doordash
Copy link

+1

@troupier88
Copy link

+1 We use terraform modules, the main dev set the default value at "true", that's not my use case :(

@crw
Copy link
Collaborator

crw commented Nov 3, 2022

Just a reminder to please use the 👍 reaction on the original post to upvote issues - we do sort by most upvoted to understand which issues are the most important. This also reduces "noise" in the notification feed for folks following this issue. Thanks!

@rbjoergensen

Has Hashicorp given any reasoning as to why they're not fixing this?

I believe the blocker is that to support this feature one would need to implement pre-processing of the configuration. This would be a major design change to the underlying fundamentals of Terraform. Not impossible, but not something that is likely to happen without a major product design effort. Again, please do not quote me on that technical explanation; this is how I understand the underlying issue but I may be a little off-base.

You might also check out these adjacent issues:
#3116
#4149
#30937

@richardgragasin
Copy link

its 2023 guys, any updates?

@bgshacklett
Copy link

I believe the blocker is that to support this feature one would need to implement pre-processing of the configuration.

Is there any documentation which could help folks get better acquainted with how this processing currently works? With a better understanding of the current difficulties/blockers, it would be easier to discuss potential solutions.

@karthik-twilio
Copy link

+1

1 similar comment
@KamranBiglari
Copy link

+1

@crw
Copy link
Collaborator

crw commented Jan 25, 2023

Thanks for your interest in this issue! This is just a reminder to please avoid "+1" comments, and to use the upvote mechanism (click or add the 👍 emoji to the original post) to indicate your support for this issue. Thanks again for the feedback!

@algo7
Copy link

algo7 commented Feb 15, 2023

Any update on this one?

@sammalloyxydus
Copy link

+1 seems like a fairly common sense feature..

@jakuboskera
Copy link

+1

@denisp13
Copy link

denisp13 commented Apr 8, 2023

I was surprised to find such a long and old tread for such a simple issue.... +1

@zeppelinen
Copy link

You are not alone

I was surprised to find such a long and old tread for such a simple issue.... +1

@probststefan
Copy link

+1

@crw
Copy link
Collaborator

crw commented Apr 12, 2023

Thanks for your interest in this issue! This is just a reminder to please avoid "+1" comments, and to use the upvote mechanism (click or add the 👍 emoji to the original post) to indicate your support for this issue. Thanks again for the feedback!

@joe-a-t
Copy link

joe-a-t commented Apr 13, 2023

I want to call out that this is the root cause of a ton of other issues and work arounds that providers are either being asked to do or doing like:

I do understand what @crw is saying in #22544 (comment), but if the Google provider is able to implement this on their own, I don't see why Terraform core cannot as well. Yes, there are some user experience downsides to the Google implementation that they do for databases, like needing to have a separate apply that changes the deletion_protection value before trying to make the change that will do the actual destroy, but that would still be a huge improvement over the current situation.

@scheleaap
Copy link

Mentioning #30957 here as it is related.

@thatsk
Copy link

thatsk commented Apr 18, 2023

any update on this .

@KevinPecquet
Copy link

Cannot use prevent_destroy in modules without a variable so for me it cannot be used without this feature.

@nsaccente
Copy link

Running Terraform v1.4.6, still no support for variables in prevent_destroy 🙃

@aaronswainonset
Copy link

Still can't use prevent_destroy effectively without support for variables. What's happening with it?

@ecoupal-believe

This comment was marked as duplicate.

@michael-weiler
Copy link

any updates on this issue ?

@crw
Copy link
Collaborator

crw commented Jul 18, 2023

Thanks for the continued interest in this issue. As the use cases seem fairly clear and the replies primarily being of the +1 variety, I am going to lock comments to maintainers. For posterity, the last two substantive comments on this issue are: #22544 (comment) and #22544 (comment). Thanks again for your continued interest in this issue!

@hashicorp hashicorp locked as spam and limited conversation to collaborators Jul 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
config enhancement lifecycle v0.12 Issues (primarily bugs) reported against v0.12 releases
Projects
None yet
Development

No branches or pull requests