From 29d3ee39dfcbe173405f6e0010cd0dfb9468cd6e Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Mon, 20 May 2024 18:17:24 +0200 Subject: [PATCH] test: add unit tests for merge zarf state (#2522) ## Description Add tests for merge zarf state. ## Related Issue Relates to #2512 ## 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/cmd/tools/zarf.go | 12 +- src/pkg/cluster/state.go | 8 +- src/pkg/cluster/state_test.go | 317 ++++++++++++++++++++++++++++++++++ 3 files changed, 331 insertions(+), 6 deletions(-) create mode 100644 src/pkg/cluster/state_test.go diff --git a/src/cmd/tools/zarf.go b/src/cmd/tools/zarf.go index 20655089bc..bab8eb17a5 100644 --- a/src/cmd/tools/zarf.go +++ b/src/cmd/tools/zarf.go @@ -7,25 +7,27 @@ package tools import ( "fmt" "os" - "slices" "github.com/AlecAivazis/survey/v2" + "github.com/sigstore/cosign/v2/pkg/cosign" + "github.com/spf13/cobra" + "github.com/defenseunicorns/pkg/helpers" "github.com/defenseunicorns/pkg/oci" + "github.com/defenseunicorns/zarf/src/cmd/common" "github.com/defenseunicorns/zarf/src/config" "github.com/defenseunicorns/zarf/src/config/lang" "github.com/defenseunicorns/zarf/src/internal/packager/git" "github.com/defenseunicorns/zarf/src/internal/packager/helm" "github.com/defenseunicorns/zarf/src/internal/packager/template" + "github.com/defenseunicorns/zarf/src/pkg/cluster" "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/defenseunicorns/zarf/src/pkg/packager/sources" "github.com/defenseunicorns/zarf/src/pkg/pki" "github.com/defenseunicorns/zarf/src/pkg/zoci" "github.com/defenseunicorns/zarf/src/types" - "github.com/sigstore/cosign/v2/pkg/cosign" - "github.com/spf13/cobra" ) var subAltNames []string @@ -92,8 +94,8 @@ var updateCredsCmd = &cobra.Command{ // If no distro the zarf secret did not load properly message.Fatalf(nil, lang.ErrLoadState) } - var newState *types.ZarfState - if newState, err = c.MergeZarfState(oldState, updateCredsInitOpts, args); err != nil { + newState, err := cluster.MergeZarfState(oldState, updateCredsInitOpts, args) + if err != nil { message.Fatal(err, lang.CmdToolsUpdateCredsUnableUpdateCreds) } diff --git a/src/pkg/cluster/state.go b/src/pkg/cluster/state.go index 2d51aa38d4..a0414f8793 100644 --- a/src/pkg/cluster/state.go +++ b/src/pkg/cluster/state.go @@ -260,12 +260,14 @@ func (c *Cluster) SaveZarfState(ctx context.Context, state *types.ZarfState) err } // MergeZarfState merges init options for provided services into the provided state to create a new state struct -func (c *Cluster) MergeZarfState(oldState *types.ZarfState, initOptions types.ZarfInitOptions, services []string) (*types.ZarfState, error) { +func MergeZarfState(oldState *types.ZarfState, initOptions types.ZarfInitOptions, services []string) (*types.ZarfState, error) { newState := *oldState var err error if slices.Contains(services, message.RegistryKey) { + // TODO: Replace use of reflections with explicit setting newState.RegistryInfo = helpers.MergeNonZero(newState.RegistryInfo, initOptions.RegistryInfo) // Set the state of the internal registry if it has changed + // TODO: Internal registry should be a function of the address and not a property. if newState.RegistryInfo.Address == fmt.Sprintf("%s:%d", helpers.IPV4Localhost, newState.RegistryInfo.NodePort) { newState.RegistryInfo.InternalRegistry = true } else { @@ -285,9 +287,11 @@ func (c *Cluster) MergeZarfState(oldState *types.ZarfState, initOptions types.Za } } if slices.Contains(services, message.GitKey) { + // TODO: Replace use of reflections with explicit setting newState.GitServer = helpers.MergeNonZero(newState.GitServer, initOptions.GitServer) // Set the state of the internal git server if it has changed + // TODO: Internal server should be a function of the address and not a property. if newState.GitServer.Address == types.ZarfInClusterGitServiceURL { newState.GitServer.InternalServer = true } else { @@ -307,9 +311,11 @@ func (c *Cluster) MergeZarfState(oldState *types.ZarfState, initOptions types.Za } } if slices.Contains(services, message.ArtifactKey) { + // TODO: Replace use of reflections with explicit setting newState.ArtifactServer = helpers.MergeNonZero(newState.ArtifactServer, initOptions.ArtifactServer) // Set the state of the internal artifact server if it has changed + // TODO: Internal server should be a function of the address and not a property. if newState.ArtifactServer.Address == types.ZarfInClusterArtifactServiceURL { newState.ArtifactServer.InternalServer = true } else { diff --git a/src/pkg/cluster/state_test.go b/src/pkg/cluster/state_test.go new file mode 100644 index 0000000000..72f10cd873 --- /dev/null +++ b/src/pkg/cluster/state_test.go @@ -0,0 +1,317 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package cluster contains Zarf-specific cluster management functions. +package cluster + +import ( + "fmt" + "testing" + + "github.com/defenseunicorns/pkg/helpers" + "github.com/defenseunicorns/zarf/src/pkg/message" + "github.com/defenseunicorns/zarf/src/pkg/pki" + "github.com/defenseunicorns/zarf/src/types" + "github.com/stretchr/testify/require" +) + +// TODO: Change password gen method to make testing possible. +func TestMergeZarfStateRegistry(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + initRegistry types.RegistryInfo + oldRegistry types.RegistryInfo + expectedRegistry types.RegistryInfo + }{ + { + name: "username is unmodified", + oldRegistry: types.RegistryInfo{ + PushUsername: "push-user", + PullUsername: "pull-user", + }, + expectedRegistry: types.RegistryInfo{ + PushUsername: "push-user", + PullUsername: "pull-user", + }, + }, + { + name: "internal server auto generate", + oldRegistry: types.RegistryInfo{ + Address: fmt.Sprintf("%s:%d", helpers.IPV4Localhost, 1), + NodePort: 1, + InternalRegistry: true, + }, + expectedRegistry: types.RegistryInfo{ + Address: fmt.Sprintf("%s:%d", helpers.IPV4Localhost, 1), + NodePort: 1, + InternalRegistry: true, + }, + }, + { + name: "external server", + oldRegistry: types.RegistryInfo{ + Address: "example.com", + InternalRegistry: false, + PushPassword: "push", + PullPassword: "pull", + }, + expectedRegistry: types.RegistryInfo{ + Address: "example.com", + InternalRegistry: false, + PushPassword: "push", + PullPassword: "pull", + }, + }, + { + name: "init options merged", + initRegistry: types.RegistryInfo{ + PushUsername: "push-user", + PullUsername: "pull-user", + Address: "address", + NodePort: 1, + InternalRegistry: false, + Secret: "secret", + }, + expectedRegistry: types.RegistryInfo{ + PushUsername: "push-user", + PullUsername: "pull-user", + Address: "address", + NodePort: 1, + InternalRegistry: false, + Secret: "secret", + }, + }, + { + name: "init options not merged", + expectedRegistry: types.RegistryInfo{ + PushUsername: "", + PullUsername: "", + Address: "", + NodePort: 0, + InternalRegistry: false, + Secret: "", + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + oldState := &types.ZarfState{ + RegistryInfo: tt.oldRegistry, + } + newState, err := MergeZarfState(oldState, types.ZarfInitOptions{RegistryInfo: tt.initRegistry}, []string{message.RegistryKey}) + require.NoError(t, err) + require.Equal(t, tt.expectedRegistry.PushUsername, newState.RegistryInfo.PushUsername) + require.Equal(t, tt.expectedRegistry.PullUsername, newState.RegistryInfo.PullUsername) + require.Equal(t, tt.expectedRegistry.Address, newState.RegistryInfo.Address) + require.Equal(t, tt.expectedRegistry.NodePort, newState.RegistryInfo.NodePort) + require.Equal(t, tt.expectedRegistry.InternalRegistry, newState.RegistryInfo.InternalRegistry) + require.Equal(t, tt.expectedRegistry.Secret, newState.RegistryInfo.Secret) + }) + } +} + +// TODO: Change password gen method to make testing possible. +func TestMergeZarfStateGit(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + initGitServer types.GitServerInfo + oldGitServer types.GitServerInfo + expectedGitServer types.GitServerInfo + }{ + { + name: "username is unmodified", + oldGitServer: types.GitServerInfo{ + PushUsername: "push-user", + PullUsername: "pull-user", + }, + expectedGitServer: types.GitServerInfo{ + PushUsername: "push-user", + PullUsername: "pull-user", + }, + }, + { + name: "internal server auto generate", + oldGitServer: types.GitServerInfo{ + Address: types.ZarfInClusterGitServiceURL, + InternalServer: true, + }, + expectedGitServer: types.GitServerInfo{ + Address: types.ZarfInClusterGitServiceURL, + InternalServer: true, + }, + }, + { + name: "external server", + oldGitServer: types.GitServerInfo{ + Address: "example.com", + InternalServer: false, + PushPassword: "push", + PullPassword: "pull", + }, + expectedGitServer: types.GitServerInfo{ + Address: "example.com", + InternalServer: false, + PushPassword: "push", + PullPassword: "pull", + }, + }, + { + name: "init options merged", + initGitServer: types.GitServerInfo{ + PushUsername: "push-user", + PullUsername: "pull-user", + Address: "address", + InternalServer: false, + }, + expectedGitServer: types.GitServerInfo{ + PushUsername: "push-user", + PullUsername: "pull-user", + Address: "address", + InternalServer: false, + }, + }, + { + name: "empty init options not merged", + expectedGitServer: types.GitServerInfo{ + PushUsername: "", + PullUsername: "", + Address: "", + InternalServer: false, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + oldState := &types.ZarfState{ + GitServer: tt.oldGitServer, + } + newState, err := MergeZarfState(oldState, types.ZarfInitOptions{GitServer: tt.initGitServer}, []string{message.GitKey}) + require.NoError(t, err) + require.Equal(t, tt.expectedGitServer.PushUsername, newState.GitServer.PushUsername) + require.Equal(t, tt.expectedGitServer.PullUsername, newState.GitServer.PullUsername) + require.Equal(t, tt.expectedGitServer.Address, newState.GitServer.Address) + require.Equal(t, tt.expectedGitServer.InternalServer, newState.GitServer.InternalServer) + }) + } +} + +func TestMergeZarfStateArtifact(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + initArtifactServer types.ArtifactServerInfo + oldArtifactServer types.ArtifactServerInfo + expectedArtifactServer types.ArtifactServerInfo + }{ + { + name: "username is unmodified", + oldArtifactServer: types.ArtifactServerInfo{ + PushUsername: "push-user", + }, + expectedArtifactServer: types.ArtifactServerInfo{ + PushUsername: "push-user", + }, + }, + { + name: "old state is internal server auto generate push token", + oldArtifactServer: types.ArtifactServerInfo{ + PushToken: "foobar", + Address: types.ZarfInClusterArtifactServiceURL, + InternalServer: true, + }, + expectedArtifactServer: types.ArtifactServerInfo{ + PushToken: "", + Address: types.ZarfInClusterArtifactServiceURL, + InternalServer: true, + }, + }, + { + name: "old state is not internal server auto generate push token but init options does not match", + initArtifactServer: types.ArtifactServerInfo{ + PushToken: "hello world", + }, + oldArtifactServer: types.ArtifactServerInfo{ + PushToken: "foobar", + Address: types.ZarfInClusterArtifactServiceURL, + InternalServer: false, + }, + expectedArtifactServer: types.ArtifactServerInfo{ + PushToken: "hello world", + Address: types.ZarfInClusterArtifactServiceURL, + InternalServer: true, + }, + }, + { + name: "external server same push token", + oldArtifactServer: types.ArtifactServerInfo{ + PushToken: "foobar", + Address: "http://example.com", + InternalServer: false, + }, + expectedArtifactServer: types.ArtifactServerInfo{ + PushToken: "foobar", + Address: "http://example.com", + InternalServer: false, + }, + }, + { + name: "init options merged", + initArtifactServer: types.ArtifactServerInfo{ + PushUsername: "user", + PushToken: "token", + Address: "address", + InternalServer: false, + }, + expectedArtifactServer: types.ArtifactServerInfo{ + PushUsername: "user", + PushToken: "token", + Address: "address", + InternalServer: false, + }, + }, + { + name: "empty init options not merged", + expectedArtifactServer: types.ArtifactServerInfo{ + PushUsername: "", + PushToken: "", + Address: "", + InternalServer: false, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + oldState := &types.ZarfState{ + ArtifactServer: tt.oldArtifactServer, + } + newState, err := MergeZarfState(oldState, types.ZarfInitOptions{ArtifactServer: tt.initArtifactServer}, []string{message.ArtifactKey}) + require.NoError(t, err) + require.Equal(t, tt.expectedArtifactServer, newState.ArtifactServer) + }) + } +} + +func TestMergeZarfStateAgent(t *testing.T) { + t.Parallel() + + oldState := &types.ZarfState{ + AgentTLS: pki.GeneratePKI("example.com"), + } + newState, err := MergeZarfState(oldState, types.ZarfInitOptions{}, []string{message.AgentKey}) + require.NoError(t, err) + require.NotEqual(t, oldState.AgentTLS, newState.AgentTLS) +}