From 099293b690f56bd04f8f684d1ad14e19bdd7fc95 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 25 Aug 2016 14:43:57 -0700 Subject: [PATCH 1/3] config: outputs must be unique --- config/config.go | 70 +++++++++++-------- config/config_test.go | 7 ++ .../test-fixtures/validate-output-dup/main.tf | 10 +++ 3 files changed, 58 insertions(+), 29 deletions(-) create mode 100644 config/test-fixtures/validate-output-dup/main.tf diff --git a/config/config.go b/config/config.go index 54ecd49205..ac6749a319 100644 --- a/config/config.go +++ b/config/config.go @@ -586,43 +586,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..81bd11b234 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 { 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" +} From fbf06e2a59c59250915c5848c0303c1737bbd937 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 25 Aug 2016 14:51:49 -0700 Subject: [PATCH 2/3] config: vars must be unique --- config/config.go | 6 ++++++ config/config_test.go | 7 +++++++ config/loader_hcl.go | 7 ++++--- config/test-fixtures/validate-var-default/main.tf | 2 +- config/test-fixtures/validate-var-dup/main.tf | 2 ++ 5 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 config/test-fixtures/validate-var-dup/main.tf diff --git a/config/config.go b/config/config.go index ac6749a319..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 } diff --git a/config/config_test.go b/config/config_test.go index 81bd11b234..bbbb7fa06c 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -447,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 8c81156e47..4adf5ab45f 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/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" {} From 0fceeaaeb05426ceba3721baa8b920253c105015 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 26 Aug 2016 13:48:21 -0700 Subject: [PATCH 3/3] config: test for var uniqueness in overrides --- config/loader_test.go | 22 +++++++++++++++++++ config/test-fixtures/dir-override-var/main.tf | 4 ++++ .../dir-override-var/main_override.tf | 3 +++ 3 files changed, 29 insertions(+) create mode 100644 config/test-fixtures/dir-override-var/main.tf create mode 100644 config/test-fixtures/dir-override-var/main_override.tf diff --git a/config/loader_test.go b/config/loader_test.go index b7d53dee52..d2a1208eca 100644 --- a/config/loader_test.go +++ b/config/loader_test.go @@ -446,6 +446,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 { @@ -922,6 +938,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" +}