From 1a9d7802ed09fb702f73f81175370b75af6d6358 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 15 Jun 2020 13:33:55 -0400 Subject: [PATCH] Merge pull request #8101 from hashicorp/dnephin/decode-hook-slice-interfaces decode: recursively unslice opaque config --- lib/decode/decode.go | 70 ++++++++++++++++++++++++----- lib/decode/decode_test.go | 92 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 147 insertions(+), 15 deletions(-) diff --git a/lib/decode/decode.go b/lib/decode/decode.go index b098e43135..6730d47f85 100644 --- a/lib/decode/decode.go +++ b/lib/decode/decode.go @@ -7,6 +7,8 @@ package decode import ( "reflect" "strings" + + "github.com/mitchellh/reflectwalk" ) // HookTranslateKeys is a mapstructure decode hook which translates keys in a @@ -92,11 +94,13 @@ func canonicalFieldKey(field reflect.StructField) string { return parts[0] } -// HookWeakDecodeFromSlice looks for []map[string]interface{} in the source -// data. If the target is not a slice or array it attempts to unpack 1 item -// out of the slice. If there are more items the source data is left unmodified, -// allowing mapstructure to handle and report the decode error caused by -// mismatched types. +// HookWeakDecodeFromSlice looks for []map[string]interface{} and []interface{} +// in the source data. If the target is not a slice or array it attempts to unpack +// 1 item out of the slice. If there are more items the source data is left +// unmodified, allowing mapstructure to handle and report the decode error caused by +// mismatched types. The []interface{} is handled so that all slice types are +// behave the same way, and for the rare case when a raw structure is re-encoded +// to JSON, which will produce the []interface{}. // // If this hook is being used on a "second pass" decode to decode an opaque // configuration into a type, the DecodeConfig should set WeaklyTypedInput=true, @@ -121,14 +125,56 @@ func HookWeakDecodeFromSlice(from, to reflect.Type, data interface{}) (interface switch d := data.(type) { case []map[string]interface{}: switch { - case len(d) == 0: - return nil, nil - case len(d) == 1: - return d[0], nil - default: + case len(d) != 1: return data, nil + case to == typeOfEmptyInterface: + return unSlice(d[0]) + default: + return d[0], nil + } + + // a slice map be encoded as []interface{} in some cases + case []interface{}: + switch { + case len(d) != 1: + return data, nil + case to == typeOfEmptyInterface: + return unSlice(d[0]) + default: + return d[0], nil } - default: - return data, nil } + return data, nil +} + +var typeOfEmptyInterface = reflect.TypeOf((*interface{})(nil)).Elem() + +func unSlice(data interface{}) (interface{}, error) { + err := reflectwalk.Walk(data, &unSliceWalker{}) + return data, err +} + +type unSliceWalker struct{} + +func (u *unSliceWalker) Map(_ reflect.Value) error { + return nil +} + +func (u *unSliceWalker) MapElem(m, k, v reflect.Value) error { + if !v.IsValid() || v.Kind() != reflect.Interface { + return nil + } + + v = v.Elem() // unpack the value from the interface{} + if v.Kind() != reflect.Slice || v.Len() != 1 { + return nil + } + + first := v.Index(0) + // The value should always be assignable, but double check to avoid a panic. + if !first.Type().AssignableTo(m.Type().Elem()) { + return nil + } + m.SetMapIndex(k, first) + return nil } diff --git a/lib/decode/decode_test.go b/lib/decode/decode_test.go index cc2ca1bfb0..72e012b39c 100644 --- a/lib/decode/decode_test.go +++ b/lib/decode/decode_test.go @@ -198,9 +198,11 @@ func TestTranslationsForType(t *testing.T) { } type nested struct { - O map[string]interface{} - Slice []Item - Item Item + O map[string]interface{} + Slice []Item + Item Item + OSlice []map[string]interface{} + Sub *nested } type Item struct { @@ -215,6 +217,14 @@ slice { slice { name = "second" } +item { + name = "solo" +} +sub { + oslice { + something = "v1" + } +} ` target := &nested{} err := decodeHCLToMapStructure(source, target) @@ -222,6 +232,12 @@ slice { expected := &nested{ Slice: []Item{{Name: "first"}, {Name: "second"}}, + Item: Item{Name: "solo"}, + Sub: &nested{ + OSlice: []map[string]interface{}{ + {"something": "v1"}, + }, + }, } require.Equal(t, target, expected) } @@ -242,6 +258,40 @@ func decodeHCLToMapStructure(source string, target interface{}) error { return decoder.Decode(&raw) } +func TestHookWeakDecodeFromSlice_DoesNotModifySliceTargetsFromSliceInterface(t *testing.T) { + raw := map[string]interface{}{ + "slice": []interface{}{map[string]interface{}{"name": "first"}}, + "item": []interface{}{map[string]interface{}{"name": "solo"}}, + "sub": []interface{}{ + map[string]interface{}{ + "OSlice": []interface{}{ + map[string]interface{}{"something": "v1"}, + }, + "item": []interface{}{map[string]interface{}{"name": "subitem"}}, + }, + }, + } + target := &nested{} + decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: HookWeakDecodeFromSlice, + Result: target, + }) + require.NoError(t, err) + err = decoder.Decode(&raw) + + expected := &nested{ + Slice: []Item{{Name: "first"}}, + Item: Item{Name: "solo"}, + Sub: &nested{ + OSlice: []map[string]interface{}{ + {"something": "v1"}, + }, + Item: Item{Name: "subitem"}, + }, + } + require.Equal(t, target, expected) +} + func TestHookWeakDecodeFromSlice_ErrorsWithMultipleNestedBlocks(t *testing.T) { source := ` item { @@ -272,3 +322,39 @@ item { } require.Equal(t, target, expected) } + +func TestHookWeakDecodeFromSlice_NestedOpaqueConfig(t *testing.T) { + source := ` +service { + proxy { + config { + envoy_gateway_bind_addresses { + all-interfaces { + address = "0.0.0.0" + port = 8443 + } + } + } + } +}` + + target := map[string]interface{}{} + err := decodeHCLToMapStructure(source, &target) + require.NoError(t, err) + + expected := map[string]interface{}{ + "service": map[string]interface{}{ + "proxy": map[string]interface{}{ + "config": map[string]interface{}{ + "envoy_gateway_bind_addresses": map[string]interface{}{ + "all-interfaces": map[string]interface{}{ + "address": "0.0.0.0", + "port": 8443, + }, + }, + }, + }, + }, + } + require.Equal(t, target, expected) +}