Terraform 0.12.0-alpha no longer allows assignments in terraform.tfvars without a corresponding variable declaration

[lubars]

Terraform Version

Terraform v0.12.0-alpha2
+ provider.google v1.19.1-4-gf3de5334
+ provider.null v1.0.0-5-gf54ff98

Expected Behavior

Terraform v0.11.10 and earlier allowed assignments to occur in terraform.tfvars without a corresponding variable declaration in infrastructure.tf.

Actual Behavior

This now results in errors such as the following:

Error: Value for undeclared variable

  on terraform.tfvars line 4:
   4: AMI = "ami-18726478"

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

Steps to Reproduce

1. Add an entry to terraform.tfvars without a corresponding variable declaration in infrastructure.tf
2. terraform plan

Additional Context

We generate a common terraform.tfvars file shared by multiple provider-specific infrastucture.tf files. Only a subset of the assignments in terraform.tfvars are referenced in a given infrastructure.tf file. Up until v0.11.10, assignments in terraform.tfvars without a corresponding variable declaration in a given infrastructure.tf were ignored. The previous behavior should be supported, or perhaps a flag (e.g. "-ignore-undeclared") added. I am submitting this as a bug rather than a feature because we considered the behavior up to v.0.11.10 as the feature because it allowed use of a common file and, by allowing new assignments without a corresponding variable declaration, provided a certain amount of forward- and backward-compatiblity.

Update

I am now deleting from terraform.tfvars every line that does not match the following regular expression in infrastructure.tf:

^[\s]*variable[\s]+\"$var\".*$"

So I am unblocked for the moment.

[apparentlymart]

Hi @lubars! Thanks for reporting this and sharing your use-case.

We intentionally made this change to improve usability in situations where a user makes a typo in a .tfvars file, where the previous silent failure was unhelpful and time-wasting. However, I do see your use-case of potentially sharing files between multiple configurations; that is not something we ever intended be possible, but we'll think about this and decide what to do before the final v0.12.0 release.

It's unlikely that the outcome here will be a new command-line option since each new option increases complexity and hurts usability. Therefore the decision to be made here is whether the unintended convenience of sharing a single "superset" file between configurations outweighs the poor usability of having a silent failure.

[lubars]

Thanks for the response @apparentlymart. I agree with what you said about command-line options; Terraform has done a good job of keeping them in check, such that the --help for most commands fits on one screen.

I did want to add that it's not just a superset file (otherwise we could break it up into provider-specific and shared portions), but that we're generating it from a database from which we generate config files intended for consumption by a variety of applications; what these applications all have in common is that they ignore unrecognized fields, allowing the master to be upgraded without breaking existing applications (as long as fields have been added but not deleted).

[apparentlymart]

Hi @lubars,

One other thing I noticed looking at your opening comment again was an inconsistency in the error message you shared:

Error: Value for undeclared variable

  on terraform.tfvars line 4:
   4: AMI = "ami-18726478"

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

The snippet is showing the AMI value but the paragraph below is talking about Label. Is this exactly the error message that Terraform produced, or was this inconsistency the result of some simplification of the message to hide sensitive information/etc?

If it was inconsistent then that suggests a bug in the error message handling here which I'll open as a separate issue so we can investigate further, but I just wanted to check first to make sure that wasn't "doctored" output!

[lubars]

Hi @apparentlymart - that was indeed a cut-and-paste error - the "Label" contained something semi-proprietary, so I (clumsily) pasted over it with an earlier error. Sorry to confuse the issue! I will fix it now.

[lubars]

Closing because the regex filter I put on terraform.tfvars generation is working well.

[Legogris]

We have been using a common global.tfvars file between several workloads where each of them uses only a subset of variables.
Rather than require all variables in the tfvars file to be declared, how about requiring all variables that are actually used to be declared in the root module?
I can imagine there must be many others who will be unable to move to 0.12 because they have built their workflow around being able to share global variables across modules who use them selectively.

[z0mbix]

The last 3 places I’ve worked all had similar usage where this would cause problems, so surely this change will affect many users making the terraform upgrade not backwards compatible.

[ricoli]

Same, here - this is surely a very common pattern - every single project I've worked on has had a similar pattern as described by @Legogris : a global.tfvars, and also one tfvars file per environment, with all the values across all the states that are specific to that environment. It would be a nightmare to have to change this pattern when upgrading to 0.12.

[ricoli]

@lubars any chance you could reopen this for visibility? Having to devise workarounds for this is very undesirable in my opinion

[apparentlymart]

Hi all,

As a compromise, we will make this situation be a warning for 0.12, and then plan on making it an error in 0.13.

As I noted above, we made this change so that users can get feedback when they make a typo in a variable name either in the main configuration or in the tfvars file, and giving good feedback to those using Terraform directly usually takes priority over making automation easier to implement, because automation by definition makes repetitive tasks easier.

However, I can see that right now there is a hole here where it's not clear what exactly such an automation should do, because there's no way to ask Terraform which variables are needed for a particular configuration. For now the easiest answer would be to declare the same variables in every configuration if you intend to pass them all there, but I know that's not ideal for a number of reasons.

In future we intend to have a built-in command to "inspect" a configuration to get a machine-readable description of its top-level objects, which could potentially help here. For now, we have a separate codebase terraform-config-inspect which could be used for this in principle.

The scope of this issue will just be to make the error into a warning. We'll tackle further improvements in this area via other issues once we've got 0.12.0 released.

[ricoli]

thank you very much @apparentlymart - that is a reasonable compromise.

[apparentlymart]

I realize I forgot to mention the other possible approach here:

For pragmatic reasons the TF_VAR_xxx environment variable scheme is not subject to this checking of declaredness, on the assumption that environment variables tend to be session-wide and so it'd be wrong to have Terraform refuse to run just because you have some irrelevant variables set ambiently in the environment.

As a consequence, it's possible to use environment variables to set this sort of "global" variable that is provided by the automation wrapper, and reserve the CLI arguments for any run-specific overrides. In a previous role (before joining the Terraform team) this is how my team arranged for the automation wrapper to pass in certain "global" values like the environment name, application name, etc. This approach is particularly handy if your automation tool already has first-class support for setting environment variables for runs.

[Legogris]

Would you consider adding an opt-in flag to terraform to allow unused global variables?

[apparentlymart]

I don't think an opt-in is required here now that it's a warning. Terraform will just print a message telling you it's deprecated (which it is) and move on as before. You can transition to using environment variables instead at your leisure any time before the future major release that will turn this back into an error again (which is not yet decided).

[Zordrak]

@apparentlymart Where can I track the decision making process for this for 0.13?

tfscaffold explicitly depends on the capability to inherit and share configuration files as required between the scopes of "global" "region" "group" and "environment". If this is implemented as an absolute error, it will completely break many projects, requiring every component to be forced to code unnecessary variable blocks to account for any variable it does not use.

I completely understand the desire to support users who have made a mistake, I do not understand the need to enforce further backward-incompatible change on users who intentionally work in this manner to provide an enterprise-grade project management structure around terraform.

Frankly I do not understand the need to introduce backward-incompatible changes at every major release. I would really appreciate you guys considering this more in the future. Additional features do not have to be breaking. For example, the introduction of the -auto-approve flag as a requirement to continue to act in the pre-existing manner, instead of simply adding -require-approval as an optional feature.

[diroussel]

The more backwards compatibility changes there are, the more likely we are to look for alternatives to terraform. Wouldn't just a warning do in this case?

[pcj]

I can understand this feature from a correctness perspective but like other people on this thread think users should be able to opt-out via a flag or environment variable.

…

On Fri, Mar 8, 2019 at 7:58 AM David Roussel ***@***.***> wrote: The more backwards compatibility changes there are, the more likely we are to look for alternatives to terraform. Wouldn't just a warning do in this case? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#19424 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADFlPvlQ7cy98FDeH_2qjc_VrjBb0u_ks5vUnqpgaJpZM4YsC6s> .

[apparentlymart]

Thanks for your feedback here, everyone.

What is implemented right now is already a warning for the initial 0.12 release. The migration path for existing software relying on this is to transition to using the TF_VAR_ environment variables instead of a generated .tfvars file, which is backward compatible for several past major versions and so will not require different behavior to support both 0.11 and 0.12 at the same time.

We make these carefully-considered breaking changes because real-world feedback tells us that the previous behavior was not good enough. Not making these changes would prevent fixing real-world pitfalls that cause frustration and real damage. We always try to ensure there is a path forward for existing valid use-cases, which we did both in changing the default behavior of terraform apply (by adding -auto-approve first as an opt-out to transition and then as an opt-in, so wrappers could force to behavior they need across both versions) and in introducing this validation feedback (warning first with backward-compatible alternative, then breaking change in a future release.) We didn't get the migration path for this change quite right in the initial implementation, so we responded to that feedback with the compromise of using a deprecation warning for 0.12 and delaying the breaking change for 0.13 so there is time to respond to it.

Helpful feedback about mistakes is never helpful if you have to know to opt in to see it, because the person who needs the feedback doesn't know to ask for it yet. It is therefore important that Terraform's default behavior is safe and helpful, while providing opt-in alternative paths where needed.

Because the TF_VAR_... environment variables already work in 0.11, any wrapper script or other automation affected by this change can migrate as soon as convenient and have behavior that will work for both 0.11 and 0.12. This avoids the need to introduce a separate, specialized opt-out flag which would not be 0.11-compatible and thus require special cases in your automation.

If you find that other products suit your needs better than Terraform for whatever reason then if course we invite you to use them. Terraform is one product in a busy space, and we make these decisions to make it as user-friendly as possible but a single product will never meet every use-cases and we would rather see you use the right tool for your job (whether it be an older version of Terraform that is already meeting your needs or another similar product in the space) than to be frustrated by forcing Terraform into uses it is less suited for.

[edmundcraske]

@apparentlymart how can we pass more complex variables such as maps and lists via environment variables? I’m sure they work fine for some use cases, but I cannot see them working in my case.

[apparentlymart]

@edmundcraske it should be possible to set any variable that way. If you are seeing strange/unexpected behavior, please open an issue about it and we will investigate what is going on.

