Skip to content

Commit

Permalink
refactor: remove warnings property from packager
Browse files Browse the repository at this point in the history
  • Loading branch information
phillebaba committed Jul 2, 2024
1 parent 5a2ada1 commit fa0b127
Show file tree
Hide file tree
Showing 11 changed files with 57 additions and 84 deletions.
22 changes: 6 additions & 16 deletions src/pkg/packager/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ type Packager struct {
state *types.ZarfState
cluster *cluster.Cluster
layout *layout.PackagePaths
warnings []string
hpaModified bool
connectStrings types.ConnectStrings
sbomViewFiles []string
Expand Down Expand Up @@ -235,35 +234,26 @@ func (p *Packager) validatePackageArchitecture(ctx context.Context) error {
}

// 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

func validateLastNonBreakingVersion(cliVersion, lastNonBreakingVersion string) ([]string, error) {
if lastNonBreakingVersion == "" {
return nil
return nil, 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)
return nil, fmt.Errorf("unable to parse lastNonBreakingVersion '%s' from Zarf package build data : %w", lastNonBreakingVersion, err)
}

cliSemVer, err := semver.NewVersion(cliVersion)
if err != nil {
warning := fmt.Sprintf(lang.CmdPackageDeployInvalidCLIVersionWarn, config.CLIVersion)
p.warnings = append(p.warnings, warning)
return nil
return []string{fmt.Sprintf(lang.CmdPackageDeployInvalidCLIVersionWarn, config.CLIVersion)}, nil
}

if cliSemVer.LessThan(lastNonBreakingSemVer) {
warning := fmt.Sprintf(
lang.CmdPackageDeployValidateLastNonBreakingVersionWarn,
cliVersion,
lastNonBreakingVersion,
lastNonBreakingVersion,
)
p.warnings = append(p.warnings, warning)
return []string{warning}, nil
}

return nil
return nil, nil
}
45 changes: 14 additions & 31 deletions src/pkg/packager/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"

