From e273c861fcaaaf93627010b0fcf82df486212975 Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Mon, 21 Aug 2023 16:33:04 -0500 Subject: [PATCH 1/6] Ignore default CLIVersion when validating last non-breaking version --- src/pkg/packager/common.go | 2 +- src/pkg/packager/common_test.go | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/pkg/packager/common.go b/src/pkg/packager/common.go index 0d93429bf0..a9ef57ecad 100644 --- a/src/pkg/packager/common.go +++ b/src/pkg/packager/common.go @@ -503,7 +503,7 @@ func (p *Packager) validateLastNonBreakingVersion() (err error) { cliVersion := config.CLIVersion lastNonBreakingVersion := p.cfg.Pkg.Build.LastNonBreakingVersion - if lastNonBreakingVersion == "" || cliVersion == "UnknownVersion" { + if lastNonBreakingVersion == "" || cliVersion == "UnknownVersion" || cliVersion == "unset" { return nil } diff --git a/src/pkg/packager/common_test.go b/src/pkg/packager/common_test.go index 9665864b15..52032da1db 100644 --- a/src/pkg/packager/common_test.go +++ b/src/pkg/packager/common_test.go @@ -178,6 +178,13 @@ func TestValidateLastNonBreakingVersion(t *testing.T) { returnError: false, throwWarning: false, }, + { + name: "default CLI version", + cliVersion: "unset", + lastNonBreakingVersion: "v0.27.0", + returnError: false, + throwWarning: false, + }, } for _, testCase := range testCases { From 3618dd1bb42864ef7feca48bf544fe634c499f3f Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Wed, 23 Aug 2023 11:28:45 -0500 Subject: [PATCH 2/6] Throw warning when CLIVersion is unset when using Zarf as a library --- src/config/lang/english.go | 1 + src/pkg/packager/common.go | 12 ++++++++---- src/pkg/packager/common_test.go | 20 ++++++++++++-------- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/config/lang/english.go b/src/config/lang/english.go index 9bfe870250..f4f72fb053 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -256,6 +256,7 @@ const ( CmdPackageDeployFlagPublicKey = "Path to public key file for validating signed packages" CmdPackageDeployValidateArchitectureErr = "this package architecture is %s, but the target cluster has the %s architecture. These architectures must be the same" CmdPackageDeployValidateLastNonBreakingVersionWarn = "the version of this Zarf binary '%s' is less than the LastNonBreakingVersion of '%s'. You may need to upgrade your Zarf version to at least '%s' to deploy this package" + CmdPackageDeployUnsetCLIVersionWarn = "CLIVersion is set to the default value of '%s'. This could potentially cause issues with package creation and deployment. To avoid such issues, please set the value to a valid semantic version." CmdPackageDeployErr = "Failed to deploy package: %s" CmdPackageInspectFlagSbom = "View SBOM contents while inspecting the package" diff --git a/src/pkg/packager/common.go b/src/pkg/packager/common.go index a9ef57ecad..f26ebece42 100644 --- a/src/pkg/packager/common.go +++ b/src/pkg/packager/common.go @@ -496,14 +496,18 @@ func (p *Packager) validatePackageArchitecture() (err error) { return nil } -// validateLastNonBreakingVersion compares the Zarf CLI version against a package's LastNonBreakingVersion. -// It will return an error if there is an error parsing either of the two versions, -// and will throw a warning if the CLI version is less than the LastNonBreakingVersion. +// validateLastNonBreakingVersion validates the Zarf CLI version against a package's LastNonBreakingVersion. func (p *Packager) validateLastNonBreakingVersion() (err error) { cliVersion := config.CLIVersion lastNonBreakingVersion := p.cfg.Pkg.Build.LastNonBreakingVersion - if lastNonBreakingVersion == "" || cliVersion == "UnknownVersion" || cliVersion == "unset" { + if lastNonBreakingVersion == "" || cliVersion == "UnknownVersion" { + return nil + } + + if cliVersion == "unset" { + unsetWarning := fmt.Sprintf(lang.CmdPackageDeployUnsetCLIVersionWarn, config.CLIVersion) + p.warnings = append(p.warnings, unsetWarning) return nil } diff --git a/src/pkg/packager/common_test.go b/src/pkg/packager/common_test.go index 52032da1db..d2c8c9b886 100644 --- a/src/pkg/packager/common_test.go +++ b/src/pkg/packager/common_test.go @@ -10,7 +10,6 @@ import ( "github.com/defenseunicorns/zarf/src/internal/cluster" "github.com/defenseunicorns/zarf/src/pkg/k8s" "github.com/defenseunicorns/zarf/src/types" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -183,12 +182,17 @@ func TestValidateLastNonBreakingVersion(t *testing.T) { cliVersion: "unset", lastNonBreakingVersion: "v0.27.0", returnError: false, - throwWarning: false, + throwWarning: true, + expectedWarningMessage: fmt.Sprintf(lang.CmdPackageDeployUnsetCLIVersionWarn, config.CLIVersion), }, } for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + config.CLIVersion = testCase.cliVersion p := &Packager{ @@ -205,14 +209,14 @@ func TestValidateLastNonBreakingVersion(t *testing.T) { switch { case testCase.returnError: - assert.ErrorContains(t, err, testCase.expectedErrorMessage) - assert.Empty(t, p.warnings, "Expected no warnings for test case: %s", testCase.name) + require.ErrorContains(t, err, testCase.expectedErrorMessage) + require.Empty(t, p.warnings, "Expected no warnings for test case: %s", testCase.name) case testCase.throwWarning: - assert.Contains(t, p.warnings, testCase.expectedWarningMessage) - assert.NoError(t, err, "Expected no error for test case: %s", testCase.name) + require.Contains(t, p.warnings, testCase.expectedWarningMessage) + require.NoError(t, err, "Expected no error for test case: %s", testCase.name) default: - assert.NoError(t, err, "Expected no error for test case: %s", testCase.name) - assert.Empty(t, p.warnings, "Expected no warnings for test case: %s", testCase.name) + require.NoError(t, err, "Expected no error for test case: %s", testCase.name) + require.Empty(t, p.warnings, "Expected no warnings for test case: %s", testCase.name) } }) } From 2db29cfc3ac0bef96a5147025de27de023b746e5 Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Wed, 23 Aug 2023 15:36:30 -0500 Subject: [PATCH 3/6] Update src/config/lang/english.go Co-authored-by: Wayne Starr --- src/config/lang/english.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/lang/english.go b/src/config/lang/english.go index f4f72fb053..17f706d681 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -256,7 +256,7 @@ const ( CmdPackageDeployFlagPublicKey = "Path to public key file for validating signed packages" CmdPackageDeployValidateArchitectureErr = "this package architecture is %s, but the target cluster has the %s architecture. These architectures must be the same" CmdPackageDeployValidateLastNonBreakingVersionWarn = "the version of this Zarf binary '%s' is less than the LastNonBreakingVersion of '%s'. You may need to upgrade your Zarf version to at least '%s' to deploy this package" - CmdPackageDeployUnsetCLIVersionWarn = "CLIVersion is set to the default value of '%s'. This could potentially cause issues with package creation and deployment. To avoid such issues, please set the value to a valid semantic version." + CmdPackageDeployUnsetCLIVersionWarn = "CLIVersion is set to '%s' which can cause issues with package creation and deployment. To avoid such issues, please set the value to the valid semantic version for this version of Zarf." CmdPackageDeployErr = "Failed to deploy package: %s" CmdPackageInspectFlagSbom = "View SBOM contents while inspecting the package" From 44f6497b9a211bd7657709c175049685713e6754 Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Wed, 23 Aug 2023 17:39:39 -0500 Subject: [PATCH 4/6] Throw warning for invalid CLI version instead of return error --- src/pkg/packager/common.go | 13 ++++++++----- src/pkg/packager/common_test.go | 13 +++++++------ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/pkg/packager/common.go b/src/pkg/packager/common.go index f26ebece42..5559fc79f6 100644 --- a/src/pkg/packager/common.go +++ b/src/pkg/packager/common.go @@ -501,13 +501,13 @@ func (p *Packager) validateLastNonBreakingVersion() (err error) { cliVersion := config.CLIVersion lastNonBreakingVersion := p.cfg.Pkg.Build.LastNonBreakingVersion - if lastNonBreakingVersion == "" || cliVersion == "UnknownVersion" { + if lastNonBreakingVersion == "" { return nil } - if cliVersion == "unset" { - unsetWarning := fmt.Sprintf(lang.CmdPackageDeployUnsetCLIVersionWarn, config.CLIVersion) - p.warnings = append(p.warnings, unsetWarning) + if cliVersion == "unset" || cliVersion == "UnknownVersion" { + warning := fmt.Sprintf(lang.CmdPackageDeployUnsetCLIVersionWarn, config.CLIVersion) + p.warnings = append(p.warnings, warning) return nil } @@ -518,7 +518,9 @@ func (p *Packager) validateLastNonBreakingVersion() (err error) { cliSemVer, err := semver.NewVersion(cliVersion) if err != nil { - return fmt.Errorf("unable to parse Zarf CLI version '%s' : %w", cliVersion, err) + warning := fmt.Sprintf("unable to parse Zarf CLI version '%s' : %s", cliVersion, err.Error()) + p.warnings = append(p.warnings, warning) + return nil } if cliSemVer.LessThan(lastNonBreakingSemVer) { @@ -529,6 +531,7 @@ func (p *Packager) validateLastNonBreakingVersion() (err error) { lastNonBreakingVersion, ) p.warnings = append(p.warnings, warning) + return nil } return nil diff --git a/src/pkg/packager/common_test.go b/src/pkg/packager/common_test.go index d2c8c9b886..a91e634d46 100644 --- a/src/pkg/packager/common_test.go +++ b/src/pkg/packager/common_test.go @@ -137,9 +137,9 @@ func TestValidateLastNonBreakingVersion(t *testing.T) { name: "invalid semantic version (CLI version)", cliVersion: "invalidSemanticVersion", lastNonBreakingVersion: "v0.0.1", - throwWarning: false, - returnError: true, - expectedErrorMessage: "unable to parse Zarf CLI version", + returnError: false, + throwWarning: true, + expectedWarningMessage: "unable to parse Zarf CLI version 'invalidSemanticVersion' : Invalid Semantic Version", }, { name: "invalid semantic version (lastNonBreakingVersion)", @@ -171,11 +171,12 @@ func TestValidateLastNonBreakingVersion(t *testing.T) { throwWarning: false, }, { - name: "default CLI version in E2E tests", - cliVersion: "UnknownVersion", // This is used as a default version in the E2E tests + name: "CLI version in E2E tests", + cliVersion: "UnknownVersion", lastNonBreakingVersion: "v0.27.0", returnError: false, - throwWarning: false, + throwWarning: true, + expectedWarningMessage: fmt.Sprintf(lang.CmdPackageDeployUnsetCLIVersionWarn, "UnknownVersion"), }, { name: "default CLI version", From 9c0311923972b4bec9ba92be1a094927199a1aea Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Wed, 23 Aug 2023 21:18:05 -0500 Subject: [PATCH 5/6] Remove unnecessary checks --- src/config/lang/english.go | 2 +- src/pkg/packager/common.go | 8 +------- src/pkg/packager/common_test.go | 18 +----------------- 3 files changed, 3 insertions(+), 25 deletions(-) diff --git a/src/config/lang/english.go b/src/config/lang/english.go index 17f706d681..8e9ac8884f 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -256,7 +256,7 @@ const ( CmdPackageDeployFlagPublicKey = "Path to public key file for validating signed packages" CmdPackageDeployValidateArchitectureErr = "this package architecture is %s, but the target cluster has the %s architecture. These architectures must be the same" CmdPackageDeployValidateLastNonBreakingVersionWarn = "the version of this Zarf binary '%s' is less than the LastNonBreakingVersion of '%s'. You may need to upgrade your Zarf version to at least '%s' to deploy this package" - CmdPackageDeployUnsetCLIVersionWarn = "CLIVersion is set to '%s' which can cause issues with package creation and deployment. To avoid such issues, please set the value to the valid semantic version for this version of Zarf." + CmdPackageDeployInvalidCLIVersionWarn = "CLIVersion is set to '%s' which can cause issues with package creation and deployment. To avoid such issues, please set the value to the valid semantic version for this version of Zarf." CmdPackageDeployErr = "Failed to deploy package: %s" CmdPackageInspectFlagSbom = "View SBOM contents while inspecting the package" diff --git a/src/pkg/packager/common.go b/src/pkg/packager/common.go index 5559fc79f6..4fc22b9237 100644 --- a/src/pkg/packager/common.go +++ b/src/pkg/packager/common.go @@ -505,12 +505,6 @@ func (p *Packager) validateLastNonBreakingVersion() (err error) { return nil } - if cliVersion == "unset" || cliVersion == "UnknownVersion" { - warning := fmt.Sprintf(lang.CmdPackageDeployUnsetCLIVersionWarn, config.CLIVersion) - p.warnings = append(p.warnings, warning) - return nil - } - lastNonBreakingSemVer, err := semver.NewVersion(lastNonBreakingVersion) if err != nil { return fmt.Errorf("unable to parse lastNonBreakingVersion '%s' from Zarf package build data : %w", lastNonBreakingVersion, err) @@ -518,7 +512,7 @@ func (p *Packager) validateLastNonBreakingVersion() (err error) { cliSemVer, err := semver.NewVersion(cliVersion) if err != nil { - warning := fmt.Sprintf("unable to parse Zarf CLI version '%s' : %s", cliVersion, err.Error()) + warning := fmt.Sprintf(lang.CmdPackageDeployInvalidCLIVersionWarn, config.CLIVersion) p.warnings = append(p.warnings, warning) return nil } diff --git a/src/pkg/packager/common_test.go b/src/pkg/packager/common_test.go index a91e634d46..66fd6cb543 100644 --- a/src/pkg/packager/common_test.go +++ b/src/pkg/packager/common_test.go @@ -139,7 +139,7 @@ func TestValidateLastNonBreakingVersion(t *testing.T) { lastNonBreakingVersion: "v0.0.1", returnError: false, throwWarning: true, - expectedWarningMessage: "unable to parse Zarf CLI version 'invalidSemanticVersion' : Invalid Semantic Version", + expectedWarningMessage: fmt.Sprintf(lang.CmdPackageDeployInvalidCLIVersionWarn, "invalidSemanticVersion"), }, { name: "invalid semantic version (lastNonBreakingVersion)", @@ -170,22 +170,6 @@ func TestValidateLastNonBreakingVersion(t *testing.T) { returnError: false, throwWarning: false, }, - { - name: "CLI version in E2E tests", - cliVersion: "UnknownVersion", - lastNonBreakingVersion: "v0.27.0", - returnError: false, - throwWarning: true, - expectedWarningMessage: fmt.Sprintf(lang.CmdPackageDeployUnsetCLIVersionWarn, "UnknownVersion"), - }, - { - name: "default CLI version", - cliVersion: "unset", - lastNonBreakingVersion: "v0.27.0", - returnError: false, - throwWarning: true, - expectedWarningMessage: fmt.Sprintf(lang.CmdPackageDeployUnsetCLIVersionWarn, config.CLIVersion), - }, } for _, testCase := range testCases { From d2734dc508af2bc7bae9c522b47e0d5d781af9cf Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Thu, 24 Aug 2023 18:58:24 -0500 Subject: [PATCH 6/6] Update src/pkg/packager/common.go Co-authored-by: Wayne Starr --- src/pkg/packager/common.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pkg/packager/common.go b/src/pkg/packager/common.go index 4fc22b9237..1a986ffc58 100644 --- a/src/pkg/packager/common.go +++ b/src/pkg/packager/common.go @@ -525,7 +525,6 @@ func (p *Packager) validateLastNonBreakingVersion() (err error) { lastNonBreakingVersion, ) p.warnings = append(p.warnings, warning) - return nil } return nil