diff --git a/config/config.go b/config/config.go index d0177c95507e..2200325852ac 100644 --- a/config/config.go +++ b/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,25 +781,45 @@ 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.Default == nil { - return VariableTypeString + if v.DeclaredType != "" { + declaredType, ok := typeStringMap[v.DeclaredType] + if !ok { + return VariableTypeUnknown + } + + return declaredType } - var strVal string - if err := mapstructure.WeakDecode(v.Default, &strVal); err == nil { - v.Default = strVal - return VariableTypeString + 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", v.Name) + } } - var m map[string]string - if err := mapstructure.WeakDecode(v.Default, &m); err == nil { - v.Default = m - return VariableTypeMap + if v.DeclaredType == "" || v.Default == nil { + return nil } - return VariableTypeUnknown + 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 { @@ -813,3 +834,26 @@ func (v *Variable) mergerMerge(m merger) merger { 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 + } + + var strVal string + if err := mapstructure.WeakDecode(v.Default, &strVal); err == nil { + v.Default = strVal + return VariableTypeString + } + + var m map[string]string + if err := mapstructure.WeakDecode(v.Default, &m); err == nil { + v.Default = m + return VariableTypeMap + } + + return VariableTypeUnknown +} diff --git a/config/config_string.go b/config/config_string.go index 2db0cbe493f1..4085d635f3f4 100644 --- a/config/config_string.go +++ b/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) } diff --git a/config/loader_hcl.go b/config/loader_hcl.go index 7868cc8ecfb8..3757e8048332 100644 --- a/config/loader_hcl.go +++ b/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) diff --git a/config/loader_test.go b/config/loader_test.go index bfe21cfb2d00..74ee7b9d55b9 100644 --- a/config/loader_test.go +++ b/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 diff --git a/config/test-fixtures/bad-variable-type.tf b/config/test-fixtures/bad-variable-type.tf new file mode 100644 index 000000000000..e6b2aad4fd14 --- /dev/null +++ b/config/test-fixtures/bad-variable-type.tf @@ -0,0 +1,3 @@ +variable "bad_type" { + type = "notatype" +} diff --git a/config/test-fixtures/basic.tf b/config/test-fixtures/basic.tf index 5afadc779755..42c1d681b777 100644 --- a/config/test-fixtures/basic.tf +++ b/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" diff --git a/config/test-fixtures/basic.tf.json b/config/test-fixtures/basic.tf.json index 1013862b3399..ba8aa97492ad 100644 --- a/config/test-fixtures/basic.tf.json +++ b/config/test-fixtures/basic.tf.json @@ -3,6 +3,15 @@ "foo": { "default": "bar", "description": "bar" + }, + "bar": { + "type": "string" + }, + "baz": { + "type": "map", + "default": { + "key": "value" + } } }, diff --git a/config/test-fixtures/variable-mismatched-type.tf b/config/test-fixtures/variable-mismatched-type.tf new file mode 100644 index 000000000000..addbff24fe8d --- /dev/null +++ b/config/test-fixtures/variable-mismatched-type.tf @@ -0,0 +1,7 @@ +variable "not_a_map" { + type = "string" + + default = { + i_am_not = "a string" + } +} diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go index f7f03b94bfcc..18d1fe427b0f 100644 --- a/terraform/context_validate_test.go +++ b/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") diff --git a/terraform/test-fixtures/validate-var-no-default-explicit-type/main.tf b/terraform/test-fixtures/validate-var-no-default-explicit-type/main.tf new file mode 100644 index 000000000000..6bd2bdd5ece3 --- /dev/null +++ b/terraform/test-fixtures/validate-var-no-default-explicit-type/main.tf @@ -0,0 +1,5 @@ +variable "maybe_a_map" { + type = "map" + + // No default +} diff --git a/website/source/docs/configuration/variables.html.md b/website/source/docs/configuration/variables.html.md index 3f4c2caa7b8e..63fbc1539a92 100644 --- a/website/source/docs/configuration/variables.html.md +++ b/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] }