hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.74k stars 9.56k forks source link

"Warning: Value for undeclared variable" #22004

Closed jmervine closed 3 years ago

jmervine commented 5 years ago

Current Terraform Version

Terraform v0.12.3

Use-cases

In our current project, we use a common.tfvars file to store shared configurations across multiple modules and plans. Up until recently, this has worked very well in allowing us to dry up configurations to a single location. With recent upgrades we've started seeing Warning: Value for undeclared variable, as not all "common" configurations are used in every plan.

Proposal

I'd like to request that a feature / flag is included to allow for ignoring the "Value for undeclared variable" warning condition, and additionally request that you not make it a breaking error in the future -- as the output indicates is the intention. Being able to include a collection of configurations and have a portion of the ignored is a valuable feature. I do understand that there is also a valid use-case for having it behave in a stricter way to many -- perhaps most. A flag or configuration allowing for opt out of strict checking seems to be an acceptable alternative.

apparentlymart commented 5 years ago

Hi @jmervine,

The warning is there for transition to give you an opportunity to migrate to the new recommended approach, which is to use TF_VAR_ environment variables for this sort of "global" or "ambient" setting. The environment variables are not and will never be subject to this existence check, because we presume that they are being set in the session with the intent of ignoring them when they are not relevant.

Using environment variables in this way allows a distinction between the ones that are set globally and the ones that are set locally, and thus allows us to have the check to report when the local ones are set incorrectly. That's important because otherwise users get no feedback when they accidentally make a typo in a variable name.

Environment variables are, therefore, the "opt-out" that is already available and what you should plan to transition to.

jmervine commented 5 years ago

@apparentlymart,

Thank you for the quick reply.

In my dealings, I've found that TF_VARS_ quickly breaks down on a distributed team or when using CI/CD pipelines to automate interactions. If required, it will be possible to work around this by checking in a .env file and leveraging something like godotenv. That said, the .tfvars file pattern is greatly preferred.

I understand that you have a large audience to consider here, just wanted to weigh in.

Cheers! Josh

kayrus commented 5 years ago

I also vote for the flag. These warning messages spoil our secrets, which are used in different modules.

svyotov commented 5 years ago

:+1:

ekini commented 5 years ago

If the only argument for keeping the warning in place is to give feedback to users who mistype, then there might be a solution that suits everyone.

Terraform has access to both declared variables and specified via named files/auto.tfvars and env vars, and therefore to detect a mistype, Damerau–Levenshtein distance can be used. I've done some testing and exploration, and it seems the distance <=2 is sufficient to catch most if not all errors.

This way:

  1. The users will get not only a warning that they mistyped, but also suggestions what variables they should use.
  2. This will restore the previous functionality and allow keeping common variables source checked. There is no sense in turning terraform to infrastructure-as-environment-variable!

An example:

Warning: Value for undeclared variable

  on /Users/ekini/project_dir/preview/service-app/../env.tfvars line 11:
  11: ecs_service_container_prot   = 8000

The root module does not declare a variable named
"ecs_service_container_prot". Did you mean one of
[ecs_service_container_port]?

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

I have a working prototype, but it seems pointless to make a PR if it's going to be rejected. @apparentlymart

jkinred commented 5 years ago

Another use case affected is the sourcing of externally generated JSON files and loading them unmodified using -var-file. I discovered this when re-factoring my code away from using jq to set TF_VAR_'s.

A flag to silence these warnings would be great.

deweysasser commented 5 years ago

Similar to jkinred, I organize my terraform into sets of related projects that build on each other to create a full stack (e.g. network, infrastructure, services, application). I use a shared secrets.tfvars file between these projects. Not all projects reference all of the variables, so this "feature" substantially impacts my working pattern.

Creating a mechanism to load these into environment variables would be a poor security practice as well as substantially more complicated.

Terraform needs some mechanism to provide "here is a body of knowledge that can be used during evaluation" -- variable files have been this for a long time. Adding this restriction effectively removes a very useful mechanism and will require either it's replacement with something else, or hacking around what becomes a tool limitation.

wrsenn commented 5 years ago

Another perspective on JSON front: we were hoping to use a single set of JSON files to serve as "set it once" configuration for both our Terraform variables and application configuration (as most of these values are used in both cases) but without being able to conditionally ignore these warnings, that's not really feasible.

wwentland commented 5 years ago

Much like @jmervine we utilise various optional var files to group shared settings for different modules, regions and environments.

Please allow users to suppress these warnings and, in particular, do not make them an error in future releases. Using TF_VAR_'s as suggested by @apparentlymart appears to not be an option and the approach outlined above seems to be the only way to achieve this very common use-case in a way that works for teams and CI/CD pipelines.

zswanson commented 5 years ago

The terrraform documentation itself states:

to set lots of variables, it is more convenient to specify their values in a variable definitions file (with a filename ending in either .tfvars or .tfvars.json) and then specify that file on the command line

as a fallback for the other ways of defining variables, Terraform searches the environment of its own process for environment variables named TFVAR

I don't see why an unused variable in a tfvars is outside the very use case you outline in your docs. Also kind of ridiculous when those variables could be complex maps and long lists.

0xDones commented 5 years ago

I have the same problem, but in my case I just want to override a variable in variables.tf file, but looks like it's not working according the documentation.

.
├── backend.tf
├── main.tf
├── modules
│   ├──api
│   │   ├── main.tf
│   │   ├── output.tf
│   │   ├── test.tfvars
│   │   └── variables.tf
├── provider.tf
└── terraform.out

variables.tf

variable "test_var" { }

test.tfvars

test_var = "my-test"

cli


# Terraform Version
terraform --version
# Output
Terraform v0.12.9
+ provider.aws v2.28.1

Terraform plan

terraform plan -var-file=modules/api/test.tfvars -out terraform.out

Output

Acquiring state lock. This may take a few moments...

Warning: Value for undeclared variable

on modules/api/test.tfvars line 1: 1: test_var = "my-test"

The root module does not declare a variable named "test_var". To use this value, add a "variable" block to the configuration.

Using a variables file to set an undeclared variable is deprecated and will become an error in a future release. If you wish to provide certain "global" settings to all configurations in your organization, use TFVAR... environment variables to set these instead.

lpakula commented 5 years ago

@dpolicastro

variable "test_var" { default="" } is solving the issue

You can then override default value

MCyprien commented 5 years ago

Hi @apparentlymart,

From what I understand the solution Hashicorp is pushing is to use scripts to source instead of tfvars file.

The need for that 'feature' is to spot typo errors.

So I need to change my tfvars file from :

MY_VAR1 = "toto"
MY_VAR2 = "tata"

to a script:

export TF_VAR_MY_VAR1 = "toto"
export TF_VAR_MY_VAR2 = "tata"

Here is the problem Now imagine, I am making a typo error here:

export TF_VAR_MY_VOR1 = "toto"
export TF_VAR_MY_VIR2 = "tata"

Terraform will not print warnings here because those are environment variables. So the whole point of this feature, to spot typo error, is not fulfilled and is causing constraints and frustrations for your users as you can read in this thread. I also think that a lot of people does not clearly see that will be a problem in futur versions. As you can see in #19424, this was already an issue.

I think good development practices is the key to avoid typo error (pair-prog, review, etc.). It looks to me you are trying to design something idiot-proof (no offense here) and I will just quote Douglas Adams:

"a common mistake that people make when trying to design something completely foolproof is to underestimate the ingenuity of complete fools"

Next time for this kind of breaking changes, can you propose a feature proposal to get feedbacks?

nicerobot commented 5 years ago

I wrote about my objections to this feature in https://github.com/hashicorp/terraform/issues/19424#issuecomment-505694536 but that issue was locked, effectively silencing further discussion so I'm going to raise my objections and supporting arguments again here.

I strongly disagree with the rationale for this warning/error! I've been using terraform for years and this goes against almost everything I've come to consider as best practices.

First, the recommendation to use TF_VAR_ goes against the philosophy of infrastructure as code! TF_VAR_ are easily overlooked when source controlling and not source controlled with .tf nor .tfvars files. They should always be discouraged or at least never recommended. And source-controlling scripts of TF_VAR_ variables is significantly less elegant than remaining in HCL.

Second, it makes no sense to be concerned about typos in the tool itself since i can simply use a map for my top-level variables and i'm right back in the same boat with typos. Linters are the right tools to try and detect typos. Something like terraform vet would be the right way to alert about this.

Third, re: https://github.com/hashicorp/terraform/issues/19424#issuecomment-440477222

your use-case of potentially sharing files between multiple configurations; that is not something we ever intended be possible

