core: Defeat backward compatibilty in mapstructure

The mapstructure library has a regrettable backward compatibility
concern whereby a WeakDecode of []interface{}{} into a target of
map[string]interface{} yields an empty map rather than an error. One
possibility is to switch to using Decode instead of WeakDecode, but this
loses the nice handling of type conversion, requiring a large volume of
code to be added to Terraform or HIL in order to retain that behaviour.

Instead we add a DecodeHook to our usage of the mapstructure library
which checks for decoding []interface{}{} or []string{} into a map and
returns an error instead.

This has the effect of defeating the code added to retain backwards
compatibility in mapstructure, giving us the correct (for our
circumstances) behaviour of Decode for empty structures and the type
conversion of WeakDecode.

The code is identical to that in the HIL library, and packaged into a
helper.

config/config.go

@@ -11,7 +11,7 @@ import (
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/hil"
 	"github.com/hashicorp/hil/ast"
-	"github.com/mitchellh/mapstructure"
+	"github.com/hashicorp/terraform/helper/hilmapstructure"
 	"github.com/mitchellh/reflectwalk"
 )
 
@@ -368,19 +368,19 @@ func (c *Config) Validate() error {
 		raw := make(map[string]interface{})
 		for k, v := range m.RawConfig.Raw {
 			var strVal string
-			if err := mapstructure.WeakDecode(v, &strVal); err == nil {
+			if err := hilmapstructure.WeakDecode(v, &strVal); err == nil {
 				raw[k] = strVal
 				continue
 			}
 
 			var mapVal map[string]interface{}
-			if err := mapstructure.WeakDecode(v, &mapVal); err == nil {
+			if err := hilmapstructure.WeakDecode(v, &mapVal); err == nil {
 				raw[k] = mapVal
 				continue
 			}
 
 			var sliceVal []interface{}
-			if err := mapstructure.WeakDecode(v, &sliceVal); err == nil {
+			if err := hilmapstructure.WeakDecode(v, &sliceVal); err == nil {
 				raw[k] = sliceVal
 				continue
 			}
@@ -919,19 +919,19 @@ func (v *Variable) inferTypeFromDefault() VariableType {
 	}
 
 	var s string
-	if err := mapstructure.WeakDecode(v.Default, &s); err == nil {
+	if err := hilmapstructure.WeakDecode(v.Default, &s); err == nil {
 		v.Default = s
 		return VariableTypeString
 	}
 
 	var m map[string]string
-	if err := mapstructure.WeakDecode(v.Default, &m); err == nil {
+	if err := hilmapstructure.WeakDecode(v.Default, &m); err == nil {
 		v.Default = m
 		return VariableTypeMap
 	}
 
 	var l []string
-	if err := mapstructure.WeakDecode(v.Default, &l); err == nil {
+	if err := hilmapstructure.WeakDecode(v.Default, &l); err == nil {
 		v.Default = l
 		return VariableTypeList
 	}

helper/hilmapstructure/hilmapstructure.go

@@ -0,0 +1,41 @@
+package hilmapstructure
+
+import (
+	"fmt"
+	"reflect"
+
+	"github.com/mitchellh/mapstructure"
+)
+
+var hilMapstructureDecodeHookEmptySlice []interface{}
+var hilMapstructureDecodeHookStringSlice []string
+var hilMapstructureDecodeHookEmptyMap map[string]interface{}
+
+// WeakDecode behaves in the same way as mapstructure.WeakDecode but has a
+// DecodeHook which defeats the backward compatibility mode of mapstructure
+// which WeakDecodes []interface{}{} into an empty map[string]interface{}. This
+// allows us to use WeakDecode (desirable), but not fail on empty lists.
+func WeakDecode(m interface{}, rawVal interface{}) error {
+	config := &mapstructure.DecoderConfig{
+		DecodeHook: func(source reflect.Type, target reflect.Type, val interface{}) (interface{}, error) {
+			sliceType := reflect.TypeOf(hilMapstructureDecodeHookEmptySlice)
+			stringSliceType := reflect.TypeOf(hilMapstructureDecodeHookStringSlice)
+			mapType := reflect.TypeOf(hilMapstructureDecodeHookEmptyMap)
+
+			if (source == sliceType || source == stringSliceType) && target == mapType {
+				return nil, fmt.Errorf("Cannot convert a []interface{} into a map[string]interface{}")
+			}
+
+			return val, nil
+		},
+		WeaklyTypedInput: true,
+		Result:           rawVal,
+	}
+
+	decoder, err := mapstructure.NewDecoder(config)
+	if err != nil {
+		return err
+	}
+
+	return decoder.Decode(m)
+}

terraform/eval_variable.go

@@ -6,7 +6,7 @@ import (
 
 	"github.com/hashicorp/terraform/config"
 	"github.com/hashicorp/terraform/config/module"
-	"github.com/mitchellh/mapstructure"
+	"github.com/hashicorp/terraform/helper/hilmapstructure"
 )
 
 // EvalTypeCheckVariable is an EvalNode which ensures that the variable
@@ -134,19 +134,19 @@ func (n *EvalVariableBlock) Eval(ctx EvalContext) (interface{}, error) {
 	rc := *n.Config
 	for k, v := range rc.Config {
 		var vString string
-		if err := mapstructure.WeakDecode(v, &vString); err == nil {
+		if err := hilmapstructure.WeakDecode(v, &vString); err == nil {
 			n.VariableValues[k] = vString
 			continue
 		}
 
 		var vMap map[string]interface{}
-		if err := mapstructure.WeakDecode(v, &vMap); err == nil {
+		if err := hilmapstructure.WeakDecode(v, &vMap); err == nil {
 			n.VariableValues[k] = vMap
 			continue
 		}
 
 		var vSlice []interface{}
-		if err := mapstructure.WeakDecode(v, &vSlice); err == nil {
+		if err := hilmapstructure.WeakDecode(v, &vSlice); err == nil {
 			n.VariableValues[k] = vSlice
 			continue
 		}

terraform/interpolate_test.go

@@ -7,6 +7,7 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/davecgh/go-spew/spew"
 	"github.com/hashicorp/hil"
 	"github.com/hashicorp/hil/ast"
 	"github.com/hashicorp/terraform/config"
@@ -527,7 +528,9 @@ func testInterpolate(
 		"foo": expectedVar,
 	}
 	if !reflect.DeepEqual(actual, expected) {
-		t.Fatalf("%q: actual: %#v\nexpected: %#v", n, actual, expected)
+		spew.Config.DisableMethods = true
+		t.Fatalf("%q: actual: %#v\nexpected: %#v\n\n%s\n\n%s\n\n", n, actual, expected,
+			spew.Sdump(actual), spew.Sdump(expected))
 	}
 }