From bb2293a2cab5511713811a5332b50c4a70feb8c3 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Thu, 21 Jan 2016 14:33:16 -0500 Subject: [PATCH] core: Support explicit variable type declaration This commit adds support for declaring variable types in Terraform configuration. Historically, the type has been inferred from the default value, defaulting to string if no default was supplied. This has caused users to devise workarounds if they wanted to declare a map but provide values from a .tfvars file (for example). The new syntax adds the "type" key to variable blocks: ``` variable "i_am_a_string" { type = "string" } variable "i_am_a_map" { type = "map" } ``` This commit does _not_ extend the type system to include bools, integers or floats - the only two types available are maps and strings. Validation is performed if a default value is provided in order to ensure that the default value type matches the declared type. In the case that a type is not declared, the old logic is used for determining the type. This allows backwards compatiblity with previous Terraform configuration. --- config/config.go | 76 +++++++++++++++---- config/config_string.go | 8 +- config/loader_hcl.go | 18 +++-- config/loader_test.go | 30 ++++++++ config/test-fixtures/bad-variable-type.tf | 3 + config/test-fixtures/basic.tf | 12 +++ config/test-fixtures/basic.tf.json | 9 +++ .../test-fixtures/variable-mismatched-type.tf | 7 ++ terraform/context_validate_test.go | 15 ++++ .../main.tf | 5 ++ .../docs/configuration/variables.html.md | 29 +++++-- 11 files changed, 184 insertions(+), 28 deletions(-) create mode 100644 config/test-fixtures/bad-variable-type.tf create mode 100644 config/test-fixtures/variable-mismatched-type.tf create mode 100644 terraform/test-fixtures/validate-var-no-default-explicit-type/main.tf 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] }