From 51fcc0336680e877a92b0dc305496ce08e500d35 Mon Sep 17 00:00:00 2001 From: Austin Abro <37223396+AustinAbro321@users.noreply.github.com> Date: Mon, 12 Aug 2024 17:01:40 -0400 Subject: [PATCH] refactor: move validate to lint (#2839) Signed-off-by: Austin Abro Co-authored-by: schristoff <28318173+schristoff@users.noreply.github.com> --- src/api/v1alpha1/package.go | 8 + src/{api/v1alpha1 => pkg/lint}/validate.go | 193 +++++---------- .../v1alpha1 => pkg/lint}/validate_test.go | 227 ++++++------------ src/pkg/packager/composer/list.go | 41 +++- src/pkg/packager/composer/list_test.go | 93 +++++++ src/pkg/packager/creator/utils.go | 2 +- src/pkg/packager/filters/os_test.go | 5 +- src/pkg/packager/generate.go | 3 +- src/pkg/packager/sources/cluster.go | 3 +- 9 files changed, 268 insertions(+), 307 deletions(-) rename src/{api/v1alpha1 => pkg/lint}/validate.go (59%) rename src/{api/v1alpha1 => pkg/lint}/validate_test.go (63%) diff --git a/src/api/v1alpha1/package.go b/src/api/v1alpha1/package.go index 92beae17d1..2ae9734ac6 100644 --- a/src/api/v1alpha1/package.go +++ b/src/api/v1alpha1/package.go @@ -25,6 +25,14 @@ var ( IsUppercaseNumberUnderscore = regexp.MustCompile(`^[A-Z0-9_]+$`).MatchString ) +// Zarf looks for these strings in zarf.yaml to make dynamic changes +const ( + ZarfPackageTemplatePrefix = "###ZARF_PKG_TMPL_" + ZarfPackageVariablePrefix = "###ZARF_PKG_VAR_" + ZarfPackageArch = "###ZARF_PKG_ARCH###" + ZarfComponentName = "###ZARF_COMPONENT_NAME###" +) + // ZarfPackageKind is an enum of the different kinds of Zarf packages. type ZarfPackageKind string diff --git a/src/api/v1alpha1/validate.go b/src/pkg/lint/validate.go similarity index 59% rename from src/api/v1alpha1/validate.go rename to src/pkg/lint/validate.go index f23be4769f..0cba4efe15 100644 --- a/src/api/v1alpha1/validate.go +++ b/src/pkg/lint/validate.go @@ -1,28 +1,19 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2021-Present The Zarf Authors -// Package v1alpha1 holds the definition of the v1alpha1 Zarf Package -package v1alpha1 +// Package lint contains functions for verifying zarf yaml files are valid +package lint import ( "errors" "fmt" - "path/filepath" "regexp" "strings" - "github.com/defenseunicorns/pkg/helpers/v2" + "github.com/zarf-dev/zarf/src/api/v1alpha1" "k8s.io/apimachinery/pkg/util/validation" ) -// Zarf looks for these strings in zarf.yaml to make dynamic changes -const ( - ZarfPackageTemplatePrefix = "###ZARF_PKG_TMPL_" - ZarfPackageVariablePrefix = "###ZARF_PKG_VAR_" - ZarfPackageArch = "###ZARF_PKG_ARCH###" - ZarfComponentName = "###ZARF_COMPONENT_NAME###" -) - var ( // IsLowercaseNumberHyphenNoStartHyphen is a regex for lowercase, numbers and hyphens that cannot start with a hyphen. // https://regex101.com/r/FLdG9G/2 @@ -47,105 +38,71 @@ const ( errChartReleaseNameEmpty = "release name empty, unable to fallback to chart name" ) +// Package errors found during validation. const ( - //nolint:revive //ignore - PkgValidateErrInitNoYOLO = "sorry, you can't YOLO an init package" - //nolint:revive //ignore - PkgValidateErrConstant = "invalid package constant: %w" - //nolint:revive //ignore - PkgValidateErrYOLONoOCI = "OCI images not allowed in YOLO" - //nolint:revive //ignore - PkgValidateErrYOLONoGit = "git repos not allowed in YOLO" - //nolint:revive //ignore - PkgValidateErrYOLONoArch = "cluster architecture not allowed in YOLO" - //nolint:revive //ignore - PkgValidateErrYOLONoDistro = "cluster distros not allowed in YOLO" - //nolint:revive //ignore - PkgValidateErrComponentNameNotUnique = "component name %q is not unique" - //nolint:revive //ignore - PkgValidateErrComponentReqDefault = "component %q cannot be both required and default" - //nolint:revive //ignore - PkgValidateErrComponentReqGrouped = "component %q cannot be both required and grouped" - //nolint:revive //ignore - PkgValidateErrChartNameNotUnique = "chart name %q is not unique" - //nolint:revive //ignore - PkgValidateErrChart = "invalid chart definition: %w" - //nolint:revive //ignore - PkgValidateErrManifestNameNotUnique = "manifest name %q is not unique" - //nolint:revive //ignore - PkgValidateErrManifest = "invalid manifest definition: %w" - //nolint:revive //ignore - PkgValidateErrGroupMultipleDefaults = "group %q has multiple defaults (%q, %q)" - //nolint:revive //ignore - PkgValidateErrGroupOneComponent = "group %q only has one component (%q)" - //nolint:revive //ignore - PkgValidateErrAction = "invalid action: %w" - //nolint:revive //ignore - PkgValidateErrActionCmdWait = "action %q cannot be both a command and wait action" - //nolint:revive //ignore - PkgValidateErrActionClusterNetwork = "a single wait action must contain only one of cluster or network" - //nolint:revive //ignore - PkgValidateErrChartName = "chart %q exceed the maximum length of %d characters" - //nolint:revive //ignore - PkgValidateErrChartNamespaceMissing = "chart %q must include a namespace" - //nolint:revive //ignore - PkgValidateErrChartURLOrPath = "chart %q must have either a url or localPath" - //nolint:revive //ignore - PkgValidateErrChartVersion = "chart %q must include a chart version" - //nolint:revive //ignore - PkgValidateErrImportDefinition = "invalid imported definition for %s: %s" - //nolint:revive //ignore + PkgValidateErrInitNoYOLO = "sorry, you can't YOLO an init package" + PkgValidateErrConstant = "invalid package constant: %w" + PkgValidateErrYOLONoOCI = "OCI images not allowed in YOLO" + PkgValidateErrYOLONoGit = "git repos not allowed in YOLO" + PkgValidateErrYOLONoArch = "cluster architecture not allowed in YOLO" + PkgValidateErrYOLONoDistro = "cluster distros not allowed in YOLO" + PkgValidateErrComponentNameNotUnique = "component name %q is not unique" + PkgValidateErrComponentReqDefault = "component %q cannot be both required and default" + PkgValidateErrComponentReqGrouped = "component %q cannot be both required and grouped" + PkgValidateErrChartNameNotUnique = "chart name %q is not unique" + PkgValidateErrChart = "invalid chart definition: %w" + PkgValidateErrManifestNameNotUnique = "manifest name %q is not unique" + PkgValidateErrManifest = "invalid manifest definition: %w" + PkgValidateErrGroupMultipleDefaults = "group %q has multiple defaults (%q, %q)" + PkgValidateErrGroupOneComponent = "group %q only has one component (%q)" + PkgValidateErrAction = "invalid action: %w" + PkgValidateErrActionCmdWait = "action %q cannot be both a command and wait action" + PkgValidateErrActionClusterNetwork = "a single wait action must contain only one of cluster or network" + PkgValidateErrChartName = "chart %q exceed the maximum length of %d characters" + PkgValidateErrChartNamespaceMissing = "chart %q must include a namespace" + PkgValidateErrChartURLOrPath = "chart %q must have either a url or localPath" + PkgValidateErrChartVersion = "chart %q must include a chart version" PkgValidateErrManifestFileOrKustomize = "manifest %q must have at least one file or kustomization" - //nolint:revive //ignore - PkgValidateErrManifestNameLength = "manifest %q exceed the maximum length of %d characters" - //nolint:revive //ignore - PkgValidateErrVariable = "invalid package variable: %w" + PkgValidateErrManifestNameLength = "manifest %q exceed the maximum length of %d characters" + PkgValidateErrVariable = "invalid package variable: %w" ) -// Validate runs all validation checks on the package. -func (pkg ZarfPackage) Validate() error { +// ValidatePackage runs all validation checks on the package. +func ValidatePackage(pkg v1alpha1.ZarfPackage) error { var err error - if pkg.Kind == ZarfInitConfig && pkg.Metadata.YOLO { + if pkg.Kind == v1alpha1.ZarfInitConfig && pkg.Metadata.YOLO { err = errors.Join(err, fmt.Errorf(PkgValidateErrInitNoYOLO)) } - for _, constant := range pkg.Constants { if varErr := constant.Validate(); varErr != nil { err = errors.Join(err, fmt.Errorf(PkgValidateErrConstant, varErr)) } } - uniqueComponentNames := make(map[string]bool) groupDefault := make(map[string]string) groupedComponents := make(map[string][]string) - if pkg.Metadata.YOLO { for _, component := range pkg.Components { if len(component.Images) > 0 { err = errors.Join(err, fmt.Errorf(PkgValidateErrYOLONoOCI)) } - if len(component.Repos) > 0 { err = errors.Join(err, fmt.Errorf(PkgValidateErrYOLONoGit)) } - if component.Only.Cluster.Architecture != "" { err = errors.Join(err, fmt.Errorf(PkgValidateErrYOLONoArch)) } - if len(component.Only.Cluster.Distros) > 0 { err = errors.Join(err, fmt.Errorf(PkgValidateErrYOLONoDistro)) } } } - for _, component := range pkg.Components { // ensure component name is unique if _, ok := uniqueComponentNames[component.Name]; ok { err = errors.Join(err, fmt.Errorf(PkgValidateErrComponentNameNotUnique, component.Name)) } uniqueComponentNames[component.Name] = true - if component.IsRequired() { if component.Default { err = errors.Join(err, fmt.Errorf(PkgValidateErrComponentReqDefault, component.Name)) @@ -154,7 +111,6 @@ func (pkg ZarfPackage) Validate() error { err = errors.Join(err, fmt.Errorf(PkgValidateErrComponentReqGrouped, component.Name)) } } - uniqueChartNames := make(map[string]bool) for _, chart := range component.Charts { // ensure chart name is unique @@ -162,12 +118,10 @@ func (pkg ZarfPackage) Validate() error { err = errors.Join(err, fmt.Errorf(PkgValidateErrChartNameNotUnique, chart.Name)) } uniqueChartNames[chart.Name] = true - - if chartErr := chart.Validate(); chartErr != nil { + if chartErr := validateChart(chart); chartErr != nil { err = errors.Join(err, fmt.Errorf(PkgValidateErrChart, chartErr)) } } - uniqueManifestNames := make(map[string]bool) for _, manifest := range component.Manifests { // ensure manifest name is unique @@ -175,16 +129,13 @@ func (pkg ZarfPackage) Validate() error { err = errors.Join(err, fmt.Errorf(PkgValidateErrManifestNameNotUnique, manifest.Name)) } uniqueManifestNames[manifest.Name] = true - - if manifestErr := manifest.Validate(); manifestErr != nil { + if manifestErr := validateManifest(manifest); manifestErr != nil { err = errors.Join(err, fmt.Errorf(PkgValidateErrManifest, manifestErr)) } } - - if actionsErr := component.Actions.validate(); actionsErr != nil { + if actionsErr := validateActions(component.Actions); actionsErr != nil { err = errors.Join(err, fmt.Errorf("%q: %w", component.Name, actionsErr)) } - // ensure groups don't have multiple defaults or only one component if component.DeprecatedGroup != "" { if component.Default { @@ -196,74 +147,38 @@ func (pkg ZarfPackage) Validate() error { groupedComponents[component.DeprecatedGroup] = append(groupedComponents[component.DeprecatedGroup], component.Name) } } - for groupKey, componentNames := range groupedComponents { if len(componentNames) == 1 { err = errors.Join(err, fmt.Errorf(PkgValidateErrGroupOneComponent, groupKey, componentNames[0])) } } - return err } -func (a ZarfComponentActions) validate() error { +// validateActions validates the actions of a component. +func validateActions(a v1alpha1.ZarfComponentActions) error { var err error - err = errors.Join(err, a.OnCreate.Validate()) + err = errors.Join(err, validateActionSet(a.OnCreate)) - if a.OnCreate.HasSetVariables() { + if hasSetVariables(a.OnCreate) { err = errors.Join(err, fmt.Errorf("cannot contain setVariables outside of onDeploy in actions")) } - err = errors.Join(err, a.OnDeploy.Validate()) + err = errors.Join(err, validateActionSet(a.OnDeploy)) - if a.OnRemove.HasSetVariables() { + if hasSetVariables(a.OnRemove) { err = errors.Join(err, fmt.Errorf("cannot contain setVariables outside of onDeploy in actions")) } - err = errors.Join(err, a.OnRemove.Validate()) - - return err -} - -// Validate validates the component trying to be imported. -func (c ZarfComponent) Validate() error { - var err error - path := c.Import.Path - url := c.Import.URL - - // ensure path or url is provided - if path == "" && url == "" { - err = errors.Join(err, fmt.Errorf(PkgValidateErrImportDefinition, c.Name, "neither a path nor a URL was provided")) - } - - // ensure path and url are not both provided - if path != "" && url != "" { - err = errors.Join(err, fmt.Errorf(PkgValidateErrImportDefinition, c.Name, "both a path and a URL were provided")) - } - - // validation for path - if url == "" && path != "" { - // ensure path is not an absolute path - if filepath.IsAbs(path) { - err = errors.Join(err, fmt.Errorf(PkgValidateErrImportDefinition, c.Name, "path cannot be an absolute path")) - } - } - - // validation for url - if url != "" && path == "" { - ok := helpers.IsOCIURL(url) - if !ok { - err = errors.Join(err, fmt.Errorf(PkgValidateErrImportDefinition, c.Name, "URL is not a valid OCI URL")) - } - } + err = errors.Join(err, validateActionSet(a.OnRemove)) return err } -// HasSetVariables returns true if any of the actions contain setVariables. -func (as ZarfComponentActionSet) HasSetVariables() bool { - check := func(actions []ZarfComponentAction) bool { +// hasSetVariables returns true if any of the actions contain setVariables. +func hasSetVariables(as v1alpha1.ZarfComponentActionSet) bool { + check := func(actions []v1alpha1.ZarfComponentAction) bool { for _, action := range actions { if len(action.SetVariables) > 0 { return true @@ -275,12 +190,12 @@ func (as ZarfComponentActionSet) HasSetVariables() bool { return check(as.Before) || check(as.After) || check(as.OnSuccess) || check(as.OnFailure) } -// Validate runs all validation checks on component action sets. -func (as ZarfComponentActionSet) Validate() error { +// validateActionSet runs all validation checks on component action sets. +func validateActionSet(as v1alpha1.ZarfComponentActionSet) error { var err error - validate := func(actions []ZarfComponentAction) { + validate := func(actions []v1alpha1.ZarfComponentAction) { for _, action := range actions { - if actionErr := action.Validate(); actionErr != nil { + if actionErr := validateAction(action); actionErr != nil { err = errors.Join(err, fmt.Errorf(PkgValidateErrAction, actionErr)) } } @@ -293,8 +208,8 @@ func (as ZarfComponentActionSet) Validate() error { return err } -// Validate runs all validation checks on an action. -func (action ZarfComponentAction) Validate() error { +// validateAction runs all validation checks on an action. +func validateAction(action v1alpha1.ZarfComponentAction) error { var err error if action.Wait != nil { @@ -340,8 +255,8 @@ func validateReleaseName(chartName, releaseName string) (err error) { return } -// Validate runs all validation checks on a chart. -func (chart ZarfChart) Validate() error { +// validateChart runs all validation checks on a chart. +func validateChart(chart v1alpha1.ZarfChart) error { var err error if len(chart.Name) > ZarfMaxChartNameLength { @@ -372,8 +287,8 @@ func (chart ZarfChart) Validate() error { return err } -// Validate runs all validation checks on a manifest. -func (manifest ZarfManifest) Validate() error { +// validateManifest runs all validation checks on a manifest. +func validateManifest(manifest v1alpha1.ZarfManifest) error { var err error if len(manifest.Name) > ZarfMaxChartNameLength { diff --git a/src/api/v1alpha1/validate_test.go b/src/pkg/lint/validate_test.go similarity index 63% rename from src/api/v1alpha1/validate_test.go rename to src/pkg/lint/validate_test.go index b41a5c522a..4e0abeda96 100644 --- a/src/api/v1alpha1/validate_test.go +++ b/src/pkg/lint/validate_test.go @@ -1,34 +1,34 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2021-Present The Zarf Authors -// Package v1alpha1 holds the definition of the v1alpha1 Zarf Package -package v1alpha1 +// Package lint contains functions for verifying zarf yaml files are valid +package lint import ( "fmt" - "path/filepath" "strings" "testing" "github.com/defenseunicorns/pkg/helpers/v2" "github.com/stretchr/testify/require" + "github.com/zarf-dev/zarf/src/api/v1alpha1" ) func TestZarfPackageValidate(t *testing.T) { t.Parallel() tests := []struct { name string - pkg ZarfPackage + pkg v1alpha1.ZarfPackage expectedErrs []string }{ { name: "valid package", - pkg: ZarfPackage{ - Kind: ZarfPackageConfig, - Metadata: ZarfMetadata{ + pkg: v1alpha1.ZarfPackage{ + Kind: v1alpha1.ZarfPackageConfig, + Metadata: v1alpha1.ZarfMetadata{ Name: "valid-package", }, - Components: []ZarfComponent{ + Components: []v1alpha1.ZarfComponent{ { Name: "component1", }, @@ -38,21 +38,21 @@ func TestZarfPackageValidate(t *testing.T) { }, { name: "invalid package", - pkg: ZarfPackage{ - Kind: ZarfPackageConfig, - Metadata: ZarfMetadata{ + pkg: v1alpha1.ZarfPackage{ + Kind: v1alpha1.ZarfPackageConfig, + Metadata: v1alpha1.ZarfMetadata{ Name: "invalid-package", }, - Components: []ZarfComponent{ + Components: []v1alpha1.ZarfComponent{ { Name: "invalid", Required: helpers.BoolPtr(true), Default: true, - Charts: []ZarfChart{ + Charts: []v1alpha1.ZarfChart{ {Name: "chart1", Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0"}, {Name: "chart1", Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0"}, }, - Manifests: []ZarfManifest{ + Manifests: []v1alpha1.ZarfManifest{ {Name: "manifest1", Files: []string{"file1"}}, {Name: "manifest1", Files: []string{"file2"}}, }, @@ -79,7 +79,7 @@ func TestZarfPackageValidate(t *testing.T) { Name: "duplicate", }, }, - Constants: []Constant{ + Constants: []v1alpha1.Constant{ { Name: "BAD", Pattern: "^good_val$", @@ -100,19 +100,19 @@ func TestZarfPackageValidate(t *testing.T) { }, { name: "invalid yolo", - pkg: ZarfPackage{ - Kind: ZarfInitConfig, - Metadata: ZarfMetadata{ + pkg: v1alpha1.ZarfPackage{ + Kind: v1alpha1.ZarfInitConfig, + Metadata: v1alpha1.ZarfMetadata{ Name: "invalid-yolo", YOLO: true, }, - Components: []ZarfComponent{ + Components: []v1alpha1.ZarfComponent{ { Name: "yolo", Images: []string{"an-image"}, Repos: []string{"a-repo"}, - Only: ZarfComponentOnlyTarget{ - Cluster: ZarfComponentOnlyCluster{ + Only: v1alpha1.ZarfComponentOnlyTarget{ + Cluster: v1alpha1.ZarfComponentOnlyCluster{ Architecture: "not-empty", Distros: []string{"not-empty"}, }, @@ -134,7 +134,7 @@ func TestZarfPackageValidate(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := tt.pkg.Validate() + err := ValidatePackage(tt.pkg) if tt.expectedErrs == nil { require.NoError(t, err) return @@ -149,23 +149,23 @@ func TestValidateManifest(t *testing.T) { t.Parallel() longName := strings.Repeat("a", ZarfMaxChartNameLength+1) tests := []struct { - manifest ZarfManifest + manifest v1alpha1.ZarfManifest expectedErrs []string name string }{ { name: "valid", - manifest: ZarfManifest{Name: "valid", Files: []string{"a-file"}}, + manifest: v1alpha1.ZarfManifest{Name: "valid", Files: []string{"a-file"}}, expectedErrs: nil, }, { name: "long name", - manifest: ZarfManifest{Name: longName, Files: []string{"a-file"}}, + manifest: v1alpha1.ZarfManifest{Name: longName, Files: []string{"a-file"}}, expectedErrs: []string{fmt.Sprintf(PkgValidateErrManifestNameLength, longName, ZarfMaxChartNameLength)}, }, { name: "no files or kustomize", - manifest: ZarfManifest{Name: "nothing-there"}, + manifest: v1alpha1.ZarfManifest{Name: "nothing-there"}, expectedErrs: []string{fmt.Sprintf(PkgValidateErrManifestFileOrKustomize, "nothing-there")}, }, } @@ -173,7 +173,7 @@ func TestValidateManifest(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := tt.manifest.Validate() + err := validateManifest(tt.manifest) if tt.expectedErrs == nil { require.NoError(t, err) return @@ -253,25 +253,25 @@ func TestValidateChart(t *testing.T) { longName := strings.Repeat("a", ZarfMaxChartNameLength+1) tests := []struct { name string - chart ZarfChart + chart v1alpha1.ZarfChart expectedErrs []string partialMatch bool }{ { name: "valid", - chart: ZarfChart{Name: "chart1", Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0", ReleaseName: "this-is-valid"}, + chart: v1alpha1.ZarfChart{Name: "chart1", Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0", ReleaseName: "this-is-valid"}, expectedErrs: nil, }, { name: "long name", - chart: ZarfChart{Name: longName, Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0"}, + chart: v1alpha1.ZarfChart{Name: longName, Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0"}, expectedErrs: []string{ fmt.Sprintf(PkgValidateErrChartName, longName, ZarfMaxChartNameLength), }, }, { name: "no url, local path, version, or namespace", - chart: ZarfChart{Name: "invalid"}, + chart: v1alpha1.ZarfChart{Name: "invalid"}, expectedErrs: []string{ fmt.Sprintf(PkgValidateErrChartNamespaceMissing, "invalid"), fmt.Sprintf(PkgValidateErrChartURLOrPath, "invalid"), @@ -280,25 +280,25 @@ func TestValidateChart(t *testing.T) { }, { name: "both url and local path", - chart: ZarfChart{Name: "invalid", Namespace: "whatever", URL: "http://whatever", LocalPath: "wherever", Version: "v1.0.0"}, + chart: v1alpha1.ZarfChart{Name: "invalid", Namespace: "whatever", URL: "http://whatever", LocalPath: "wherever", Version: "v1.0.0"}, expectedErrs: []string{ fmt.Sprintf(PkgValidateErrChartURLOrPath, "invalid"), }, }, { name: "invalid releaseName", - chart: ZarfChart{ReleaseName: "namedwithperiods-0.47.0", Name: "releaseName", Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0"}, + chart: v1alpha1.ZarfChart{ReleaseName: "namedwithperiods-0.47.0", Name: "releaseName", Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0"}, expectedErrs: []string{"invalid release name 'namedwithperiods-0.47.0'"}, partialMatch: true, }, { name: "missing releaseName fallsback to name", - chart: ZarfChart{Name: "chart3", Namespace: "namespace", URL: "http://whatever", Version: "v1.0.0"}, + chart: v1alpha1.ZarfChart{Name: "chart3", Namespace: "namespace", URL: "http://whatever", Version: "v1.0.0"}, expectedErrs: nil, }, { name: "missing name and releaseName", - chart: ZarfChart{Namespace: "namespace", URL: "http://whatever", Version: "v1.0.0"}, + chart: v1alpha1.ZarfChart{Namespace: "namespace", URL: "http://whatever", Version: "v1.0.0"}, expectedErrs: []string{errChartReleaseNameEmpty}, }, } @@ -306,7 +306,7 @@ func TestValidateChart(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := tt.chart.Validate() + err := validateChart(tt.chart) if tt.expectedErrs == nil { require.NoError(t, err) return @@ -329,21 +329,21 @@ func TestValidateComponentActions(t *testing.T) { t.Parallel() tests := []struct { name string - actions ZarfComponentActions + actions v1alpha1.ZarfComponentActions expectedErrs []string }{ { name: "valid actions", - actions: ZarfComponentActions{ - OnCreate: ZarfComponentActionSet{ - Before: []ZarfComponentAction{ + actions: v1alpha1.ZarfComponentActions{ + OnCreate: v1alpha1.ZarfComponentActionSet{ + Before: []v1alpha1.ZarfComponentAction{ { Cmd: "echo 'onCreate before valid'", }, }, }, - OnDeploy: ZarfComponentActionSet{ - Before: []ZarfComponentAction{ + OnDeploy: v1alpha1.ZarfComponentActionSet{ + Before: []v1alpha1.ZarfComponentAction{ { Cmd: "echo 'onDeploy before valid'", }, @@ -354,12 +354,12 @@ func TestValidateComponentActions(t *testing.T) { }, { name: "setVariables in onCreate", - actions: ZarfComponentActions{ - OnCreate: ZarfComponentActionSet{ - Before: []ZarfComponentAction{ + actions: v1alpha1.ZarfComponentActions{ + OnCreate: v1alpha1.ZarfComponentActionSet{ + Before: []v1alpha1.ZarfComponentAction{ { Cmd: "echo 'invalid setVariable'", - SetVariables: []Variable{{Name: "VAR"}}, + SetVariables: []v1alpha1.Variable{{Name: "VAR"}}, }, }, }, @@ -368,34 +368,34 @@ func TestValidateComponentActions(t *testing.T) { }, { name: "invalid onCreate action", - actions: ZarfComponentActions{ - OnCreate: ZarfComponentActionSet{ - Before: []ZarfComponentAction{ + actions: v1alpha1.ZarfComponentActions{ + OnCreate: v1alpha1.ZarfComponentActionSet{ + Before: []v1alpha1.ZarfComponentAction{ { Cmd: "create", - Wait: &ZarfComponentActionWait{Cluster: &ZarfComponentActionWaitCluster{}}, + Wait: &v1alpha1.ZarfComponentActionWait{Cluster: &v1alpha1.ZarfComponentActionWaitCluster{}}, }, }, }, - OnDeploy: ZarfComponentActionSet{ - After: []ZarfComponentAction{ + OnDeploy: v1alpha1.ZarfComponentActionSet{ + After: []v1alpha1.ZarfComponentAction{ { Cmd: "deploy", - Wait: &ZarfComponentActionWait{Cluster: &ZarfComponentActionWaitCluster{}}, + Wait: &v1alpha1.ZarfComponentActionWait{Cluster: &v1alpha1.ZarfComponentActionWaitCluster{}}, }, }, }, - OnRemove: ZarfComponentActionSet{ - OnSuccess: []ZarfComponentAction{ + OnRemove: v1alpha1.ZarfComponentActionSet{ + OnSuccess: []v1alpha1.ZarfComponentAction{ { Cmd: "remove", - Wait: &ZarfComponentActionWait{Cluster: &ZarfComponentActionWaitCluster{}}, + Wait: &v1alpha1.ZarfComponentActionWait{Cluster: &v1alpha1.ZarfComponentActionWaitCluster{}}, }, }, - OnFailure: []ZarfComponentAction{ + OnFailure: []v1alpha1.ZarfComponentAction{ { Cmd: "remove2", - Wait: &ZarfComponentActionWait{Cluster: &ZarfComponentActionWaitCluster{}}, + Wait: &v1alpha1.ZarfComponentActionWait{Cluster: &v1alpha1.ZarfComponentActionWaitCluster{}}, }, }, }, @@ -413,7 +413,7 @@ func TestValidateComponentActions(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := tt.actions.validate() + err := validateActions(tt.actions) if tt.expectedErrs == nil { require.NoError(t, err) return @@ -428,18 +428,18 @@ func TestValidateComponentAction(t *testing.T) { t.Parallel() tests := []struct { name string - action ZarfComponentAction + action v1alpha1.ZarfComponentAction expectedErrs []string }{ { name: "valid action no conditions", - action: ZarfComponentAction{}, + action: v1alpha1.ZarfComponentAction{}, }, { name: "cmd and wait both set, nothing in wait", - action: ZarfComponentAction{ + action: v1alpha1.ZarfComponentAction{ Cmd: "ls", - Wait: &ZarfComponentActionWait{}, + Wait: &v1alpha1.ZarfComponentActionWait{}, }, expectedErrs: []string{ fmt.Sprintf(PkgValidateErrActionCmdWait, "ls"), @@ -448,11 +448,10 @@ func TestValidateComponentAction(t *testing.T) { }, { name: "cluster and network both set", - action: ZarfComponentAction{ - Wait: &ZarfComponentActionWait{Cluster: &ZarfComponentActionWaitCluster{}, Network: &ZarfComponentActionWaitNetwork{}}, + action: v1alpha1.ZarfComponentAction{ + Wait: &v1alpha1.ZarfComponentActionWait{Cluster: &v1alpha1.ZarfComponentActionWaitCluster{}, Network: &v1alpha1.ZarfComponentActionWaitNetwork{}}, }, - //nolint:staticcheck //ignore - expectedErrs: []string{fmt.Sprintf(PkgValidateErrActionClusterNetwork)}, + expectedErrs: []string{PkgValidateErrActionClusterNetwork}, }, } @@ -460,99 +459,7 @@ func TestValidateComponentAction(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := tt.action.Validate() - if tt.expectedErrs == nil { - require.NoError(t, err) - return - } - errs := strings.Split(err.Error(), "\n") - require.ElementsMatch(t, tt.expectedErrs, errs) - }) - } -} - -func TestValidateZarfComponent(t *testing.T) { - t.Parallel() - absPath, err := filepath.Abs("abs") - require.NoError(t, err) - tests := []struct { - component ZarfComponent - expectedErrs []string - name string - }{ - { - name: "valid path", - component: ZarfComponent{ - Name: "component1", - Import: ZarfComponentImport{ - Path: "relative/path", - }, - }, - expectedErrs: nil, - }, - { - name: "valid URL", - component: ZarfComponent{ - Name: "component2", - Import: ZarfComponentImport{ - URL: "oci://example.com/package:v0.0.1", - }, - }, - expectedErrs: nil, - }, - { - name: "neither path nor URL provided", - component: ZarfComponent{ - Name: "neither", - }, - expectedErrs: []string{ - fmt.Sprintf(PkgValidateErrImportDefinition, "neither", "neither a path nor a URL was provided"), - }, - }, - { - name: "both path and URL provided", - component: ZarfComponent{ - Name: "both", - Import: ZarfComponentImport{ - Path: "relative/path", - URL: "https://example.com", - }, - }, - expectedErrs: []string{ - fmt.Sprintf(PkgValidateErrImportDefinition, "both", "both a path and a URL were provided"), - }, - }, - { - name: "absolute path provided", - component: ZarfComponent{ - Name: "abs-path", - Import: ZarfComponentImport{ - Path: absPath, - }, - }, - expectedErrs: []string{ - fmt.Sprintf(PkgValidateErrImportDefinition, "abs-path", "path cannot be an absolute path"), - }, - }, - { - name: "invalid URL provided", - component: ZarfComponent{ - Name: "bad-url", - Import: ZarfComponentImport{ - URL: "https://example.com", - }, - }, - expectedErrs: []string{ - fmt.Sprintf(PkgValidateErrImportDefinition, "bad-url", "URL is not a valid OCI URL"), - }, - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - err := tt.component.Validate() + err := validateAction(tt.action) if tt.expectedErrs == nil { require.NoError(t, err) return diff --git a/src/pkg/packager/composer/list.go b/src/pkg/packager/composer/list.go index 9d2bce9d97..291fdb71db 100644 --- a/src/pkg/packager/composer/list.go +++ b/src/pkg/packager/composer/list.go @@ -6,6 +6,7 @@ package composer import ( "context" + "errors" "fmt" "path/filepath" "strings" @@ -117,6 +118,41 @@ func (ic *ImportChain) append(c v1alpha1.ZarfComponent, index int, originalPacka } } +// validateComponentCompose validates that a component doesn't break compose rules +func validateComponentCompose(c v1alpha1.ZarfComponent) error { + var err error + path := c.Import.Path + url := c.Import.URL + + // ensure path or url is provided + if path == "" && url == "" { + err = errors.Join(err, errors.New("neither a path nor a URL was provided")) + } + + // ensure path and url are not both provided + if path != "" && url != "" { + err = errors.Join(err, errors.New("both a path and a URL were provided")) + } + + // validation for path + if url == "" && path != "" { + // ensure path is not an absolute path + if filepath.IsAbs(path) { + err = errors.Join(err, errors.New("path cannot be an absolute path")) + } + } + + // validation for url + if url != "" && path == "" { + ok := helpers.IsOCIURL(url) + if !ok { + err = errors.Join(err, errors.New("URL is not a valid OCI URL")) + } + } + + return err +} + // NewImportChain creates a new import chain from a component // Returning the chain on error so we can have additional information to use during lint func NewImportChain(ctx context.Context, head v1alpha1.ZarfComponent, index int, originalPackageName, arch, flavor string) (*ImportChain, error) { @@ -140,9 +176,8 @@ func NewImportChain(ctx context.Context, head v1alpha1.ZarfComponent, index int, return ic, nil } - // TODO: stuff like this should also happen in linting - if err := node.ZarfComponent.Validate(); err != nil { - return ic, err + if err := validateComponentCompose(node.ZarfComponent); err != nil { + return nil, fmt.Errorf("invalid imported definition for %s: %w", node.Name, err) } // ensure that remote components are not importing other remote components diff --git a/src/pkg/packager/composer/list_test.go b/src/pkg/packager/composer/list_test.go index beff63b524..1703348b3d 100644 --- a/src/pkg/packager/composer/list_test.go +++ b/src/pkg/packager/composer/list_test.go @@ -9,6 +9,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/require" @@ -540,3 +541,95 @@ func createDummyComponent(t *testing.T, name, importDir, subName string) v1alpha }, } } + +func TestValidateZarfComponent(t *testing.T) { + t.Parallel() + absPath, err := filepath.Abs("abs") + require.NoError(t, err) + tests := []struct { + component v1alpha1.ZarfComponent + expectedErrs []string + name string + }{ + { + name: "valid path", + component: v1alpha1.ZarfComponent{ + Name: "component1", + Import: v1alpha1.ZarfComponentImport{ + Path: "relative/path", + }, + }, + expectedErrs: nil, + }, + { + name: "valid URL", + component: v1alpha1.ZarfComponent{ + Name: "component2", + Import: v1alpha1.ZarfComponentImport{ + URL: "oci://example.com/package:v0.0.1", + }, + }, + expectedErrs: nil, + }, + { + name: "neither path nor URL provided", + component: v1alpha1.ZarfComponent{ + Name: "neither", + }, + expectedErrs: []string{ + "neither a path nor a URL was provided", + }, + }, + { + name: "both path and URL provided", + component: v1alpha1.ZarfComponent{ + Name: "both", + Import: v1alpha1.ZarfComponentImport{ + Path: "relative/path", + URL: "https://example.com", + }, + }, + expectedErrs: []string{ + "both a path and a URL were provided", + }, + }, + { + name: "absolute path provided", + component: v1alpha1.ZarfComponent{ + Name: "abs-path", + Import: v1alpha1.ZarfComponentImport{ + Path: absPath, + }, + }, + expectedErrs: []string{ + "path cannot be an absolute path", + }, + }, + { + name: "invalid URL provided", + component: v1alpha1.ZarfComponent{ + Name: "bad-url", + Import: v1alpha1.ZarfComponentImport{ + URL: "https://example.com", + }, + }, + expectedErrs: []string{ + "URL is not a valid OCI URL", + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := validateComponentCompose(tt.component) + if tt.expectedErrs == nil { + require.NoError(t, err) + return + } + errs := strings.Split(err.Error(), "\n") + require.ElementsMatch(t, tt.expectedErrs, errs) + }) + } +} diff --git a/src/pkg/packager/creator/utils.go b/src/pkg/packager/creator/utils.go index fdf1da49f6..b138ce7af2 100644 --- a/src/pkg/packager/creator/utils.go +++ b/src/pkg/packager/creator/utils.go @@ -20,7 +20,7 @@ import ( // Validate errors if a package violates the schema or any runtime validations // This must be run while in the parent directory of the zarf.yaml being validated func Validate(pkg v1alpha1.ZarfPackage, baseDir string) error { - if err := pkg.Validate(); err != nil { + if err := lint.ValidatePackage(pkg); err != nil { return fmt.Errorf("package validation failed: %w", err) } diff --git a/src/pkg/packager/filters/os_test.go b/src/pkg/packager/filters/os_test.go index fb81d1355a..c2e022a752 100644 --- a/src/pkg/packager/filters/os_test.go +++ b/src/pkg/packager/filters/os_test.go @@ -9,11 +9,12 @@ import ( "github.com/stretchr/testify/require" "github.com/zarf-dev/zarf/src/api/v1alpha1" + "github.com/zarf-dev/zarf/src/pkg/lint" ) func TestLocalOSFilter(t *testing.T) { pkg := v1alpha1.ZarfPackage{} - for _, os := range v1alpha1.SupportedOS() { + for _, os := range lint.SupportedOS() { pkg.Components = append(pkg.Components, v1alpha1.ZarfComponent{ Only: v1alpha1.ZarfComponentOnlyTarget{ LocalOS: os, @@ -21,7 +22,7 @@ func TestLocalOSFilter(t *testing.T) { }) } - for _, os := range v1alpha1.SupportedOS() { + for _, os := range lint.SupportedOS() { filter := ByLocalOS(os) result, err := filter.Apply(pkg) if os == "" { diff --git a/src/pkg/packager/generate.go b/src/pkg/packager/generate.go index cc3393dbb5..068808b171 100644 --- a/src/pkg/packager/generate.go +++ b/src/pkg/packager/generate.go @@ -16,6 +16,7 @@ import ( "github.com/zarf-dev/zarf/src/api/v1alpha1" "github.com/zarf-dev/zarf/src/config" "github.com/zarf-dev/zarf/src/pkg/layout" + "github.com/zarf-dev/zarf/src/pkg/lint" "github.com/zarf-dev/zarf/src/pkg/message" ) @@ -73,7 +74,7 @@ func (p *Packager) Generate(ctx context.Context) (err error) { p.cfg.Pkg.Components[i].Images = images[name] } - if err := p.cfg.Pkg.Validate(); err != nil { + if err := lint.ValidatePackage(p.cfg.Pkg); err != nil { return err } diff --git a/src/pkg/packager/sources/cluster.go b/src/pkg/packager/sources/cluster.go index ea45b254dd..5c4da94271 100644 --- a/src/pkg/packager/sources/cluster.go +++ b/src/pkg/packager/sources/cluster.go @@ -12,6 +12,7 @@ import ( "github.com/zarf-dev/zarf/src/api/v1alpha1" "github.com/zarf-dev/zarf/src/pkg/cluster" "github.com/zarf-dev/zarf/src/pkg/layout" + "github.com/zarf-dev/zarf/src/pkg/lint" "github.com/zarf-dev/zarf/src/pkg/packager/filters" "github.com/zarf-dev/zarf/src/pkg/utils" "github.com/zarf-dev/zarf/src/types" @@ -24,7 +25,7 @@ var ( // NewClusterSource creates a new cluster source. func NewClusterSource(pkgOpts *types.ZarfPackageOptions) (PackageSource, error) { - if !v1alpha1.IsLowercaseNumberHyphenNoStartHyphen(pkgOpts.PackageSource) { + if !lint.IsLowercaseNumberHyphenNoStartHyphen(pkgOpts.PackageSource) { return nil, fmt.Errorf("invalid package name %q", pkgOpts.PackageSource) }