Fix helm.values field to support mutiple values (#109) (#113)

helm.values field type used to be map[string]interface{}. Looping through the map to generate a env variable leads to a inconsistent env variable issue.
Since we change the helm.values field type to JSON already, we can pass
it through env variable directly, and process it later in helm-sync.

e2e/testcases/helm_sync_test.go

@@ -19,6 +19,7 @@ import (
 	"testing"
 
 	appsv1 "k8s.io/api/apps/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/types"
 	"kpt.dev/configsync/e2e/nomostest"
 	"kpt.dev/configsync/e2e/nomostest/gitproviders"
@@ -166,7 +167,9 @@ func TestHelmARTokenAuth(t *testing.T) {
 		nt.T.Fatalf("failed to create secret, err: %v", err)
 	}
 	nt.T.Log("Update RootSync to sync from a private Artifact Registry")
-	nt.MustMergePatch(rs, fmt.Sprintf(`{"spec": {"sourceType": "%s", "helm": {"repo": "%s", "chart": "%s", "auth": "token", "version": "%s", "releaseName": "my-coredns", "namespace": "coredns", "secretRef": {"name" : "foo"}}, "git": null}}`,
+	nt.MustMergePatch(rs, fmt.Sprintf(`{"spec": {"sourceType": "%s", "git": null, "helm": {"repo": "%s", "chart": "%s", "auth": "token", "version": "%s", "releaseName": "my-coredns", "namespace": "coredns",
+                                                                                                "values": {"image.pullPolicy": "Always", "resources.requests.memory": "250Mi", "resources.requests.cpu": "150m", "resources.limits.memory": "300Mi", "resources.limits.cpu": 1},
+                                                                                                "secretRef": {"name" : "foo"}}}}`,
 		v1beta1.HelmSource, privateARHelmRegistry, privateHelmChart, privateHelmChartVersion))
 	nt.T.Cleanup(func() {
 		// Change the rs back so that it works in the shared test environment.
@@ -176,7 +179,7 @@ func TestHelmARTokenAuth(t *testing.T) {
 	nt.WaitForRepoSyncs(nomostest.WithRootSha1Func(helmChartVersion(privateHelmChart+":"+privateHelmChartVersion)),
 		nomostest.WithSyncDirectoryMap(map[types.NamespacedName]string{nomostest.DefaultRootRepoNamespacedName: privateHelmChart}))
 	if err := nt.Validate("my-coredns-coredns", "coredns", &appsv1.Deployment{},
-		containerImagePullPolicy("IfNotPresent")); err != nil {
+		containerImagePullPolicy("Always"), nomostest.HasCorrectResourceRequestsLimits("coredns", resource.MustParse("150m"), resource.MustParse("1"), resource.MustParse("250Mi"), resource.MustParse("300Mi"))); err != nil {
 		nt.T.Error(err)
 	}
 }

pkg/helm/helm.go

@@ -16,6 +16,7 @@ package helm
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"os"
 	"os/exec"
@@ -70,7 +71,10 @@ func (h *Hydrator) templateArgs(ctx context.Context, destDir string) ([]string,
 		args = append(args, "--version", h.Version)
 	}
 	if len(h.Values) > 0 {
-		args = append(args, "--set", h.Values)
+		args, err = h.appendValuesArgs(args)
+		if err != nil {
+			return []string{}, err
+		}
 	}
 	includeCRDs, _ := strconv.ParseBool(h.IncludeCRDs)
 	if includeCRDs {
@@ -80,6 +84,18 @@ func (h *Hydrator) templateArgs(ctx context.Context, destDir string) ([]string,
 	return args, nil
 }
 
+func (h *Hydrator) appendValuesArgs(args []string) ([]string, error) {
+	var values map[string]interface{}
+	err := json.Unmarshal([]byte(h.Values), &values)
+	if err != nil {
+		return []string{}, fmt.Errorf("failed to unmarshal helm.values, error: %w", err)
+	}
+	for key, val := range values {
+		args = append(args, "--set", fmt.Sprintf("%s=%v", key, val))
+	}
+	return args, nil
+}
+
 func (h *Hydrator) registryLoginArgs(ctx context.Context) ([]string, error) {
 	args := []string{"registry", "login"}
 	args, err := h.appendAuthArgs(ctx, args)

pkg/reconcilermanager/controllers/util.go

@@ -15,15 +15,12 @@
 package controllers
 
 import (
-	"encoding/json"
 	"fmt"
 	"os"
-	"strings"
 	"time"
 
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/klog/v2"
 	"k8s.io/utils/pointer"
 	"kpt.dev/configsync/pkg/api/configsync"
 	"kpt.dev/configsync/pkg/api/configsync/v1beta1"
@@ -212,16 +209,8 @@ const (
 func helmSyncEnvs(helmBase *v1beta1.HelmBase, releaseNamespace string) []corev1.EnvVar {
 	var result []corev1.EnvVar
 	helmValues := ""
-	var values map[string]interface{}
-
 	if helmBase.Values != nil {
-		err := json.Unmarshal(helmBase.Values.Raw, &values)
-		klog.Errorf("failed to unmarshal helm.values, error: %w", err)
-		var vals []string
-		for key, val := range values {
-			vals = append(vals, fmt.Sprintf("%s=%v", key, val))
-		}
-		helmValues = strings.Join(vals, ",")
+		helmValues = string(helmBase.Values.Raw)
 	}
 	result = append(result, corev1.EnvVar{
 		Name:  reconcilermanager.HelmRepo,