Merge pull request #4795 from hashicorp/f-variable-types

core: Support explicit variable type declaration

config/config.go

@@ -98,9 +98,10 @@ type Provisioner struct {
 
 // Variable is a variable defined within the configuration.
 type Variable struct {
-	Name        string
-	Default     interface{}
-	Description string
+	Name         string
+	DeclaredType string `mapstructure:"type"`
+	Default      interface{}
+	Description  string
 }
 
 // Output is an output defined within the configuration. An output is
@@ -177,7 +178,7 @@ func (c *Config) Validate() error {
 	for _, v := range c.Variables {
 		if v.Type() == VariableTypeUnknown {
 			errs = append(errs, fmt.Errorf(
-				"Variable '%s': must be string or mapping",
+				"Variable '%s': must be a string or a map",
 				v.Name))
 			continue
 		}
@@ -780,8 +781,64 @@ func (v *Variable) Merge(v2 *Variable) *Variable {
 	return &result
 }
 
-// Type returns the type of varialbe this is.
+var typeStringMap = map[string]VariableType{
+	"string": VariableTypeString,
+	"map":    VariableTypeMap,
+}
+
+// Type returns the type of variable this is.
 func (v *Variable) Type() VariableType {
+	if v.DeclaredType != "" {
+		declaredType, ok := typeStringMap[v.DeclaredType]
+		if !ok {
+			return VariableTypeUnknown
+		}
+
+		return declaredType
+	}
+
+	return v.inferTypeFromDefault()
+}
+
+// ValidateTypeAndDefault ensures that default variable value is compatible
+// with the declared type (if one exists), and that the type is one which is
+// known to Terraform
+func (v *Variable) ValidateTypeAndDefault() error {
+	// If an explicit type is declared, ensure it is valid
+	if v.DeclaredType != "" {
+		if _, ok := typeStringMap[v.DeclaredType]; !ok {
+			return fmt.Errorf("Variable '%s' must be of type string or map - '%s' is not a valid type", v.Name, v.DeclaredType)
+		}
+	}
+
+	if v.DeclaredType == "" || v.Default == nil {
+		return nil
+	}
+
+	if v.inferTypeFromDefault() != v.Type() {
+		return fmt.Errorf("'%s' has a default value which is not of type '%s'", v.Name, v.DeclaredType)
+	}
+
+	return nil
+}
+
+func (v *Variable) mergerName() string {
+	return v.Name
+}
+
+func (v *Variable) mergerMerge(m merger) merger {
+	return v.Merge(m.(*Variable))
+}
+
+// Required tests whether a variable is required or not.
+func (v *Variable) Required() bool {
+	return v.Default == nil
+}
+
+// inferTypeFromDefault contains the logic for the old method of inferring
+// variable types - we can also use this for validating that the declared
+// type matches the type of the default value
+func (v *Variable) inferTypeFromDefault() VariableType {
 	if v.Default == nil {
 		return VariableTypeString
 	}
@@ -800,16 +857,3 @@ func (v *Variable) Type() VariableType {
 
 	return VariableTypeUnknown
 }
-
-func (v *Variable) mergerName() string {
-	return v.Name
-}
-
-func (v *Variable) mergerMerge(m merger) merger {
-	return v.Merge(m.(*Variable))
-}
-
-// Required tests whether a variable is required or not.
-func (v *Variable) Required() bool {
-	return v.Default == nil
-}

config/config_string.go

@@ -278,6 +278,11 @@ func variablesStr(vs []*Variable) string {
 			required = " (required)"
 		}
 
+		declaredType := ""
+		if v.DeclaredType != "" {
+			declaredType = fmt.Sprintf(" (%s)", v.DeclaredType)
+		}
+
 		if v.Default == nil || v.Default == "" {
 			v.Default = "<>"
 		}
@@ -286,9 +291,10 @@ func variablesStr(vs []*Variable) string {
 		}
 
 		result += fmt.Sprintf(
-			"%s%s\n  %v\n  %s\n",
+			"%s%s%s\n  %v\n  %s\n",
 			k,
 			required,
+			declaredType,
 			v.Default,
 			v.Description)
 	}

config/loader_hcl.go

@@ -28,9 +28,10 @@ func (t *hclConfigurable) Config() (*Config, error) {
 	}
 
 	type hclVariable struct {
-		Default     interface{}
-		Description string
-		Fields      []string `hcl:",decodedFields"`
+		Default      interface{}
+		Description  string
+		DeclaredType string   `hcl:"type"`
+		Fields       []string `hcl:",decodedFields"`
 	}
 
 	var rawConfig struct {
@@ -70,9 +71,14 @@ func (t *hclConfigurable) Config() (*Config, error) {
 			}
 
 			newVar := &Variable{
-				Name:        k,
-				Default:     v.Default,
-				Description: v.Description,
+				Name:         k,
+				DeclaredType: v.DeclaredType,
+				Default:      v.Default,
+				Description:  v.Description,
+			}
+
+			if err := newVar.ValidateTypeAndDefault(); err != nil {
+				return nil, err
 			}
 
 			config.Variables = append(config.Variables, newVar)

config/loader_test.go

@@ -444,6 +444,30 @@ func TestLoadDir_override(t *testing.T) {
 	}
 }
 
+func TestLoadFile_mismatchedVariableTypes(t *testing.T) {
+	_, err := LoadFile(filepath.Join(fixtureDir, "variable-mismatched-type.tf"))
+	if err == nil {
+		t.Fatalf("bad: expected error")
+	}
+
+	errorStr := err.Error()
+	if !strings.Contains(errorStr, "'not_a_map' has a default value which is not of type 'string'") {
+		t.Fatalf("bad: expected error has wrong text: %s", errorStr)
+	}
+}
+
+func TestLoadFile_badVariableTypes(t *testing.T) {
+	_, err := LoadFile(filepath.Join(fixtureDir, "bad-variable-type.tf"))
+	if err == nil {
+		t.Fatalf("bad: expected error")
+	}
+
+	errorStr := err.Error()
+	if !strings.Contains(errorStr, "'bad_type' must be of type string") {
+		t.Fatalf("bad: expected error has wrong text: %s", errorStr)
+	}
+}
+
 func TestLoadFile_provisioners(t *testing.T) {
 	c, err := LoadFile(filepath.Join(fixtureDir, "provisioners.tf"))
 	if err != nil {
@@ -802,6 +826,12 @@ aws_security_group[firewall] (x5)
 `
 
 const basicVariablesStr = `
+bar (required) (string)
+  <>
+  <>
+baz (map)
+  map[key:value]
+  <>
 foo
   bar
   bar

config/test-fixtures/bad-variable-type.tf

@@ -0,0 +1,3 @@
+variable "bad_type" {
+    type = "notatype"
+}

config/test-fixtures/basic.tf

@@ -3,6 +3,18 @@ variable "foo" {
     description = "bar"
 }
 
+variable "bar" {
+    type = "string"
+}
+
+variable "baz" {
+    type = "map"
+
+    default = {
+        key = "value"
+    }
+}
+
 provider "aws" {
   access_key = "foo"
   secret_key = "bar"

config/test-fixtures/basic.tf.json

@@ -3,6 +3,15 @@
         "foo": {
             "default": "bar",
             "description": "bar"
+        },
+        "bar": {
+            "type": "string"
+        },
+        "baz": {
+            "type": "map",
+            "default": {
+                "key": "value"
+            }
         }
     },
 

config/test-fixtures/variable-mismatched-type.tf

@@ -0,0 +1,7 @@
+variable "not_a_map" {
+    type = "string"
+
+    default = {
+        i_am_not = "a string"
+    }
+}

terraform/context_validate_test.go

@@ -25,6 +25,21 @@ func TestContext2Validate_badVar(t *testing.T) {
 	}
 }
 
+func TestContext2Validate_varNoDefaultExplicitType(t *testing.T) {
+	m := testModule(t, "validate-var-no-default-explicit-type")
+	c := testContext2(t, &ContextOpts{
+		Module: m,
+	})
+
+	w, e := c.Validate()
+	if len(w) > 0 {
+		t.Fatalf("bad: %#v", w)
+	}
+	if len(e) == 0 {
+		t.Fatalf("bad: %#v", e)
+	}
+}
+
 func TestContext2Validate_computedVar(t *testing.T) {
 	p := testProvider("aws")
 	m := testModule(t, "validate-computed-var")

terraform/test-fixtures/validate-var-no-default-explicit-type/main.tf

@@ -0,0 +1,5 @@
+variable "maybe_a_map" {
+    type = "map"
+
+    // No default
+}

website/source/docs/configuration/variables.html.md

@@ -23,9 +23,13 @@ already.
 A variable configuration looks like the following:
 
 ```
-variable "key" {}
+variable "key" {
+    type = "string"
+}
 
 variable "images" {
+    type = "map"
+
 	default = {
 		us-east-1 = "image-1234"
 		us-west-2 = "image-4567"
@@ -39,13 +43,22 @@ The `variable`  block configures a single input variable for
 a Terraform configuration. Multiple variables blocks can be used to
 add multiple variables.
 
-The `NAME` given to the variable block is the name used to
+The `name` given to the variable block is the name used to
 set the variable via the CLI as well as reference the variable
 throughout the Terraform configuration.
 
 Within the block (the `{ }`) is configuration for the variable.
 These are the parameters that can be set:
 
+  * `type` (optional) - If set this defines the type of the variable.
+    Valid values are `string` and `map`. In older versions of Terraform
+    this parameter did not exist, and the type was inferred from the
+    default value, defaulting to `string` if no default was set. If a
+    type is not specified, the previous behavior is maintained. It is
+    recommended to set variable types explicitly in preference to relying
+    on inferrence - this allows variables of type `map` to be set in the
+    `terraform.tfvars` file without requiring a default value to be set.
+
   * `default` (optional) - If set, this sets a default value
     for the variable. If this isn't set, the variable is required
     and Terraform will error if not set. The default value can be
@@ -59,15 +72,18 @@ These are the parameters that can be set:
 
 ------
 
-**Default values** can be either strings or maps. If a default
-value is omitted and the variable is required, the value assigned
-via the CLI must be a string.
+**Default values** can be either strings or maps, and if specified
+must match the declared type of the variable. If no value is supplied
+for a variable of type `map`, the values must be supplied in a
+`terraform.tfvars` file - they cannot be input via the console.
 
 String values are simple and represent a basic key to value
 mapping where the key is the variable name. An example is:
 
 ```
 variable "key" {
+    type = "string"
+
 	default = "value"
 }
 ```
@@ -79,6 +95,8 @@ An example:
 
 ```
 variable "images" {
+    type = "map"
+
 	default = {
 		us-east-1 = "image-1234"
 		us-west-2 = "image-4567"
@@ -115,6 +133,7 @@ The full syntax is:
 
 ```
 variable NAME {
+	[type = TYPE]
 	[default = DEFAULT]
 	[description = DESCRIPTION]
 }