From 9239fe6f1f3e09793f3a6794595269013f7bff3b Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Tue, 21 May 2024 17:30:03 -0500 Subject: [PATCH 1/6] chore: add unit tests for creator.LoadPackageDefinition --- src/pkg/packager/create.go | 5 -- src/pkg/packager/creator/creator.go | 2 +- src/pkg/packager/creator/normal.go | 11 ++-- src/pkg/packager/creator/normal_test.go | 42 +++++++++++++++ src/pkg/packager/creator/skeleton.go | 11 ++-- src/pkg/packager/creator/skeleton_test.go | 54 +++++++++++++++++++ .../creator/testdata/invalid/zarf.yaml | 5 ++ .../packager/creator/testdata/valid/zarf.yaml | 6 +++ src/types/validate.go | 5 ++ 9 files changed, 129 insertions(+), 12 deletions(-) create mode 100644 src/pkg/packager/creator/skeleton_test.go create mode 100644 src/pkg/packager/creator/testdata/invalid/zarf.yaml create mode 100644 src/pkg/packager/creator/testdata/valid/zarf.yaml diff --git a/src/pkg/packager/create.go b/src/pkg/packager/create.go index bc10fe8d73..bad88d5d4e 100755 --- a/src/pkg/packager/create.go +++ b/src/pkg/packager/create.go @@ -39,11 +39,6 @@ func (p *Packager) Create() (err error) { return err } - // Perform early package validation. - if err := p.cfg.Pkg.Validate(); err != nil { - return fmt.Errorf("unable to validate package: %w", err) - } - if !p.confirmAction(config.ZarfCreateStage) { return fmt.Errorf("package creation canceled") } diff --git a/src/pkg/packager/creator/creator.go b/src/pkg/packager/creator/creator.go index 5b0f3c27b1..5e34fd46c1 100644 --- a/src/pkg/packager/creator/creator.go +++ b/src/pkg/packager/creator/creator.go @@ -11,7 +11,7 @@ import ( // Creator is an interface for creating Zarf packages. type Creator interface { - LoadPackageDefinition(dst *layout.PackagePaths) (pkg types.ZarfPackage, warnings []string, err error) + LoadPackageDefinition(src *layout.PackagePaths) (pkg types.ZarfPackage, warnings []string, err error) Assemble(dst *layout.PackagePaths, components []types.ZarfComponent, arch string) error Output(dst *layout.PackagePaths, pkg *types.ZarfPackage) error } diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index 0cb5c7d921..79e0c01087 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -61,12 +61,17 @@ func NewPackageCreator(createOpts types.ZarfCreateOptions, cwd string) *PackageC } // LoadPackageDefinition loads and configures a zarf.yaml file during package create. -func (pc *PackageCreator) LoadPackageDefinition(dst *layout.PackagePaths) (pkg types.ZarfPackage, warnings []string, err error) { - pkg, warnings, err = dst.ReadZarfYAML() +func (pc *PackageCreator) LoadPackageDefinition(src *layout.PackagePaths) (pkg types.ZarfPackage, warnings []string, err error) { + pkg, warnings, err = src.ReadZarfYAML() if err != nil { return types.ZarfPackage{}, nil, err } + // Perform early package validation. + if err := pkg.Validate(); err != nil { + return types.ZarfPackage{}, nil, fmt.Errorf("unable to validate package: %w", err) + } + pkg.Metadata.Architecture = config.GetArch(pkg.Metadata.Architecture) // Compose components into a single zarf.yaml file @@ -86,7 +91,7 @@ func (pc *PackageCreator) LoadPackageDefinition(dst *layout.PackagePaths) (pkg t warnings = append(warnings, templateWarnings...) // After templates are filled process any create extensions - pkg.Components, err = pc.processExtensions(pkg.Components, dst, pkg.Metadata.YOLO) + pkg.Components, err = pc.processExtensions(pkg.Components, src, pkg.Metadata.YOLO) if err != nil { return types.ZarfPackage{}, nil, err } diff --git a/src/pkg/packager/creator/normal_test.go b/src/pkg/packager/creator/normal_test.go index 9a4830cb1f..dc7d5ebfe3 100644 --- a/src/pkg/packager/creator/normal_test.go +++ b/src/pkg/packager/creator/normal_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "testing" + "github.com/defenseunicorns/zarf/src/pkg/layout" + "github.com/defenseunicorns/zarf/src/types" "github.com/stretchr/testify/require" ) @@ -56,3 +58,43 @@ func TestDifferentialPackagePathSetCorrectly(t *testing.T) { }) } } + +func TestLoadPackageDefinition(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + testDir string + expectErr bool + }{ + { + name: "valid package definition", + testDir: "valid", + expectErr: false, + }, + { + name: "invalid package definition", + testDir: "invalid", + expectErr: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + src := layout.New(filepath.Join("testdata", tt.testDir)) + pc := NewPackageCreator(types.ZarfCreateOptions{}, "") + pkg, _, err := pc.LoadPackageDefinition(src) + + switch { + case tt.expectErr: + require.Error(t, err) + default: + require.NoError(t, err) + require.NotEmpty(t, pkg) + } + }) + } +} diff --git a/src/pkg/packager/creator/skeleton.go b/src/pkg/packager/creator/skeleton.go index 74dc19fd35..a576d3dcda 100644 --- a/src/pkg/packager/creator/skeleton.go +++ b/src/pkg/packager/creator/skeleton.go @@ -42,12 +42,17 @@ func NewSkeletonCreator(createOpts types.ZarfCreateOptions, publishOpts types.Za } // LoadPackageDefinition loads and configure a zarf.yaml file when creating and publishing a skeleton package. -func (sc *SkeletonCreator) LoadPackageDefinition(dst *layout.PackagePaths) (pkg types.ZarfPackage, warnings []string, err error) { - pkg, warnings, err = dst.ReadZarfYAML() +func (sc *SkeletonCreator) LoadPackageDefinition(src *layout.PackagePaths) (pkg types.ZarfPackage, warnings []string, err error) { + pkg, warnings, err = src.ReadZarfYAML() if err != nil { return types.ZarfPackage{}, nil, err } + // Perform early package validation. + if err := pkg.Validate(); err != nil { + return types.ZarfPackage{}, nil, fmt.Errorf("unable to validate package: %w", err) + } + pkg.Metadata.Architecture = config.GetArch() // Compose components into a single zarf.yaml file @@ -60,7 +65,7 @@ func (sc *SkeletonCreator) LoadPackageDefinition(dst *layout.PackagePaths) (pkg warnings = append(warnings, composeWarnings...) - pkg.Components, err = sc.processExtensions(pkg.Components, dst) + pkg.Components, err = sc.processExtensions(pkg.Components, src) if err != nil { return types.ZarfPackage{}, nil, err } diff --git a/src/pkg/packager/creator/skeleton_test.go b/src/pkg/packager/creator/skeleton_test.go new file mode 100644 index 0000000000..c9ddc2f9cc --- /dev/null +++ b/src/pkg/packager/creator/skeleton_test.go @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package creator contains functions for creating Zarf packages. +package creator + +import ( + "path/filepath" + "testing" + + "github.com/defenseunicorns/zarf/src/pkg/layout" + "github.com/defenseunicorns/zarf/src/types" + "github.com/stretchr/testify/require" +) + +func TestSkeletonLoadPackageDefinition(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + testDir string + expectErr bool + }{ + { + name: "valid package definition", + testDir: "valid", + expectErr: false, + }, + { + name: "invalid package definition", + testDir: "invalid", + expectErr: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + src := layout.New(filepath.Join("testdata", tt.testDir)) + pc := NewSkeletonCreator(types.ZarfCreateOptions{}, types.ZarfPublishOptions{}) + pkg, _, err := pc.LoadPackageDefinition(src) + + switch { + case tt.expectErr: + require.Error(t, err) + default: + require.NoError(t, err) + require.NotEmpty(t, pkg) + } + }) + } +} diff --git a/src/pkg/packager/creator/testdata/invalid/zarf.yaml b/src/pkg/packager/creator/testdata/invalid/zarf.yaml new file mode 100644 index 0000000000..0ba4630bde --- /dev/null +++ b/src/pkg/packager/creator/testdata/invalid/zarf.yaml @@ -0,0 +1,5 @@ +kind: ZarfPackageConfig +metadata: + name: minimal-invalid + # must have at least 1 component + description: Minimal invalid package diff --git a/src/pkg/packager/creator/testdata/valid/zarf.yaml b/src/pkg/packager/creator/testdata/valid/zarf.yaml new file mode 100644 index 0000000000..a9b00bb04d --- /dev/null +++ b/src/pkg/packager/creator/testdata/valid/zarf.yaml @@ -0,0 +1,6 @@ +kind: ZarfPackageConfig +metadata: + name: minimal-valid + description: Minimal valid package +components: + - name: component1 diff --git a/src/types/validate.go b/src/types/validate.go index ae6236227f..f1ab7cdb56 100644 --- a/src/types/validate.go +++ b/src/types/validate.go @@ -5,6 +5,7 @@ package types import ( + "errors" "fmt" "path/filepath" "regexp" @@ -47,6 +48,10 @@ func (pkg ZarfPackage) Validate() error { return fmt.Errorf(lang.PkgValidateErrPkgName, pkg.Metadata.Name) } + if pkg.Components == nil { + return errors.New("package must have at least 1 component") + } + for _, variable := range pkg.Variables { if err := variable.Validate(); err != nil { return fmt.Errorf(lang.PkgValidateErrVariable, err) From aa27f4fce92b1a24e9807a629b8873930360bd8d Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Tue, 21 May 2024 17:40:20 -0500 Subject: [PATCH 2/6] validate pkg after templating --- src/pkg/packager/creator/normal.go | 10 +++++----- src/pkg/packager/creator/skeleton.go | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index 79e0c01087..dbed7089e0 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -67,11 +67,6 @@ func (pc *PackageCreator) LoadPackageDefinition(src *layout.PackagePaths) (pkg t return types.ZarfPackage{}, nil, err } - // Perform early package validation. - if err := pkg.Validate(); err != nil { - return types.ZarfPackage{}, nil, fmt.Errorf("unable to validate package: %w", err) - } - pkg.Metadata.Architecture = config.GetArch(pkg.Metadata.Architecture) // Compose components into a single zarf.yaml file @@ -124,6 +119,11 @@ func (pc *PackageCreator) LoadPackageDefinition(src *layout.PackagePaths) (pkg t } } + // Perform early package validation. + if err := pkg.Validate(); err != nil { + return types.ZarfPackage{}, nil, fmt.Errorf("unable to validate package: %w", err) + } + return pkg, warnings, nil } diff --git a/src/pkg/packager/creator/skeleton.go b/src/pkg/packager/creator/skeleton.go index a576d3dcda..2d4adc82f2 100644 --- a/src/pkg/packager/creator/skeleton.go +++ b/src/pkg/packager/creator/skeleton.go @@ -48,11 +48,6 @@ func (sc *SkeletonCreator) LoadPackageDefinition(src *layout.PackagePaths) (pkg return types.ZarfPackage{}, nil, err } - // Perform early package validation. - if err := pkg.Validate(); err != nil { - return types.ZarfPackage{}, nil, fmt.Errorf("unable to validate package: %w", err) - } - pkg.Metadata.Architecture = config.GetArch() // Compose components into a single zarf.yaml file @@ -70,6 +65,11 @@ func (sc *SkeletonCreator) LoadPackageDefinition(src *layout.PackagePaths) (pkg return types.ZarfPackage{}, nil, err } + // Perform early package validation. + if err := pkg.Validate(); err != nil { + return types.ZarfPackage{}, nil, fmt.Errorf("unable to validate package: %w", err) + } + for _, warning := range warnings { message.Warn(warning) } From 44f2ab8a77c80a9270591c93ea45195d8906d7e3 Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Wed, 22 May 2024 09:35:55 -0500 Subject: [PATCH 3/6] require empty pkg on error --- src/pkg/packager/creator/normal_test.go | 1 + src/pkg/packager/creator/skeleton_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/src/pkg/packager/creator/normal_test.go b/src/pkg/packager/creator/normal_test.go index dc7d5ebfe3..967751fa6f 100644 --- a/src/pkg/packager/creator/normal_test.go +++ b/src/pkg/packager/creator/normal_test.go @@ -91,6 +91,7 @@ func TestLoadPackageDefinition(t *testing.T) { switch { case tt.expectErr: require.Error(t, err) + require.Empty(t, pkg) default: require.NoError(t, err) require.NotEmpty(t, pkg) diff --git a/src/pkg/packager/creator/skeleton_test.go b/src/pkg/packager/creator/skeleton_test.go index c9ddc2f9cc..9a21d0efdd 100644 --- a/src/pkg/packager/creator/skeleton_test.go +++ b/src/pkg/packager/creator/skeleton_test.go @@ -45,6 +45,7 @@ func TestSkeletonLoadPackageDefinition(t *testing.T) { switch { case tt.expectErr: require.Error(t, err) + require.Empty(t, pkg) default: require.NoError(t, err) require.NotEmpty(t, pkg) From 9116524368e0527093e94ab3c1b5a92bf44a563a Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Wed, 22 May 2024 10:22:49 -0500 Subject: [PATCH 4/6] sc --- src/pkg/packager/creator/skeleton_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pkg/packager/creator/skeleton_test.go b/src/pkg/packager/creator/skeleton_test.go index 9a21d0efdd..53736103aa 100644 --- a/src/pkg/packager/creator/skeleton_test.go +++ b/src/pkg/packager/creator/skeleton_test.go @@ -39,8 +39,8 @@ func TestSkeletonLoadPackageDefinition(t *testing.T) { t.Parallel() src := layout.New(filepath.Join("testdata", tt.testDir)) - pc := NewSkeletonCreator(types.ZarfCreateOptions{}, types.ZarfPublishOptions{}) - pkg, _, err := pc.LoadPackageDefinition(src) + sc := NewSkeletonCreator(types.ZarfCreateOptions{}, types.ZarfPublishOptions{}) + pkg, _, err := sc.LoadPackageDefinition(src) switch { case tt.expectErr: From d96a09927cb4f7d12aa47a5e6e0f8ac44bfbc134 Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Wed, 22 May 2024 12:02:07 -0500 Subject: [PATCH 5/6] apply PR feedback --- src/pkg/packager/creator/normal.go | 3 +-- src/pkg/packager/creator/normal_test.go | 28 +++++++++++------------ src/pkg/packager/creator/skeleton.go | 9 ++++---- src/pkg/packager/creator/skeleton_test.go | 28 +++++++++++------------ 4 files changed, 33 insertions(+), 35 deletions(-) diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index dbed7089e0..7747cfdd4d 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -119,9 +119,8 @@ func (pc *PackageCreator) LoadPackageDefinition(src *layout.PackagePaths) (pkg t } } - // Perform early package validation. if err := pkg.Validate(); err != nil { - return types.ZarfPackage{}, nil, fmt.Errorf("unable to validate package: %w", err) + return types.ZarfPackage{}, nil, err } return pkg, warnings, nil diff --git a/src/pkg/packager/creator/normal_test.go b/src/pkg/packager/creator/normal_test.go index 967751fa6f..95998ba1ca 100644 --- a/src/pkg/packager/creator/normal_test.go +++ b/src/pkg/packager/creator/normal_test.go @@ -63,19 +63,19 @@ func TestLoadPackageDefinition(t *testing.T) { t.Parallel() tests := []struct { - name string - testDir string - expectErr bool + name string + testDir string + expectedErr string }{ { - name: "valid package definition", - testDir: "valid", - expectErr: false, + name: "valid package definition", + testDir: "valid", + expectedErr: "", }, { - name: "invalid package definition", - testDir: "invalid", - expectErr: true, + name: "invalid package definition", + testDir: "invalid", + expectedErr: "package must have at least 1 component", }, } @@ -88,14 +88,14 @@ func TestLoadPackageDefinition(t *testing.T) { pc := NewPackageCreator(types.ZarfCreateOptions{}, "") pkg, _, err := pc.LoadPackageDefinition(src) - switch { - case tt.expectErr: - require.Error(t, err) - require.Empty(t, pkg) - default: + if tt.expectedErr == "" { require.NoError(t, err) require.NotEmpty(t, pkg) + return } + + require.EqualError(t, err, tt.expectedErr) + require.Empty(t, pkg) }) } } diff --git a/src/pkg/packager/creator/skeleton.go b/src/pkg/packager/creator/skeleton.go index 2d4adc82f2..b6e5498322 100644 --- a/src/pkg/packager/creator/skeleton.go +++ b/src/pkg/packager/creator/skeleton.go @@ -65,15 +65,14 @@ func (sc *SkeletonCreator) LoadPackageDefinition(src *layout.PackagePaths) (pkg return types.ZarfPackage{}, nil, err } - // Perform early package validation. - if err := pkg.Validate(); err != nil { - return types.ZarfPackage{}, nil, fmt.Errorf("unable to validate package: %w", err) - } - for _, warning := range warnings { message.Warn(warning) } + if err := pkg.Validate(); err != nil { + return types.ZarfPackage{}, nil, err + } + return pkg, warnings, nil } diff --git a/src/pkg/packager/creator/skeleton_test.go b/src/pkg/packager/creator/skeleton_test.go index 53736103aa..1fed6f985d 100644 --- a/src/pkg/packager/creator/skeleton_test.go +++ b/src/pkg/packager/creator/skeleton_test.go @@ -17,19 +17,19 @@ func TestSkeletonLoadPackageDefinition(t *testing.T) { t.Parallel() tests := []struct { - name string - testDir string - expectErr bool + name string + testDir string + expectedErr string }{ { - name: "valid package definition", - testDir: "valid", - expectErr: false, + name: "valid package definition", + testDir: "valid", + expectedErr: "", }, { - name: "invalid package definition", - testDir: "invalid", - expectErr: true, + name: "invalid package definition", + testDir: "invalid", + expectedErr: "package must have at least 1 component", }, } @@ -42,14 +42,14 @@ func TestSkeletonLoadPackageDefinition(t *testing.T) { sc := NewSkeletonCreator(types.ZarfCreateOptions{}, types.ZarfPublishOptions{}) pkg, _, err := sc.LoadPackageDefinition(src) - switch { - case tt.expectErr: - require.Error(t, err) - require.Empty(t, pkg) - default: + if tt.expectedErr == "" { require.NoError(t, err) require.NotEmpty(t, pkg) + return } + + require.EqualError(t, err, tt.expectedErr) + require.Empty(t, pkg) }) } } From b9fc08080b32f92d0f6435018780cc4e180900bf Mon Sep 17 00:00:00 2001 From: Lucas Rodriguez Date: Wed, 22 May 2024 12:23:06 -0500 Subject: [PATCH 6/6] apply PR feedback --- src/pkg/packager/creator/testdata/invalid/zarf.yaml | 3 +-- src/types/validate.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/pkg/packager/creator/testdata/invalid/zarf.yaml b/src/pkg/packager/creator/testdata/invalid/zarf.yaml index 0ba4630bde..ae4c915b6e 100644 --- a/src/pkg/packager/creator/testdata/invalid/zarf.yaml +++ b/src/pkg/packager/creator/testdata/invalid/zarf.yaml @@ -1,5 +1,4 @@ kind: ZarfPackageConfig metadata: name: minimal-invalid - # must have at least 1 component - description: Minimal invalid package + description: Must have at least 1 component diff --git a/src/types/validate.go b/src/types/validate.go index f1ab7cdb56..fbef1dab65 100644 --- a/src/types/validate.go +++ b/src/types/validate.go @@ -48,7 +48,7 @@ func (pkg ZarfPackage) Validate() error { return fmt.Errorf(lang.PkgValidateErrPkgName, pkg.Metadata.Name) } - if pkg.Components == nil { + if len(pkg.Components) == 0 { return errors.New("package must have at least 1 component") }