diff --git a/config/config.go b/config/config.go index 54ecd49205..d98ad854b5 100644 --- a/config/config.go +++ b/config/config.go @@ -239,6 +239,12 @@ func (c *Config) Validate() error { vars := c.InterpolatedVariables() varMap := make(map[string]*Variable) for _, v := range c.Variables { + if _, ok := varMap[v.Name]; ok { + errs = append(errs, fmt.Errorf( + "Variable '%s': duplicate found. Variable names must be unique.", + v.Name)) + } + varMap[v.Name] = v } @@ -586,43 +592,55 @@ func (c *Config) Validate() error { } // Check that all outputs are valid - for _, o := range c.Outputs { - var invalidKeys []string - valueKeyFound := false - for k := range o.RawConfig.Raw { - if k == "value" { - valueKeyFound = true + { + found := make(map[string]struct{}) + for _, o := range c.Outputs { + // Verify the output is new + if _, ok := found[o.Name]; ok { + errs = append(errs, fmt.Errorf( + "%s: duplicate output. output names must be unique.", + o.Name)) continue } - if k == "sensitive" { - if sensitive, ok := o.RawConfig.config[k].(bool); ok { - if sensitive { - o.Sensitive = true - } + found[o.Name] = struct{}{} + + var invalidKeys []string + valueKeyFound := false + for k := range o.RawConfig.Raw { + if k == "value" { + valueKeyFound = true continue } + if k == "sensitive" { + if sensitive, ok := o.RawConfig.config[k].(bool); ok { + if sensitive { + o.Sensitive = true + } + continue + } + errs = append(errs, fmt.Errorf( + "%s: value for 'sensitive' must be boolean", + o.Name)) + continue + } + invalidKeys = append(invalidKeys, k) + } + if len(invalidKeys) > 0 { errs = append(errs, fmt.Errorf( - "%s: value for 'sensitive' must be boolean", - o.Name)) - continue + "%s: output has invalid keys: %s", + o.Name, strings.Join(invalidKeys, ", "))) } - invalidKeys = append(invalidKeys, k) - } - if len(invalidKeys) > 0 { - errs = append(errs, fmt.Errorf( - "%s: output has invalid keys: %s", - o.Name, strings.Join(invalidKeys, ", "))) - } - if !valueKeyFound { - errs = append(errs, fmt.Errorf( - "%s: output is missing required 'value' key", o.Name)) - } - - for _, v := range o.RawConfig.Variables { - if _, ok := v.(*CountVariable); ok { + if !valueKeyFound { errs = append(errs, fmt.Errorf( - "%s: count variables are only valid within resources", o.Name)) + "%s: output is missing required 'value' key", o.Name)) + } + + for _, v := range o.RawConfig.Variables { + if _, ok := v.(*CountVariable); ok { + errs = append(errs, fmt.Errorf( + "%s: count variables are only valid within resources", o.Name)) + } } } } diff --git a/config/config_test.go b/config/config_test.go index 41690733d9..bbbb7fa06c 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -300,6 +300,13 @@ func TestConfigValidate_outputBadField(t *testing.T) { } } +func TestConfigValidate_outputDuplicate(t *testing.T) { + c := testConfig(t, "validate-output-dup") + if err := c.Validate(); err == nil { + t.Fatal("should not be valid") + } +} + func TestConfigValidate_pathVar(t *testing.T) { c := testConfig(t, "validate-path-var") if err := c.Validate(); err != nil { @@ -440,6 +447,13 @@ func TestConfigValidate_varDefaultInterpolate(t *testing.T) { } } +func TestConfigValidate_varDup(t *testing.T) { + c := testConfig(t, "validate-var-dup") + if err := c.Validate(); err == nil { + t.Fatal("should not be valid") + } +} + func TestConfigValidate_varMultiExactNonSlice(t *testing.T) { c := testConfig(t, "validate-var-multi-exact-non-slice") if err := c.Validate(); err != nil { diff --git a/config/loader_hcl.go b/config/loader_hcl.go index 2988db19fc..9f85e8c07d 100644 --- a/config/loader_hcl.go +++ b/config/loader_hcl.go @@ -29,6 +29,7 @@ func (t *hclConfigurable) Config() (*Config, error) { } type hclVariable struct { + Name string `hcl:",key"` Default interface{} Description string DeclaredType string `hcl:"type"` @@ -36,7 +37,7 @@ func (t *hclConfigurable) Config() (*Config, error) { } var rawConfig struct { - Variable map[string]*hclVariable + Variable []*hclVariable } // Top-level item should be the object list @@ -56,7 +57,7 @@ func (t *hclConfigurable) Config() (*Config, error) { config := new(Config) if len(rawConfig.Variable) > 0 { config.Variables = make([]*Variable, 0, len(rawConfig.Variable)) - for k, v := range rawConfig.Variable { + for _, v := range rawConfig.Variable { // Defaults turn into a slice of map[string]interface{} and // we need to make sure to convert that down into the // proper type for Config. @@ -72,7 +73,7 @@ func (t *hclConfigurable) Config() (*Config, error) { } newVar := &Variable{ - Name: k, + Name: v.Name, DeclaredType: v.DeclaredType, Default: v.Default, Description: v.Description, diff --git a/config/loader_test.go b/config/loader_test.go index 09b16f352a..1da5292353 100644 --- a/config/loader_test.go +++ b/config/loader_test.go @@ -462,6 +462,22 @@ func TestLoadDir_override(t *testing.T) { } } +func TestLoadDir_overrideVar(t *testing.T) { + c, err := LoadDir(filepath.Join(fixtureDir, "dir-override-var")) + if err != nil { + t.Fatalf("err: %s", err) + } + + if c == nil { + t.Fatal("config should not be nil") + } + + actual := variablesStr(c.Variables) + if actual != strings.TrimSpace(dirOverrideVarsVariablesStr) { + t.Fatalf("bad:\n%s", actual) + } +} + func TestLoadFile_mismatchedVariableTypes(t *testing.T) { _, err := LoadFile(filepath.Join(fixtureDir, "variable-mismatched-type.tf")) if err == nil { @@ -943,6 +959,12 @@ foo bar ` +const dirOverrideVarsVariablesStr = ` +foo + baz + bar +` + const importProvidersStr = ` aws bar diff --git a/config/test-fixtures/dir-override-var/main.tf b/config/test-fixtures/dir-override-var/main.tf new file mode 100644 index 0000000000..f902801bdf --- /dev/null +++ b/config/test-fixtures/dir-override-var/main.tf @@ -0,0 +1,4 @@ +variable "foo" { + default = "bar" + description = "bar" +} diff --git a/config/test-fixtures/dir-override-var/main_override.tf b/config/test-fixtures/dir-override-var/main_override.tf new file mode 100644 index 0000000000..a3fd68abb8 --- /dev/null +++ b/config/test-fixtures/dir-override-var/main_override.tf @@ -0,0 +1,3 @@ +variable "foo" { + default = "baz" +} diff --git a/config/test-fixtures/validate-output-dup/main.tf b/config/test-fixtures/validate-output-dup/main.tf new file mode 100644 index 0000000000..10b598f8aa --- /dev/null +++ b/config/test-fixtures/validate-output-dup/main.tf @@ -0,0 +1,10 @@ +resource "aws_instance" "web" { +} + +output "ip" { + value = "foo" +} + +output "ip" { + value = "bar" +} diff --git a/config/test-fixtures/validate-var-default/main.tf b/config/test-fixtures/validate-var-default/main.tf index 0222275016..80db1843dd 100644 --- a/config/test-fixtures/validate-var-default/main.tf +++ b/config/test-fixtures/validate-var-default/main.tf @@ -2,7 +2,7 @@ variable "foo" { default = "bar" } -variable "foo" { +variable "foo2" { default = { foo = "bar" } diff --git a/config/test-fixtures/validate-var-dup/main.tf b/config/test-fixtures/validate-var-dup/main.tf new file mode 100644 index 0000000000..6f75d8d473 --- /dev/null +++ b/config/test-fixtures/validate-var-dup/main.tf @@ -0,0 +1,2 @@ +variable "foo" {} +variable "foo" {}