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

Impending error when supplying value for undeclared variable #22337

Open
jbergknoff-rival opened this issue Aug 5, 2019 · 3 comments
Open

Impending error when supplying value for undeclared variable #22337

jbergknoff-rival opened this issue Aug 5, 2019 · 3 comments

Comments

@jbergknoff-rival
Copy link

jbergknoff-rival commented Aug 5, 2019

Hi, in TF 0.12 we now receive warnings to the effect of

The root module does not declare a variable named "subdomain". 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.

First off, this warning is a great addition, and we've already found it useful enough (for identifying dead code/tfvars) that we relay these warnings from our Jenkins pipeline to Slack for manual review. (Though I'll note that providing the full list of warnings would be more useful than the current behavior of just providing a few)

However, the decision to convert these warnings to errors in a future release should really be revisited. This has been covered already in #19424 (and I would have chimed in there had it not already been closed). In particular, @nicerobot makes some excellent points, chief among them that encouraging the use of environment variables undermines the ideals of infrastructure as code.

Now we'll need to keep track of which TF_VAR_ environment variables to set when running Terraform in automation. Of course we'll have those in source control. But not in a tfvars file? A tfvars-rejects file? Seems like keeping track of them in the tfvars file continues to be the most natural solution by far. In my opinion, this will be a major regression in usability, and doing that in service of an edge case (protecting users from typos) is really questionable.

I'll chip in our use case: we have several environments, e.g. dev/stage/prod. As we promote a new set of Terraform definitions ("stack") to production via an automated pipeline, we update the tfvars in preparation. If we were to update the tfvars to accomodate a new required variable, and then need to apply an old version of the TF stack before the new version is rolled out, we'd be blocked with an error. Yes, it's easy to work around, but very awkward in a highly automated process.

@philomory
Copy link
Contributor

philomory commented Aug 12, 2019

I just want to chime in here and say that we run into this problem too, and, unlike a lot of the users in #19424, we do not generate our tfvars files programmatically. We have multiple configurations in a hierarchy of layers (e.g. top/region1/topic1/) and at each layer there is a tfvars file for the settings at that layer (e.g. there's a top/shared.tfvars file for common values, a top/region1/shared.tfvars file for regional settings for region 1, etc). We invoke all this from the command line, not from scripts or pipelines.

Also, environment variables are not a usable replacement, at least, not according to the documentation, which states that they can only be used for string values:

Note: Environment variables can only populate string-type variables. List and map type variables must be populated via one of the other mechanisms.

Even if environment variables do work, they're not really an acceptable solution for all the reasons that @jbergknoff-rival noted: this stuff belongs in source control. Forcing us to put these values in environment variables, and then find (on our own) a cross-platform manner of automatically enforcing environment variables based on text files, is definitely the wrong road to go down.

I think one reasonable variation on this would be to only make providing a value for an undeclared variable a warning/error if the tfvars file that provides that value comes from inside the configuration root. If a tfvars file comes from outside the configuration root, it's probably shared between multiple configurations.

@mingzhaodotname
Copy link

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.

@martinkellydigital
Copy link

Simply put, this breaks a lot of the improved methodologies around automation. It's not controversial that workspaces are a poor method for managing multiple environments, and global tfvars files combined with separate components allows you to avoid that swamp.

Presented as a 'user experience improvement' fix, without the option to opt out, is heavy handed.

It's I'm sure entirely coincidental that if you use workspaces and the new commercial automation, you're forced into a mode that doesn't interact with this unfortunate new feature.

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

6 participants