Expecting a single .tfvars to be associated to a single definition is utter nonsense. A practice I generally prefer and promote is to separate definitions based on frequency of use. For example, networks may rarely apply but application services may apply many times a day so it makes sense to separate these definitions. Extrapolating that to a full, complex infrastructure can result in dozens of folders of terraform definitions (making use of dozens of modules), most with very similar, but rarely identical, variables needs. To require a custom, unique .tfvars for each of these is simply ridiculous! .tfvars files should not align with the .tf definitions! .tf files align with the infrastructure design (like deployment frequency already mentioned) while .tfvars align more with application layout like allowing developers to define variables needed for their services. So .tf and .tfvars are and should be organized differently. To require that developers define only the values used in the infrastructure results in a disastrously tight coupling between the developers and the infrastructure and also means they'll have to maintain multiple .tfvars (which almost certainly means duplicating values) just so they can be used in various deployment scenarios.

Lastly, the diff output of terraform is absolutely critical. It's immensely more important than warning about a typo and I would even argue that it's the most important aspect of the tool. In lieu of a specific command to report problems, like validate, vet, ..., the diff is actually the right place to discover that there is a typo. To clutter that output with warnings about situations that the user may intentionally desire is amazingly irresponsible.

This has a very serious negative impact on productivity. I don't completely disagree with the intent of providing safer infrastructure definitions. But I strongly disagree with the proposed solution. And while I do also agree with your concern about restraining the complexity of the CLI flags, I believe this is a valid reason for an exception of making this a selectable feature. Something like -[no]-strict-mode, or even just TF_STRICT_MODE=true|false. To simply ignore all the valid use-cases outlined in the comments posted here and elsewhere and to believe your usage is the only right use-case is incredibly heavy-handed and makes me seriously question continuing to rely on this tool in the long term.

cc: @apparentlymart

udondan commented 5 years ago

These warnings confused our users when running our tooling (based on terragrunt). We're now using a wrapper script to get rid of these warnings. Leaving it here as others might find this helpful:

#!/usr/bin/env bash
ansi="(\x1b\\[[0-9;]+[a-zA-Z])*"
terraform "$@" | pcregrep -Mv "\n?${ansi}Warning: ${ansi}Values? for undeclared variables?(\n|.)*set these instead.\n?"

If you use terragrunt, you can just point to this script via TERRAGRUNT_TFPATH.

On MacOS you can install pcregrep via brew install pcre

kolly83 commented 5 years ago

Hello Guys

Can somebody tell how can be fixed this error\warning message and run the script?

Interestingly I have used the totally same terraform.tfvars file and procider.tf file in my previous learning lessons and it did work.

Though I am struggling with this warn message in my current learning lesson.

PS: I have tried the below but now I've got an Error message: Invalid block definition.

The equals sign "=" indicates an argument definition, and must not be used when defining a block.

export TF_VAR_MY_VAR1 = "toto" export TF_VAR_MY_VAR2 = "tata"

It is really annoying.

Terraform version: 0.12.9

Thank you.

udondan commented 5 years ago

fixed this error\warning message and run the script?

It only is a warning not an error (yet). So your code is still working. It might not in some future version of terraform.

export TF_VAR_MY_VAR1 = "toto"
export TF_VAR_MY_VAR2 = "tata"

This would not go into your terraform files. You need to export these vars before calling terraform, like so:

export TF_VAR_MY_VAR1="toto"
export TF_VAR_MY_VAR2="tata"
terraform plan ....

or putting it right in front of the tf call:

TF_VAR_MY_VAR1="toto" TF_VAR_MY_VAR2="tata" terraform plan ....
kolly83 commented 5 years ago

Hello udondan

Thanks for your message, though after I amended the terraform.tfvars as you suggested I've still got an error: Error: Invalid block definition

The equals sign "=" indicates an argument definition, and must not be used when defining a block.

Thanks.

udondan commented 5 years ago

The whole thing should not be in your tfvars. You have to export it outside of tf.

kolly83 commented 5 years ago

What do you mean?

Like that?

terraform plan -out out.terraform # terraform plan and write the plan to out file

In the lesson it is not said that it has to be exported.

Thank you.

kolly83 commented 5 years ago

I could sorted out it.

Thank you for your help anyway.

DTTerastar commented 5 years ago

Please just add an option to ignore these! Pulumi is going to get a lot of new users unless you listen...

mingzhaodotname commented 5 years ago

Instead of forcing it, a better option might be to provide a command flag (like many other terraform flags), e.g. --allow-undeclared-variable or --allow-set-undeclared-variable

In this way, users have the judgement call to enforce or not. I think this will have a better user experience. As a tool, terraform should make it easy for users to do their job, instead of blocking them. Terraform can have some best practices, and can make them as the default behavior too. However, it should give users option to do their work, as there are so many other use cases.

lqueryvg commented 5 years ago

