Merge pull request #8101 from hashicorp/dnephin/decode-hook-slice-interfaces

decode: recursively unslice opaque config

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
 }

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)
+}