From e16f85944b9caff98e5e22164bcb80e494e65389 Mon Sep 17 00:00:00 2001 From: Alejandro Ruiz Date: Thu, 24 Apr 2025 12:29:29 +0200 Subject: [PATCH] Small refactor in tests --- pkg/resources/formatters/formatter_test.go | 268 +++++++++++---------- 1 file changed, 144 insertions(+), 124 deletions(-) diff --git a/pkg/resources/formatters/formatter_test.go b/pkg/resources/formatters/formatter_test.go index 7bf045b2..68c54266 100644 --- a/pkg/resources/formatters/formatter_test.go +++ b/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 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 +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{}{ - "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 +244,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 +291,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 +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,24 +377,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 {