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

Allows undeclared variables from named files (-var-file=...) #21697

Closed
wants to merge 1 commit into from

Conversation

ekini
Copy link
Contributor

@ekini ekini commented Jun 11, 2019

Rationale

Many people use terraform together with terragrunt or other tools.
With terragrunt specifically, a common project structure can be as follows:

|---- infrastructure
       |
       |---- production
       |          |---- env.tfvars
       |          |---- ecs-cluster
       |          |          |- terragrunt.hcl
       |          |---- ecs-service
       |                     |- terragrunt.hcl
       |---- staging
       |          |---- env.tfvars
       |          |---- ecs-cluster
       |          |          |-terragrunt.hcl
       |          |---- ecs-service
       |                     |- terragrunt.hcl
       |
       |--- common.tfvars

Here, ecs-cluster and ecs-service are terragrunt modules.

The files common.tfvars are plugged together with correct env.tfvars depending on the environment as var files:

terraform plan -var-file=/some/project/infra/common.tfvars -var-file=/some/project/infra/production/env.tfvars

This allows setting some variables globally for the project, plus it makes it possible to override them in env.tfvars or on per-module basis.

However, some modules don't have all variables declared, for example ecs-service module doesn't need to declare a variable like ecs_cluster_instance_type, because it doesn't use it. But it is convenient to set it in env.vars to make it a source of configuration for this environment, rather than setting variables in every module folder.

Since terraform 0.12 there is a warning for every undeclared variable, which can get really messy if you use it with terragrunt:

...
Plan: 2 to add, 0 to change, 0 to destroy.

Warning: Values for undeclared variables

In addition to the other similar warnings shown, 10 other variable(s) defined
without being declared.

<<3 more warning with large text>>

I actually raised this some time ago: gruntwork-io/terragrunt#466 (comment)

To make it work in the current state, some nasty hacks has to be introduced as per gruntwork-io/terragrunt#737 like having terragrunt to convert variables to env vars.

Instead, this PR just allows undeclared variables set via named files.
Plus adds a few more test cases to check for other undeclared var sources.

@hashicorp-cla
Copy link

hashicorp-cla commented Jun 11, 2019

CLA assistant check
All committers have signed the CLA.

@apparentlymart
Copy link
Contributor

Hi @ekini! Thanks for working on this.

You can see in #19424 the background for why this check is present, and why we don't intend to relax this. Because environment variables already provide a way to set "global defaults" that may or may not be used, we think it's more important to give feedback to users about potential typos that would otherwise lead to strange behavior that is difficult to diagnose.

As you saw, we made this a warning for Terraform 0.12 so that existing tools (including Terragrunt) would be able to transition over to using environment variables instead before this becomes a full error in Terraform 0.13.

We have no intention of ever applying this restriction to TF_VAR_... environment variables because we expect those to be set in a global or session-wide way where they are ignored when not relevant, as is common practice for environment variables -- it would be very unexpected for a program to refuse to run because an environment variable is set that it doesn't understand.

It looks like there's some good discussion underway in gruntwork-io/terragrunt#737 about different ways for Terragrunt to approach this, including the possibility of using environment variables (the most straightforward equivalent of the old behavior) I'm also glad to see the discussion of the possibility of making things more explicit on the Terragrunt side too, so that Terragrunt users might enjoy the same benefit of quick feedback if they set a variable name incorrectly. I'm happy to leave that tradeoff in the hands of the Terragrunt team, since it's very much a compromise with no obvious right answer.

However, I can say that we do not intend to remove this warning because we think the frustration of debugging the implied strange behavior of an incorrectly-set argument with no feedback is worse than seeing this warning in Terraform output until automation tools are updated in some way.

Thank you for proposing this, and I hope the above rationale gives good context for why we cannot merge it.

@ghost
Copy link

ghost commented Jul 25, 2019

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.

@ghost ghost locked and limited conversation to collaborators Jul 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants