From cb6cb8b96a731f4a4594dabc394a61bd32523bc8 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 | 80 ++++++++++++++----- 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, 186 insertions(+), 30 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 d0177c955..220032585 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,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", v.Name) + } + } + + 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 -} diff --git a/config/config_string.go b/config/config_string.go index 2db0cbe49..4085d635f 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 7868cc8ec..3757e8048 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 bfe21cfb2..74ee7b9d5 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 000000000..e6b2aad4f --- /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 5afadc779..42c1d681b 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 1013862b3..ba8aa9749 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 000000000..addbff24f --- /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 f7f03b94b..18d1fe427 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 000000000..6bd2bdd5e --- /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 3f4c2caa7..79d4060fa 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] }