Merge pull request #8482 from hashicorp/b-output-dup

config: variable names and outputs must be unique

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
+		found := make(map[string]struct{})
-		valueKeyFound := false
+		for _, o := range c.Outputs {
-		for k := range o.RawConfig.Raw {
+			// Verify the output is new
-			if k == "value" {
+			if _, ok := found[o.Name]; ok {
-				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",
+					"%s: duplicate output. output names must be unique.",
 					o.Name))
 				continue
 			}
-			invalidKeys = append(invalidKeys, k)
+			found[o.Name] = struct{}{}
-		}
-		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 {
+			var invalidKeys []string
-			if _, ok := v.(*CountVariable); ok {
+			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: count variables are only valid within resources", o.Name))
+					"%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 {
+					errs = append(errs, fmt.Errorf(
+						"%s: count variables are only valid within resources", o.Name))
+				}
 			}
 		}
 	}

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 {

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,

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

config/test-fixtures/dir-override-var/main.tf

@@ -0,0 +1,4 @@
+variable "foo" {
+    default = "bar"
+    description = "bar"
+}

config/test-fixtures/dir-override-var/main_override.tf

@@ -0,0 +1,3 @@
+variable "foo" {
+    default = "baz"
+}

config/test-fixtures/validate-output-dup/main.tf

@@ -0,0 +1,10 @@
+resource "aws_instance" "web" {
+}
+
+output "ip" {
+  value = "foo"
+}
+
+output "ip" {
+  value = "bar"
+}

config/test-fixtures/validate-var-default/main.tf

@@ -2,7 +2,7 @@ variable "foo" {
   default = "bar"
 }
 
-variable "foo" {
+variable "foo2" {
   default = {
     foo = "bar"
   }

config/test-fixtures/validate-var-dup/main.tf

@@ -0,0 +1,2 @@
+variable "foo" {}
+variable "foo" {}