Small refactor in tests

pkg/resources/formatters/formatter_test.go

@@ -5,31 +5,19 @@ 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"
 	"testing"
 
+	"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"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	pbchart "k8s.io/helm/pkg/proto/hapi/chart"
+	rspb "k8s.io/helm/pkg/proto/hapi/release"
 )
 
-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",
 	Chart: &pbchart.Chart{
@@ -48,41 +36,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: "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{}{
+			resource: newSecret("helm", map[string]any{
-				"release": base64.StdEncoding.EncodeToString([]byte(newV3Release())),
+				"release": toGzippedBase64(t, helmRelease),
 			}),
 			request: newRequest("true"),
-			want: newSecret("helm", map[string]interface{}{
+			want: newSecret("helm", map[string]any{
-				"release": &r,
+				"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{}{
+			resource: newSecret("helm", map[string]any{
-				"release": base64.StdEncoding.EncodeToString([]byte(newV3Release())),
+				"release": toGzippedBase64(t, helmRelease),
 			}),
 			request: newRequest("false"),
-			want:        newSecret("helm", map[string]interface{}{}),
+			want:    newSecret("helm", map[string]any{}),
-			helmVersion: 3,
 		},
 		{
 			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{}{
+			resource: newSecret("helm", map[string]any{
-				"release": base64.StdEncoding.EncodeToString([]byte(newV3Release())),
+				"release": base64.StdEncoding.EncodeToString([]byte(toGzippedBase64(t, helmRelease))),
 			}),
 			request: newRequest(""),
-			want:        newSecret("helm", map[string]interface{}{}),
+			want:    newSecret("helm", map[string]any{}),
-			helmVersion: 3,
 		},
 		{
 			name: "When receiving a non-helm SECRET or CONFIGMAP resource with includeHelmData set to TRUE, it shouldn't change the resource",
@@ -107,7 +107,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 +131,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 +155,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{}{
+			resource: newConfigMap("helm", map[string]any{
-				"release": newV3Release(),
+				"release": toGzippedBase64(t, helmRelease),
 			}),
 			request: newRequest("true"),
-			want: newConfigMap("helm", map[string]interface{}{
+			want: newConfigMap("helm", map[string]any{
-				"release": &r,
+				"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{}{
+			resource: newConfigMap("helm", map[string]any{
-				"release": newV3Release(),
+				"release": toGzippedBase64(t, helmRelease),
 			}),
 			request: newRequest("false"),
-			want:        newConfigMap("helm", map[string]interface{}{}),
+			want:    newConfigMap("helm", map[string]any{}),
-			helmVersion: 3,
 		},
 		{
 			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{}{
+			resource: newConfigMap("helm", map[string]any{
-				"release": newV3Release(),
+				"release": toGzippedBase64(t, helmRelease),
 			}),
 			request: newRequest(""),
-			want:        newConfigMap("helm", map[string]interface{}{}),
+			want:    newConfigMap("helm", map[string]any{}),
-			helmVersion: 3,
 		},
-		//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,
 		},
 		{
 			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,
 		},
 		{
 			name: "When receiving a CONFIGMAP resource with includeHelmData set to TRUE and owner set to TILLER, it should decode the helm2 release",
@@ -227,7 +244,6 @@ 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",
@@ -236,7 +252,6 @@ func Test_HandleHelmData(t *testing.T) {
 			}),
 			request: newRequest("false"),
 			want:    newConfigMap("TILLER", map[string]interface{}{}),
-			helmVersion: 2,
 		},
 		{
 			name: "When receiving a CONFIGMAP resource WITHOUT the includeHelmData query parameter and owner set to TILLER, it should drop the helm data",
@@ -245,35 +260,21 @@ func Test_HandleHelmData(t *testing.T) {
 			}),
 			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,
 		},
 		{
 			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)
 			assert.True(t, ok)
 			rl, ok := u.UnstructuredContent()["data"].(map[string]interface{})["release"]
@@ -286,24 +287,50 @@ func Test_HandleHelmData(t *testing.T) {
 			} else {
 				assert.Equal(t, tt.resource, tt.want)
 			}
-			} else {
-				assert.Equal(t, tt.resource, tt.want)
-			}
 		})
 	}
 }
 
-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 +344,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,26 +377,20 @@ func newV2ReleaseWithoutGzip() string {
 	return base64.StdEncoding.EncodeToString(b)
 }
 
-func newV3Release() string {
+func toGzippedBase64(t *testing.T, v string) string {
-	b, err := json.Marshal(r)
+	t.Helper()
-	if err != nil {
+	var buf bytes.Buffer
-		logrus.Errorf("Failed to marshal release: %v", err)
-	}
-	buf := bytes.Buffer{}
 	gz := gzip.NewWriter(&buf)
-	gz.Write(b)
+	defer gz.Close()
-	gz.Close()
+	if _, err := gz.Write([]byte(v)); err != nil {
+		t.Fatal(err)
+	}
+	if err := gz.Close(); err != nil {
+		t.Fatal(err)
+	}
 	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)
-	}
-	return base64.StdEncoding.EncodeToString(b)
-}
-
 func newRequest(value string) *types.APIRequest {
 	req := &types.APIRequest{Query: url.Values{}}
 	if value != "" {