[Legogris]

@apparentlymart I for one appreciate the detailed response.

Helpful feedback about mistakes is never helpful if you have to know to opt in to see it, because the person who needs the feedback doesn't know to ask for it yet. It is therefore important that Terraform's default behavior is safe and helpful, while providing opt-in alternative paths where needed.

Definitely. The suggestion some of us made here is therefore to introduce an opt-in flag for the pre-v0.12 behavior. Something like --allow-undeclared-vars. This would still make v0.13 breaking, but the fix for someone relying on having shared global varfiles between workloads would simply be to add this flag rather than remodel into environment variables.

Would you be open to this? If not, what do you see as the downside here?

[Stretch96]

I understand this hasn't been accepted yet, but made a WIP PR for a little fun ☝️ (Using the -allow-undeclared-vars flag as proposed by @Legogris )

I agree with @Legogris on this, I don't think that this feature should be enforced.

It feels more like a strict linting/styling feature than core functionality, so would feel better to have the option to ignore it.

[apparentlymart]

We do not intend to have two different ways to specify undeclared variables, because that increases the complexity of the software, and thus increases the documentation, testing, and ongoing maintenance overhead. A command line argument also doesn't solve the problem for any wrapper program that is trying to work with both v0.11 and v0.12 at the same time, because -allow-undeclared-vars would be rejected by v0.11 as an error.

The environment variables solution is the documented way to do this, it already works in Terraform v0.11 (so automation wrappers don't need to do any special version switching to detect when v0.12 is in use, and can embrace this solution today), and is what we plan to ship in v0.12.

As I noted above, if you find that setting variables by environment variables is non-functional for some reason then please let us know in a separate issue, and we will fix it.

[EricMCornelius]

Really feels like disallowing superset variable configuration files and forcing users to jam everything into environment variables is going to exacerbate the original problem you're setting out to fix.

Most people here likely have machine generated configurations that are being passed to several different terraform projects for managing subsets of their infrastructure.

Forcing all of that to be passed via environment just to hide warnings (and eventually, errors) is just sweeping the typo problem under a different, even more opaque, and less user friendly rug...

[EricMCornelius]

If the argument is you don't want to support two different methods, you'd probably be better off rejecting undeclared TF_VAR_* environment variables and leaving configuration file superset behavior as is

The latter is a far, far more likely source of accidental typos, as it'll usually be utilized for passing secrets on the command line.

[ekini]

The best compromise would be to allow undeclared variables from var files and reject undeclared env vars.

This way:

1. There will be only one way to specify undeclared variables
2. The functionality that people relied on for years will be restored.
3. It will be compatible with both terraform 0.11 and 0.12

It's good to try make a tool fool-proof, but it's just not cool to break everything.
As another argument, take a look at gruntwork-io/terragrunt#740 (comment)
Taking into account converting typed vars to untyped env vars and hope terraform will guess the type correctly is counter-intuitive and inefficient.

@apparentlymart

[steveardis]

Not allowing variables in tfvars files that are undeclared as variables would break the magic we've put together. Would it not be enough to just list unused variables as an "info" message when running the terraform command?

[lostick]

Really feels like disallowing superset variable configuration files and forcing users to jam everything into environment variables is going to exacerbate the original problem you're setting out to fix.

Absolutely, having to set all global vars as TF_VAR_* environment variables adds a level of complexity and is prone to errors.

[ekini]

There are different absurd solutions to this problem:

1. Create a wrapper for terraform 0.12 make a regex that will catch and cut the warning from terraform output. For example,

$terraform plan | sed -e '/Value for undeclared variable/,/environment variables to set these instead/d' -e '/Values for undeclared variabl/,/without being declared./d'

cuts it off nicely.
2. Create a wrapper for 0.13 to pre-run terraform, catch the errors and generate declarations for undeclared variables, then run terraform
3. Create a wrapper that will read terraform files and create missing declarations
4. In package managers, say for brew, compile terraform from source with a patch that removes terraform.ValueFromNamedFile from https://github.com/hashicorp/terraform/blob/master/backend/unparsed_value.go#L54

But in reality, just one line needs to change.

[ekini]

Meanwhile, I took the formula from brew and modified it to apply the patch. Now the patched version can be installed as simple as brew install springload/tools/terraform
(or upgrade or reinstall depending on whether you have it installed already)

Check this out: https://github.com/springload/homebrew-tools/blob/master/Formula/terraform.rb
There are no bottled versions obviously. If people are keen, I could make a PR to the main brew repo to include this patch.

Linux is out of scope, unfortunately. Well, brew works on Linux...

[nicerobot]

I strongly disagree with the rationale for this change! 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.

Second, it makes no sense to be concerned about typos since i can simply use a map for my top-level variables and i'm right back in the same boat with typos.

Lastly, 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 us 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, most with very similar, but rarely identical, variable needs. To require a .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 disastrous tight coupling between the developers and the infrastructure and also means they'll have to maintain multiple .tfvars (that almost certainly means duplicating values) just so they can be used in various deployment scenarios.

Also, 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. 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, only the solution. While I do 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 to this issue and 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

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.