From 6cc9c57bed34ae1c0a3eafd0a1a453a80ff193d5 Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Fri, 21 Jun 2024 19:21:50 +0200 Subject: [PATCH] refactor: remove use of reflections (#2634) ## Description This change removes any direct import of the reflection package. Use of reflections still exists but these are wrapped in the helpers package. Use of deep equals has been removed from tests as require equals uses deep equals as part of its test. ## Related Issue N/A ## Checklist before merging - [x] Test, docs, adr added or updated as needed - [x] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/.github/CONTRIBUTING.md#developer-workflow) followed Co-authored-by: Austin Abro <37223396+AustinAbro321@users.noreply.github.com> --- src/internal/packager/helm/post-render.go | 5 ++-- src/pkg/cluster/secrets.go | 7 ++--- src/pkg/packager/deploy_test.go | 5 +--- src/pkg/packager/filters/deploy_test.go | 31 +++++------------------ src/pkg/utils/sort_test.go | 5 +--- 5 files changed, 16 insertions(+), 37 deletions(-) diff --git a/src/internal/packager/helm/post-render.go b/src/internal/packager/helm/post-render.go index 92b912ae63..1dcdbc6912 100644 --- a/src/internal/packager/helm/post-render.go +++ b/src/internal/packager/helm/post-render.go @@ -8,9 +8,9 @@ import ( "bytes" "context" "fmt" + "maps" "os" "path/filepath" - "reflect" "slices" "github.com/defenseunicorns/pkg/helpers/v2" @@ -163,7 +163,8 @@ func (r *renderer) adoptAndUpdateNamespaces(ctx context.Context) error { validRegistrySecret := c.GenerateRegistryPullCreds(name, config.ZarfImagePullSecretName, r.state.RegistryInfo) // TODO: Refactor as error is not checked instead of checking for not found error. currentRegistrySecret, _ := c.Clientset.CoreV1().Secrets(name).Get(ctx, config.ZarfImagePullSecretName, metav1.GetOptions{}) - if currentRegistrySecret.Name != config.ZarfImagePullSecretName || !reflect.DeepEqual(currentRegistrySecret.Data, validRegistrySecret.Data) { + sameSecretData := maps.EqualFunc(currentRegistrySecret.Data, validRegistrySecret.Data, func(v1, v2 []byte) bool { return bytes.Equal(v1, v2) }) + if currentRegistrySecret.Name != config.ZarfImagePullSecretName || !sameSecretData { err := func() error { _, err := c.Clientset.CoreV1().Secrets(validRegistrySecret.Namespace).Create(ctx, validRegistrySecret, metav1.CreateOptions{}) if err != nil && !kerrors.IsAlreadyExists(err) { diff --git a/src/pkg/cluster/secrets.go b/src/pkg/cluster/secrets.go index cfbb90953e..a7af5449f6 100644 --- a/src/pkg/cluster/secrets.go +++ b/src/pkg/cluster/secrets.go @@ -5,10 +5,11 @@ package cluster import ( + "bytes" "context" "encoding/base64" "encoding/json" - "reflect" + "maps" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -123,7 +124,7 @@ func (c *Cluster) UpdateZarfManagedImageSecrets(ctx context.Context, state *type spinner.Updatef("Updating existing Zarf-managed image secret for namespace: '%s'", namespace.Name) newRegistrySecret := c.GenerateRegistryPullCreds(namespace.Name, config.ZarfImagePullSecretName, state.RegistryInfo) - if !reflect.DeepEqual(currentRegistrySecret.Data, newRegistrySecret.Data) { + if !maps.EqualFunc(currentRegistrySecret.Data, newRegistrySecret.Data, func(v1, v2 []byte) bool { return bytes.Equal(v1, v2) }) { _, err := c.Clientset.CoreV1().Secrets(newRegistrySecret.Namespace).Update(ctx, newRegistrySecret, metav1.UpdateOptions{}) if err != nil { message.WarnErrf(err, "Problem creating registry secret for the %s namespace", namespace.Name) @@ -159,7 +160,7 @@ func (c *Cluster) UpdateZarfManagedGitSecrets(ctx context.Context, state *types. // Create the secret newGitSecret := c.GenerateGitPullCreds(namespace.Name, config.ZarfGitServerSecretName, state.GitServer) - if !reflect.DeepEqual(currentGitSecret.StringData, newGitSecret.StringData) { + if !maps.Equal(currentGitSecret.StringData, newGitSecret.StringData) { _, err := c.Clientset.CoreV1().Secrets(newGitSecret.Namespace).Update(ctx, newGitSecret, metav1.UpdateOptions{}) if err != nil { message.WarnErrf(err, "Problem creating git server secret for the %s namespace", namespace.Name) diff --git a/src/pkg/packager/deploy_test.go b/src/pkg/packager/deploy_test.go index b4b7cb9df2..8c69d1682f 100644 --- a/src/pkg/packager/deploy_test.go +++ b/src/pkg/packager/deploy_test.go @@ -4,7 +4,6 @@ package packager import ( - "reflect" "testing" "github.com/defenseunicorns/zarf/src/pkg/packager/sources" @@ -222,9 +221,7 @@ func TestGenerateValuesOverrides(t *testing.T) { if err != nil { t.Errorf("%s: generateValuesOverrides() error = %v", tt.name, err) } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("%s: generateValuesOverrides() got = %v, want %v", tt.name, got, tt.want) - } + require.Equal(t, tt.want, got) }) } } diff --git a/src/pkg/packager/filters/deploy_test.go b/src/pkg/packager/filters/deploy_test.go index 029a2a9d53..1bb548f1cb 100644 --- a/src/pkg/packager/filters/deploy_test.go +++ b/src/pkg/packager/filters/deploy_test.go @@ -6,7 +6,6 @@ package filters import ( "fmt" - "reflect" "strings" "testing" @@ -115,7 +114,7 @@ func TestDeployFilter_Apply(t *testing.T) { possibilities := componentMatrix(t) - testCases := map[string]struct { + tests := map[string]struct { pkg types.ZarfPackage optionalComponents string want []types.ZarfComponent @@ -196,35 +195,19 @@ func TestDeployFilter_Apply(t *testing.T) { }, } - for name, tc := range testCases { + for name, tt := range tests { t.Run(name, func(t *testing.T) { // we do not currently support interactive mode in unit tests isInteractive := false - filter := ForDeploy(tc.optionalComponents, isInteractive) + filter := ForDeploy(tt.optionalComponents, isInteractive) - result, err := filter.Apply(tc.pkg) - if tc.expectedErr != nil { - require.ErrorIs(t, err, tc.expectedErr) + result, err := filter.Apply(tt.pkg) + if tt.expectedErr != nil { + require.ErrorIs(t, err, tt.expectedErr) } else { require.NoError(t, err) } - equal := reflect.DeepEqual(tc.want, result) - if !equal { - left := []string{} - right := []string{} - - for _, c := range tc.want { - left = append(left, c.Name) - } - - for _, c := range result { - right = append(right, c.Name) - fmt.Printf("componentFromQuery(t, %q),\n", strings.TrimSpace(c.Name)) - } - - // cause the test to fail - require.FailNow(t, "expected and actual are not equal", "\n\nexpected: %#v\n\nactual: %#v", left, right) - } + require.Equal(t, tt.want, result) }) } } diff --git a/src/pkg/utils/sort_test.go b/src/pkg/utils/sort_test.go index 2cd2a29c42..beee3e160b 100644 --- a/src/pkg/utils/sort_test.go +++ b/src/pkg/utils/sort_test.go @@ -5,7 +5,6 @@ package utils import ( - "reflect" "testing" "github.com/stretchr/testify/require" @@ -169,9 +168,7 @@ func TestSortDependencies(t *testing.T) { } else { require.Error(t, err) } - if !reflect.DeepEqual(result, tt.expected) { - t.Errorf("expected %v but got %v", tt.expected, result) - } + require.Equal(t, tt.expected, result) }) } }