Merge pull request #7493 from hashicorp/b-pass-map-to-module

terraform: allow literal maps to be passed to modules

terraform/context_plan_test.go

@@ -2286,3 +2286,42 @@ func TestContext2Plan_ignoreChanges(t *testing.T) {
 		t.Fatalf("bad:\n%s\n\nexpected\n\n%s", actual, expected)
 	}
 }
+
+func TestContext2Plan_moduleMapLiteral(t *testing.T) {
+	m := testModule(t, "plan-module-map-literal")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = func(i *InstanceInfo, s *InstanceState, c *ResourceConfig) (*InstanceDiff, error) {
+		// Here we verify that both the populated and empty map literals made it
+		// through to the resource attributes
+		val, _ := c.Get("tags")
+		m, ok := val.(map[string]interface{})
+		if !ok {
+			t.Fatalf("Tags attr not map: %#v", val)
+		}
+		if m["foo"] != "bar" {
+			t.Fatalf("Bad value in tags attr: %#v", m)
+		}
+		{
+			val, _ := c.Get("meta")
+			m, ok := val.(map[string]interface{})
+			if !ok {
+				t.Fatalf("Meta attr not map: %#v", val)
+			}
+			if len(m) != 0 {
+				t.Fatalf("Meta attr not empty: %#v", val)
+			}
+		}
+		return nil, nil
+	}
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+	})
+
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+}

terraform/eval_variable.go

