From 9938801f79ca19664b17ae44898c072c6f27dd33 Mon Sep 17 00:00:00 2001
From: Renzo Rojas <renzor@google.com>
Date: Mon, 28 Nov 2022 12:42:49 -0800
Subject: [PATCH] fix: change to make buildFlatMap support arrays as YAML
 values (#8152)

* fix: change to make buildFlatMap support arrays as YAML values

* test: add unit test for unmarshal logic with nested arrays
---
 pkg/skaffold/schema/util/util.go      | 55 +++++++++-------------
 pkg/skaffold/schema/util/util_test.go | 68 +++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 33 deletions(-)

diff --git a/pkg/skaffold/schema/util/util.go b/pkg/skaffold/schema/util/util.go
index 76547887652..15373367099 100644
--- a/pkg/skaffold/schema/util/util.go
+++ b/pkg/skaffold/schema/util/util.go
@@ -92,45 +92,34 @@ func (m *FlatMap) UnmarshalYAML(unmarshal func(interface{}) error) error {
 		return err
 	}
 	result := make(map[string]string)
-	if err := buildFlatMap(obj, result, ""); err != nil {
-		return err
-	}
+	buildFlatMap(obj, result, "")
 	*m = result
 	return nil
 }
 
-func buildFlatMap(obj map[string]interface{}, result map[string]string, currK string) (err error) {
-	var prevK string
-	for k, v := range obj {
-		prevK = currK
-		if currK == "" {
-			currK = fmt.Sprintf("%v", k)
-		} else {
-			currK = fmt.Sprintf("%v.%v", currK, k)
-		}
+func buildFlatList(list []interface{}, result map[string]string, parentK string) {
+	for idx, item := range list {
+		currK := fmt.Sprintf("%v[%d]", parentK, idx)
+		processItem(item, result, currK)
+	}
+}
 
-		switch v := v.(type) {
-		case map[string]interface{}:
-			if err = buildFlatMap(v, result, currK); err != nil {
-				return
-			}
-		case []interface{}:
-			for idx, i := range v {
-				if m, ok := i.(map[string]interface{}); ok {
-					currIdx := fmt.Sprintf("%v[%d]", currK, idx)
-					if err = buildFlatMap(m, result, currIdx); err != nil {
-						return
-					}
-				}
-			}
-		case string:
-			result[currK] = v
-		default:
-			result[currK] = fmt.Sprintf("%v", v)
-		}
-		currK = prevK
+func buildFlatMap(obj map[string]interface{}, result map[string]string, parentK string) {
+	for key, item := range obj {
+		currK := fmt.Sprintf("%v%v", parentK, key)
+		processItem(item, result, currK)
+	}
+}
+
+func processItem(item interface{}, result map[string]string, currK string) {
+	switch v := item.(type) {
+	case map[string]interface{}:
+		buildFlatMap(v, result, fmt.Sprintf("%v.", currK))
+	case []interface{}:
+		buildFlatList(v, result, currK)
+	default:
+		result[currK] = fmt.Sprintf("%v", v)
 	}
-	return err
 }
 
 func marshalInlineYaml(in interface{}) ([]byte, error) {
diff --git a/pkg/skaffold/schema/util/util_test.go b/pkg/skaffold/schema/util/util_test.go
index 9a4a41623e3..269ab39b1c3 100644
--- a/pkg/skaffold/schema/util/util_test.go
+++ b/pkg/skaffold/schema/util/util_test.go
@@ -174,3 +174,71 @@ values.val3[0].val9: foo4
   values.val3[0].val9: foo4
 `, string(out))
 }
+
+func TestFlatMap_UnmarshalYAMLNestedArrays(t *testing.T) {
+	inputYaml := `
+name: nameValue
+configs:
+  name: configName
+  defaults:
+    - prop1: bar1
+      prop2: foo1
+    - prop1: bar2
+      prop2: foo2
+  mixlist:
+    - prop1: bar3
+      prop2:
+       - foo3
+    - element2
+  matrix:
+    - - i1
+      - i2
+    - - j1
+      - j2
+`
+
+	expected := `name: nameValue
+configs.name: configName
+configs.defaults[0].prop1: bar1
+configs.defaults[0].prop2: foo1
+configs.defaults[1].prop1: bar2
+configs.defaults[1].prop2: foo2
+configs.mixlist[0].prop1: bar3
+configs.mixlist[0].prop2[0]: foo3
+configs.mixlist[1]: element2
+configs.matrix[0][0]: i1
+configs.matrix[0][1]: i2
+configs.matrix[1][0]: j1
+configs.matrix[1][1]: j2
+`
+	inputFlatmap := &FlatMap{}
+	expectedFlatmap := &FlatMap{}
+
+	err := yaml.Unmarshal([]byte(inputYaml), &inputFlatmap)
+	testutil.CheckError(t, false, err)
+
+	err = yaml.Unmarshal([]byte(expected), &expectedFlatmap)
+	testutil.CheckError(t, false, err)
+
+	testutil.CheckDeepEqual(t, *inputFlatmap, *expectedFlatmap)
+
+	out, err := yaml.Marshal(struct {
+		M *FlatMap `yaml:"value,omitempty"`
+	}{inputFlatmap})
+
+	testutil.CheckErrorAndDeepEqual(t, false, err, `value:
+  configs.defaults[0].prop1: bar1
+  configs.defaults[0].prop2: foo1
+  configs.defaults[1].prop1: bar2
+  configs.defaults[1].prop2: foo2
+  configs.matrix[0][0]: i1
+  configs.matrix[0][1]: i2
+  configs.matrix[1][0]: j1
+  configs.matrix[1][1]: j2
+  configs.mixlist[0].prop1: bar3
+  configs.mixlist[0].prop2[0]: foo3
+  configs.mixlist[1]: element2
+  configs.name: configName
+  name: nameValue
+`, string(out))
+}