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: Report duplicate output definitions as invalid #5052

Closed
radeksimko opened this issue Feb 8, 2016 · 8 comments · Fixed by #8482
Closed

config: Report duplicate output definitions as invalid #5052

radeksimko opened this issue Feb 8, 2016 · 8 comments · Fixed by #8482
Assignees

Comments

@radeksimko
Copy link
Member

The following is currently considered as valid config:

variable "var1" {
    default = "Yoda"
}
variable "var2" {
    default = "Pug"
}

output "yada" {
    value = "${var.var1}"
}
output "yada" {
    value = "${var.var2}"
}
$ terraform apply
Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

  yada = Yoda

I believe we should be returning error in these cases as outputs are saved as key/value map - i.e. there will always be only 1 unique value returned for a single key.

@radeksimko
Copy link
Member Author

variables have the same issue:

variable "var1" {
    default = "Yoda"
}
variable "var1" {
    default = "Pug"
}

is reported as valid.

@phinze
Copy link
Contributor

phinze commented Feb 8, 2016

Yep agreed terraform should protect against these config mistakes. 👍

@sorenmat
Copy link
Contributor

I've looked into this, and have a fix for the output issue.
But it seems like having multiple variables with the same name is a feature, and it should just override the values. See
https://github.com/hashicorp/terraform/blob/master/config/config_test.go#L371 and the test-fixture https://github.com/hashicorp/terraform/blob/master/config/test-fixtures/validate-var-default/main.tf

Should we still make a check for duplicate variables ?

@radeksimko
Copy link
Member Author

@sorenmat I don't see how duplicate variable definitions can be useful as we'll always pick one at the end... so I'd suggest we check for this too.

@billf
Copy link
Contributor

billf commented Feb 11, 2016

some amount of duplicates need to be supported for https://www.terraform.io/docs/configuration/override.html to work.

@radeksimko
Copy link
Member Author

@billf good point, we should definitely have tests covering these cases as these are valid.
However 2 duplicate variables in the same file should be reported as invalid.

@brainsik
Copy link

Duplicates in different files should also be invalid. That's the case I'm really worried about as the configurations grow. Is there ever a time where you'd want to re-use a var/output name outside of an override file?

@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
Development

Successfully merging a pull request may close this issue.

6 participants