Refactor HelmData formatter to avoid importing helm dependencies (#607)

* Small refactor in tests

* Small refactor around decoding

* Refactor decodeHelm3 so we can drop the helm dependency

* Vendor helmv2 protobuf types so we can drop the helmv2 old dependency

* go mod tidy

* Add test with sample HelmV2 data

pkg/resources/formatters/formatter_test.go

@@ -5,33 +5,25 @@ import (
 	"compress/gzip"
 	"encoding/base64"
 	"encoding/json"
-	"github.com/golang/protobuf/proto"
-	"github.com/sirupsen/logrus"
-	"github.com/stretchr/testify/assert"
-	"helm.sh/helm/v3/pkg/chart"
-	"helm.sh/helm/v3/pkg/release"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	pbchart "k8s.io/helm/pkg/proto/hapi/chart"
-	rspb "k8s.io/helm/pkg/proto/hapi/release"
 	"net/url"
+	"os"
 	"testing"
 
+	pbchart "github.com/rancher/steve/pkg/resources/formatters/internal/legacytypes/helmv2api"
+	rspb "github.com/rancher/steve/pkg/resources/formatters/internal/legacytypes/helmv2api"
+
+	"github.com/golang/protobuf/proto"
+	"github.com/google/go-cmp/cmp"
 	"github.com/rancher/apiserver/pkg/types"
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	yaml "sigs.k8s.io/yaml/goyaml.v3"
 )
 
-var r = release.Release{
-	Name: "helmV3Release",
-	Chart: &chart.Chart{
-		Values: map[string]interface{}{
-			"key": "value",
-		},
-	},
-	Version:   1,
-	Namespace: "default",
-}
-
 var rv2 = rspb.Release{
-	Name: "helmV3Release",
+	Name: "helmV2Release",
 	Chart: &pbchart.Chart{
 		Metadata: &pbchart.Metadata{
 			Name:    "chartName",
@@ -48,41 +40,53 @@ var rv2 = rspb.Release{
 }
 
 func Test_HandleHelmData(t *testing.T) {
+	const helmRelease = `{
+  "name": "helmV3Release",
+  "chart": {
+    "values": {
+      "key": "value"
+    }
+  },
+  "version": 1,
+  "namespace": "default"
+}
+`
+	var r map[string]any
+	if err := json.Unmarshal([]byte(helmRelease), &r); err != nil {
+		t.Fatal(err)
+	}
+
 	tests := []struct {
-		name        string
-		resource    *types.RawResource
-		request     *types.APIRequest
-		want        *types.RawResource
-		helmVersion int
-	}{ //helm v3
+		name     string
+		resource *types.RawResource
+		request  *types.APIRequest
+		want     *types.RawResource
+	}{
 		{
 			name: "When receiving a SECRET resource with includeHelmData set to TRUE and owner set to HELM, it should decode the helm3 release",
-			resource: newSecret("helm", map[string]interface{}{
-				"release": base64.StdEncoding.EncodeToString([]byte(newV3Release())),
+			resource: newSecret("helm", map[string]any{
+				"release": toGzippedBase64(t, helmRelease),
 			}),
 			request: newRequest("true"),
-			want: newSecret("helm", map[string]interface{}{
-				"release": &r,
+			want: newSecret("helm", map[string]any{
+				"release": r,
 			}),
-			helmVersion: 3,
 		},
 		{
 			name: "When receiving a SECRET resource with includeHelmData set to FALSE and owner set to HELM, it should drop the helm data",
-			resource: newSecret("helm", map[string]interface{}{
-				"release": base64.StdEncoding.EncodeToString([]byte(newV3Release())),
+			resource: newSecret("helm", map[string]any{
+				"release": toGzippedBase64(t, helmRelease),
 			}),
-			request:     newRequest("false"),
-			want:        newSecret("helm", map[string]interface{}{}),
-			helmVersion: 3,
+			request: newRequest("false"),
+			want:    newSecret("helm", map[string]any{}),
 		},
 		{
 			name: "When receiving a SECRET resource WITHOUT the includeHelmData query parameter and owner set to HELM, it should drop the helm data",
-			resource: newSecret("helm", map[string]interface{}{
-				"release": base64.StdEncoding.EncodeToString([]byte(newV3Release())),
+			resource: newSecret("helm", map[string]any{
+				"release": base64.StdEncoding.EncodeToString([]byte(toGzippedBase64(t, helmRelease))),
 			}),
-			request:     newRequest(""),
-			want:        newSecret("helm", map[string]interface{}{}),
-			helmVersion: 3,
+			request: newRequest(""),
+			want:    newSecret("helm", map[string]any{}),
 		},
 		{
 			name: "When receiving a non-helm SECRET or CONFIGMAP resource with includeHelmData set to TRUE, it shouldn't change the resource",
@@ -107,7 +111,6 @@ func Test_HandleHelmData(t *testing.T) {
 					},
 				}}},
 			},
-			helmVersion: 3,
 		},
 		{
 			name: "When receiving a non-helm SECRET or CONFIGMAP resource with includeHelmData set to FALSE, it shouldn't change the resource",
@@ -132,7 +135,6 @@ func Test_HandleHelmData(t *testing.T) {
 					},
 				}}},
 			},
-			helmVersion: 3,
 		},
 		{
 			name: "When receiving a non-helm SECRET or CONFIGMAP resource WITHOUT the includeHelmData query parameter, it shouldn't change the resource",
@@ -157,66 +159,85 @@ func Test_HandleHelmData(t *testing.T) {
 					},
 				}}},
 			},
-			helmVersion: 3,
 		},
 		{
 			name: "When receiving a CONFIGMAP resource with includeHelmData set to TRUE and owner set to HELM, it should decode the helm3 release",
-			resource: newConfigMap("helm", map[string]interface{}{
-				"release": newV3Release(),
+			resource: newConfigMap("helm", map[string]any{
+				"release": toGzippedBase64(t, helmRelease),
 			}),
 			request: newRequest("true"),
-			want: newConfigMap("helm", map[string]interface{}{
-				"release": &r,
+			want: newConfigMap("helm", map[string]any{
+				"release": r,
 			}),
-			helmVersion: 3,
 		},
 		{
 			name: "When receiving a CONFIGMAP resource with includeHelmData set to FALSE and owner set to HELM, it should drop the helm data",
-			resource: newConfigMap("helm", map[string]interface{}{
-				"release": newV3Release(),
+			resource: newConfigMap("helm", map[string]any{
+				"release": toGzippedBase64(t, helmRelease),
 			}),
-			request:     newRequest("false"),
-			want:        newConfigMap("helm", map[string]interface{}{}),
-			helmVersion: 3,
+			request: newRequest("false"),
+			want:    newConfigMap("helm", map[string]any{}),
 		},
 		{
 			name: "When receiving a CONFIGMAP resource WITHOUT the includeHelmData query parameter and owner set to HELM, it should drop the helm data",
-			resource: newConfigMap("helm", map[string]interface{}{
-				"release": newV3Release(),
+			resource: newConfigMap("helm", map[string]any{
+				"release": toGzippedBase64(t, helmRelease),
 			}),
-			request:     newRequest(""),
-			want:        newConfigMap("helm", map[string]interface{}{}),
-			helmVersion: 3,
+			request: newRequest(""),
+			want:    newConfigMap("helm", map[string]any{}),
 		},
-		//helm v2
+		{
+			name: "[no magic gzip] When receiving a SECRET resource with includeHelmData set to TRUE and owner set to HELM, it should decode the helm3 release",
+			resource: newSecret("helm", map[string]any{
+				"release": base64.StdEncoding.EncodeToString([]byte(helmRelease)),
+			}),
+			request: newRequest("true"),
+			want: newSecret("helm", map[string]any{
+				"release": r,
+			}),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			HandleHelmData(tt.request, tt.resource)
+			// data will be serialized, so it makes sense to compare their JSON representation
+			assert.Empty(t, cmp.Diff(toPlainMap(t, tt.resource), toPlainMap(t, tt.want)))
+		})
+	}
+}
+
+func Test_HandleLegacyHelmV2Data(t *testing.T) {
+	tests := []struct {
+		name     string
+		resource *types.RawResource
+		request  *types.APIRequest
+		want     *types.RawResource
+	}{
 		{
 			name: "When receiving a SECRET resource with includeHelmData set to TRUE and owner set to TILLER, it should decode the helm2 release",
 			resource: newSecret("TILLER", map[string]interface{}{
-				"release": base64.StdEncoding.EncodeToString([]byte(newV2Release())),
+				"release": newV2Release(),
 			}),
 			request: newRequest("true"),
 			want: newSecret("TILLER", map[string]interface{}{
 				"release": &rv2,
 			}),
-			helmVersion: 2,
 		},
 		{
 			name: "When receiving a SECRET resource with includeHelmData set to FALSE and owner set to TILLER, it should drop the helm data",
 			resource: newSecret("TILLER", map[string]interface{}{
-				"release": base64.StdEncoding.EncodeToString([]byte(newV2Release())),
+				"release": newV2Release(),
 			}),
-			request:     newRequest("false"),
-			want:        newSecret("TILLER", map[string]interface{}{}),
-			helmVersion: 2,
+			request: newRequest("false"),
+			want:    newSecret("TILLER", map[string]interface{}{}),
 		},
 		{
 			name: "When receiving a SECRET resource WITHOUT the includeHelmData query parameter and owner set to TILLER, it should drop the helm data",
 			resource: newSecret("TILLER", map[string]interface{}{
-				"release": base64.StdEncoding.EncodeToString([]byte(newV2Release())),
+				"release": newV2Release(),
 			}),
-			request:     newRequest(""),
-			want:        newSecret("TILLER", map[string]interface{}{}),
-			helmVersion: 2,
+			request: newRequest(""),
+			want:    newSecret("TILLER", map[string]interface{}{}),
 		},
 		{
 			name: "When receiving a CONFIGMAP resource with includeHelmData set to TRUE and owner set to TILLER, it should decode the helm2 release",
@@ -227,65 +248,46 @@ func Test_HandleHelmData(t *testing.T) {
 			want: newConfigMap("TILLER", map[string]interface{}{
 				"release": &rv2,
 			}),
-			helmVersion: 2,
 		},
 		{
 			name: "When receiving a CONFIGMAP resource with includeHelmData set to FALSE and owner set to TILLER, it should drop the helm data",
 			resource: newConfigMap("TILLER", map[string]interface{}{
 				"release": newV2Release(),
 			}),
-			request:     newRequest("false"),
-			want:        newConfigMap("TILLER", map[string]interface{}{}),
-			helmVersion: 2,
+			request: newRequest("false"),
+			want:    newConfigMap("TILLER", map[string]interface{}{}),
 		},
 		{
 			name: "When receiving a CONFIGMAP resource WITHOUT the includeHelmData query parameter and owner set to TILLER, it should drop the helm data",
 			resource: newConfigMap("TILLER", map[string]interface{}{
 				"release": newV2Release(),
 			}),
-			request:     newRequest(""),
-			want:        newConfigMap("TILLER", map[string]interface{}{}),
-			helmVersion: 2,
-		},
-		{
-			name: "[no magic gzip] When receiving a SECRET resource with includeHelmData set to TRUE and owner set to HELM, it should decode the helm3 release",
-			resource: newSecret("helm", map[string]interface{}{
-				"release": base64.StdEncoding.EncodeToString([]byte(newV3ReleaseWithoutGzip())),
-			}),
-			request: newRequest("true"),
-			want: newSecret("helm", map[string]interface{}{
-				"release": &r,
-			}),
-			helmVersion: 3,
+			request: newRequest(""),
+			want:    newConfigMap("TILLER", map[string]interface{}{}),
 		},
 		{
 			name: "[no magic gzip] When receiving a SECRET resource with includeHelmData set to TRUE and owner set to TILLER, it should decode the helm2 release",
 			resource: newSecret("TILLER", map[string]interface{}{
-				"release": base64.StdEncoding.EncodeToString([]byte(newV2ReleaseWithoutGzip())),
+				"release": newV2ReleaseWithoutGzip(),
 			}),
 			request: newRequest("true"),
 			want: newSecret("TILLER", map[string]interface{}{
 				"release": &rv2,
 			}),
-			helmVersion: 2,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			HandleHelmData(tt.request, tt.resource)
-			if tt.helmVersion == 2 {
-				u, ok := tt.resource.APIObject.Object.(*unstructured.Unstructured)
+			u, ok := tt.resource.APIObject.Object.(*unstructured.Unstructured)
+			assert.True(t, ok)
+			rl, ok := u.UnstructuredContent()["data"].(map[string]interface{})["release"]
+			if ok {
+				u, ok = tt.want.APIObject.Object.(*unstructured.Unstructured)
 				assert.True(t, ok)
-				rl, ok := u.UnstructuredContent()["data"].(map[string]interface{})["release"]
-				if ok {
-					u, ok = tt.want.APIObject.Object.(*unstructured.Unstructured)
-					assert.True(t, ok)
-					rl2, ok := u.UnstructuredContent()["data"].(map[string]interface{})["release"]
-					assert.True(t, ok)
-					assert.True(t, proto.Equal(rl.(proto.Message), rl2.(proto.Message)))
-				} else {
-					assert.Equal(t, tt.resource, tt.want)
-				}
+				rl2, ok := u.UnstructuredContent()["data"].(map[string]interface{})["release"]
+				assert.True(t, ok)
+				assert.True(t, proto.Equal(rl.(proto.Message), rl2.(proto.Message)))
 			} else {
 				assert.Equal(t, tt.resource, tt.want)
 			}
@@ -293,17 +295,46 @@ func Test_HandleHelmData(t *testing.T) {
 	}
 }
 
-func newSecret(owner string, data map[string]interface{}) *types.RawResource {
+func toPlainMap(t *testing.T, v interface{}) map[string]any {
+	t.Helper()
+	var buf bytes.Buffer
+	if err := json.NewEncoder(&buf).Encode(v); err != nil {
+		t.Fatal(err)
+	}
+	var res map[string]any
+	if err := json.NewDecoder(&buf).Decode(&res); err != nil {
+		t.Fatal(err)
+	}
+	pruneNullValues(res)
+	return res
+}
+
+func pruneNullValues(m map[string]any) {
+	for k, v := range m {
+		if v == nil {
+			delete(m, k)
+		} else if mm, ok := v.(map[string]any); ok {
+			pruneNullValues(mm)
+		}
+	}
+	return
+}
+
+func newSecret(owner string, data map[string]any) *types.RawResource {
+	for k, v := range data {
+		if s, ok := v.(string); ok {
+			data[k] = base64.StdEncoding.EncodeToString([]byte(s))
+		}
+	}
 	secret := &unstructured.Unstructured{Object: map[string]interface{}{
 		"apiVersion": "v1",
 		"kind":       "Secret",
 		"data":       data,
 	}}
-	if owner == "helm" {
-		secret.SetLabels(map[string]string{"owner": owner})
-	}
 	if owner == "TILLER" {
 		secret.SetLabels(map[string]string{"OWNER": owner})
+	} else {
+		secret.SetLabels(map[string]string{"owner": owner})
 	}
 	return &types.RawResource{
 		Type:      "secret",
@@ -317,11 +348,10 @@ func newConfigMap(owner string, data map[string]interface{}) *types.RawResource
 		"kind":       "configmap",
 		"data":       data,
 	}}
-	if owner == "helm" {
-		cfgMap.SetLabels(map[string]string{"owner": owner})
-	}
 	if owner == "TILLER" {
 		cfgMap.SetLabels(map[string]string{"OWNER": owner})
+	} else {
+		cfgMap.SetLabels(map[string]string{"owner": owner})
 	}
 	return &types.RawResource{
 		Type:      "configmap",
@@ -351,24 +381,18 @@ func newV2ReleaseWithoutGzip() string {
 	return base64.StdEncoding.EncodeToString(b)
 }
 
-func newV3Release() string {
-	b, err := json.Marshal(r)
-	if err != nil {
-		logrus.Errorf("Failed to marshal release: %v", err)
-	}
-	buf := bytes.Buffer{}
+func toGzippedBase64(t *testing.T, v string) string {
+	t.Helper()
+	var buf bytes.Buffer
 	gz := gzip.NewWriter(&buf)
-	gz.Write(b)
-	gz.Close()
-	return base64.StdEncoding.EncodeToString(buf.Bytes())
-}
-
-func newV3ReleaseWithoutGzip() string {
-	b, err := json.Marshal(r)
-	if err != nil {
-		logrus.Errorf("Failed to marshal release: %v", err)
+	defer gz.Close()
+	if _, err := gz.Write([]byte(v)); err != nil {
+		t.Fatal(err)
 	}
-	return base64.StdEncoding.EncodeToString(b)
+	if err := gz.Close(); err != nil {
+		t.Fatal(err)
+	}
+	return base64.StdEncoding.EncodeToString(buf.Bytes())
 }
 
 func newRequest(value string) *types.APIRequest {
@@ -378,3 +402,39 @@ func newRequest(value string) *types.APIRequest {
 	}
 	return req
 }
+
+func loadTestYamlFile(t *testing.T, path string, into any) {
+	t.Helper()
+	f, err := os.Open(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close()
+
+	if err := yaml.NewDecoder(f).Decode(into); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func Test_decodeHelm2(t *testing.T) {
+	// sample ConfigMap from installing a "mariadb" chart using helm v2.1.7
+	var cm corev1.ConfigMap
+	loadTestYamlFile(t, "testdata/release-cm.yaml", &cm)
+
+	releaseData := cm.Data["release"]
+	if releaseData == "" {
+		t.Fatalf("release data is empty")
+	}
+
+	release, err := decodeHelm2(releaseData)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assert.Equal(t, "mariadb", release.Name)
+	assert.Equal(t, "default", release.Namespace)
+	assert.Equal(t, "mariadb", release.Chart.Metadata.Name)
+	assert.Equal(t, "7.3.14", release.Chart.Metadata.Version)
+	assert.NotEmpty(t, release.Chart.GetValues().GetRaw())
+	assert.Equal(t, "{}\n", release.GetConfig().GetRaw())
+}