@@ -2,6 +2,7 @@ package terraform
 
 import (
 	"fmt"
+	"log"
 	"reflect"
 	"strings"
 
@@ -21,10 +22,6 @@ import (
 //       declared
 //     - the path to the module (so we know which part of the tree to
 //       compare the values against).
-//
-// Currently since the type system is simple, we currently do not make
-// use of the values since it is only valid to pass string values. The
-// structure is in place for extension of the type system, however.
 type EvalTypeCheckVariable struct {
 	Variables  map[string]interface{}
 	ModulePath []string
@@ -50,10 +47,6 @@ func (n *EvalTypeCheckVariable) Eval(ctx EvalContext) (interface{}, error) {
 	}
 
 	for name, declaredType := range prototypes {
-		// This is only necessary when we _actually_ check. It is left as a reminder
-		// that at the current time we are dealing with a type system consisting only
-		// of strings and maps - where the only valid inter-module variable type is
-		// string.
 		proposedValue, ok := n.Variables[name]
 		if !ok {
 			// This means the default value should be used as no overriding value
@@ -67,8 +60,6 @@ func (n *EvalTypeCheckVariable) Eval(ctx EvalContext) (interface{}, error) {
 
 		switch declaredType {
 		case config.VariableTypeString:
-			// This will need actual verification once we aren't dealing with
-			// a map[string]string but this is sufficient for now.
 			switch proposedValue.(type) {
 			case string:
 				continue
@@ -93,8 +84,6 @@ func (n *EvalTypeCheckVariable) Eval(ctx EvalContext) (interface{}, error) {
 					name, modulePathDescription, declaredType.Printable(), hclTypeName(proposedValue))
 			}
 		default:
-			// This will need the actual type substituting when we have more than
-			// just strings and maps.
 			return nil, fmt.Errorf("variable %s%s should be type %s, got type string",
 				name, modulePathDescription, declaredType.Printable())
 		}
@@ -163,6 +152,54 @@ func (n *EvalVariableBlock) Eval(ctx EvalContext) (interface{}, error) {
 	return nil, nil
 }
 
+// EvalCoerceMapVariable is an EvalNode implementation that recognizes a
+// specific ambiguous HCL parsing situation and resolves it. In HCL parsing, a
+// bare map literal is indistinguishable from a list of maps w/ one element.
+//
+// We take all the same inputs as EvalTypeCheckVariable above, since we need
+// both the target type and the proposed value in order to properly coerce.
+type EvalCoerceMapVariable struct {
+	Variables  map[string]interface{}
+	ModulePath []string
+	ModuleTree *module.Tree
+}
+
+// Eval implements the EvalNode interface. See EvalCoerceMapVariable for
+// details.
+func (n *EvalCoerceMapVariable) Eval(ctx EvalContext) (interface{}, error) {
+	currentTree := n.ModuleTree
+	for _, pathComponent := range n.ModulePath[1:] {
+		currentTree = currentTree.Children()[pathComponent]
+	}
+	targetConfig := currentTree.Config()
+
+	prototypes := make(map[string]config.VariableType)
+	for _, variable := range targetConfig.Variables {
+		prototypes[variable.Name] = variable.Type()
+	}
+
+	for name, declaredType := range prototypes {
+		if declaredType != config.VariableTypeMap {
+			continue
+		}
+
+		proposedValue, ok := n.Variables[name]
+		if !ok {
+			continue
+		}
+
+		if list, ok := proposedValue.([]interface{}); ok && len(list) == 1 {
+			if m, ok := list[0].(map[string]interface{}); ok {
+				log.Printf("[DEBUG] EvalCoerceMapVariable: "+
+					"Coercing single element list into map: %#v", m)
+				n.Variables[name] = m
+			}
+		}
+	}
+
+	return nil, nil
+}
+
 // hclTypeName returns the name of the type that would represent this value in
 // a config file, or falls back to the Go type name if there's no corresponding
 // HCL type. This is used for formatted output, not for comparing types.

terraform/eval_variable_test.go

@@ -0,0 +1,142 @@
+package terraform
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestCoerceMapVariable(t *testing.T) {
+	cases := map[string]struct {
+		Input      *EvalCoerceMapVariable
+		ExpectVars map[string]interface{}
+	}{
+		"a valid map is untouched": {
+			Input: &EvalCoerceMapVariable{
+				Variables: map[string]interface{}{
+					"amap": map[string]interface{}{"foo": "bar"},
+				},
+				ModulePath: []string{"root"},
+				ModuleTree: testModuleInline(t, map[string]string{
+					"main.tf": `
+						variable "amap" {
+							type = "map"
+						}
+					`,
+				}),
+			},
+			ExpectVars: map[string]interface{}{
+				"amap": map[string]interface{}{"foo": "bar"},
+			},
+		},
+		"a list w/ a single map element is coerced": {
+			Input: &EvalCoerceMapVariable{
+				Variables: map[string]interface{}{
+					"amap": []interface{}{
+						map[string]interface{}{"foo": "bar"},
+					},
+				},
+				ModulePath: []string{"root"},
+				ModuleTree: testModuleInline(t, map[string]string{
+					"main.tf": `
+						variable "amap" {
+							type = "map"
+						}
+					`,
+				}),
+			},
+			ExpectVars: map[string]interface{}{
+				"amap": map[string]interface{}{"foo": "bar"},
+			},
+		},
+		"a list w/ more than one map element is untouched": {
+			Input: &EvalCoerceMapVariable{
+				Variables: map[string]interface{}{
+					"amap": []interface{}{
+						map[string]interface{}{"foo": "bar"},
+						map[string]interface{}{"baz": "qux"},
+					},
+				},
+				ModulePath: []string{"root"},
+				ModuleTree: testModuleInline(t, map[string]string{
+					"main.tf": `
+						variable "amap" {
+							type = "map"
+						}
+					`,
+				}),
+			},
+			ExpectVars: map[string]interface{}{
+				"amap": []interface{}{
+					map[string]interface{}{"foo": "bar"},
+					map[string]interface{}{"baz": "qux"},
+				},
+			},
+		},
+		"list coercion also works in a module": {
+			Input: &EvalCoerceMapVariable{
+				Variables: map[string]interface{}{
+					"amap": []interface{}{
+						map[string]interface{}{"foo": "bar"},
+					},
+				},
+				ModulePath: []string{"root", "middle", "bottom"},
+				ModuleTree: testModuleInline(t, map[string]string{
+					"top.tf": `
+						module "middle" {
+							source = "./middle"
+						}
+					`,
+					"middle/mid.tf": `
+						module "bottom" {
+							source = "./bottom"
+							amap {
+								foo = "bar"
+							}
+						}
+					`,
+					"middle/bottom/bot.tf": `
+						variable "amap" {
+							type = "map"
+						}
+					`,
+				}),
+			},
+			ExpectVars: map[string]interface{}{
+				"amap": map[string]interface{}{"foo": "bar"},
+			},
+		},
+		"coercion only occurs when target var is a map": {
+			Input: &EvalCoerceMapVariable{
+				Variables: map[string]interface{}{
+					"alist": []interface{}{
+						map[string]interface{}{"foo": "bar"},
+					},
+				},
+				ModulePath: []string{"root"},
+				ModuleTree: testModuleInline(t, map[string]string{
+					"main.tf": `
+						variable "alist" {
+							type = "list"
+						}
+					`,
+				}),
+			},
+			ExpectVars: map[string]interface{}{
+				"alist": []interface{}{
+					map[string]interface{}{"foo": "bar"},
+				},
+			},
+		},
+	}
+
+	for tn, tc := range cases {
+		_, err := tc.Input.Eval(&MockEvalContext{})
+		if err != nil {
+			t.Fatalf("%s: Unexpected err: %s", tn, err)
+		}
+		if !reflect.DeepEqual(tc.Input.Variables, tc.ExpectVars) {
+			t.Fatalf("%s: Expected variables:\n\n%#v\n\nGot:\n\n%#v",
+				tn, tc.ExpectVars, tc.Input.Variables)
+		}
+	}
+}

terraform/graph_config_node_variable.go

@@ -179,6 +179,12 @@ func (n *GraphNodeConfigVariable) EvalTree() EvalNode {
 				VariableValues: variables,
 			},
 
+			&EvalCoerceMapVariable{
+				Variables:  variables,
+				ModulePath: n.ModulePath,
+				ModuleTree: n.ModuleTree,
+			},
+
 			&EvalTypeCheckVariable{
 				Variables:  variables,
 				ModulePath: n.ModulePath,

terraform/test-fixtures/plan-module-map-literal/child/main.tf

@@ -0,0 +1,12 @@
+variable "amap" {
+  type = "map"
+}
+
+variable "othermap" {
+  type = "map"
+}
+
+resource "aws_instance" "foo" {
+  tags = "${var.amap}"
+  meta = "${var.othermap}"
+}

terraform/test-fixtures/plan-module-map-literal/main.tf

@@ -0,0 +1,7 @@
+module "child" {
+  source = "./child"
+  amap {
+    foo = "bar"
+  }
+  othermap {}
+}