Sadly we are at 0.12.13 and still no resolution for this.

But it doesn't make any sense...

The use case for being able to specify input variables which don't have to be declared and used by all configurations is fairly real and obvious. Yet Terraform seems to be trying to prevent users from doing this.

The Terraform .11 behaviour was:

All bases are covered, aren't they ? No problem, correct ? So, what problem are Hashicorp trying to solve with this warning in Terraform .12 ?

Also, if there is a real problem which I've missed, then how does forcing users to use TFVAR variables help ? We'll surely still hit the same problem (whatever that is!) after suffering the inconvenience of having to export environment variables (in countless wrapper scripts) because we are no longer allowed to use tfvars files for this use case.

What's wrong with giving the users the flexibility and power to structure their project in their own way ?

I'm baffled !

voltzop commented 4 years ago

Another use case I have not seen mentioned in these threads:

We are doing multithreaded Terraform deployments on our workers as part of our CICD pipeline - requiring system wide environment variables will simply not work if we don't otherwise work around this unnecessary, breaking change. We supply tfvars to components based on arguments, environment files, and platform specifications and may be running one or more deployments to one or more accounts at any given time.

I sincerely hope HashiCorp listens on this one.

SerhiiSokolov commented 4 years ago

I have big variables (objects for environments). One map may include 20 environments and each environment should include about 50 properties. Move those variables to ENV is terrifying me.

kayrus commented 4 years ago

Just in case, there is another trend, like removing of using env vars: https://github.com/kubernetes/kubernetes/issues/81117

lqueryvg commented 4 years ago

I've noticed this Warning: Value for undeclared variable only appears when with apply, not plan.

Surely, for consistency, it should happen on both.

Has anyone else noticed that ?

I'm using 0.12.12

lqueryvg commented 4 years ago

@SerhiiSokolov, I wonder if a custom data source my help in your situation ? @kayrys, are you saying that while Terraform is encouraging us to use environment variables, other projects are encouraging us not to ?

max-rocket-internet commented 4 years ago

migrate to the new recommended approach, which is to use TFVAR environment variables

That sounds really terrible 👎👎👎 And AFAIK, we can't even suppress the annoying warnings by setting something for TF_LOG.

Anyway, we could work around it by moving many variables inside maps. For us, many variables could be grouped anyway, so it wasn't too painful. Here's an example...

regions/region01.tfvars:

region_config = {
  region_name       = "asia"
  aws_region        = "ap-southeast-1"
  kms_master_key_id = "arn:aws:kms:ap-southeast-1:xxxxx:key/xxxxx"
}

environments/production.tfvars:

env_config = {
  env_name       = "production"
  short_env_name = "prd"
}

Now every terraform code base is passed both of those tfvar files and must take both of these variables to stop the warnings:

variable "region_config" {  default = {} }

variable "env_config" { default = {} }

But not every terraform code base must use all keys in these 2 maps. Not the best solution but worked OK for us.

wwentland commented 4 years ago

I can only re-iterate my hope that Terraform's behaviour in this regard will be user configurable and that the warning will not be treated as an error in future versions.

There are many legitimate use cases build around being able to move configuration shared by environments, regions or instances to a single place and assign them to all subsumed entities. Modules can then use whatever data they need and only throw an error when something they need is missing, rather than throwing a warning/error when the data at their perusal contains more elements than they need.

Please, please, please reconsider the choice of not making this configurable.

alessio-torelli commented 4 years ago

I vote to remove the warning (and not turning it into an error, of course) too

jim-leary commented 4 years ago

This warning is a real pain and causes lots of problems for us in our automated environments. There should be a flag to disable it.

studees commented 4 years ago

yet one more comment to request that this behaviour is configurable. For our use case, the set of TFVARs is a result of stitching together multiple dimensions (aws account, environment type, vpc details etc), with quite a large number of root modules that utilise a subset of each dimension vars. Making the variable declaration mandatory will lead to a huge amount of boilerplate in our case

mstrejczek commented 4 years ago

+1 for removing the warning and the whole deprecation thing. The existing approach is fine and allows building nice variable hierarchies in easy and convenient way. Environment variables looks more fragile and prone to undesired interactions in CI/CD scenarios.

robzr commented 4 years ago

If I cannot put lists or maps in environment variables, then TFVARS env vars do not provide sufficient functionality to replace input variable files.

