Allow deeply nested property definition for Helm properties (#4511)

* Allow deeply nested property definition for Helm properties like `ArtifactOverrides`, `SetValues` and `SetValueTemplates`

Author: gsquared94

docs/content/en/schemas/v2beta6.json

@@ -1243,13 +1243,8 @@
       ],
       "properties": {
         "artifactOverrides": {
-          "additionalProperties": {
-            "type": "string"
-          },
-          "type": "object",
           "description": "key value pairs. If present, Skaffold will send `--set-string` flag to Helm CLI and append all pairs after the flag.",
-          "x-intellij-html-description": "key value pairs. If present, Skaffold will send <code>--set-string</code> flag to Helm CLI and append all pairs after the flag.",
-          "default": "{}"
+          "x-intellij-html-description": "key value pairs. If present, Skaffold will send <code>--set-string</code> flag to Helm CLI and append all pairs after the flag."
         },
         "chartPath": {
           "type": "string",
@@ -1302,22 +1297,12 @@
           "default": "{}"
         },
         "setValueTemplates": {
-          "additionalProperties": {
-            "type": "string"
-          },
-          "type": "object",
           "description": "key-value pairs. If present, Skaffold will try to parse the value part of each key-value pair using environment variables in the system, then send `--set` flag to Helm CLI and append all parsed pairs after the flag.",
-          "x-intellij-html-description": "key-value pairs. If present, Skaffold will try to parse the value part of each key-value pair using environment variables in the system, then send <code>--set</code> flag to Helm CLI and append all parsed pairs after the flag.",
-          "default": "{}"
+          "x-intellij-html-description": "key-value pairs. If present, Skaffold will try to parse the value part of each key-value pair using environment variables in the system, then send <code>--set</code> flag to Helm CLI and append all parsed pairs after the flag."
         },
         "setValues": {
-          "additionalProperties": {
-            "type": "string"
-          },
-          "type": "object",
           "description": "key-value pairs. If present, Skaffold will send `--set` flag to Helm CLI and append all pairs after the flag.",
-          "x-intellij-html-description": "key-value pairs. If present, Skaffold will send <code>--set</code> flag to Helm CLI and append all pairs after the flag.",
-          "default": "{}"
+          "x-intellij-html-description": "key-value pairs. If present, Skaffold will send <code>--set</code> flag to Helm CLI and append all pairs after the flag."
         },
         "skipBuildDependencies": {
           "type": "boolean",

integration/examples/helm-deployment-dependencies/skaffold.yaml

@@ -16,7 +16,8 @@ deploy:
       skipBuildDependencies: true # Skip helm dep build
       artifactOverrides:
         image: skaffold-helm
-        "skaffold-helm-subchart.image": skaffold-helm
+        skaffold-helm-subchart:
+          image: skaffold-helm
       #recreatePods will pass --recreate-pods to helm upgrade
       #recreatePods: true
       #overrides builds an override values.yaml file to run with the helm deploy
@@ -25,4 +26,5 @@ deploy:
       #   key: someValue
       #setValues get appended to the helm deploy with --set.
       #setValues:
-        #some.key: someValue
+      # some:
+      #   key: someValue

pkg/skaffold/schema/latest/config.go

@@ -510,7 +510,7 @@ type HelmRelease struct {
 
 	// ArtifactOverrides are key value pairs.
 	// If present, Skaffold will send `--set-string` flag to Helm CLI and append all pairs after the flag.
-	ArtifactOverrides map[string]string `yaml:"artifactOverrides,omitempty,omitempty"`
+	ArtifactOverrides util.FlatMap `yaml:"artifactOverrides,omitempty,omitempty"`
 
 	// Namespace is the Kubernetes namespace.
 	Namespace string `yaml:"namespace,omitempty"`
@@ -520,13 +520,13 @@ type HelmRelease struct {
 
 	// SetValues are key-value pairs.
 	// If present, Skaffold will send `--set` flag to Helm CLI and append all pairs after the flag.
-	SetValues map[string]string `yaml:"setValues,omitempty"`
+	SetValues util.FlatMap `yaml:"setValues,omitempty"`
 
 	// SetValueTemplates are key-value pairs.
 	// If present, Skaffold will try to parse the value part of each key-value pair using
 	// environment variables in the system, then send `--set` flag to Helm CLI and append
 	// all parsed pairs after the flag.
-	SetValueTemplates map[string]string `yaml:"setValueTemplates,omitempty"`
+	SetValueTemplates util.FlatMap `yaml:"setValueTemplates,omitempty"`
 
 	// SetFiles are key-value pairs.
 	// If present, Skaffold will send `--set-file` flag to Helm CLI and append all pairs after the flag.

pkg/skaffold/schema/util/util.go

@@ -18,6 +18,7 @@ package util
 
 import (
 	"encoding/json"
+	"fmt"
 	"reflect"
 	"strings"
 
@@ -36,6 +37,9 @@ type HelmOverrides struct {
 	Values map[string]interface{} `yaml:",inline"`
 }
 
+// FlatMap flattens deeply nested yaml into a map with corresponding dot separated keys
+type FlatMap map[string]string
+
 // MarshalJSON implements JSON marshalling by including the value as an inline yaml fragment.
 func (h *HelmOverrides) MarshalJSON() ([]byte, error) {
 	return marshalInlineYaml(h)
@@ -82,6 +86,44 @@ func (n *YamlpatchNode) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	return n.Node.UnmarshalYAML(unmarshal)
 }
 
+func (m *FlatMap) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	var obj map[string]interface{}
+	if err := unmarshal(&obj); err != nil {
+		return err
+	}
+	result := make(map[string]string)
+	if err := buildFlatMap(obj, result, ""); err != nil {
+		return err
+	}
+	*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)
+		}
+
+		switch v := v.(type) {
+		case map[string]interface{}:
+			if err = buildFlatMap(v, result, currK); err != nil {
+				return
+			}
+		case string:
+			result[currK] = v
+		default:
+			result[currK] = fmt.Sprintf("%v", v)
+		}
+		currK = prevK
+	}
+	return
+}
+
 func marshalInlineYaml(in interface{}) ([]byte, error) {
 	yaml, err := yaml.Marshal(in)
 	if err != nil {

pkg/skaffold/schema/util/util_test.go

@@ -90,3 +90,40 @@ func TestYamlpatchNodeWhenEmbedded(t *testing.T) {
     localstack: {}
 `, string(out))
 }
+
+func TestFlatMap_UnmarshalYAML(t *testing.T) {
+	y1 := `val1: foo1
+val2: 
+  val3: bar1
+  val4: foo2
+  val5:
+    val6: bar2
+`
+	y2 := `val1: foo1
+val2.val3: bar1
+val2.val4: foo2
+val2.val5.val6: bar2
+`
+
+	f1 := &FlatMap{}
+	f2 := &FlatMap{}
+
+	err := yaml.Unmarshal([]byte(y1), &f1)
+	testutil.CheckError(t, false, err)
+
+	err = yaml.Unmarshal([]byte(y2), &f2)
+	testutil.CheckError(t, false, err)
+
+	testutil.CheckDeepEqual(t, *f1, *f2)
+
+	out, err := yaml.Marshal(struct {
+		M *FlatMap `yaml:"value,omitempty"`
+	}{f1})
+
+	testutil.CheckErrorAndDeepEqual(t, false, err, `value:
+  val1: foo1
+  val2.val3: bar1
+  val2.val4: foo2
+  val2.val5.val6: bar2
+`, string(out))
+}