"github.com/defenseunicorns/zarf/src/config"
"github.com/defenseunicorns/zarf/src/config/lang"
"github.com/defenseunicorns/zarf/src/pkg/cluster"
"github.com/defenseunicorns/zarf/src/types"
Expand Down Expand Up @@ -120,17 +119,15 @@ func TestValidatePackageArchitecture(t *testing.T) {
func TestValidateLastNonBreakingVersion(t *testing.T) {
t.Parallel()

type testCase struct {
tests := []struct {
name string
cliVersion string
lastNonBreakingVersion string
expectedErrorMessage string
expectedWarningMessage string
returnError bool
throwWarning bool
}

testCases := []testCase{
}{
{
name: "CLI version less than lastNonBreakingVersion",
cliVersion: "v0.26.4",
Expand Down Expand Up @@ -182,35 +179,21 @@ func TestValidateLastNonBreakingVersion(t *testing.T) {
throwWarning: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

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

t.Run(testCase.name, func(t *testing.T) {
config.CLIVersion = testCase.cliVersion

p := &Packager{
cfg: &types.PackagerConfig{
Pkg: types.ZarfPackage{
Build: types.ZarfBuildData{
LastNonBreakingVersion: testCase.lastNonBreakingVersion,
},
},
},
}

err := p.validateLastNonBreakingVersion()

warnings, err := validateLastNonBreakingVersion(tt.cliVersion, tt.lastNonBreakingVersion)
switch {
case testCase.returnError:
require.ErrorContains(t, err, testCase.expectedErrorMessage)
require.Empty(t, p.warnings, "Expected no warnings for test case: %s", testCase.name)
case testCase.throwWarning:
require.Contains(t, p.warnings, testCase.expectedWarningMessage)
require.NoError(t, err, "Expected no error for test case: %s", testCase.name)
case tt.returnError:
require.ErrorContains(t, err, tt.expectedErrorMessage)
require.Empty(t, warnings, "Expected no warnings for test case: %s", tt.name)
case tt.throwWarning:
require.Contains(t, warnings, tt.expectedWarningMessage)
require.NoError(t, err, "Expected no error for test case: %s", tt.name)
default:
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)
require.NoError(t, err, "Expected no error for test case: %s", tt.name)
require.Empty(t, warnings, "Expected no warnings for test case: %s", tt.name)
}
})
}
Expand Down
7 changes: 4 additions & 3 deletions src/pkg/packager/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
)

// Create generates a Zarf package tarball for a given PackageConfig and optional base directory.
func (p *Packager) Create(ctx context.Context) (err error) {
func (p *Packager) Create(ctx context.Context) error {
cwd, err := os.Getwd()
if err != nil {
return err
Expand All @@ -35,12 +35,13 @@ func (p *Packager) Create(ctx context.Context) (err error) {
return err
}

p.cfg.Pkg, p.warnings, err = pc.LoadPackageDefinition(ctx, p.layout)
pkg, warnings, err := pc.LoadPackageDefinition(ctx, p.layout)
if err != nil {
return err
}
p.cfg.Pkg = pkg

if !p.confirmAction(config.ZarfCreateStage) {
if !p.confirmAction(config.ZarfCreateStage, warnings) {
return fmt.Errorf("package creation canceled")
}

Expand Down
25 changes: 13 additions & 12 deletions src/pkg/packager/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,47 +47,48 @@ func (p *Packager) resetRegistryHPA(ctx context.Context) {
}

// Deploy attempts to deploy the given PackageConfig.
func (p *Packager) Deploy(ctx context.Context) (err error) {

func (p *Packager) Deploy(ctx context.Context) error {
isInteractive := !config.CommonOptions.Confirm

deployFilter := filters.Combine(
filters.ByLocalOS(runtime.GOOS),
filters.ForDeploy(p.cfg.PkgOpts.OptionalComponents, isInteractive),
)

warnings := []string{}
if isInteractive {
filter := filters.Empty()

p.cfg.Pkg, p.warnings, err = p.source.LoadPackage(ctx, p.layout, filter, true)
var err error
p.cfg.Pkg, warnings, err = p.source.LoadPackage(ctx, p.layout, filter, true)
if err != nil {
return fmt.Errorf("unable to load the package: %w", err)
}
} else {
p.cfg.Pkg, p.warnings, err = p.source.LoadPackage(ctx, p.layout, deployFilter, true)
var err error
p.cfg.Pkg, warnings, err = p.source.LoadPackage(ctx, p.layout, deployFilter, true)
if err != nil {
return fmt.Errorf("unable to load the package: %w", err)
}

if err := p.populatePackageVariableConfig(); err != nil {
return fmt.Errorf("unable to set the active variables: %w", err)
}
}

if err := p.validateLastNonBreakingVersion(); err != nil {
validateWarnings, err := validateLastNonBreakingVersion(config.CLIVersion, p.cfg.Pkg.Build.LastNonBreakingVersion)
if err != nil {
return err
}
warnings = append(warnings, validateWarnings...)

var sbomWarnings []string
p.sbomViewFiles, sbomWarnings, err = p.layout.SBOMs.StageSBOMViewFiles()
sbomViewFiles, sbomWarnings, err := p.layout.SBOMs.StageSBOMViewFiles()
if err != nil {
return err
}

p.warnings = append(p.warnings, sbomWarnings...)
p.sbomViewFiles = sbomViewFiles
warnings = append(warnings, sbomWarnings...)

// Confirm the overall package deployment
if !p.confirmAction(config.ZarfDeployStage) {
if !p.confirmAction(config.ZarfDeployStage, warnings) {
return fmt.Errorf("deployment cancelled")
}

Expand Down
2 changes: 1 addition & 1 deletion src/pkg/packager/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (p *Packager) DevDeploy(ctx context.Context) error {
return err
}

p.cfg.Pkg, p.warnings, err = pc.LoadPackageDefinition(ctx, p.layout)
p.cfg.Pkg, _, err = pc.LoadPackageDefinition(ctx, p.layout)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion src/pkg/packager/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
func (p *Packager) Inspect(ctx context.Context) (err error) {
wantSBOM := p.cfg.InspectOpts.ViewSBOM || p.cfg.InspectOpts.SBOMOutputDir != ""

p.cfg.Pkg, p.warnings, err = p.source.LoadPackageMetadata(ctx, p.layout, wantSBOM, true)
p.cfg.Pkg, _, err = p.source.LoadPackageMetadata(ctx, p.layout, wantSBOM, true)
if err != nil {
return err
}
Expand Down
7 changes: 3 additions & 4 deletions src/pkg/packager/interactive.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ import (
"github.com/pterm/pterm"
)

func (p *Packager) confirmAction(stage string) (confirm bool) {

func (p *Packager) confirmAction(stage string, warnings []string) (confirm bool) {
pterm.Println()
message.HeaderInfof("📦 PACKAGE DEFINITION")
utils.ColorPrintYAML(p.cfg.Pkg, p.getPackageYAMLHints(stage), true)
Expand Down Expand Up @@ -54,10 +53,10 @@ func (p *Packager) confirmAction(stage string) (confirm bool) {
}
}

if len(p.warnings) > 0 {
if len(warnings) > 0 {
message.HorizontalRule()
message.Title("Package Warnings", "the following warnings were flagged while reading the package")
for _, warning := range p.warnings {
for _, warning := range warnings {
message.Warn(warning)
}
}
Expand Down
14 changes: 7 additions & 7 deletions src/pkg/packager/mirror.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,27 @@ import (
)

// Mirror pulls resources from a package (images, git repositories, etc) and pushes them to remotes in the air gap without deploying them
func (p *Packager) Mirror(ctx context.Context) (err error) {
func (p *Packager) Mirror(ctx context.Context) error {
filter := filters.Combine(
filters.ByLocalOS(runtime.GOOS),
filters.BySelectState(p.cfg.PkgOpts.OptionalComponents),
)

p.cfg.Pkg, p.warnings, err = p.source.LoadPackage(ctx, p.layout, filter, true)
pkg, warnings, err := p.source.LoadPackage(ctx, p.layout, filter, true)
if err != nil {
return fmt.Errorf("unable to load the package: %w", err)
}
p.cfg.Pkg = pkg

var sbomWarnings []string
p.sbomViewFiles, sbomWarnings, err = p.layout.SBOMs.StageSBOMViewFiles()
sbomViewFiles, sbomWarnings, err := p.layout.SBOMs.StageSBOMViewFiles()
if err != nil {
return err
}

p.warnings = append(p.warnings, sbomWarnings...)
p.sbomViewFiles = sbomViewFiles
warnings = append(warnings, sbomWarnings...)

// Confirm the overall package mirror
if !p.confirmAction(config.ZarfMirrorStage) {
if !p.confirmAction(config.ZarfMirrorStage, warnings) {
return fmt.Errorf("mirror cancelled")
}

Expand Down
7 changes: 4 additions & 3 deletions src/pkg/packager/prepare.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import (
type imageMap map[string]bool

// FindImages iterates over a Zarf.yaml and attempts to parse any images.
func (p *Packager) FindImages(ctx context.Context) (imgMap map[string][]string, err error) {
func (p *Packager) FindImages(ctx context.Context) (map[string][]string, error) {
cwd, err := os.Getwd()
if err != nil {
return nil, err
Expand All @@ -60,12 +60,13 @@ func (p *Packager) FindImages(ctx context.Context) (imgMap map[string][]string,
return nil, err
}

p.cfg.Pkg, p.warnings, err = c.LoadPackageDefinition(ctx, p.layout)
pkg, warnings, err := c.LoadPackageDefinition(ctx, p.layout)
if err != nil {
return nil, err
}
p.cfg.Pkg = pkg

for _, warning := range p.warnings {
for _, warning := range warnings {
message.Warn(warning)
}

Expand Down
4 changes: 2 additions & 2 deletions src/pkg/packager/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (p *Packager) Publish(ctx context.Context) (err error) {
return err
}

p.cfg.Pkg, p.warnings, err = sc.LoadPackageDefinition(ctx, p.layout)
p.cfg.Pkg, _, err = sc.LoadPackageDefinition(ctx, p.layout)
if err != nil {
return err
}
Expand All @@ -72,7 +72,7 @@ func (p *Packager) Publish(ctx context.Context) (err error) {
}
} else {
filter := filters.Empty()
p.cfg.Pkg, p.warnings, err = p.source.LoadPackage(ctx, p.layout, filter, false)
p.cfg.Pkg, _, err = p.source.LoadPackage(ctx, p.layout, filter, false)
if err != nil {
return fmt.Errorf("unable to load the package: %w", err)
}
Expand Down
6 changes: 2 additions & 4 deletions src/pkg/packager/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,13 @@ func (p *Packager) Remove(ctx context.Context) (err error) {
spinner := message.NewProgressSpinner("Removing Zarf package %s", p.cfg.PkgOpts.PackageSource)
defer spinner.Stop()

var packageName string

// we do not want to allow removal of signed packages without a signature if there are remove actions
// as this is arbitrary code execution from an untrusted source
p.cfg.Pkg, p.warnings, err = p.source.LoadPackageMetadata(ctx, p.layout, false, false)
p.cfg.Pkg, _, err = p.source.LoadPackageMetadata(ctx, p.layout, false, false)
if err != nil {
return err
}
packageName = p.cfg.Pkg.Metadata.Name
packageName := p.cfg.Pkg.Metadata.Name

// Build a list of components to remove and determine if we need a cluster connection
componentsToRemove := []string{}
Expand Down

0 comments on commit fa0b127

Please sign in to comment.