From 6ed1b2618cd3b4914eb47f6229f7edadc5daa099 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Tue, 23 Jul 2024 14:13:55 +0000 Subject: [PATCH 1/4] feat: run schema validation on create Signed-off-by: Austin Abro --- main.go | 2 +- src/cmd/dev.go | 4 +- src/config/lang/english.go | 7 - src/pkg/lint/findings.go | 115 ++++++++ src/pkg/lint/findings_test.go | 105 +++++++ src/pkg/lint/lint.go | 136 +++++++++ src/pkg/lint/lint_test.go | 71 +++++ src/pkg/lint/rules.go | 108 +++++++ src/pkg/lint/rules_test.go | 141 +++++++++ src/pkg/lint/schema.go | 83 ++++++ .../lint/lint_test.go => lint/schema_test.go} | 162 +--------- src/pkg/packager/creator/creator_test.go | 38 ++- src/pkg/packager/creator/normal.go | 5 +- src/pkg/packager/creator/skeleton.go | 4 +- src/pkg/packager/creator/utils.go | 21 ++ src/pkg/packager/dev.go | 77 +---- src/pkg/packager/lint/findings.go | 41 --- src/pkg/packager/lint/findings_test.go | 122 -------- src/pkg/packager/lint/lint.go | 278 ------------------ src/pkg/variables/types.go | 13 - src/types/package.go | 2 + src/types/validate.go | 34 --- src/types/validate_test.go | 45 +-- 23 files changed, 837 insertions(+), 777 deletions(-) create mode 100644 src/pkg/lint/findings.go create mode 100644 src/pkg/lint/findings_test.go create mode 100644 src/pkg/lint/lint.go create mode 100644 src/pkg/lint/lint_test.go create mode 100644 src/pkg/lint/rules.go create mode 100644 src/pkg/lint/rules_test.go create mode 100644 src/pkg/lint/schema.go rename src/pkg/{packager/lint/lint_test.go => lint/schema_test.go} (56%) delete mode 100644 src/pkg/packager/lint/findings.go delete mode 100644 src/pkg/packager/lint/findings_test.go delete mode 100644 src/pkg/packager/lint/lint.go diff --git a/main.go b/main.go index 70feb480f4..f424789b54 100644 --- a/main.go +++ b/main.go @@ -13,7 +13,7 @@ import ( "github.com/zarf-dev/zarf/src/cmd" "github.com/zarf-dev/zarf/src/config" - "github.com/zarf-dev/zarf/src/pkg/packager/lint" + "github.com/zarf-dev/zarf/src/pkg/lint" ) //go:embed cosign.pub diff --git a/src/cmd/dev.go b/src/cmd/dev.go index b10f2dde51..631b4de315 100644 --- a/src/cmd/dev.go +++ b/src/cmd/dev.go @@ -22,6 +22,7 @@ import ( "github.com/zarf-dev/zarf/src/cmd/common" "github.com/zarf-dev/zarf/src/config" "github.com/zarf-dev/zarf/src/config/lang" + "github.com/zarf-dev/zarf/src/pkg/lint" "github.com/zarf-dev/zarf/src/pkg/message" "github.com/zarf-dev/zarf/src/pkg/packager" "github.com/zarf-dev/zarf/src/pkg/transform" @@ -269,6 +270,7 @@ var devLintCmd = &cobra.Command{ Short: lang.CmdDevLintShort, Long: lang.CmdDevLintLong, RunE: func(cmd *cobra.Command, args []string) error { + config.CommonOptions.Confirm = true pkgConfig.CreateOpts.BaseDir = common.SetBaseDirectory(args) v := common.GetViper() pkgConfig.CreateOpts.SetVariables = helpers.TransformAndMergeMap( @@ -280,7 +282,7 @@ var devLintCmd = &cobra.Command{ } defer pkgClient.ClearTempPaths() - return pkgClient.Lint(cmd.Context()) + return lint.Validate(cmd.Context(), pkgConfig.CreateOpts) }, } diff --git a/src/config/lang/english.go b/src/config/lang/english.go index df39428f48..87e569242b 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -632,19 +632,15 @@ const ( // Package validate const ( PkgValidateTemplateDeprecation = "Package template %q is using the deprecated syntax ###ZARF_PKG_VAR_%s###. This will be removed in Zarf v1.0.0. Please update to ###ZARF_PKG_TMPL_%s###." - PkgValidateMustBeUppercase = "variable name %q must be all uppercase and contain no special characters except _" 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" PkgValidateErrChart = "invalid chart definition: %w" PkgValidateErrChartName = "chart %q exceed the maximum length of %d characters" - PkgValidateErrChartNameMissing = "chart must include a name" PkgValidateErrChartNameNotUnique = "chart name %q is not unique" 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" - PkgValidateErrComponentName = "component name %q must be all lowercase and contain no special characters except '-' and cannot start with a '-'" - PkgValidateErrComponentLocalOS = "component %q contains a localOS value that is not supported: %s (supported: %s)" 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" @@ -656,11 +652,8 @@ const ( PkgValidateErrManifest = "invalid manifest definition: %w" PkgValidateErrManifestFileOrKustomize = "manifest %q must have at least one file or kustomization" PkgValidateErrManifestNameLength = "manifest %q exceed the maximum length of %d characters" - PkgValidateErrManifestNameMissing = "manifest must include a name" PkgValidateErrManifestNameNotUnique = "manifest name %q is not unique" - PkgValidateErrPkgConstantName = "constant name %q must be all uppercase and contain no special characters except _" PkgValidateErrPkgConstantPattern = "provided value for constant %q does not match pattern %q" - PkgValidateErrPkgName = "package name %q must be all lowercase and contain no special characters except '-' and cannot start with a '-'" PkgValidateErrVariable = "invalid package variable: %w" PkgValidateErrYOLONoArch = "cluster architecture not allowed in YOLO" PkgValidateErrYOLONoDistro = "cluster distros not allowed in YOLO" diff --git a/src/pkg/lint/findings.go b/src/pkg/lint/findings.go new file mode 100644 index 0000000000..a8ad9b5eac --- /dev/null +++ b/src/pkg/lint/findings.go @@ -0,0 +1,115 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package lint contains functions for verifying zarf yaml files are valid +package lint + +import ( + "fmt" + "path/filepath" + + "github.com/defenseunicorns/pkg/helpers/v2" + "github.com/fatih/color" + "github.com/zarf-dev/zarf/src/pkg/message" +) + +// PackageFinding is a struct that contains a finding about something wrong with a package +type PackageFinding struct { + // YqPath is the path to the key where the error originated from, this is sometimes empty in the case of a general error + YqPath string + Description string + // Item is the value of a key that is causing an error, for example a bad image name + Item string + // PackageNameOverride shows the name of the package that the error originated from + // If it is not set the base package will be used when displaying the error + PackageNameOverride string + // PackagePathOverride shows the path to the package that the error originated from + // If it is not set the base package will be used when displaying the error + PackagePathOverride string + Severity Severity +} + +// Severity is the type of finding +type Severity int + +// different severities of package errors +const ( + SevErr Severity = iota + 1 + SevWarn +) + +func (f PackageFinding) itemizedDescription() string { + if f.Item == "" { + return f.Description + } + return fmt.Sprintf("%s - %s", f.Description, f.Item) +} + +func colorWrapSev(s Severity) string { + if s == SevErr { + return message.ColorWrap("Error", color.FgRed) + } else if s == SevWarn { + return message.ColorWrap("Warning", color.FgYellow) + } + return "unknown" +} + +func filterLowerSeverity(findings []PackageFinding, severity Severity) []PackageFinding { + findings = helpers.RemoveMatches(findings, func(finding PackageFinding) bool { + return finding.Severity > severity + }) + return findings +} + +// PrintFindings prints the findings of the given severity in a table +func PrintFindings(findings []PackageFinding, severity Severity, baseDir string, packageName string) { + findings = filterLowerSeverity(findings, severity) + if len(findings) == 0 { + return + } + mapOfFindingsByPath := GroupFindingsByPath(findings, packageName) + + header := []string{"Type", "Path", "Message"} + + for _, findings := range mapOfFindingsByPath { + lintData := [][]string{} + for _, finding := range findings { + lintData = append(lintData, []string{ + colorWrapSev(finding.Severity), + message.ColorWrap(finding.YqPath, color.FgCyan), + finding.itemizedDescription(), + }) + } + var packagePathFromUser string + if helpers.IsOCIURL(findings[0].PackagePathOverride) { + packagePathFromUser = findings[0].PackagePathOverride + } else { + packagePathFromUser = filepath.Join(baseDir, findings[0].PackagePathOverride) + } + message.Notef("Linting package %q at %s", findings[0].PackageNameOverride, packagePathFromUser) + message.Table(header, lintData) + } +} + +// GroupFindingsByPath groups findings by their package path +func GroupFindingsByPath(findings []PackageFinding, packageName string) map[string][]PackageFinding { + for i := range findings { + if findings[i].PackageNameOverride == "" { + findings[i].PackageNameOverride = packageName + } + if findings[i].PackagePathOverride == "" { + findings[i].PackagePathOverride = "." + } + } + + mapOfFindingsByPath := make(map[string][]PackageFinding) + for _, finding := range findings { + mapOfFindingsByPath[finding.PackagePathOverride] = append(mapOfFindingsByPath[finding.PackagePathOverride], finding) + } + return mapOfFindingsByPath +} + +// HasSevOrHigher returns true if the findings contain a severity equal to or greater than the given severity +func HasSevOrHigher(findings []PackageFinding, severity Severity) bool { + return len(filterLowerSeverity(findings, severity)) > 0 +} diff --git a/src/pkg/lint/findings_test.go b/src/pkg/lint/findings_test.go new file mode 100644 index 0000000000..f3c09673c8 --- /dev/null +++ b/src/pkg/lint/findings_test.go @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package lint contains functions for verifying zarf yaml files are valid +package lint + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestGroupFindingsByPath(t *testing.T) { + t.Parallel() + tests := []struct { + name string + findings []PackageFinding + severity Severity + packageName string + want map[string][]PackageFinding + }{ + { + name: "same package multiple findings", + findings: []PackageFinding{ + {Severity: SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"}, + {Severity: SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"}, + }, + packageName: "testPackage", + want: map[string][]PackageFinding{ + "path": { + {Severity: SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"}, + {Severity: SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"}, + }, + }, + }, + { + name: "different packages single finding", + findings: []PackageFinding{ + {Severity: SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"}, + {Severity: SevErr, PackageNameOverride: "", PackagePathOverride: ""}, + }, + packageName: "testPackage", + want: map[string][]PackageFinding{ + "path": {{Severity: SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"}}, + ".": {{Severity: SevErr, PackageNameOverride: "testPackage", PackagePathOverride: "."}}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + require.Equal(t, tt.want, GroupFindingsByPath(tt.findings, tt.packageName)) + }) + } +} + +func TestHasSeverity(t *testing.T) { + t.Parallel() + tests := []struct { + name string + severity Severity + expected bool + findings []PackageFinding + }{ + { + name: "error severity present", + findings: []PackageFinding{ + { + Severity: SevErr, + }, + }, + severity: SevErr, + expected: true, + }, + { + name: "error severity not present", + findings: []PackageFinding{ + { + Severity: SevWarn, + }, + }, + severity: SevErr, + expected: false, + }, + { + name: "err and warning severity present", + findings: []PackageFinding{ + { + Severity: SevWarn, + }, + { + Severity: SevErr, + }, + }, + severity: SevErr, + expected: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + require.Equal(t, tt.expected, HasSevOrHigher(tt.findings, tt.severity)) + }) + } +} diff --git a/src/pkg/lint/lint.go b/src/pkg/lint/lint.go new file mode 100644 index 0000000000..6b48951f48 --- /dev/null +++ b/src/pkg/lint/lint.go @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package lint contains functions for verifying zarf yaml files are valid +package lint + +import ( + "context" + "errors" + "fmt" + "os" + + "github.com/zarf-dev/zarf/src/config" + "github.com/zarf-dev/zarf/src/config/lang" + "github.com/zarf-dev/zarf/src/pkg/layout" + "github.com/zarf-dev/zarf/src/pkg/message" + "github.com/zarf-dev/zarf/src/pkg/packager/composer" + "github.com/zarf-dev/zarf/src/pkg/utils" + "github.com/zarf-dev/zarf/src/types" +) + +// Validate lints the given Zarf package +func Validate(ctx context.Context, createOpts types.ZarfCreateOptions) error { + var findings []PackageFinding + if err := os.Chdir(createOpts.BaseDir); err != nil { + return fmt.Errorf("unable to access directory %q: %w", createOpts.BaseDir, err) + } + var pkg types.ZarfPackage + if err := utils.ReadYaml(layout.ZarfYAML, &pkg); err != nil { + return err + } + + compFindings, err := lintComponents(ctx, pkg, createOpts) + if err != nil { + return err + } + findings = append(findings, compFindings...) + schemaFindings, err := ValidateSchema() + if err != nil { + return err + } + findings = append(findings, schemaFindings...) + + if len(findings) == 0 { + message.Successf("0 findings for %q", pkg.Metadata.Name) + return nil + } + PrintFindings(findings, SevWarn, createOpts.BaseDir, pkg.Metadata.Name) + if HasSevOrHigher(findings, SevErr) { + return errors.New("errors during lint") + } + return nil +} + +func lintComponents(ctx context.Context, pkg types.ZarfPackage, createOpts types.ZarfCreateOptions) ([]PackageFinding, error) { + var findings []PackageFinding + + for i, component := range pkg.Components { + arch := config.GetArch(pkg.Metadata.Architecture) + if !composer.CompatibleComponent(component, arch, createOpts.Flavor) { + continue + } + + chain, err := composer.NewImportChain(ctx, component, i, pkg.Metadata.Name, arch, createOpts.Flavor) + + if err != nil { + return nil, err + } + + node := chain.Head() + for node != nil { + component := node.ZarfComponent + compFindings, err := fillComponentTemplate(&component, createOpts) + if err != nil { + return nil, err + } + compFindings = append(compFindings, CheckComponentValues(component, node.Index())...) + for i := range compFindings { + compFindings[i].PackagePathOverride = node.ImportLocation() + compFindings[i].PackageNameOverride = node.OriginalPackageName() + } + findings = append(findings, compFindings...) + node = node.Next() + } + } + return findings, nil +} + +func fillComponentTemplate(c *types.ZarfComponent, createOpts types.ZarfCreateOptions) ([]PackageFinding, error) { + var findings []PackageFinding + templateMap := map[string]string{} + + setVarsAndWarn := func(templatePrefix string, deprecated bool) error { + yamlTemplates, err := utils.FindYamlTemplates(c, templatePrefix, "###") + if err != nil { + return err + } + + var unSetTemplates bool + for key := range yamlTemplates { + if deprecated { + findings = append(findings, PackageFinding{ + Description: fmt.Sprintf(lang.PkgValidateTemplateDeprecation, key, key, key), + Severity: SevWarn, + }) + } + if _, present := createOpts.SetVariables[key]; !present { + unSetTemplates = true + } + } + if unSetTemplates { + findings = append(findings, PackageFinding{ + Description: lang.UnsetVarLintWarning, + Severity: SevWarn, + }) + } + for key, value := range createOpts.SetVariables { + templateMap[fmt.Sprintf("%s%s###", templatePrefix, key)] = value + } + return nil + } + + if err := setVarsAndWarn(types.ZarfPackageTemplatePrefix, false); err != nil { + return nil, err + } + + // [DEPRECATION] Set the Package Variable syntax as well for backward compatibility + if err := setVarsAndWarn(types.ZarfPackageVariablePrefix, true); err != nil { + return nil, err + } + + if err := utils.ReloadYamlTemplate(c, templateMap); err != nil { + return nil, err + } + return findings, nil +} diff --git a/src/pkg/lint/lint_test.go b/src/pkg/lint/lint_test.go new file mode 100644 index 0000000000..d5c5a03495 --- /dev/null +++ b/src/pkg/lint/lint_test.go @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package lint contains functions for verifying zarf yaml files are valid +package lint + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + "github.com/zarf-dev/zarf/src/config/lang" + "github.com/zarf-dev/zarf/src/types" +) + +func TestLintComponents(t *testing.T) { + t.Run("Test composable components with bad path", func(t *testing.T) { + t.Parallel() + zarfPackage := types.ZarfPackage{ + Components: []types.ZarfComponent{ + { + Import: types.ZarfComponentImport{Path: "bad-path"}, + }, + }, + Metadata: types.ZarfMetadata{Name: "test-zarf-package"}, + } + + createOpts := types.ZarfCreateOptions{Flavor: "", BaseDir: "."} + _, err := lintComponents(context.Background(), zarfPackage, createOpts) + require.Error(t, err) + }) +} +func TestFillComponentTemplate(t *testing.T) { + createOpts := types.ZarfCreateOptions{ + SetVariables: map[string]string{ + "KEY1": "value1", + "KEY2": "value2", + }, + } + + component := types.ZarfComponent{ + Images: []string{ + fmt.Sprintf("%s%s###", types.ZarfPackageTemplatePrefix, "KEY1"), + fmt.Sprintf("%s%s###", types.ZarfPackageVariablePrefix, "KEY2"), + fmt.Sprintf("%s%s###", types.ZarfPackageTemplatePrefix, "KEY3"), + }, + } + + findings, err := fillComponentTemplate(&component, createOpts) + require.NoError(t, err) + expectedFindings := []PackageFinding{ + { + Severity: SevWarn, + Description: "There are templates that are not set and won't be evaluated during lint", + }, + { + Severity: SevWarn, + Description: fmt.Sprintf(lang.PkgValidateTemplateDeprecation, "KEY2", "KEY2", "KEY2"), + }, + } + expectedComponent := types.ZarfComponent{ + Images: []string{ + "value1", + "value2", + fmt.Sprintf("%s%s###", types.ZarfPackageTemplatePrefix, "KEY3"), + }, + } + require.ElementsMatch(t, expectedFindings, findings) + require.Equal(t, expectedComponent, component) +} diff --git a/src/pkg/lint/rules.go b/src/pkg/lint/rules.go new file mode 100644 index 0000000000..17c22256a4 --- /dev/null +++ b/src/pkg/lint/rules.go @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package lint contains functions for verifying zarf yaml files are valid +package lint + +import ( + "fmt" + "strings" + + "github.com/defenseunicorns/pkg/helpers/v2" + "github.com/zarf-dev/zarf/src/pkg/transform" + "github.com/zarf-dev/zarf/src/types" +) + +func isPinnedImage(image string) (bool, error) { + transformedImage, err := transform.ParseImageRef(image) + if err != nil { + if strings.Contains(image, types.ZarfPackageTemplatePrefix) || + strings.Contains(image, types.ZarfPackageVariablePrefix) { + return true, nil + } + return false, err + } + if isCosignSignature(transformedImage.Tag) || isCosignAttestation(transformedImage.Tag) { + return true, nil + } + return (transformedImage.Digest != ""), err +} + +func isCosignSignature(image string) bool { + return strings.HasSuffix(image, ".sig") +} + +func isCosignAttestation(image string) bool { + return strings.HasSuffix(image, ".att") +} + +func isPinnedRepo(repo string) bool { + return (strings.Contains(repo, "@")) +} + +// CheckComponentValues runs lint rules validating values on component keys, should be run after templating +func CheckComponentValues(c types.ZarfComponent, i int) []PackageFinding { + var findings []PackageFinding + findings = append(findings, checkForUnpinnedRepos(c, i)...) + findings = append(findings, checkForUnpinnedImages(c, i)...) + findings = append(findings, checkForUnpinnedFiles(c, i)...) + return findings +} + +func checkForUnpinnedRepos(c types.ZarfComponent, i int) []PackageFinding { + var findings []PackageFinding + for j, repo := range c.Repos { + repoYqPath := fmt.Sprintf(".components.[%d].repos.[%d]", i, j) + if !isPinnedRepo(repo) { + findings = append(findings, PackageFinding{ + YqPath: repoYqPath, + Description: "Unpinned repository", + Item: repo, + Severity: SevWarn, + }) + } + } + return findings +} + +func checkForUnpinnedImages(c types.ZarfComponent, i int) []PackageFinding { + var findings []PackageFinding + for j, image := range c.Images { + imageYqPath := fmt.Sprintf(".components.[%d].images.[%d]", i, j) + pinnedImage, err := isPinnedImage(image) + if err != nil { + findings = append(findings, PackageFinding{ + YqPath: imageYqPath, + Description: "Failed to parse image reference", + Item: image, + Severity: SevWarn, + }) + continue + } + if !pinnedImage { + findings = append(findings, PackageFinding{ + YqPath: imageYqPath, + Description: "Image not pinned with digest", + Item: image, + Severity: SevWarn, + }) + } + } + return findings +} + +func checkForUnpinnedFiles(c types.ZarfComponent, i int) []PackageFinding { + var findings []PackageFinding + for j, file := range c.Files { + fileYqPath := fmt.Sprintf(".components.[%d].files.[%d]", i, j) + if file.Shasum == "" && helpers.IsURL(file.Source) { + findings = append(findings, PackageFinding{ + YqPath: fileYqPath, + Description: "No shasum for remote file", + Item: file.Source, + Severity: SevWarn, + }) + } + } + return findings +} diff --git a/src/pkg/lint/rules_test.go b/src/pkg/lint/rules_test.go new file mode 100644 index 0000000000..a73b275d1c --- /dev/null +++ b/src/pkg/lint/rules_test.go @@ -0,0 +1,141 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package lint contains functions for verifying zarf yaml files are valid +package lint + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" + "github.com/zarf-dev/zarf/src/types" +) + +func TestValidateComponent(t *testing.T) { + t.Parallel() + + t.Run("Unpinnned repo warning", func(t *testing.T) { + t.Parallel() + unpinnedRepo := "https://github.com/zarf-dev/zarf-public-test.git" + component := types.ZarfComponent{Repos: []string{ + unpinnedRepo, + "https://dev.azure.com/zarf-dev/zarf-public-test/_git/zarf-public-test@v0.0.1", + }} + findings := checkForUnpinnedRepos(component, 0) + expected := []PackageFinding{ + { + Item: unpinnedRepo, + Description: "Unpinned repository", + Severity: SevWarn, + YqPath: ".components.[0].repos.[0]", + }, + } + require.Equal(t, expected, findings) + }) + + t.Run("Unpinnned image warning", func(t *testing.T) { + t.Parallel() + unpinnedImage := "registry.com:9001/whatever/image:1.0.0" + badImage := "badimage:badimage@@sha256:3fbc632167424a6d997e74f5" + cosignSignature := "ghcr.io/stefanprodan/podinfo:sha256-57a654ace69ec02ba8973093b6a786faa15640575fbf0dbb603db55aca2ccec8.sig" + cosignAttestation := "ghcr.io/stefanprodan/podinfo:sha256-57a654ace69ec02ba8973093b6a786faa15640575fbf0dbb603db55aca2ccec8.att" + component := types.ZarfComponent{Images: []string{ + unpinnedImage, + "busybox:latest@sha256:3fbc632167424a6d997e74f52b878d7cc478225cffac6bc977eedfe51c7f4e79", + badImage, + cosignSignature, + cosignAttestation, + }} + findings := checkForUnpinnedImages(component, 0) + expected := []PackageFinding{ + { + Item: unpinnedImage, + Description: "Image not pinned with digest", + Severity: SevWarn, + YqPath: ".components.[0].images.[0]", + }, + { + Item: badImage, + Description: "Failed to parse image reference", + Severity: SevWarn, + YqPath: ".components.[0].images.[2]", + }, + } + require.Equal(t, expected, findings) + }) + + t.Run("Unpinnned file warning", func(t *testing.T) { + t.Parallel() + fileURL := "http://example.com/file.zip" + localFile := "local.txt" + zarfFiles := []types.ZarfFile{ + { + Source: fileURL, + }, + { + Source: localFile, + }, + { + Source: fileURL, + Shasum: "fake-shasum", + }, + } + component := types.ZarfComponent{Files: zarfFiles} + findings := checkForUnpinnedFiles(component, 0) + expected := []PackageFinding{ + { + Item: fileURL, + Description: "No shasum for remote file", + Severity: SevWarn, + YqPath: ".components.[0].files.[0]", + }, + } + require.Equal(t, expected, findings) + require.Len(t, findings, 1) + }) + + t.Run("isImagePinned", func(t *testing.T) { + t.Parallel() + tests := []struct { + input string + expected bool + err error + }{ + { + input: "registry.com:8080/zarf-dev/whatever", + expected: false, + err: nil, + }, + { + input: "ghcr.io/zarf-dev/pepr/controller:v0.15.0", + expected: false, + err: nil, + }, + { + input: "busybox:latest@sha256:3fbc632167424a6d997e74f52b878d7cc478225cffac6bc977eedfe51c7f4e79", + expected: true, + err: nil, + }, + { + input: "busybox:bad/image", + expected: false, + err: errors.New("invalid reference format"), + }, + { + input: "busybox:###ZARF_PKG_TMPL_BUSYBOX_IMAGE###", + expected: true, + err: nil, + }, + } + for _, tc := range tests { + t.Run(tc.input, func(t *testing.T) { + actual, err := isPinnedImage(tc.input) + if err != nil { + require.EqualError(t, err, tc.err.Error()) + } + require.Equal(t, tc.expected, actual) + }) + } + }) +} diff --git a/src/pkg/lint/schema.go b/src/pkg/lint/schema.go new file mode 100644 index 0000000000..136bd2b46d --- /dev/null +++ b/src/pkg/lint/schema.go @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package lint contains functions for verifying zarf yaml files are valid +package lint + +import ( + "fmt" + "io/fs" + "regexp" + + "github.com/xeipuuv/gojsonschema" + "github.com/zarf-dev/zarf/src/pkg/layout" + "github.com/zarf-dev/zarf/src/pkg/utils" +) + +// ZarfSchema is exported so main.go can embed the schema file +var ZarfSchema fs.ReadFileFS + +// ValidateSchema checks the Zarf package in the current directory against the Zarf schema +func ValidateSchema() ([]PackageFinding, error) { + + var untypedZarfPackage interface{} + if err := utils.ReadYaml(layout.ZarfYAML, &untypedZarfPackage); err != nil { + return nil, err + } + + jsonSchema, err := ZarfSchema.ReadFile("zarf.schema.json") + if err != nil { + return nil, err + } + + return validateSchema(jsonSchema, untypedZarfPackage) +} + +func makeFieldPathYqCompat(field string) string { + if field == "(root)" { + return field + } + // \b is a metacharacter that will stop at the next non-word character (including .) + // https://regex101.com/r/pIRPk0/1 + re := regexp.MustCompile(`(\b\d+\b)`) + + wrappedField := re.ReplaceAllString(field, "[$1]") + + return fmt.Sprintf(".%s", wrappedField) +} + +func validateSchema(jsonSchema []byte, untypedZarfPackage interface{}) ([]PackageFinding, error) { + var findings []PackageFinding + + schemaErrors, err := runSchema(jsonSchema, untypedZarfPackage) + if err != nil { + return nil, err + } + + if len(schemaErrors) != 0 { + for _, schemaErr := range schemaErrors { + findings = append(findings, PackageFinding{ + YqPath: makeFieldPathYqCompat(schemaErr.Field()), + Description: schemaErr.Description(), + Severity: SevErr, + }) + } + } + + return findings, err +} + +func runSchema(jsonSchema []byte, pkg interface{}) ([]gojsonschema.ResultError, error) { + schemaLoader := gojsonschema.NewBytesLoader(jsonSchema) + documentLoader := gojsonschema.NewGoLoader(pkg) + + result, err := gojsonschema.Validate(schemaLoader, documentLoader) + if err != nil { + return nil, err + } + + if !result.Valid() { + return result.Errors(), nil + } + return nil, nil +} diff --git a/src/pkg/packager/lint/lint_test.go b/src/pkg/lint/schema_test.go similarity index 56% rename from src/pkg/packager/lint/lint_test.go rename to src/pkg/lint/schema_test.go index 213c0938d2..1827310c32 100644 --- a/src/pkg/packager/lint/lint_test.go +++ b/src/pkg/lint/schema_test.go @@ -5,8 +5,6 @@ package lint import ( - "context" - "errors" "fmt" "os" "testing" @@ -19,7 +17,7 @@ import ( func TestZarfSchema(t *testing.T) { t.Parallel() - zarfSchema, err := os.ReadFile("../../../../zarf.schema.json") + zarfSchema, err := os.ReadFile("../../../zarf.schema.json") require.NoError(t, err) tests := []struct { @@ -119,7 +117,6 @@ func TestZarfSchema(t *testing.T) { }, } for _, tt := range tests { - tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() findings, err := runSchema(zarfSchema, tt.pkg) @@ -180,10 +177,10 @@ components: }, }) require.NoError(t, err) - expected := []types.PackageFinding{ + expected := []PackageFinding{ { Description: "Invalid type. Expected: array, given: null", - Severity: types.SevErr, + Severity: SevErr, YqPath: ".components", }, } @@ -191,89 +188,8 @@ components: }) } -func TestValidateComponent(t *testing.T) { +func TestYqCompat(t *testing.T) { t.Parallel() - - t.Run("Unpinnned repo warning", func(t *testing.T) { - t.Parallel() - unpinnedRepo := "https://github.com/zarf-dev/zarf-public-test.git" - component := types.ZarfComponent{Repos: []string{ - unpinnedRepo, - "https://dev.azure.com/defenseunicorns/zarf-public-test/_git/zarf-public-test@v0.0.1", - }} - findings := checkForUnpinnedRepos(component, 0) - expected := []types.PackageFinding{ - { - Item: unpinnedRepo, - Description: "Unpinned repository", - Severity: types.SevWarn, - YqPath: ".components.[0].repos.[0]", - }, - } - require.Equal(t, expected, findings) - }) - - t.Run("Unpinnned image warning", func(t *testing.T) { - t.Parallel() - unpinnedImage := "registry.com:9001/whatever/image:1.0.0" - badImage := "badimage:badimage@@sha256:3fbc632167424a6d997e74f5" - cosignSignature := "ghcr.io/stefanprodan/podinfo:sha256-57a654ace69ec02ba8973093b6a786faa15640575fbf0dbb603db55aca2ccec8.sig" - cosignAttestation := "ghcr.io/stefanprodan/podinfo:sha256-57a654ace69ec02ba8973093b6a786faa15640575fbf0dbb603db55aca2ccec8.att" - component := types.ZarfComponent{Images: []string{ - unpinnedImage, - "busybox:latest@sha256:3fbc632167424a6d997e74f52b878d7cc478225cffac6bc977eedfe51c7f4e79", - badImage, - cosignSignature, - cosignAttestation, - }} - findings := checkForUnpinnedImages(component, 0) - expected := []types.PackageFinding{ - { - Item: unpinnedImage, - Description: "Image not pinned with digest", - Severity: types.SevWarn, - YqPath: ".components.[0].images.[0]", - }, - { - Item: badImage, - Description: "Failed to parse image reference", - Severity: types.SevWarn, - YqPath: ".components.[0].images.[2]", - }, - } - require.Equal(t, expected, findings) - }) - - t.Run("Unpinnned file warning", func(t *testing.T) { - t.Parallel() - fileURL := "http://example.com/file.zip" - localFile := "local.txt" - zarfFiles := []types.ZarfFile{ - { - Source: fileURL, - }, - { - Source: localFile, - }, - { - Source: fileURL, - Shasum: "fake-shasum", - }, - } - component := types.ZarfComponent{Files: zarfFiles} - findings := checkForUnpinnedFiles(component, 0) - expectedErr := []types.PackageFinding{ - { - Item: fileURL, - Description: "No shasum for remote file", - Severity: types.SevWarn, - YqPath: ".components.[0].files.[0]", - }, - } - require.Equal(t, expectedErr, findings) - require.Len(t, findings, 1) - }) - t.Run("Wrap standalone numbers in bracket", func(t *testing.T) { t.Parallel() input := "components12.12.import.path" @@ -288,74 +204,4 @@ func TestValidateComponent(t *testing.T) { actual := makeFieldPathYqCompat(input) require.Equal(t, input, actual) }) - - t.Run("Test composable components with bad path", func(t *testing.T) { - t.Parallel() - zarfPackage := types.ZarfPackage{ - Components: []types.ZarfComponent{ - { - Import: types.ZarfComponentImport{Path: "bad-path"}, - }, - }, - Metadata: types.ZarfMetadata{Name: "test-zarf-package"}, - } - - createOpts := types.ZarfCreateOptions{Flavor: "", BaseDir: "."} - _, err := lintComponents(context.Background(), zarfPackage, createOpts) - require.Error(t, err) - }) - - t.Run("isImagePinned", func(t *testing.T) { - t.Parallel() - tests := []struct { - input string - expected bool - err error - }{ - { - input: "registry.com:8080/defenseunicorns/whatever", - expected: false, - err: nil, - }, - { - input: "ghcr.io/defenseunicorns/pepr/controller:v0.15.0", - expected: false, - err: nil, - }, - { - input: "busybox:latest@sha256:3fbc632167424a6d997e74f52b878d7cc478225cffac6bc977eedfe51c7f4e79", - expected: true, - err: nil, - }, - { - input: "busybox:bad/image", - expected: false, - err: errors.New("invalid reference format"), - }, - { - input: "busybox:###ZARF_PKG_TMPL_BUSYBOX_IMAGE###", - expected: true, - err: nil, - }, - { - input: "ghcr.io/stefanprodan/podinfo:sha256-57a654ace69ec02ba8973093b6a786faa15640575fbf0dbb603db55aca2ccec8.sig", - expected: true, - err: nil, - }, - { - input: "ghcr.io/stefanprodan/podinfo:sha256-57a654ace69ec02ba8973093b6a786faa15640575fbf0dbb603db55aca2ccec8.att", - expected: true, - err: nil, - }, - } - for _, tc := range tests { - t.Run(tc.input, func(t *testing.T) { - actual, err := isPinnedImage(tc.input) - if err != nil { - require.EqualError(t, err, tc.err.Error()) - } - require.Equal(t, tc.expected, actual) - }) - } - }) } diff --git a/src/pkg/packager/creator/creator_test.go b/src/pkg/packager/creator/creator_test.go index 1c2e3b7153..7248fe1cc9 100644 --- a/src/pkg/packager/creator/creator_test.go +++ b/src/pkg/packager/creator/creator_test.go @@ -6,16 +6,33 @@ package creator import ( "context" + "io/fs" + "os" "path/filepath" "testing" "github.com/stretchr/testify/require" "github.com/zarf-dev/zarf/src/pkg/layout" + "github.com/zarf-dev/zarf/src/pkg/lint" "github.com/zarf-dev/zarf/src/types" ) +type mockSchemaLoader struct { + b []byte +} + +func (m *mockSchemaLoader) ReadFile(_ string) ([]byte, error) { + return m.b, nil +} + +// Satisfy fs.ReadFileFS interface +func (m *mockSchemaLoader) Open(_ string) (fs.File, error) { + return nil, nil +} + func TestLoadPackageDefinition(t *testing.T) { - t.Parallel() + // TODO once creator is refactored to not expect to be in the same directory as the zarf.yaml file + // this test can be re-parallelized tests := []struct { name string testDir string @@ -31,7 +48,7 @@ func TestLoadPackageDefinition(t *testing.T) { { name: "invalid package definition", testDir: "invalid", - expectedErr: "package must have at least 1 component", + expectedErr: "found errors in package", creator: NewPackageCreator(types.ZarfCreateOptions{}, ""), }, { @@ -43,17 +60,28 @@ func TestLoadPackageDefinition(t *testing.T) { { name: "invalid package definition", testDir: "invalid", - expectedErr: "package must have at least 1 component", + expectedErr: "found errors in package", creator: NewSkeletonCreator(types.ZarfCreateOptions{}, types.ZarfPublishOptions{}), }, } + b, err := os.ReadFile("../../../../zarf.schema.json") + require.NoError(t, err) + lint.ZarfSchema = &mockSchemaLoader{b: b} for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - t.Parallel() + cwd, err := os.Getwd() + require.NoError(t, err) + defer func() { + err = os.Chdir(cwd) + require.NoError(t, err) + }() + path := filepath.Join("testdata", tt.testDir) + err = os.Chdir(path) + require.NoError(t, err) - src := layout.New(filepath.Join("testdata", tt.testDir)) + src := layout.New(".") pkg, _, err := tt.creator.LoadPackageDefinition(context.Background(), src) if tt.expectedErr == "" { diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index 3b34b7e846..8671a891df 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -27,6 +27,7 @@ import ( "github.com/zarf-dev/zarf/src/internal/packager/kustomize" "github.com/zarf-dev/zarf/src/internal/packager/sbom" "github.com/zarf-dev/zarf/src/pkg/layout" + "github.com/zarf-dev/zarf/src/pkg/lint" "github.com/zarf-dev/zarf/src/pkg/message" "github.com/zarf-dev/zarf/src/pkg/packager/actions" "github.com/zarf-dev/zarf/src/pkg/packager/filters" @@ -74,7 +75,6 @@ func (pc *PackageCreator) LoadPackageDefinition(ctx context.Context, src *layout if err != nil { return types.ZarfPackage{}, nil, err } - warnings = append(warnings, composeWarnings...) // After components are composed, template the active package. @@ -119,7 +119,8 @@ func (pc *PackageCreator) LoadPackageDefinition(ctx context.Context, src *layout } } - if err := pkg.Validate(); err != nil { + if findings, err := Validate(pkg); err != nil { + lint.PrintFindings(findings, lint.SevErr, pc.createOpts.BaseDir, pkg.Metadata.Name) return types.ZarfPackage{}, nil, err } diff --git a/src/pkg/packager/creator/skeleton.go b/src/pkg/packager/creator/skeleton.go index 091874c84f..12c7b14671 100644 --- a/src/pkg/packager/creator/skeleton.go +++ b/src/pkg/packager/creator/skeleton.go @@ -20,6 +20,7 @@ import ( "github.com/zarf-dev/zarf/src/internal/packager/helm" "github.com/zarf-dev/zarf/src/internal/packager/kustomize" "github.com/zarf-dev/zarf/src/pkg/layout" + "github.com/zarf-dev/zarf/src/pkg/lint" "github.com/zarf-dev/zarf/src/pkg/message" "github.com/zarf-dev/zarf/src/pkg/utils" "github.com/zarf-dev/zarf/src/pkg/zoci" @@ -70,7 +71,8 @@ func (sc *SkeletonCreator) LoadPackageDefinition(ctx context.Context, src *layou message.Warn(warning) } - if err := pkg.Validate(); err != nil { + if findings, err := Validate(pkg); err != nil { + lint.PrintFindings(findings, lint.SevErr, sc.createOpts.BaseDir, pkg.Metadata.Name) return types.ZarfPackage{}, nil, err } diff --git a/src/pkg/packager/creator/utils.go b/src/pkg/packager/creator/utils.go index 5c3f962766..3b485e170b 100644 --- a/src/pkg/packager/creator/utils.go +++ b/src/pkg/packager/creator/utils.go @@ -5,15 +5,36 @@ package creator import ( + "fmt" "os" "runtime" "time" "github.com/zarf-dev/zarf/src/config" + "github.com/zarf-dev/zarf/src/pkg/lint" "github.com/zarf-dev/zarf/src/pkg/packager/deprecated" "github.com/zarf-dev/zarf/src/types" ) +// 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 types.ZarfPackage) ([]lint.PackageFinding, error) { + if err := pkg.Validate(); err != nil { + return nil, fmt.Errorf("package validation failed: %w", err) + } + + findings, err := lint.ValidateSchema() + if err != nil { + return nil, fmt.Errorf("unable to check schema: %w", err) + } + + if lint.HasSevOrHigher(findings, lint.SevErr) { + return findings, fmt.Errorf("found errors in package") + } + + return nil, nil +} + // recordPackageMetadata records various package metadata during package create. func recordPackageMetadata(pkg *types.ZarfPackage, createOpts types.ZarfCreateOptions) error { now := time.Now() diff --git a/src/pkg/packager/dev.go b/src/pkg/packager/dev.go index 4363c0b49a..7713565482 100644 --- a/src/pkg/packager/dev.go +++ b/src/pkg/packager/dev.go @@ -6,21 +6,17 @@ package packager import ( "context" - "errors" "fmt" "os" - "path/filepath" "runtime" "github.com/defenseunicorns/pkg/helpers/v2" - "github.com/fatih/color" "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" "github.com/zarf-dev/zarf/src/pkg/packager/creator" "github.com/zarf-dev/zarf/src/pkg/packager/filters" - "github.com/zarf-dev/zarf/src/pkg/packager/lint" - "github.com/zarf-dev/zarf/src/pkg/utils" "github.com/zarf-dev/zarf/src/types" ) @@ -58,8 +54,9 @@ func (p *Packager) DevDeploy(ctx context.Context) error { return err } - if err := p.cfg.Pkg.Validate(); err != nil { - return fmt.Errorf("unable to validate package: %w", err) + if findings, err := creator.Validate(p.cfg.Pkg); err != nil { + lint.PrintFindings(findings, lint.SevErr, p.cfg.CreateOpts.BaseDir, p.cfg.Pkg.Metadata.Name) + return fmt.Errorf("package validation failed: %w", err) } if err := p.populatePackageVariableConfig(); err != nil { @@ -110,69 +107,3 @@ func (p *Packager) DevDeploy(ctx context.Context) error { // cd back return os.Chdir(cwd) } - -// Lint ensures a package is valid & follows suggested conventions -func (p *Packager) Lint(ctx context.Context) error { - if err := os.Chdir(p.cfg.CreateOpts.BaseDir); err != nil { - return fmt.Errorf("unable to access directory %q: %w", p.cfg.CreateOpts.BaseDir, err) - } - - if err := utils.ReadYaml(layout.ZarfYAML, &p.cfg.Pkg); err != nil { - return err - } - - findings, err := lint.Validate(ctx, p.cfg.Pkg, p.cfg.CreateOpts) - if err != nil { - return fmt.Errorf("linting failed: %w", err) - } - - if len(findings) == 0 { - message.Successf("0 findings for %q", p.cfg.Pkg.Metadata.Name) - return nil - } - - mapOfFindingsByPath := lint.GroupFindingsByPath(findings, types.SevWarn, p.cfg.Pkg.Metadata.Name) - - header := []string{"Type", "Path", "Message"} - - for _, findings := range mapOfFindingsByPath { - lintData := [][]string{} - for _, finding := range findings { - lintData = append(lintData, []string{ - colorWrapSev(finding.Severity), - message.ColorWrap(finding.YqPath, color.FgCyan), - itemizedDescription(finding.Description, finding.Item), - }) - } - var packagePathFromUser string - if helpers.IsOCIURL(findings[0].PackagePathOverride) { - packagePathFromUser = findings[0].PackagePathOverride - } else { - packagePathFromUser = filepath.Join(p.cfg.CreateOpts.BaseDir, findings[0].PackagePathOverride) - } - message.Notef("Linting package %q at %s", findings[0].PackageNameOverride, packagePathFromUser) - message.Table(header, lintData) - } - - if lint.HasSeverity(findings, types.SevErr) { - return errors.New("errors during lint") - } - - return nil -} - -func itemizedDescription(description string, item string) string { - if item == "" { - return description - } - return fmt.Sprintf("%s - %s", description, item) -} - -func colorWrapSev(s types.Severity) string { - if s == types.SevErr { - return message.ColorWrap("Error", color.FgRed) - } else if s == types.SevWarn { - return message.ColorWrap("Warning", color.FgYellow) - } - return "unknown" -} diff --git a/src/pkg/packager/lint/findings.go b/src/pkg/packager/lint/findings.go deleted file mode 100644 index 8b48bdea78..0000000000 --- a/src/pkg/packager/lint/findings.go +++ /dev/null @@ -1,41 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: 2021-Present The Zarf Authors - -// Package lint contains functions for verifying zarf yaml files are valid -package lint - -import ( - "github.com/defenseunicorns/pkg/helpers/v2" - "github.com/zarf-dev/zarf/src/types" -) - -// GroupFindingsByPath groups findings by their package path -func GroupFindingsByPath(findings []types.PackageFinding, severity types.Severity, packageName string) map[string][]types.PackageFinding { - findings = helpers.RemoveMatches(findings, func(finding types.PackageFinding) bool { - return finding.Severity > severity - }) - for i := range findings { - if findings[i].PackageNameOverride == "" { - findings[i].PackageNameOverride = packageName - } - if findings[i].PackagePathOverride == "" { - findings[i].PackagePathOverride = "." - } - } - - mapOfFindingsByPath := make(map[string][]types.PackageFinding) - for _, finding := range findings { - mapOfFindingsByPath[finding.PackagePathOverride] = append(mapOfFindingsByPath[finding.PackagePathOverride], finding) - } - return mapOfFindingsByPath -} - -// HasSeverity returns true if the findings contain a severity equal to or greater than the given severity -func HasSeverity(findings []types.PackageFinding, severity types.Severity) bool { - for _, finding := range findings { - if finding.Severity <= severity { - return true - } - } - return false -} diff --git a/src/pkg/packager/lint/findings_test.go b/src/pkg/packager/lint/findings_test.go deleted file mode 100644 index 522135eb96..0000000000 --- a/src/pkg/packager/lint/findings_test.go +++ /dev/null @@ -1,122 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: 2021-Present The Zarf Authors - -// Package lint contains functions for verifying zarf yaml files are valid -package lint - -import ( - "testing" - - "github.com/stretchr/testify/require" - "github.com/zarf-dev/zarf/src/types" -) - -func TestGroupFindingsByPath(t *testing.T) { - t.Parallel() - tests := []struct { - name string - findings []types.PackageFinding - severity types.Severity - packageName string - want map[string][]types.PackageFinding - }{ - { - name: "same package multiple findings", - findings: []types.PackageFinding{ - {Severity: types.SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"}, - {Severity: types.SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"}, - }, - severity: types.SevWarn, - packageName: "testPackage", - want: map[string][]types.PackageFinding{ - "path": { - {Severity: types.SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"}, - {Severity: types.SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"}, - }, - }, - }, - { - name: "different packages single finding", - findings: []types.PackageFinding{ - {Severity: types.SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"}, - {Severity: types.SevErr, PackageNameOverride: "", PackagePathOverride: ""}, - }, - severity: types.SevWarn, - packageName: "testPackage", - want: map[string][]types.PackageFinding{ - "path": {{Severity: types.SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"}}, - ".": {{Severity: types.SevErr, PackageNameOverride: "testPackage", PackagePathOverride: "."}}, - }, - }, - { - name: "Multiple findings, mixed severity", - findings: []types.PackageFinding{ - {Severity: types.SevWarn, PackageNameOverride: "", PackagePathOverride: ""}, - {Severity: types.SevErr, PackageNameOverride: "", PackagePathOverride: ""}, - }, - severity: types.SevErr, - packageName: "testPackage", - want: map[string][]types.PackageFinding{ - ".": {{Severity: types.SevErr, PackageNameOverride: "testPackage", PackagePathOverride: "."}}, - }, - }, - } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - require.Equal(t, tt.want, GroupFindingsByPath(tt.findings, tt.severity, tt.packageName)) - }) - } -} - -func TestHasSeverity(t *testing.T) { - t.Parallel() - tests := []struct { - name string - severity types.Severity - expected bool - findings []types.PackageFinding - }{ - { - name: "error severity present", - findings: []types.PackageFinding{ - { - Severity: types.SevErr, - }, - }, - severity: types.SevErr, - expected: true, - }, - { - name: "error severity not present", - findings: []types.PackageFinding{ - { - Severity: types.SevWarn, - }, - }, - severity: types.SevErr, - expected: false, - }, - { - name: "err and warning severity present", - findings: []types.PackageFinding{ - { - Severity: types.SevWarn, - }, - { - Severity: types.SevErr, - }, - }, - severity: types.SevErr, - expected: true, - }, - } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - require.Equal(t, tt.expected, HasSeverity(tt.findings, tt.severity)) - }) - } -} diff --git a/src/pkg/packager/lint/lint.go b/src/pkg/packager/lint/lint.go deleted file mode 100644 index 34ab432fe0..0000000000 --- a/src/pkg/packager/lint/lint.go +++ /dev/null @@ -1,278 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: 2021-Present The Zarf Authors - -// Package lint contains functions for verifying zarf yaml files are valid -package lint - -import ( - "context" - "fmt" - "io/fs" - "regexp" - "strings" - - "github.com/defenseunicorns/pkg/helpers/v2" - "github.com/xeipuuv/gojsonschema" - "github.com/zarf-dev/zarf/src/config" - "github.com/zarf-dev/zarf/src/config/lang" - "github.com/zarf-dev/zarf/src/pkg/layout" - "github.com/zarf-dev/zarf/src/pkg/packager/composer" - "github.com/zarf-dev/zarf/src/pkg/packager/creator" - "github.com/zarf-dev/zarf/src/pkg/transform" - "github.com/zarf-dev/zarf/src/pkg/utils" - "github.com/zarf-dev/zarf/src/types" -) - -// ZarfSchema is exported so main.go can embed the schema file -var ZarfSchema fs.ReadFileFS - -// Validate the given Zarf package. The Zarf package should not already be composed when sent to this function. -func Validate(ctx context.Context, pkg types.ZarfPackage, createOpts types.ZarfCreateOptions) ([]types.PackageFinding, error) { - var findings []types.PackageFinding - compFindings, err := lintComponents(ctx, pkg, createOpts) - if err != nil { - return nil, err - } - findings = append(findings, compFindings...) - - jsonSchema, err := ZarfSchema.ReadFile("zarf.schema.json") - if err != nil { - return nil, err - } - - var untypedZarfPackage interface{} - if err := utils.ReadYaml(layout.ZarfYAML, &untypedZarfPackage); err != nil { - return nil, err - } - - schemaFindings, err := validateSchema(jsonSchema, untypedZarfPackage) - if err != nil { - return nil, err - } - findings = append(findings, schemaFindings...) - - return findings, nil -} - -func lintComponents(ctx context.Context, pkg types.ZarfPackage, createOpts types.ZarfCreateOptions) ([]types.PackageFinding, error) { - var findings []types.PackageFinding - - for i, component := range pkg.Components { - arch := config.GetArch(pkg.Metadata.Architecture) - if !composer.CompatibleComponent(component, arch, createOpts.Flavor) { - continue - } - - chain, err := composer.NewImportChain(ctx, component, i, pkg.Metadata.Name, arch, createOpts.Flavor) - if err != nil { - return nil, err - } - - node := chain.Head() - for node != nil { - component := node.ZarfComponent - compFindings := fillComponentTemplate(&component, &createOpts) - compFindings = append(compFindings, checkComponent(component, node.Index())...) - for i := range compFindings { - compFindings[i].PackagePathOverride = node.ImportLocation() - compFindings[i].PackageNameOverride = node.OriginalPackageName() - } - findings = append(findings, compFindings...) - node = node.Next() - } - } - return findings, nil -} - -func fillComponentTemplate(c *types.ZarfComponent, createOpts *types.ZarfCreateOptions) []types.PackageFinding { - var findings []types.PackageFinding - err := creator.ReloadComponentTemplate(c) - if err != nil { - findings = append(findings, types.PackageFinding{ - Description: err.Error(), - Severity: types.SevWarn, - }) - } - templateMap := map[string]string{} - - setVarsAndWarn := func(templatePrefix string, deprecated bool) { - yamlTemplates, err := utils.FindYamlTemplates(c, templatePrefix, "###") - if err != nil { - findings = append(findings, types.PackageFinding{ - Description: err.Error(), - Severity: types.SevWarn, - }) - } - - for key := range yamlTemplates { - if deprecated { - findings = append(findings, types.PackageFinding{ - Description: fmt.Sprintf(lang.PkgValidateTemplateDeprecation, key, key, key), - Severity: types.SevWarn, - }) - } - _, present := createOpts.SetVariables[key] - if !present { - findings = append(findings, types.PackageFinding{ - Description: lang.UnsetVarLintWarning, - Severity: types.SevWarn, - }) - } - } - for key, value := range createOpts.SetVariables { - templateMap[fmt.Sprintf("%s%s###", templatePrefix, key)] = value - } - } - - setVarsAndWarn(types.ZarfPackageTemplatePrefix, false) - - // [DEPRECATION] Set the Package Variable syntax as well for backward compatibility - setVarsAndWarn(types.ZarfPackageVariablePrefix, true) - - //nolint: errcheck // This error should bubble up - utils.ReloadYamlTemplate(c, templateMap) - return findings -} - -func isPinnedImage(image string) (bool, error) { - transformedImage, err := transform.ParseImageRef(image) - if err != nil { - if strings.Contains(image, types.ZarfPackageTemplatePrefix) || - strings.Contains(image, types.ZarfPackageVariablePrefix) { - return true, nil - } - return false, err - } - if isCosignSignature(transformedImage.Tag) || isCosignAttestation(transformedImage.Tag) { - return true, nil - } - return (transformedImage.Digest != ""), err -} - -func isCosignSignature(image string) bool { - return strings.HasSuffix(image, ".sig") -} - -func isCosignAttestation(image string) bool { - return strings.HasSuffix(image, ".att") -} - -func isPinnedRepo(repo string) bool { - return (strings.Contains(repo, "@")) -} - -// checkComponent runs lint rules against a component -func checkComponent(c types.ZarfComponent, i int) []types.PackageFinding { - var findings []types.PackageFinding - findings = append(findings, checkForUnpinnedRepos(c, i)...) - findings = append(findings, checkForUnpinnedImages(c, i)...) - findings = append(findings, checkForUnpinnedFiles(c, i)...) - return findings -} - -func checkForUnpinnedRepos(c types.ZarfComponent, i int) []types.PackageFinding { - var findings []types.PackageFinding - for j, repo := range c.Repos { - repoYqPath := fmt.Sprintf(".components.[%d].repos.[%d]", i, j) - if !isPinnedRepo(repo) { - findings = append(findings, types.PackageFinding{ - YqPath: repoYqPath, - Description: "Unpinned repository", - Item: repo, - Severity: types.SevWarn, - }) - } - } - return findings -} - -func checkForUnpinnedImages(c types.ZarfComponent, i int) []types.PackageFinding { - var findings []types.PackageFinding - for j, image := range c.Images { - imageYqPath := fmt.Sprintf(".components.[%d].images.[%d]", i, j) - pinnedImage, err := isPinnedImage(image) - if err != nil { - findings = append(findings, types.PackageFinding{ - YqPath: imageYqPath, - Description: "Failed to parse image reference", - Item: image, - Severity: types.SevWarn, - }) - continue - } - if !pinnedImage { - findings = append(findings, types.PackageFinding{ - YqPath: imageYqPath, - Description: "Image not pinned with digest", - Item: image, - Severity: types.SevWarn, - }) - } - } - return findings -} - -func checkForUnpinnedFiles(c types.ZarfComponent, i int) []types.PackageFinding { - var findings []types.PackageFinding - for j, file := range c.Files { - fileYqPath := fmt.Sprintf(".components.[%d].files.[%d]", i, j) - if file.Shasum == "" && helpers.IsURL(file.Source) { - findings = append(findings, types.PackageFinding{ - YqPath: fileYqPath, - Description: "No shasum for remote file", - Item: file.Source, - Severity: types.SevWarn, - }) - } - } - return findings -} - -func makeFieldPathYqCompat(field string) string { - if field == "(root)" { - return field - } - // \b is a metacharacter that will stop at the next non-word character (including .) - // https://regex101.com/r/pIRPk0/1 - re := regexp.MustCompile(`(\b\d+\b)`) - - wrappedField := re.ReplaceAllString(field, "[$1]") - - return fmt.Sprintf(".%s", wrappedField) -} - -func validateSchema(jsonSchema []byte, untypedZarfPackage interface{}) ([]types.PackageFinding, error) { - var findings []types.PackageFinding - - schemaErrors, err := runSchema(jsonSchema, untypedZarfPackage) - if err != nil { - return nil, err - } - - if len(schemaErrors) != 0 { - for _, schemaErr := range schemaErrors { - findings = append(findings, types.PackageFinding{ - YqPath: makeFieldPathYqCompat(schemaErr.Field()), - Description: schemaErr.Description(), - Severity: types.SevErr, - }) - } - } - - return findings, err -} - -func runSchema(jsonSchema []byte, pkg interface{}) ([]gojsonschema.ResultError, error) { - schemaLoader := gojsonschema.NewBytesLoader(jsonSchema) - documentLoader := gojsonschema.NewGoLoader(pkg) - - result, err := gojsonschema.Validate(schemaLoader, documentLoader) - if err != nil { - return nil, err - } - - if !result.Valid() { - return result.Errors(), nil - } - return nil, nil -} diff --git a/src/pkg/variables/types.go b/src/pkg/variables/types.go index 3fb8d18d80..b106e911fa 100644 --- a/src/pkg/variables/types.go +++ b/src/pkg/variables/types.go @@ -60,21 +60,8 @@ type SetVariable struct { Value string `json:"value" jsonschema:"description=The value the variable is currently set with"` } -// Validate runs all validation checks on a package variable. -func (v Variable) Validate() error { - if !IsUppercaseNumberUnderscore(v.Name) { - return fmt.Errorf(lang.PkgValidateMustBeUppercase, v.Name) - } - return nil -} - // Validate runs all validation checks on a package constant. func (c Constant) Validate() error { - // ensure the constant name is only capitals and underscores - if !IsUppercaseNumberUnderscore(c.Name) { - return fmt.Errorf(lang.PkgValidateErrPkgConstantName, c.Name) - } - if !regexp.MustCompile(c.Pattern).MatchString(c.Value) { return fmt.Errorf(lang.PkgValidateErrPkgConstantPattern, c.Name, c.Pattern) } diff --git a/src/types/package.go b/src/types/package.go index 17419dd6e3..cb097d6e42 100644 --- a/src/types/package.go +++ b/src/types/package.go @@ -43,6 +43,8 @@ func (pkg ZarfPackage) IsSBOMAble() bool { // ZarfMetadata lists information about the current ZarfPackage. type ZarfMetadata struct { + // The Name regex permits lowercase letters, numbers, and hyphens not at the start + // https://regex101.com/r/FLdG9G/2 Name string `json:"name" jsonschema:"description=Name to identify this Zarf package,pattern=^[a-z0-9][a-z0-9\\-]*$"` Description string `json:"description,omitempty" jsonschema:"description=Additional information about this package"` Version string `json:"version,omitempty" jsonschema:"description=Generic string set by a package author to track the package version (Note: ZarfInitConfigs will always be versioned to the CLIVersion they were created with)"` diff --git a/src/types/validate.go b/src/types/validate.go index 4d39d36b23..a589ed802d 100644 --- a/src/types/validate.go +++ b/src/types/validate.go @@ -9,7 +9,6 @@ import ( "fmt" "path/filepath" "regexp" - "slices" "github.com/defenseunicorns/pkg/helpers/v2" "github.com/zarf-dev/zarf/src/config/lang" @@ -45,20 +44,6 @@ func (pkg ZarfPackage) Validate() error { err = errors.Join(err, fmt.Errorf(lang.PkgValidateErrInitNoYOLO)) } - if !IsLowercaseNumberHyphenNoStartHyphen(pkg.Metadata.Name) { - err = errors.Join(err, fmt.Errorf(lang.PkgValidateErrPkgName, pkg.Metadata.Name)) - } - - if len(pkg.Components) == 0 { - err = errors.Join(err, fmt.Errorf("package must have at least 1 component")) - } - - for _, variable := range pkg.Variables { - if varErr := variable.Validate(); varErr != nil { - err = errors.Join(err, fmt.Errorf(lang.PkgValidateErrVariable, varErr)) - } - } - for _, constant := range pkg.Constants { if varErr := constant.Validate(); varErr != nil { err = errors.Join(err, fmt.Errorf(lang.PkgValidateErrConstant, varErr)) @@ -96,14 +81,6 @@ func (pkg ZarfPackage) Validate() error { } uniqueComponentNames[component.Name] = true - if !IsLowercaseNumberHyphenNoStartHyphen(component.Name) { - err = errors.Join(err, fmt.Errorf(lang.PkgValidateErrComponentName, component.Name)) - } - - if !slices.Contains(supportedOS, component.Only.LocalOS) { - err = errors.Join(err, fmt.Errorf(lang.PkgValidateErrComponentLocalOS, component.Name, component.Only.LocalOS, supportedOS)) - } - if component.IsRequired() { if component.Default { err = errors.Join(err, fmt.Errorf(lang.PkgValidateErrComponentReqDefault, component.Name)) @@ -255,9 +232,6 @@ func (as ZarfComponentActionSet) Validate() error { // Validate runs all validation checks on an action. func (action ZarfComponentAction) Validate() error { var err error - for _, variable := range action.SetVariables { - err = errors.Join(err, variable.Validate()) - } if action.Wait != nil { // Validate only cmd or wait, not both @@ -283,10 +257,6 @@ func (action ZarfComponentAction) Validate() error { func (chart ZarfChart) Validate() error { var err error - if chart.Name == "" { - err = errors.Join(err, fmt.Errorf(lang.PkgValidateErrChartNameMissing)) - } - if len(chart.Name) > ZarfMaxChartNameLength { err = errors.Join(err, fmt.Errorf(lang.PkgValidateErrChartName, chart.Name, ZarfMaxChartNameLength)) } @@ -315,10 +285,6 @@ func (chart ZarfChart) Validate() error { func (manifest ZarfManifest) Validate() error { var err error - if manifest.Name == "" { - err = errors.Join(err, fmt.Errorf(lang.PkgValidateErrManifestNameMissing)) - } - if len(manifest.Name) > ZarfMaxChartNameLength { err = errors.Join(err, fmt.Errorf(lang.PkgValidateErrManifestNameLength, manifest.Name, ZarfMaxChartNameLength)) } diff --git a/src/types/validate_test.go b/src/types/validate_test.go index f283302675..b0ebc60e8f 100644 --- a/src/types/validate_test.go +++ b/src/types/validate_test.go @@ -38,30 +38,16 @@ func TestZarfPackageValidate(t *testing.T) { }, expectedErrs: nil, }, - { - name: "no components", - pkg: ZarfPackage{ - Kind: ZarfPackageConfig, - Metadata: ZarfMetadata{ - Name: "empty-components", - }, - Components: []ZarfComponent{}, - }, - expectedErrs: []string{"package must have at least 1 component"}, - }, { name: "invalid package", pkg: ZarfPackage{ Kind: ZarfPackageConfig, Metadata: ZarfMetadata{ - Name: "-invalid-package", + Name: "invalid-package", }, Components: []ZarfComponent{ { - Name: "-invalid", - Only: ZarfComponentOnlyTarget{ - LocalOS: "unsupportedOS", - }, + Name: "invalid", Required: helpers.BoolPtr(true), Default: true, Charts: []ZarfChart{ @@ -95,15 +81,7 @@ func TestZarfPackageValidate(t *testing.T) { Name: "duplicate", }, }, - Variables: []variables.InteractiveVariable{ - { - Variable: variables.Variable{Name: "not_uppercase"}, - }, - }, Constants: []variables.Constant{ - { - Name: "not_uppercase", - }, { Name: "BAD", Pattern: "^good_val$", @@ -112,13 +90,8 @@ func TestZarfPackageValidate(t *testing.T) { }, }, expectedErrs: []string{ - fmt.Sprintf(lang.PkgValidateErrPkgName, "-invalid-package"), - fmt.Errorf(lang.PkgValidateErrVariable, fmt.Errorf(lang.PkgValidateMustBeUppercase, "not_uppercase")).Error(), - fmt.Errorf(lang.PkgValidateErrConstant, fmt.Errorf(lang.PkgValidateErrPkgConstantName, "not_uppercase")).Error(), fmt.Errorf(lang.PkgValidateErrConstant, fmt.Errorf(lang.PkgValidateErrPkgConstantPattern, "BAD", "^good_val$")).Error(), - fmt.Sprintf(lang.PkgValidateErrComponentName, "-invalid"), - fmt.Sprintf(lang.PkgValidateErrComponentLocalOS, "-invalid", "unsupportedOS", supportedOS), - fmt.Sprintf(lang.PkgValidateErrComponentReqDefault, "-invalid"), + fmt.Sprintf(lang.PkgValidateErrComponentReqDefault, "invalid"), fmt.Sprintf(lang.PkgValidateErrChartNameNotUnique, "chart1"), fmt.Sprintf(lang.PkgValidateErrManifestNameNotUnique, "manifest1"), fmt.Sprintf(lang.PkgValidateErrComponentReqGrouped, "required-in-group"), @@ -187,11 +160,6 @@ func TestValidateManifest(t *testing.T) { manifest: ZarfManifest{Name: "valid", Files: []string{"a-file"}}, expectedErrs: nil, }, - { - name: "empty name", - manifest: ZarfManifest{Name: "", Files: []string{"a-file"}}, - expectedErrs: []string{lang.PkgValidateErrManifestNameMissing}, - }, { name: "long name", manifest: ZarfManifest{Name: longName, Files: []string{"a-file"}}, @@ -231,11 +199,6 @@ func TestValidateChart(t *testing.T) { chart: ZarfChart{Name: "chart1", Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0"}, expectedErrs: nil, }, - { - name: "empty name", - chart: ZarfChart{Name: "", Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0"}, - expectedErrs: []string{lang.PkgValidateErrChartNameMissing}, - }, { name: "long name", chart: ZarfChart{Name: longName, Namespace: "whatever", URL: "http://whatever", Version: "v1.0.0"}, @@ -244,7 +207,7 @@ func TestValidateChart(t *testing.T) { }, }, { - name: "no url or local path", + name: "no url, local path, version, or namespace", chart: ZarfChart{Name: "invalid"}, expectedErrs: []string{ fmt.Sprintf(lang.PkgValidateErrChartNamespaceMissing, "invalid"), From 689034eee810fadff4f627382c3accae95dc0d3f Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Tue, 23 Jul 2024 15:45:19 +0000 Subject: [PATCH 2/4] rewordings Signed-off-by: Austin Abro --- src/pkg/lint/lint.go | 2 +- src/pkg/lint/schema.go | 20 +++++++++----------- src/pkg/packager/creator/creator_test.go | 4 ++-- src/pkg/packager/creator/utils.go | 4 ++-- 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/pkg/lint/lint.go b/src/pkg/lint/lint.go index 6b48951f48..fda3da01f9 100644 --- a/src/pkg/lint/lint.go +++ b/src/pkg/lint/lint.go @@ -35,7 +35,7 @@ func Validate(ctx context.Context, createOpts types.ZarfCreateOptions) error { return err } findings = append(findings, compFindings...) - schemaFindings, err := ValidateSchema() + schemaFindings, err := ValidatePackageSchema() if err != nil { return err } diff --git a/src/pkg/lint/schema.go b/src/pkg/lint/schema.go index 136bd2b46d..ffdafdac0f 100644 --- a/src/pkg/lint/schema.go +++ b/src/pkg/lint/schema.go @@ -17,8 +17,8 @@ import ( // ZarfSchema is exported so main.go can embed the schema file var ZarfSchema fs.ReadFileFS -// ValidateSchema checks the Zarf package in the current directory against the Zarf schema -func ValidateSchema() ([]PackageFinding, error) { +// ValidatePackageSchema checks the Zarf package in the current directory against the Zarf schema +func ValidatePackageSchema() ([]PackageFinding, error) { var untypedZarfPackage interface{} if err := utils.ReadYaml(layout.ZarfYAML, &untypedZarfPackage); err != nil { @@ -54,17 +54,15 @@ func validateSchema(jsonSchema []byte, untypedZarfPackage interface{}) ([]Packag return nil, err } - if len(schemaErrors) != 0 { - for _, schemaErr := range schemaErrors { - findings = append(findings, PackageFinding{ - YqPath: makeFieldPathYqCompat(schemaErr.Field()), - Description: schemaErr.Description(), - Severity: SevErr, - }) - } + for _, schemaErr := range schemaErrors { + findings = append(findings, PackageFinding{ + YqPath: makeFieldPathYqCompat(schemaErr.Field()), + Description: schemaErr.Description(), + Severity: SevErr, + }) } - return findings, err + return findings, nil } func runSchema(jsonSchema []byte, pkg interface{}) ([]gojsonschema.ResultError, error) { diff --git a/src/pkg/packager/creator/creator_test.go b/src/pkg/packager/creator/creator_test.go index 7248fe1cc9..9312fe0a2c 100644 --- a/src/pkg/packager/creator/creator_test.go +++ b/src/pkg/packager/creator/creator_test.go @@ -48,7 +48,7 @@ func TestLoadPackageDefinition(t *testing.T) { { name: "invalid package definition", testDir: "invalid", - expectedErr: "found errors in package", + expectedErr: "found errors in schema", creator: NewPackageCreator(types.ZarfCreateOptions{}, ""), }, { @@ -60,7 +60,7 @@ func TestLoadPackageDefinition(t *testing.T) { { name: "invalid package definition", testDir: "invalid", - expectedErr: "found errors in package", + expectedErr: "found errors in schema", creator: NewSkeletonCreator(types.ZarfCreateOptions{}, types.ZarfPublishOptions{}), }, } diff --git a/src/pkg/packager/creator/utils.go b/src/pkg/packager/creator/utils.go index 3b485e170b..77d28096b1 100644 --- a/src/pkg/packager/creator/utils.go +++ b/src/pkg/packager/creator/utils.go @@ -23,13 +23,13 @@ func Validate(pkg types.ZarfPackage) ([]lint.PackageFinding, error) { return nil, fmt.Errorf("package validation failed: %w", err) } - findings, err := lint.ValidateSchema() + findings, err := lint.ValidatePackageSchema() if err != nil { return nil, fmt.Errorf("unable to check schema: %w", err) } if lint.HasSevOrHigher(findings, lint.SevErr) { - return findings, fmt.Errorf("found errors in package") + return findings, fmt.Errorf("found errors in schema") } return nil, nil From 330fc5cd7a04543d2440c1bcdd53e26224659ff3 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Thu, 25 Jul 2024 13:22:33 +0000 Subject: [PATCH 3/4] feedback Signed-off-by: Austin Abro --- src/pkg/lint/rules_test.go | 240 +++++++++++++-------------- src/pkg/lint/schema.go | 8 +- src/pkg/lint/schema_test.go | 2 +- src/pkg/packager/creator/normal.go | 4 +- src/pkg/packager/creator/skeleton.go | 4 +- src/pkg/packager/creator/utils.go | 11 +- src/pkg/packager/dev.go | 4 +- 7 files changed, 131 insertions(+), 142 deletions(-) diff --git a/src/pkg/lint/rules_test.go b/src/pkg/lint/rules_test.go index a73b275d1c..bf080ac506 100644 --- a/src/pkg/lint/rules_test.go +++ b/src/pkg/lint/rules_test.go @@ -12,130 +12,126 @@ import ( "github.com/zarf-dev/zarf/src/types" ) -func TestValidateComponent(t *testing.T) { +func TestUnpinnedRepo(t *testing.T) { t.Parallel() + unpinnedRepo := "https://github.com/zarf-dev/zarf-public-test.git" + component := types.ZarfComponent{Repos: []string{ + unpinnedRepo, + "https://dev.azure.com/zarf-dev/zarf-public-test/_git/zarf-public-test@v0.0.1", + }} + findings := checkForUnpinnedRepos(component, 0) + expected := []PackageFinding{ + { + Item: unpinnedRepo, + Description: "Unpinned repository", + Severity: SevWarn, + YqPath: ".components.[0].repos.[0]", + }, + } + require.Equal(t, expected, findings) +} - t.Run("Unpinnned repo warning", func(t *testing.T) { - t.Parallel() - unpinnedRepo := "https://github.com/zarf-dev/zarf-public-test.git" - component := types.ZarfComponent{Repos: []string{ - unpinnedRepo, - "https://dev.azure.com/zarf-dev/zarf-public-test/_git/zarf-public-test@v0.0.1", - }} - findings := checkForUnpinnedRepos(component, 0) - expected := []PackageFinding{ - { - Item: unpinnedRepo, - Description: "Unpinned repository", - Severity: SevWarn, - YqPath: ".components.[0].repos.[0]", - }, - } - require.Equal(t, expected, findings) - }) - - t.Run("Unpinnned image warning", func(t *testing.T) { - t.Parallel() - unpinnedImage := "registry.com:9001/whatever/image:1.0.0" - badImage := "badimage:badimage@@sha256:3fbc632167424a6d997e74f5" - cosignSignature := "ghcr.io/stefanprodan/podinfo:sha256-57a654ace69ec02ba8973093b6a786faa15640575fbf0dbb603db55aca2ccec8.sig" - cosignAttestation := "ghcr.io/stefanprodan/podinfo:sha256-57a654ace69ec02ba8973093b6a786faa15640575fbf0dbb603db55aca2ccec8.att" - component := types.ZarfComponent{Images: []string{ - unpinnedImage, - "busybox:latest@sha256:3fbc632167424a6d997e74f52b878d7cc478225cffac6bc977eedfe51c7f4e79", - badImage, - cosignSignature, - cosignAttestation, - }} - findings := checkForUnpinnedImages(component, 0) - expected := []PackageFinding{ - { - Item: unpinnedImage, - Description: "Image not pinned with digest", - Severity: SevWarn, - YqPath: ".components.[0].images.[0]", - }, - { - Item: badImage, - Description: "Failed to parse image reference", - Severity: SevWarn, - YqPath: ".components.[0].images.[2]", - }, - } - require.Equal(t, expected, findings) - }) +func TestUnpinnedImageWarning(t *testing.T) { + t.Parallel() + unpinnedImage := "registry.com:9001/whatever/image:1.0.0" + badImage := "badimage:badimage@@sha256:3fbc632167424a6d997e74f5" + cosignSignature := "ghcr.io/stefanprodan/podinfo:sha256-57a654ace69ec02ba8973093b6a786faa15640575fbf0dbb603db55aca2ccec8.sig" + cosignAttestation := "ghcr.io/stefanprodan/podinfo:sha256-57a654ace69ec02ba8973093b6a786faa15640575fbf0dbb603db55aca2ccec8.att" + component := types.ZarfComponent{Images: []string{ + unpinnedImage, + "busybox:latest@sha256:3fbc632167424a6d997e74f52b878d7cc478225cffac6bc977eedfe51c7f4e79", + badImage, + cosignSignature, + cosignAttestation, + }} + findings := checkForUnpinnedImages(component, 0) + expected := []PackageFinding{ + { + Item: unpinnedImage, + Description: "Image not pinned with digest", + Severity: SevWarn, + YqPath: ".components.[0].images.[0]", + }, + { + Item: badImage, + Description: "Failed to parse image reference", + Severity: SevWarn, + YqPath: ".components.[0].images.[2]", + }, + } + require.Equal(t, expected, findings) +} - t.Run("Unpinnned file warning", func(t *testing.T) { - t.Parallel() - fileURL := "http://example.com/file.zip" - localFile := "local.txt" - zarfFiles := []types.ZarfFile{ - { - Source: fileURL, - }, - { - Source: localFile, - }, - { - Source: fileURL, - Shasum: "fake-shasum", - }, - } - component := types.ZarfComponent{Files: zarfFiles} - findings := checkForUnpinnedFiles(component, 0) - expected := []PackageFinding{ - { - Item: fileURL, - Description: "No shasum for remote file", - Severity: SevWarn, - YqPath: ".components.[0].files.[0]", - }, - } - require.Equal(t, expected, findings) - require.Len(t, findings, 1) - }) +func TestUnpinnnedFileWarning(t *testing.T) { + t.Parallel() + fileURL := "http://example.com/file.zip" + localFile := "local.txt" + zarfFiles := []types.ZarfFile{ + { + Source: fileURL, + }, + { + Source: localFile, + }, + { + Source: fileURL, + Shasum: "fake-shasum", + }, + } + component := types.ZarfComponent{Files: zarfFiles} + findings := checkForUnpinnedFiles(component, 0) + expected := []PackageFinding{ + { + Item: fileURL, + Description: "No shasum for remote file", + Severity: SevWarn, + YqPath: ".components.[0].files.[0]", + }, + } + require.Equal(t, expected, findings) + require.Len(t, findings, 1) +} - t.Run("isImagePinned", func(t *testing.T) { - t.Parallel() - tests := []struct { - input string - expected bool - err error - }{ - { - input: "registry.com:8080/zarf-dev/whatever", - expected: false, - err: nil, - }, - { - input: "ghcr.io/zarf-dev/pepr/controller:v0.15.0", - expected: false, - err: nil, - }, - { - input: "busybox:latest@sha256:3fbc632167424a6d997e74f52b878d7cc478225cffac6bc977eedfe51c7f4e79", - expected: true, - err: nil, - }, - { - input: "busybox:bad/image", - expected: false, - err: errors.New("invalid reference format"), - }, - { - input: "busybox:###ZARF_PKG_TMPL_BUSYBOX_IMAGE###", - expected: true, - err: nil, - }, - } - for _, tc := range tests { - t.Run(tc.input, func(t *testing.T) { - actual, err := isPinnedImage(tc.input) - if err != nil { - require.EqualError(t, err, tc.err.Error()) - } - require.Equal(t, tc.expected, actual) - }) - } - }) +func TestIsImagePinned(t *testing.T) { + t.Parallel() + tests := []struct { + input string + expected bool + err error + }{ + { + input: "registry.com:8080/zarf-dev/whatever", + expected: false, + err: nil, + }, + { + input: "ghcr.io/zarf-dev/pepr/controller:v0.15.0", + expected: false, + err: nil, + }, + { + input: "busybox:latest@sha256:3fbc632167424a6d997e74f52b878d7cc478225cffac6bc977eedfe51c7f4e79", + expected: true, + err: nil, + }, + { + input: "busybox:bad/image", + expected: false, + err: errors.New("invalid reference format"), + }, + { + input: "busybox:###ZARF_PKG_TMPL_BUSYBOX_IMAGE###", + expected: true, + err: nil, + }, + } + for _, tc := range tests { + t.Run(tc.input, func(t *testing.T) { + actual, err := isPinnedImage(tc.input) + if err != nil { + require.EqualError(t, err, tc.err.Error()) + } + require.Equal(t, tc.expected, actual) + }) + } } diff --git a/src/pkg/lint/schema.go b/src/pkg/lint/schema.go index ffdafdac0f..ae3e991863 100644 --- a/src/pkg/lint/schema.go +++ b/src/pkg/lint/schema.go @@ -19,7 +19,6 @@ var ZarfSchema fs.ReadFileFS // ValidatePackageSchema checks the Zarf package in the current directory against the Zarf schema func ValidatePackageSchema() ([]PackageFinding, error) { - var untypedZarfPackage interface{} if err := utils.ReadYaml(layout.ZarfYAML, &untypedZarfPackage); err != nil { return nil, err @@ -30,7 +29,7 @@ func ValidatePackageSchema() ([]PackageFinding, error) { return nil, err } - return validateSchema(jsonSchema, untypedZarfPackage) + return getSchemaFindings(jsonSchema, untypedZarfPackage) } func makeFieldPathYqCompat(field string) string { @@ -46,10 +45,9 @@ func makeFieldPathYqCompat(field string) string { return fmt.Sprintf(".%s", wrappedField) } -func validateSchema(jsonSchema []byte, untypedZarfPackage interface{}) ([]PackageFinding, error) { +func getSchemaFindings(jsonSchema []byte, obj interface{}) ([]PackageFinding, error) { var findings []PackageFinding - - schemaErrors, err := runSchema(jsonSchema, untypedZarfPackage) + schemaErrors, err := runSchema(jsonSchema, obj) if err != nil { return nil, err } diff --git a/src/pkg/lint/schema_test.go b/src/pkg/lint/schema_test.go index 1827310c32..bb6adde541 100644 --- a/src/pkg/lint/schema_test.go +++ b/src/pkg/lint/schema_test.go @@ -170,7 +170,7 @@ components: t.Run("test schema findings is created as expected", func(t *testing.T) { t.Parallel() - findings, err := validateSchema(zarfSchema, types.ZarfPackage{ + findings, err := getSchemaFindings(zarfSchema, types.ZarfPackage{ Kind: types.ZarfInitConfig, Metadata: types.ZarfMetadata{ Name: "invalid", diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index 8671a891df..cc50a5309e 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -27,7 +27,6 @@ import ( "github.com/zarf-dev/zarf/src/internal/packager/kustomize" "github.com/zarf-dev/zarf/src/internal/packager/sbom" "github.com/zarf-dev/zarf/src/pkg/layout" - "github.com/zarf-dev/zarf/src/pkg/lint" "github.com/zarf-dev/zarf/src/pkg/message" "github.com/zarf-dev/zarf/src/pkg/packager/actions" "github.com/zarf-dev/zarf/src/pkg/packager/filters" @@ -119,8 +118,7 @@ func (pc *PackageCreator) LoadPackageDefinition(ctx context.Context, src *layout } } - if findings, err := Validate(pkg); err != nil { - lint.PrintFindings(findings, lint.SevErr, pc.createOpts.BaseDir, pkg.Metadata.Name) + if err := Validate(pkg, pc.createOpts.BaseDir, pkg.Metadata.Name); err != nil { return types.ZarfPackage{}, nil, err } diff --git a/src/pkg/packager/creator/skeleton.go b/src/pkg/packager/creator/skeleton.go index 12c7b14671..4835602738 100644 --- a/src/pkg/packager/creator/skeleton.go +++ b/src/pkg/packager/creator/skeleton.go @@ -20,7 +20,6 @@ import ( "github.com/zarf-dev/zarf/src/internal/packager/helm" "github.com/zarf-dev/zarf/src/internal/packager/kustomize" "github.com/zarf-dev/zarf/src/pkg/layout" - "github.com/zarf-dev/zarf/src/pkg/lint" "github.com/zarf-dev/zarf/src/pkg/message" "github.com/zarf-dev/zarf/src/pkg/utils" "github.com/zarf-dev/zarf/src/pkg/zoci" @@ -71,8 +70,7 @@ func (sc *SkeletonCreator) LoadPackageDefinition(ctx context.Context, src *layou message.Warn(warning) } - if findings, err := Validate(pkg); err != nil { - lint.PrintFindings(findings, lint.SevErr, sc.createOpts.BaseDir, pkg.Metadata.Name) + if err := Validate(pkg, sc.createOpts.BaseDir, pkg.Metadata.Name); err != nil { return types.ZarfPackage{}, nil, err } diff --git a/src/pkg/packager/creator/utils.go b/src/pkg/packager/creator/utils.go index 77d28096b1..23b22f9bc5 100644 --- a/src/pkg/packager/creator/utils.go +++ b/src/pkg/packager/creator/utils.go @@ -18,21 +18,22 @@ 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 types.ZarfPackage) ([]lint.PackageFinding, error) { +func Validate(pkg types.ZarfPackage, baseDir string, pkgName string) error { if err := pkg.Validate(); err != nil { - return nil, fmt.Errorf("package validation failed: %w", err) + return fmt.Errorf("package validation failed: %w", err) } findings, err := lint.ValidatePackageSchema() if err != nil { - return nil, fmt.Errorf("unable to check schema: %w", err) + return fmt.Errorf("unable to check schema: %w", err) } if lint.HasSevOrHigher(findings, lint.SevErr) { - return findings, fmt.Errorf("found errors in schema") + lint.PrintFindings(findings, lint.SevErr, baseDir, pkgName) + return fmt.Errorf("found errors in schema") } - return nil, nil + return nil } // recordPackageMetadata records various package metadata during package create. diff --git a/src/pkg/packager/dev.go b/src/pkg/packager/dev.go index 7713565482..bb74fe16ab 100644 --- a/src/pkg/packager/dev.go +++ b/src/pkg/packager/dev.go @@ -13,7 +13,6 @@ import ( "github.com/defenseunicorns/pkg/helpers/v2" "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" "github.com/zarf-dev/zarf/src/pkg/packager/creator" "github.com/zarf-dev/zarf/src/pkg/packager/filters" @@ -54,8 +53,7 @@ func (p *Packager) DevDeploy(ctx context.Context) error { return err } - if findings, err := creator.Validate(p.cfg.Pkg); err != nil { - lint.PrintFindings(findings, lint.SevErr, p.cfg.CreateOpts.BaseDir, p.cfg.Pkg.Metadata.Name) + if err := creator.Validate(p.cfg.Pkg, p.cfg.CreateOpts.BaseDir, p.cfg.Pkg.Metadata.Name); err != nil { return fmt.Errorf("package validation failed: %w", err) } From 47c477de6a3f48c1c27a6c20ec0440a4d9ecdbd4 Mon Sep 17 00:00:00 2001 From: Austin Abro Date: Thu, 25 Jul 2024 13:25:07 +0000 Subject: [PATCH 4/4] removed unnecessary param Signed-off-by: Austin Abro --- src/pkg/packager/creator/normal.go | 2 +- src/pkg/packager/creator/skeleton.go | 2 +- src/pkg/packager/creator/utils.go | 4 ++-- src/pkg/packager/dev.go | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index cc50a5309e..27c9691196 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -118,7 +118,7 @@ func (pc *PackageCreator) LoadPackageDefinition(ctx context.Context, src *layout } } - if err := Validate(pkg, pc.createOpts.BaseDir, pkg.Metadata.Name); err != nil { + if err := Validate(pkg, pc.createOpts.BaseDir); err != nil { return types.ZarfPackage{}, nil, err } diff --git a/src/pkg/packager/creator/skeleton.go b/src/pkg/packager/creator/skeleton.go index 4835602738..cfa435b102 100644 --- a/src/pkg/packager/creator/skeleton.go +++ b/src/pkg/packager/creator/skeleton.go @@ -70,7 +70,7 @@ func (sc *SkeletonCreator) LoadPackageDefinition(ctx context.Context, src *layou message.Warn(warning) } - if err := Validate(pkg, sc.createOpts.BaseDir, pkg.Metadata.Name); err != nil { + if err := Validate(pkg, sc.createOpts.BaseDir); err != nil { return types.ZarfPackage{}, nil, err } diff --git a/src/pkg/packager/creator/utils.go b/src/pkg/packager/creator/utils.go index 23b22f9bc5..4d72edb59f 100644 --- a/src/pkg/packager/creator/utils.go +++ b/src/pkg/packager/creator/utils.go @@ -18,7 +18,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 types.ZarfPackage, baseDir string, pkgName string) error { +func Validate(pkg types.ZarfPackage, baseDir string) error { if err := pkg.Validate(); err != nil { return fmt.Errorf("package validation failed: %w", err) } @@ -29,7 +29,7 @@ func Validate(pkg types.ZarfPackage, baseDir string, pkgName string) error { } if lint.HasSevOrHigher(findings, lint.SevErr) { - lint.PrintFindings(findings, lint.SevErr, baseDir, pkgName) + lint.PrintFindings(findings, lint.SevErr, baseDir, pkg.Metadata.Name) return fmt.Errorf("found errors in schema") } diff --git a/src/pkg/packager/dev.go b/src/pkg/packager/dev.go index bb74fe16ab..1c1208be14 100644 --- a/src/pkg/packager/dev.go +++ b/src/pkg/packager/dev.go @@ -53,7 +53,7 @@ func (p *Packager) DevDeploy(ctx context.Context) error { return err } - if err := creator.Validate(p.cfg.Pkg, p.cfg.CreateOpts.BaseDir, p.cfg.Pkg.Metadata.Name); err != nil { + if err := creator.Validate(p.cfg.Pkg, p.cfg.CreateOpts.BaseDir); err != nil { return fmt.Errorf("package validation failed: %w", err) }