Definitely would like a flag to silence this, as it causes a lot of unnecessary (and unhelpful) output in our deployment tooling. We use a Jenkins shared library step that bootstraps Terraform - sets up remote state/workspace, auth's via STS to AWS, and provides a dynamically built set of available input variables for the developers to use. Git tags are used to communicate the environment and deployment name to Jenkins, so this is the flow to provide that data into Terraform. It lets us avoid maintaining duplicated static tfvars files, which is helpful in a large environment with dozens of AWS accounts, multiple environments per account, and centralized metadata storage.

robzr commented 4 years ago

I'm just going to say that I've lost respect for Hashicorp on this one.

First for making this change in the first place. It's bad functionality to implement without an override. Was it really a problem in earlier versions???

But mostly for the way they have ignored the feedback, and provided absurd reasoning for it. Really? This is how you support people using your product at scale? A poorly informed, dismissive take in June? WTF. Do you guys even use your own product at scale?

jim-leary commented 4 years ago

No one uses environment variables as “global variables” for any at scale, production-level Terraform code that I’m aware of. Environment variables are typically overrides. Hashicorp should pay more attention to their user base needs and own up to their mistake here. I vote for a flag to suppress the warning.

kayrus commented 4 years ago

@robzr https://github.com/hashicorp/terraform/issues?q=is%3Aissue+is%3Aopen+sort%3Acomments-desc probably we need more comments. sarcasm.

feraudet commented 4 years ago

@kayrus If it work, here another comment to have it on top 😬

robzr commented 4 years ago

It's bizarre because with all the work that has gone into both this issue and Hashicorp's response, they could have just added this, it's a 5 minute fix.

dansali commented 4 years ago

I don't understand the logic behind this method.

If I typo the variable name, it will then throw an error on anytime I reference the variable. Please revert or let us mute this warning.

jdeluyck commented 4 years ago

We have a landingzone deployment system that uses a lot of entries in AWS SSM for configuration purposes, and our make files then export these to tfvars files. This works fine and scales incredibly. Having to define 100+ environment variables is NOT FEASIBLE.

hunt3ri commented 4 years ago

It does seem unreasonable not to include a flag to disable this warning. I'm guessing currently Terraform makes no distinction between variables loaded from a variables.tf file and those loaded from a .tfvars file which presumably makes adding a flag non-trivial??

I wrote a few lines of python to convert my existing .tfvars file into a .tfvars.sh file which I can then stat in my session to set the global TF_VARS.

It may help people affected by this, contributions always welcome 😄

https://github.com/hunt3ri/gen-terraform-env-vars

etwillbefine commented 4 years ago

In my case I need to rely on the precedence how variables can be overwritten. Using TF_VAR_ still allows for overwrite via .tfvars file. In my case I want to pass cli flags to ensure that value will be used. I even get an error in terraform 0.12.20 saying Error: Value for undeclared variable and the plan fails. Reporting that variables are defined and not used is something I personally dont need.

tumblago commented 4 years ago

I wanted to chime in favor of restoring back the old behavior (no warning, no error, potentially through a cli flag). We are using a file shared between different programs only one of which is terraform. The warnings (future errors, god forbid) are really unnecessary and hard to avoid in any elegant and satisfactory way. Forcing terraform users to export env vars from files (by prefixing them with TFVAR on top) is not anyone's idea of user friendliness and helpfulness. I really hope the terraform team hears the feedback and reconsiders this decision. Thanks.

ghost commented 4 years ago

@apparentlymart We use terragrunt and parent file has lots of org wide used variables . My screen is full of all these warnings only. Please provide support to suppress warnings pertaining to Warning: Value for undeclared variable

Thanks

jmervine commented 4 years ago

Seconding what @smeena667 said, we're in the process of moving to Terragrunt to resolve this issue, as well as many others.

conzy commented 4 years ago

This is a really disappointing change. I typically use terragrunt to paper over some of the cracks in functionality missing from terraform. Thats fine. On a current project I didn't want to introduce the additional complexity of another tool like terragrunt and wanted to give "vanilla" terraform a go.

After some battling and creative use of symlinks and terraform.tfvars I finally reach a reasonably elegant solution and I was hit with this deprecation warning.

I'm not trying to do anything groundbreaking. Multiple AWS accounts with a handful of states each with some shared configuration to keep the code DRY. Its looking like adding terragrunt to this project is actually far less complex than trying to achieve this with vanilla terraform

tphan18 commented 4 years ago

In my case, I integrated the SAM CLI tool to build my lambdas:

  1. SAM uses template.json as its configuration to build and upload lambdas to S3.
  2. Terraform uses template.json as variable file so I can dynamically build my lambda resources.

I don't want to declare unused variables like AWSTemplateFormatVersion and Transform.