Merge pull request #7710 from hashicorp/f-parse-tfenvvars-as-hcl

core: Allow TF_VAR_x to contain lists and maps

command/flag_kv.go

@@ -3,21 +3,46 @@ package command
 import (
 	"fmt"
 	"io/ioutil"
+	"regexp"
 	"strings"
 
 	"github.com/hashicorp/hcl"
 	"github.com/mitchellh/go-homedir"
 )
 
-// FlagKV is a flag.Value implementation for parsing user variables
+// FlagTypedKVis a flag.Value implementation for parsing user variables
-// from the command-line in the format of '-var key=value'.
+// from the command-line in the format of '-var key=value', where value is
-type FlagKV map[string]string
+// a type intended for use as a Terraform variable
+type FlagTypedKV map[string]interface{}
 
-func (v *FlagKV) String() string {
+func (v *FlagTypedKV) String() string {
 	return ""
 }
 
-func (v *FlagKV) Set(raw string) error {
+func (v *FlagTypedKV) Set(raw string) error {
+	key, value, err := parseVarFlagAsHCL(raw)
+	if err != nil {
+		return err
+	}
+
+	if *v == nil {
+		*v = make(map[string]interface{})
+	}
+
+	(*v)[key] = value
+	return nil
+}
+
+// FlagStringKV is a flag.Value implementation for parsing user variables
+// from the command-line in the format of '-var key=value', where value is
+// only ever a primitive.
+type FlagStringKV map[string]string
+
+func (v *FlagStringKV) String() string {
+	return ""
+}
+
+func (v *FlagStringKV) Set(raw string) error {
 	idx := strings.Index(raw, "=")
 	if idx == -1 {
 		return fmt.Errorf("No '=' value in arg: %s", raw)
@@ -34,7 +59,7 @@ func (v *FlagKV) Set(raw string) error {
 
 // FlagKVFile is a flag.Value implementation for parsing user variables
 // from the command line in the form of files. i.e. '-var-file=foo'
-type FlagKVFile map[string]string
+type FlagKVFile map[string]interface{}
 
 func (v *FlagKVFile) String() string {
 	return ""
@@ -47,7 +72,7 @@ func (v *FlagKVFile) Set(raw string) error {
 	}
 
 	if *v == nil {
-		*v = make(map[string]string)
+		*v = make(map[string]interface{})
 	}
 
 	for key, value := range vs {
@@ -57,7 +82,7 @@ func (v *FlagKVFile) Set(raw string) error {
 	return nil
 }
 
-func loadKVFile(rawPath string) (map[string]string, error) {
+func loadKVFile(rawPath string) (map[string]interface{}, error) {
 	path, err := homedir.Expand(rawPath)
 	if err != nil {
 		return nil, fmt.Errorf(
@@ -78,7 +103,7 @@ func loadKVFile(rawPath string) (map[string]string, error) {
 			"Error parsing %s: %s", path, err)
 	}
 
-	var result map[string]string
+	var result map[string]interface{}
 	if err := hcl.DecodeObject(&result, obj); err != nil {
 		return nil, fmt.Errorf(
 			"Error decoding Terraform vars file: %s\n\n"+
@@ -103,3 +128,49 @@ func (v *FlagStringSlice) Set(raw string) error {
 
 	return nil
 }
+
+// parseVarFlagAsHCL parses the value of a single variable as would have been specified
+// on the command line via -var or in an environment variable named TF_VAR_x, where x is
+// the name of the variable. In order to get around the restriction of HCL requiring a
+// top level object, we prepend a sentinel key, decode the user-specified value as its
+// value and pull the value back out of the resulting map.
+func parseVarFlagAsHCL(input string) (string, interface{}, error) {
+	idx := strings.Index(input, "=")
+	if idx == -1 {
+		return "", nil, fmt.Errorf("No '=' value in variable: %s", input)
+	}
+	probablyName := input[0:idx]
+
+	parsed, err := hcl.Parse(input)
+	if err != nil {
+		// This covers flags of the form `foo=bar` which is not valid HCL
+		// At this point, probablyName is actually the name, and the remainder
+		// of the expression after the equals sign is the value.
+		if regexp.MustCompile(`Unknown token: \d+:\d+ IDENT`).Match([]byte(err.Error())) {
+			value := input[idx+1:]
+			return probablyName, value, nil
+		}
+		return "", nil, fmt.Errorf("Cannot parse value for variable %s (%q) as valid HCL: %s", probablyName, input, err)
+	}
+
+	var decoded map[string]interface{}
+	if hcl.DecodeObject(&decoded, parsed); err != nil {
+		return "", nil, fmt.Errorf("Cannot parse value for variable %s (%q) as valid HCL: %s", probablyName, input, err)
+	}
+
+	// Cover cases such as key=
+	if len(decoded) == 0 {
+		return probablyName, "", nil
+	}
+
+	if len(decoded) > 1 {
+		return "", nil, fmt.Errorf("Cannot parse value for variable %s (%q) as valid HCL. Only one value may be specified.", probablyName, input)
+	}
+
+	for k, v := range decoded {
+		return k, v, nil
+	}
+
+	// Should be unreachable
+	return "", nil, fmt.Errorf("No value for variable: %s", input)
+}

command/flag_kv_test.go

@@ -2,16 +2,17 @@ package command
 
 import (
 	"flag"
+	"github.com/davecgh/go-spew/spew"
 	"io/ioutil"
 	"reflect"
 	"testing"
 )
 
-func TestFlagKV_impl(t *testing.T) {
+func TestFlagStringKV_impl(t *testing.T) {
-	var _ flag.Value = new(FlagKV)
+	var _ flag.Value = new(FlagStringKV)
 }
 
-func TestFlagKV(t *testing.T) {
+func TestFlagStringKV(t *testing.T) {
 	cases := []struct {
 		Input  string
 		Output map[string]string
@@ -49,10 +50,10 @@ func TestFlagKV(t *testing.T) {
 	}
 
 	for _, tc := range cases {
-		f := new(FlagKV)
+		f := new(FlagStringKV)
 		err := f.Set(tc.Input)
 		if err != nil != tc.Error {
-			t.Fatalf("bad error. Input: %#v", tc.Input)
+			t.Fatalf("bad error. Input: %#v\n\nError: %s", tc.Input, err)
 		}
 
 		actual := map[string]string(*f)
@@ -62,6 +63,86 @@ func TestFlagKV(t *testing.T) {
 	}
 }
 
+func TestFlagTypedKV_impl(t *testing.T) {
+	var _ flag.Value = new(FlagTypedKV)
+}
+
+func TestFlagTypedKV(t *testing.T) {
+	cases := []struct {
+		Input  string
+		Output map[string]interface{}
+		Error  bool
+	}{
+		{
+			"key=value",
+			map[string]interface{}{"key": "value"},
+			false,
+		},
+
+		{
+			"key=",
+			map[string]interface{}{"key": ""},
+			false,
+		},
+
+		{
+			"key=foo=bar",
+			map[string]interface{}{"key": "foo=bar"},
+			false,
+		},
+
+		{
+			"map.key=foo",
+			map[string]interface{}{"map.key": "foo"},
+			false,
+		},
+
+		{
+			"key",
+			nil,
+			true,
+		},
+
+		{
+			`key=["hello", "world"]`,
+			map[string]interface{}{"key": []interface{}{"hello", "world"}},
+			false,
+		},
+
+		{
+			`key={"hello" = "world", "foo" = "bar"}`,
+			map[string]interface{}{
+				"key": []map[string]interface{}{
+					map[string]interface{}{
+						"hello": "world",
+						"foo":   "bar",
+					},
+				},
+			},
+			false,
+		},
+
+		{
+			`key={"hello" = "world", "foo" = "bar"}\nkey2="invalid"`,
+			nil,
+			true,
+		},
+	}
+
+	for _, tc := range cases {
+		f := new(FlagTypedKV)
+		err := f.Set(tc.Input)
+		if err != nil != tc.Error {
+			t.Fatalf("bad error. Input: %#v\n\nError: %s", tc.Input, err)
+		}
+
+		actual := map[string]interface{}(*f)
+		if !reflect.DeepEqual(actual, tc.Output) {
+			t.Fatalf("bad:\nexpected: %s\n\n     got: %s\n", spew.Sdump(tc.Output), spew.Sdump(actual))
+		}
+	}
+}
+
 func TestFlagKVFile_impl(t *testing.T) {
 	var _ flag.Value = new(FlagKVFile)
 }
@@ -76,24 +157,24 @@ foo = "bar"
 
 	cases := []struct {
 		Input  string
-		Output map[string]string
+		Output map[string]interface{}
 		Error  bool
 	}{
 		{
 			inputLibucl,
-			map[string]string{"foo": "bar"},
+			map[string]interface{}{"foo": "bar"},
 			false,
 		},
 
 		{
 			inputJson,
-			map[string]string{"foo": "bar"},
+			map[string]interface{}{"foo": "bar"},
 			false,
 		},
 
 		{
 			`map.key = "foo"`,
-			map[string]string{"map.key": "foo"},
+			map[string]interface{}{"map.key": "foo"},
 			false,
 		},
 	}
@@ -111,7 +192,7 @@ foo = "bar"
 			t.Fatalf("bad error. Input: %#v, err: %s", tc.Input, err)
 		}
 
-		actual := map[string]string(*f)
+		actual := map[string]interface{}(*f)
 		if !reflect.DeepEqual(actual, tc.Output) {
 			t.Fatalf("bad: %#v", actual)
 		}

command/init.go

@@ -25,7 +25,7 @@ func (c *InitCommand) Run(args []string) int {
 	remoteConfig := make(map[string]string)
 	cmdFlags := flag.NewFlagSet("init", flag.ContinueOnError)
 	cmdFlags.StringVar(&remoteBackend, "backend", "", "")
-	cmdFlags.Var((*FlagKV)(&remoteConfig), "backend-config", "config")
+	cmdFlags.Var((*FlagStringKV)(&remoteConfig), "backend-config", "config")
 	cmdFlags.Usage = func() { c.Ui.Error(c.Help()) }
 	if err := cmdFlags.Parse(args); err != nil {
 		return 1

command/meta.go

@@ -36,9 +36,9 @@ type Meta struct {
 
 	// Variables for the context (private)
 	autoKey       string
-	autoVariables map[string]string
+	autoVariables map[string]interface{}
 	input         bool
-	variables     map[string]string
+	variables     map[string]interface{}
 
 	// Targets for this context (private)
 	targets []string
@@ -315,7 +315,7 @@ func (m *Meta) contextOpts() *terraform.ContextOpts {
 func (m *Meta) flagSet(n string) *flag.FlagSet {
 	f := flag.NewFlagSet(n, flag.ContinueOnError)
 	f.BoolVar(&m.input, "input", true, "input")
-	f.Var((*FlagKV)(&m.variables), "var", "variables")
+	f.Var((*FlagTypedKV)(&m.variables), "var", "variables")
 	f.Var((*FlagKVFile)(&m.variables), "var-file", "variable file")
 	f.Var((*FlagStringSlice)(&m.targets), "target", "resource to target")
 

command/remote_config.go

@@ -38,7 +38,7 @@ func (c *RemoteConfigCommand) Run(args []string) int {
 	cmdFlags.StringVar(&c.conf.statePath, "state", DefaultStateFilename, "path")
 	cmdFlags.StringVar(&c.conf.backupPath, "backup", "", "path")
 	cmdFlags.StringVar(&c.remoteConf.Type, "backend", "atlas", "")
-	cmdFlags.Var((*FlagKV)(&config), "backend-config", "config")
+	cmdFlags.Var((*FlagStringKV)(&config), "backend-config", "config")
 	cmdFlags.Usage = func() { c.Ui.Error(c.Help()) }
 	if err := cmdFlags.Parse(args); err != nil {
 		c.Ui.Error(fmt.Sprintf("\nError parsing CLI flags: %s", err))

terraform/context.go

@@ -9,6 +9,7 @@ import (
 	"sync"
 
 	"github.com/hashicorp/go-multierror"
+	"github.com/hashicorp/hcl"
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/config/module"
 )
@@ -119,24 +120,109 @@ func NewContext(opts *ContextOpts) (*Context, error) {
 		par = 10
 	}
 
-	// Setup the variables. We first take the variables given to us.
+	// Set up the variables in the following sequence:
-	// We then merge in the variables set in the environment.
+	//    0 - Take default values from the configuration
+	//    1 - Take values from TF_VAR_x environment variables
+	//    2 - Take values specified in -var flags, overriding values
+	//        set by environment variables if necessary. This includes
+	//        values taken from -var-file in addition.
 	variables := make(map[string]interface{})
-	for _, v := range os.Environ() {
+
-		if !strings.HasPrefix(v, VarEnvPrefix) {
+	if opts.Module != nil {
-			continue
+		for _, v := range opts.Module.Config().Variables {
+			if v.Default != nil {
+				if v.Type() == config.VariableTypeString {
+					// v.Default has already been parsed as HCL so there may be
+					// some stray ints in there
+					switch typedDefault := v.Default.(type) {
+					case string:
+						if typedDefault == "" {
+							continue
+						}
+						variables[v.Name] = typedDefault
+					case int, int64:
+						variables[v.Name] = fmt.Sprintf("%d", typedDefault)
+					case float32, float64:
+						variables[v.Name] = fmt.Sprintf("%f", typedDefault)
+					case bool:
+						variables[v.Name] = fmt.Sprintf("%t", typedDefault)
+					}
+				} else {
+					variables[v.Name] = v.Default
+				}
+			}
 		}
 
-		// Strip off the prefix and get the value after the first "="
+		for _, v := range os.Environ() {
-		idx := strings.Index(v, "=")
+			if !strings.HasPrefix(v, VarEnvPrefix) {
-		k := v[len(VarEnvPrefix):idx]
+				continue
-		v = v[idx+1:]
+			}
 
-		// Override the command-line set variable
+			// Strip off the prefix and get the value after the first "="
-		variables[k] = v
+			idx := strings.Index(v, "=")
-	}
+			k := v[len(VarEnvPrefix):idx]
-	for k, v := range opts.Variables {
+			v = v[idx+1:]
-		variables[k] = v
+
+			// Override the configuration-default values. Note that *not* finding the variable
+			// in configuration is OK, as we don't want to preclude people from having multiple
+			// sets of TF_VAR_whatever in their environment even if it is a little weird.
+			for _, schema := range opts.Module.Config().Variables {
+				if schema.Name == k {
+					varType := schema.Type()
+					varVal, err := parseVariableAsHCL(k, v, varType)
+					if err != nil {
+						return nil, err
+					}
+					switch varType {
+					case config.VariableTypeMap:
+						if existing, hasMap := variables[k]; !hasMap {
+							variables[k] = varVal
+						} else {
+							if existingMap, ok := existing.(map[string]interface{}); !ok {
+								panic(fmt.Sprintf("%s is not a map, this is a bug in Terraform.", k))
+							} else {
+								if newMap, ok := varVal.(map[string]interface{}); ok {
+									for newKey, newVal := range newMap {
+										existingMap[newKey] = newVal
+									}
+								} else {
+									panic(fmt.Sprintf("%s is not a map, this is a bug in Terraform.", k))
+								}
+							}
+						}
+					default:
+						variables[k] = varVal
+					}
+				}
+			}
+		}
+
+		for k, v := range opts.Variables {
+			for _, schema := range opts.Module.Config().Variables {
+				if schema.Name == k {
+					switch schema.Type() {
+					case config.VariableTypeMap:
+						if existing, hasMap := variables[k]; !hasMap {
+							variables[k] = v
+						} else {
+							if existingMap, ok := existing.(map[string]interface{}); !ok {
+								panic(fmt.Sprintf("%s is not a map, this is a bug in Terraform.", k))
+							} else {
+								if newMap, ok := v.([]map[string]interface{}); ok {
+									for newKey, newVal := range newMap[0] {
+										existingMap[newKey] = newVal
+									}
+								} else {
+									panic(fmt.Sprintf("%s is not a map, this is a bug in Terraform.", k))
+								}
+							}
+						}
+					default:
+						variables[k] = v
+					}
+				}
+			}
+		}
 	}
 
 	return &Context{
@@ -548,3 +634,45 @@ func (c *Context) walk(
 	walker := &ContextGraphWalker{Context: c, Operation: operation}
 	return walker, graph.Walk(walker)
 }
+
+// parseVariableAsHCL parses the value of a single variable as would have been specified
+// on the command line via -var or in an environment variable named TF_VAR_x, where x is
+// the name of the variable. In order to get around the restriction of HCL requiring a
+// top level object, we prepend a sentinel key, decode the user-specified value as its
+// value and pull the value back out of the resulting map.
+func parseVariableAsHCL(name string, input interface{}, targetType config.VariableType) (interface{}, error) {
+	if targetType == config.VariableTypeString {
+		return input, nil
+	}
+
+	const sentinelValue = "SENTINEL_TERRAFORM_VAR_OVERRIDE_KEY"
+	inputWithSentinal := fmt.Sprintf("%s = %s", sentinelValue, input)
+
+	var decoded map[string]interface{}
+	err := hcl.Decode(&decoded, inputWithSentinal)
+	if err != nil {
+		return nil, fmt.Errorf("Cannot parse value for variable %s (%q) as valid HCL: %s", name, input, err)
+	}
+
+	if len(decoded) != 1 {
+		return nil, fmt.Errorf("Cannot parse value for variable %s (%q) as valid HCL. Only one value may be specified.", name, input)
+	}
+
+	parsedValue, ok := decoded[sentinelValue]
+	if !ok {
+		return nil, fmt.Errorf("Cannot parse value for variable %s (%q) as valid HCL. One value must be specified.", name, input)
+	}
+
+	switch targetType {
+	case config.VariableTypeList:
+		return parsedValue, nil
+	case config.VariableTypeMap:
+		if list, ok := parsedValue.([]map[string]interface{}); ok {
+			return list[0], nil
+		}
+
+		return nil, fmt.Errorf("Cannot parse value for variable %s (%q) as valid HCL. One value must be specified.", name, input)
+	default:
+		panic(fmt.Errorf("unknown type %s", targetType))
+	}
+}

terraform/context_apply_test.go

@@ -1135,7 +1135,11 @@ func TestContext2Apply_mapVariableOverride(t *testing.T) {
 			"aws": testProviderFuncFixed(p),
 		},
 		Variables: map[string]interface{}{
-			"images.us-west-2": "overridden",
+			"images": []map[string]interface{}{
+				map[string]interface{}{
+					"us-west-2": "overridden",
+				},
+			},
 		},
 	})
 
@@ -4269,8 +4273,18 @@ func TestContext2Apply_vars(t *testing.T) {
 			"aws": testProviderFuncFixed(p),
 		},
 		Variables: map[string]interface{}{
-			"foo":            "us-west-2",
+			"foo":       "us-west-2",
-			"amis.us-east-1": "override",
+			"test_list": []interface{}{"Hello", "World"},
+			"test_map": map[string]interface{}{
+				"Hello": "World",
+				"Foo":   "Bar",
+				"Baz":   "Foo",
+			},
+			"amis": []map[string]interface{}{
+				map[string]interface{}{
+					"us-east-1": "override",
+				},
+			},
 		},
 	})
 
@@ -4300,8 +4314,13 @@ func TestContext2Apply_vars(t *testing.T) {
 
 func TestContext2Apply_varsEnv(t *testing.T) {
 	// Set the env var
-	old := tempEnv(t, "TF_VAR_ami", "baz")
+	old_ami := tempEnv(t, "TF_VAR_ami", "baz")
-	defer os.Setenv("TF_VAR_ami", old)
+	old_list := tempEnv(t, "TF_VAR_list", `["Hello", "World"]`)
+	old_map := tempEnv(t, "TF_VAR_map", `{"Hello" = "World", "Foo" = "Bar", "Baz" = "Foo"}`)
+
+	defer os.Setenv("TF_VAR_ami", old_ami)
+	defer os.Setenv("TF_VAR_list", old_list)
+	defer os.Setenv("TF_VAR_list", old_map)
 
 	m := testModule(t, "apply-vars-env")
 	p := testProvider("aws")

terraform/context_input_test.go

@@ -19,8 +19,12 @@ func TestContext2Input(t *testing.T) {
 			"aws": testProviderFuncFixed(p),
 		},
 		Variables: map[string]interface{}{
-			"foo":            "us-west-2",
+			"foo": "us-west-2",
-			"amis.us-east-1": "override",
+			"amis": []map[string]interface{}{
+				map[string]interface{}{
+					"us-east-1": "override",
+				},
+			},
 		},
 		UIInput: input,
 	})

terraform/semantics.go

@@ -95,16 +95,42 @@ func smcUserVariables(c *config.Config, vs map[string]interface{}) []error {
 	}
 
 	// Check that types match up
-	for k, _ := range vs {
+	for name, proposedValue := range vs {
-		v, ok := cvs[k]
+		schema, ok := cvs[name]
 		if !ok {
 			continue
 		}
 
-		if v.Type() != config.VariableTypeString {
+		declaredType := schema.Type()
-			errs = append(errs, fmt.Errorf(
+
-				"%s: cannot assign string value to map type",
+		switch declaredType {
-				k))
+		case config.VariableTypeString:
+			switch proposedValue.(type) {
+			case string:
+				continue
+			default:
+				errs = append(errs, fmt.Errorf("variable %s should be type %s, got %s",
+					name, declaredType.Printable(), hclTypeName(proposedValue)))
+			}
+		case config.VariableTypeMap:
+			switch proposedValue.(type) {
+			case map[string]interface{}:
+				continue
+			default:
+				errs = append(errs, fmt.Errorf("variable %s should be type %s, got %s",
+					name, declaredType.Printable(), hclTypeName(proposedValue)))
+			}
+		case config.VariableTypeList:
+			switch proposedValue.(type) {
+			case []interface{}:
+				continue
+			default:
+				errs = append(errs, fmt.Errorf("variable %s should be type %s, got %s",
+					name, declaredType.Printable(), hclTypeName(proposedValue)))
+			}
+		default:
+			errs = append(errs, fmt.Errorf("variable %s should be type %s, got %s",
+				name, declaredType.Printable(), hclTypeName(proposedValue)))
 		}
 	}
 

terraform/terraform_test.go

@@ -705,6 +705,8 @@ aws_instance.bar:
 aws_instance.foo:
   ID = foo
   bar = baz
+  list = Hello,World
+  map = Baz,Foo,Hello
   num = 2
   type = aws_instance
 `
@@ -712,6 +714,8 @@ aws_instance.foo:
 const testTerraformApplyVarsEnvStr = `
 aws_instance.bar:
   ID = foo
+  bar = Hello,World
+  baz = Baz,Foo,Hello
   foo = baz
   type = aws_instance
 `

terraform/test-fixtures/apply-vars-env/main.tf

@@ -1,7 +1,20 @@
 variable "ami" {
-    default = "foo"
+  default = "foo"
+  type    = "string"
+}
+
+variable "list" {
+  default = []
+  type    = "list"
+}
+
+variable "map" {
+  default = {}
+  type = "map"
 }
 
 resource "aws_instance" "bar" {
-    foo = "${var.ami}"
+  foo = "${var.ami}"
+  bar = "${join(",", var.list)}"
+  baz = "${join(",", keys(var.map))}"
 }

terraform/test-fixtures/apply-vars/main.tf

@@ -5,6 +5,14 @@ variable "amis" {
     }
 }
 
+variable "test_list" {
+    type = "list"
+}
+
+variable "test_map" {
+    type = "map"
+}
+
 variable "bar" {
     default = "baz"
 }
@@ -14,6 +22,8 @@ variable "foo" {}
 resource "aws_instance" "foo" {
     num = "2"
     bar = "${var.bar}"
+    list = "${join(",", var.test_list)}"
+    map = "${join(",", keys(var.test_map))}"
 }
 
 resource "aws_instance" "bar" {