From cb9ef298f3946fd6f55f231b993e1a54c3887cda Mon Sep 17 00:00:00 2001 From: James Nugent Date: Fri, 3 Jun 2016 15:57:04 -0500 Subject: [PATCH] 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 | 14 ++++---- helper/hilmapstructure/hilmapstructure.go | 41 +++++++++++++++++++++++ terraform/eval_variable.go | 8 ++--- terraform/interpolate_test.go | 5 ++- 4 files changed, 56 insertions(+), 12 deletions(-) create mode 100644 helper/hilmapstructure/hilmapstructure.go diff --git a/config/config.go b/config/config.go index 848552aac..0dd5d3ff3 100644 --- a/config/config.go +++ b/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 } diff --git a/helper/hilmapstructure/hilmapstructure.go b/helper/hilmapstructure/hilmapstructure.go new file mode 100644 index 000000000..67be1df1f --- /dev/null +++ b/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) +} diff --git a/terraform/eval_variable.go b/terraform/eval_variable.go index 114f8eaa0..635c237e0 100644 --- a/terraform/eval_variable.go +++ b/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 } diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index d85f7b36c..df87890f4 100644 --- a/terraform/interpolate_test.go +++ b/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)) } }