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

config: variable names and outputs must be unique #8482

Merged
merged 3 commits into from
Aug 26, 2016
Merged

Conversation

mitchellh
Copy link
Contributor

@mitchellh mitchellh commented Aug 25, 2016

Fixes #5052
Fixes #5144

This adds new config validation to verify output names and variable names are unique.

@mitchellh mitchellh changed the title config: outputs must be unique config: variable names and outputs must be unique Aug 25, 2016
@darrensony
Copy link

Regarding #5144, and looking at your test code, this may or may not be the right solution.

Say I have two .tf files, A.tf and B.tf

A.tf sets the value of the variable
variable "foo" { default = 1 }

B.tf needs to use the value of "foo" but not define it.
variable "foo" {}

The core issue of #5144 was that the above worked fine, but if A.tf was renamed to C.tf, it no longer worked because Terraform reads files in alphabetical order and thus sees the reference before the declaration.

Throwing an error because there is both a declaration and a reference is not the correct behaviour.

@mitchellh
Copy link
Contributor Author

mitchellh commented Aug 26, 2016

There is no such thing as a "declaration" and a "reference". Every variable is a declaration, you don't need to "reference" anything (and that doesn't exist in Terraform). Every Terraform file in the same directory is loaded into the same namespace.

Unfortunately what you were doing before should've never worked. Terraform loads and merges all files together. Having a config in A.tf and B.tf is the same as concatenating B.tf to A.tf. Terraform currently happens to load files in alphabetical order which caused your behavior.

We never intended two variables to be defined, as they're all in the same namespace, so this is the correct behavior.

Actually, to fight against this in the future, we should probably load all the files in a random order in each load. The declarative nature of Terraform means the load order shouldn't matter. I'll probably do that in 0.8...

@mitchellh
Copy link
Contributor Author

@jen20 brought up a good point in our Slack channel that I need to test how this interacts with override files. I'm going to add test cases for this since its currently an unknown.

@jen20
Copy link
Contributor

jen20 commented Aug 26, 2016

LGTM pending Travis with the new test.

@ghost
Copy link

ghost commented Apr 22, 2020

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 Apr 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants