Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw warning instead of error when default CLIVersion is used when validating last non-breaking version #1981

Merged
merged 6 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/config/lang/english.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
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"
Expand Down
10 changes: 5 additions & 5 deletions src/pkg/packager/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -496,14 +496,12 @@ 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" {
if lastNonBreakingVersion == "" {
return nil
}

Expand All @@ -514,7 +512,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(lang.CmdPackageDeployInvalidCLIVersionWarn, config.CLIVersion)
p.warnings = append(p.warnings, warning)
return nil
}

if cliSemVer.LessThan(lastNonBreakingSemVer) {
Expand Down
30 changes: 13 additions & 17 deletions src/pkg/packager/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -138,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: fmt.Sprintf(lang.CmdPackageDeployInvalidCLIVersionWarn, "invalidSemanticVersion"),
},
{
name: "invalid semantic version (lastNonBreakingVersion)",
Expand Down Expand Up @@ -171,17 +170,14 @@ func TestValidateLastNonBreakingVersion(t *testing.T) {
returnError: false,
throwWarning: false,
},
{
name: "default CLI version in E2E tests",
cliVersion: "UnknownVersion", // This is used as a default version in the E2E tests
lastNonBreakingVersion: "v0.27.0",
returnError: false,
throwWarning: false,
},
}

for _, testCase := range testCases {
testCase := testCase

t.Run(testCase.name, func(t *testing.T) {
t.Parallel()

config.CLIVersion = testCase.cliVersion

p := &Packager{
Expand All @@ -198,14 +194,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)
}
})
}
Expand Down