diff --git a/src/cmd/dev.go b/src/cmd/dev.go index adc5420a15..3714759c0d 100644 --- a/src/cmd/dev.go +++ b/src/cmd/dev.go @@ -18,7 +18,6 @@ import ( "github.com/defenseunicorns/zarf/src/config/lang" "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/defenseunicorns/zarf/src/pkg/packager" - "github.com/defenseunicorns/zarf/src/pkg/packager/lint" "github.com/defenseunicorns/zarf/src/pkg/transform" "github.com/defenseunicorns/zarf/src/pkg/utils" "github.com/defenseunicorns/zarf/src/types" @@ -249,19 +248,19 @@ var devLintCmd = &cobra.Command{ Aliases: []string{"l"}, Short: lang.CmdDevLintShort, Long: lang.CmdDevLintLong, - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { pkgConfig.CreateOpts.BaseDir = common.SetBaseDirectory(args) v := common.GetViper() pkgConfig.CreateOpts.SetVariables = helpers.TransformAndMergeMap( v.GetStringMapString(common.VPkgCreateSet), pkgConfig.CreateOpts.SetVariables, strings.ToUpper) - validator, err := lint.Validate(cmd.Context(), pkgConfig.CreateOpts) + + pkgClient, err := packager.New(&pkgConfig) if err != nil { - message.Fatal(err, err.Error()) - } - validator.DisplayFormattedMessage() - if !validator.IsSuccess() { - os.Exit(1) + return err } + defer pkgClient.ClearTempPaths() + + return pkgClient.Lint(cmd.Context()) }, } diff --git a/src/pkg/message/message.go b/src/pkg/message/message.go index 206176813f..68bf711219 100644 --- a/src/pkg/message/message.go +++ b/src/pkg/message/message.go @@ -327,7 +327,7 @@ func Table(header []string, data [][]string) { // preventing future characters from taking on the given color // returns string as normal if color is disabled func ColorWrap(str string, attr color.Attribute) string { - if config.NoColor { + if config.NoColor || str == "" { return str } return fmt.Sprintf("\x1b[%dm%s\x1b[0m", attr, str) diff --git a/src/pkg/packager/dev.go b/src/pkg/packager/dev.go index 4bb07fcb0c..5332d10c10 100644 --- a/src/pkg/packager/dev.go +++ b/src/pkg/packager/dev.go @@ -6,8 +6,10 @@ package packager import ( "context" + "errors" "fmt" "os" + "path/filepath" "runtime" "github.com/defenseunicorns/pkg/helpers/v2" @@ -16,7 +18,10 @@ import ( "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/defenseunicorns/zarf/src/pkg/packager/creator" "github.com/defenseunicorns/zarf/src/pkg/packager/filters" + "github.com/defenseunicorns/zarf/src/pkg/packager/lint" + "github.com/defenseunicorns/zarf/src/pkg/utils" "github.com/defenseunicorns/zarf/src/types" + "github.com/fatih/color" ) // DevDeploy creates + deploys a package in one shot @@ -105,3 +110,69 @@ 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 new file mode 100644 index 0000000000..5b184146ed --- /dev/null +++ b/src/pkg/packager/lint/findings.go @@ -0,0 +1,41 @@ +// 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/defenseunicorns/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 new file mode 100644 index 0000000000..99f0b7a652 --- /dev/null +++ b/src/pkg/packager/lint/findings_test.go @@ -0,0 +1,122 @@ +// 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/defenseunicorns/zarf/src/types" + "github.com/stretchr/testify/require" +) + +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 index c6e1c4dde0..ddcdea76db 100644 --- a/src/pkg/packager/lint/lint.go +++ b/src/pkg/packager/lint/lint.go @@ -6,10 +6,8 @@ package lint import ( "context" - "embed" "fmt" - "os" - "path/filepath" + "io/fs" "regexp" "strings" @@ -26,119 +24,99 @@ import ( ) // ZarfSchema is exported so main.go can embed the schema file -var ZarfSchema embed.FS +var ZarfSchema fs.ReadFileFS -func getSchemaFile() ([]byte, error) { - return ZarfSchema.ReadFile("zarf.schema.json") -} - -// Validate validates a zarf file against the zarf schema, returns *validator with warnings or errors if they exist -// along with an error if the validation itself failed -func Validate(ctx context.Context, createOpts types.ZarfCreateOptions) (*Validator, error) { - validator := Validator{} - var err error - - if err := utils.ReadYaml(filepath.Join(createOpts.BaseDir, layout.ZarfYAML), &validator.typedZarfPackage); err != nil { +// 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...) - if err := utils.ReadYaml(filepath.Join(createOpts.BaseDir, layout.ZarfYAML), &validator.untypedZarfPackage); err != nil { + jsonSchema, err := ZarfSchema.ReadFile("zarf.schema.json") + if err != nil { return nil, err } - if err := os.Chdir(createOpts.BaseDir); err != nil { - return nil, fmt.Errorf("unable to access directory '%s': %w", createOpts.BaseDir, err) - } - - validator.baseDir = createOpts.BaseDir - - lintComponents(ctx, &validator, &createOpts) - - if validator.jsonSchema, err = getSchemaFile(); err != nil { + var untypedZarfPackage interface{} + if err := utils.ReadYaml(layout.ZarfYAML, &untypedZarfPackage); err != nil { return nil, err } - if err = validateSchema(&validator); err != nil { + schemaFindings, err := validateSchema(jsonSchema, untypedZarfPackage) + if err != nil { return nil, err } + findings = append(findings, schemaFindings...) - return &validator, nil + return findings, nil } -func lintComponents(ctx context.Context, validator *Validator, createOpts *types.ZarfCreateOptions) { - for i, component := range validator.typedZarfPackage.Components { - arch := config.GetArch(validator.typedZarfPackage.Metadata.Architecture) +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, validator.typedZarfPackage.Metadata.Name, arch, createOpts.Flavor) - baseComponent := chain.Head() + chain, err := composer.NewImportChain(ctx, component, i, pkg.Metadata.Name, arch, createOpts.Flavor) - var badImportYqPath string - if baseComponent != nil { - if baseComponent.Import.URL != "" { - badImportYqPath = fmt.Sprintf(".components.[%d].import.url", i) - } - if baseComponent.Import.Path != "" { - badImportYqPath = fmt.Sprintf(".components.[%d].import.path", i) - } - } if err != nil { - validator.addError(validatorMessage{ - description: err.Error(), - packageRelPath: ".", - packageName: validator.typedZarfPackage.Metadata.Name, - yqPath: badImportYqPath, - }) + return nil, err } - node := baseComponent + node := chain.Head() for node != nil { - checkForVarInComponentImport(validator, node) - fillComponentTemplate(validator, node, createOpts) - lintComponent(validator, node) + 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(validator *Validator, node *composer.Node, createOpts *types.ZarfCreateOptions) { - err := creator.ReloadComponentTemplate(&node.ZarfComponent) +func fillComponentTemplate(c *types.ZarfComponent, createOpts *types.ZarfCreateOptions) []types.PackageFinding { + var findings []types.PackageFinding + err := creator.ReloadComponentTemplate(c) if err != nil { - validator.addWarning(validatorMessage{ - description: err.Error(), - packageRelPath: node.ImportLocation(), - packageName: node.OriginalPackageName(), + 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(node, templatePrefix, "###") + yamlTemplates, err := utils.FindYamlTemplates(c, templatePrefix, "###") if err != nil { - validator.addWarning(validatorMessage{ - description: err.Error(), - packageRelPath: node.ImportLocation(), - packageName: node.OriginalPackageName(), + findings = append(findings, types.PackageFinding{ + Description: err.Error(), + Severity: types.SevWarn, }) } for key := range yamlTemplates { if deprecated { - validator.addWarning(validatorMessage{ - description: fmt.Sprintf(lang.PkgValidateTemplateDeprecation, key, key, key), - packageRelPath: node.ImportLocation(), - packageName: node.OriginalPackageName(), + findings = append(findings, types.PackageFinding{ + Description: fmt.Sprintf(lang.PkgValidateTemplateDeprecation, key, key, key), + Severity: types.SevWarn, }) } _, present := createOpts.SetVariables[key] if !present { - validator.addWarning(validatorMessage{ - description: lang.UnsetVarLintWarning, - packageRelPath: node.ImportLocation(), - packageName: node.OriginalPackageName(), + findings = append(findings, types.PackageFinding{ + Description: lang.UnsetVarLintWarning, + Severity: types.SevWarn, }) } } @@ -153,7 +131,8 @@ func fillComponentTemplate(validator *Validator, node *composer.Node, createOpts setVarsAndWarn(types.ZarfPackageVariablePrefix, true) //nolint: errcheck // This error should bubble up - utils.ReloadYamlTemplate(node, templateMap) + utils.ReloadYamlTemplate(c, templateMap) + return findings } func isPinnedImage(image string) (bool, error) { @@ -172,87 +151,71 @@ func isPinnedRepo(repo string) bool { return (strings.Contains(repo, "@")) } -func lintComponent(validator *Validator, node *composer.Node) { - checkForUnpinnedRepos(validator, node) - checkForUnpinnedImages(validator, node) - checkForUnpinnedFiles(validator, node) +// 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(validator *Validator, node *composer.Node) { - for j, repo := range node.Repos { - repoYqPath := fmt.Sprintf(".components.[%d].repos.[%d]", node.Index(), j) +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) { - validator.addWarning(validatorMessage{ - yqPath: repoYqPath, - packageRelPath: node.ImportLocation(), - packageName: node.OriginalPackageName(), - description: "Unpinned repository", - item: repo, + findings = append(findings, types.PackageFinding{ + YqPath: repoYqPath, + Description: "Unpinned repository", + Item: repo, + Severity: types.SevWarn, }) } } + return findings } -func checkForUnpinnedImages(validator *Validator, node *composer.Node) { - for j, image := range node.Images { - imageYqPath := fmt.Sprintf(".components.[%d].images.[%d]", node.Index(), j) +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 { - validator.addError(validatorMessage{ - yqPath: imageYqPath, - packageRelPath: node.ImportLocation(), - packageName: node.OriginalPackageName(), - description: "Invalid image reference", - item: image, + findings = append(findings, types.PackageFinding{ + YqPath: imageYqPath, + Description: "Failed to parse image reference", + Item: image, + Severity: types.SevWarn, }) continue } if !pinnedImage { - validator.addWarning(validatorMessage{ - yqPath: imageYqPath, - packageRelPath: node.ImportLocation(), - packageName: node.OriginalPackageName(), - description: "Image not pinned with digest", - item: image, + findings = append(findings, types.PackageFinding{ + YqPath: imageYqPath, + Description: "Image not pinned with digest", + Item: image, + Severity: types.SevWarn, }) } } + return findings } -func checkForUnpinnedFiles(validator *Validator, node *composer.Node) { - for j, file := range node.Files { - fileYqPath := fmt.Sprintf(".components.[%d].files.[%d]", node.Index(), j) +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) { - validator.addWarning(validatorMessage{ - yqPath: fileYqPath, - packageRelPath: node.ImportLocation(), - packageName: node.OriginalPackageName(), - description: "No shasum for remote file", - item: file.Source, + findings = append(findings, types.PackageFinding{ + YqPath: fileYqPath, + Description: "No shasum for remote file", + Item: file.Source, + Severity: types.SevWarn, }) } } -} - -func checkForVarInComponentImport(validator *Validator, node *composer.Node) { - if strings.Contains(node.Import.Path, types.ZarfPackageTemplatePrefix) { - validator.addWarning(validatorMessage{ - yqPath: fmt.Sprintf(".components.[%d].import.path", node.Index()), - packageRelPath: node.ImportLocation(), - packageName: node.OriginalPackageName(), - description: "Zarf does not evaluate variables at component.x.import.path", - item: node.Import.Path, - }) - } - if strings.Contains(node.Import.URL, types.ZarfPackageTemplatePrefix) { - validator.addWarning(validatorMessage{ - yqPath: fmt.Sprintf(".components.[%d].import.url", node.Index()), - packageRelPath: node.ImportLocation(), - packageName: node.OriginalPackageName(), - description: "Zarf does not evaluate variables at component.x.import.url", - item: node.Import.URL, - }) - } + return findings } func makeFieldPathYqCompat(field string) string { @@ -268,25 +231,38 @@ func makeFieldPathYqCompat(field string) string { return fmt.Sprintf(".%s", wrappedField) } -func validateSchema(validator *Validator) error { - schemaLoader := gojsonschema.NewBytesLoader(validator.jsonSchema) - documentLoader := gojsonschema.NewGoLoader(validator.untypedZarfPackage) +func validateSchema(jsonSchema []byte, untypedZarfPackage interface{}) ([]types.PackageFinding, error) { + var findings []types.PackageFinding - result, err := gojsonschema.Validate(schemaLoader, documentLoader) + schemaErrors, err := runSchema(jsonSchema, untypedZarfPackage) if err != nil { - return err + return nil, err } - if !result.Valid() { - for _, desc := range result.Errors() { - validator.addError(validatorMessage{ - yqPath: makeFieldPathYqCompat(desc.Field()), - description: desc.Description(), - packageRelPath: ".", - packageName: validator.typedZarfPackage.Metadata.Name, + if len(schemaErrors) != 0 { + for _, schemaErr := range schemaErrors { + findings = append(findings, types.PackageFinding{ + YqPath: makeFieldPathYqCompat(schemaErr.Field()), + Description: schemaErr.Description(), + Severity: types.SevErr, }) } } - return err + 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/packager/lint/lint_test.go index b259fd89ed..78d4c4102d 100644 --- a/src/pkg/packager/lint/lint_test.go +++ b/src/pkg/packager/lint/lint_test.go @@ -9,136 +9,239 @@ import ( "errors" "fmt" "os" - "path/filepath" "testing" - "github.com/defenseunicorns/zarf/src/config" - "github.com/defenseunicorns/zarf/src/pkg/packager/composer" + "github.com/defenseunicorns/zarf/src/pkg/variables" "github.com/defenseunicorns/zarf/src/types" goyaml "github.com/goccy/go-yaml" "github.com/stretchr/testify/require" ) -const badZarfPackage = ` +func TestZarfSchema(t *testing.T) { + t.Parallel() + zarfSchema, err := os.ReadFile("../../../../zarf.schema.json") + require.NoError(t, err) + + tests := []struct { + name string + pkg types.ZarfPackage + expectedSchemaStrings []string + }{ + { + name: "valid package", + pkg: types.ZarfPackage{ + Kind: types.ZarfInitConfig, + Metadata: types.ZarfMetadata{ + Name: "valid-name", + }, + Components: []types.ZarfComponent{ + { + Name: "valid-comp", + }, + }, + }, + expectedSchemaStrings: nil, + }, + { + name: "no comp or kind", + pkg: types.ZarfPackage{ + Metadata: types.ZarfMetadata{ + Name: "no-comp-or-kind", + }, + Components: []types.ZarfComponent{}, + }, + expectedSchemaStrings: []string{ + "kind: kind must be one of the following: \"ZarfInitConfig\", \"ZarfPackageConfig\"", + "components: Array must have at least 1 items", + }, + }, + { + name: "invalid package", + pkg: types.ZarfPackage{ + Kind: types.ZarfInitConfig, + Metadata: types.ZarfMetadata{ + Name: "-invalid-name", + }, + Components: []types.ZarfComponent{ + { + Name: "invalid-name", + Only: types.ZarfComponentOnlyTarget{ + LocalOS: "unsupportedOS", + }, + Import: types.ZarfComponentImport{ + Path: fmt.Sprintf("start%send", types.ZarfPackageTemplatePrefix), + URL: fmt.Sprintf("oci://start%send", types.ZarfPackageTemplatePrefix), + }, + }, + { + Name: "actions", + Actions: types.ZarfComponentActions{ + OnCreate: types.ZarfComponentActionSet{ + Before: []types.ZarfComponentAction{ + { + Cmd: "echo 'invalid setVariable'", + SetVariables: []variables.Variable{{Name: "not_uppercase"}}, + }, + }, + }, + OnRemove: types.ZarfComponentActionSet{ + OnSuccess: []types.ZarfComponentAction{ + { + Cmd: "echo 'invalid setVariable'", + SetVariables: []variables.Variable{{Name: "not_uppercase"}}, + }, + }, + }, + }, + }, + }, + Variables: []variables.InteractiveVariable{ + { + Variable: variables.Variable{Name: "not_uppercase"}, + }, + }, + Constants: []variables.Constant{ + { + Name: "not_uppercase", + }, + }, + }, + expectedSchemaStrings: []string{ + "metadata.name: Does not match pattern '^[a-z0-9][a-z0-9\\-]*$'", + "variables.0.name: Does not match pattern '^[A-Z0-9_]+$'", + "constants.0.name: Does not match pattern '^[A-Z0-9_]+$'", + "components.0.only.localOS: components.0.only.localOS must be one of the following: \"linux\", \"darwin\", \"windows\"", + "components.1.actions.onCreate.before.0.setVariables.0.name: Does not match pattern '^[A-Z0-9_]+$'", + "components.1.actions.onRemove.onSuccess.0.setVariables.0.name: Does not match pattern '^[A-Z0-9_]+$'", + "components.0.import.path: Must not validate the schema (not)", + "components.0.import.url: Must not validate the schema (not)", + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + findings, err := runSchema(zarfSchema, tt.pkg) + require.NoError(t, err) + var schemaStrings []string + for _, schemaErr := range findings { + schemaStrings = append(schemaStrings, schemaErr.String()) + } + require.ElementsMatch(t, tt.expectedSchemaStrings, schemaStrings) + }) + } + + t.Run("validate schema fail with errors not possible from object", func(t *testing.T) { + t.Parallel() + // When we want to test the absence of a field, an incorrect type, or an extra field + // we can't do it through a struct since non pointer fields will have a zero value of their type + const badZarfPackage = ` kind: ZarfInitConfig +extraField: whatever metadata: - name: init + name: invalid description: Testing bad yaml components: -- name: first-test-component - import: - not-path: packages/distros/k3s - name: import-test import: path: 123123 + charts: + - noWait: true + manifests: + - namespace: no-name-for-manifest ` - -const goodZarfPackage = ` -x-name: &name good-zarf-package - -kind: ZarfPackageConfig -metadata: - name: *name - x-description: Testing good yaml with yaml extension - -components: - - name: baseline - required: true - x-foo: bar - -` - -func readAndUnmarshalYaml[T interface{}](t *testing.T, yamlString string) T { - t.Helper() - var unmarshalledYaml T - err := goyaml.Unmarshal([]byte(yamlString), &unmarshalledYaml) - if err != nil { - t.Errorf("error unmarshalling yaml: %v", err) - } - return unmarshalledYaml -} - -func TestValidateSchema(t *testing.T) { - getZarfSchema := func(t *testing.T) []byte { - t.Helper() - file, err := os.ReadFile("../../../../zarf.schema.json") - if err != nil { - t.Errorf("error reading file: %v", err) + var unmarshalledYaml interface{} + err := goyaml.Unmarshal([]byte(badZarfPackage), &unmarshalledYaml) + require.NoError(t, err) + schemaErrs, err := runSchema(zarfSchema, unmarshalledYaml) + require.NoError(t, err) + var schemaStrings []string + for _, schemaErr := range schemaErrs { + schemaStrings = append(schemaStrings, schemaErr.String()) + } + expectedSchemaStrings := []string{ + "(root): Additional property extraField is not allowed", + "components.0.import.path: Invalid type. Expected: string, given: integer", + "components.0.charts.0: name is required", + "components.0.manifests.0: name is required", } - return file - } - t.Run("validate schema success", func(t *testing.T) { - unmarshalledYaml := readAndUnmarshalYaml[interface{}](t, goodZarfPackage) - validator := Validator{untypedZarfPackage: unmarshalledYaml, jsonSchema: getZarfSchema(t)} - err := validateSchema(&validator) - require.NoError(t, err) - require.Empty(t, validator.findings) + require.ElementsMatch(t, expectedSchemaStrings, schemaStrings) }) - t.Run("validate schema fail", func(t *testing.T) { - unmarshalledYaml := readAndUnmarshalYaml[interface{}](t, badZarfPackage) - validator := Validator{untypedZarfPackage: unmarshalledYaml, jsonSchema: getZarfSchema(t)} - err := validateSchema(&validator) + t.Run("test schema findings is created as expected", func(t *testing.T) { + t.Parallel() + findings, err := validateSchema(zarfSchema, types.ZarfPackage{ + Kind: types.ZarfInitConfig, + Metadata: types.ZarfMetadata{ + Name: "invalid", + }, + }) require.NoError(t, err) - config.NoColor = true - require.Equal(t, "Additional property not-path is not allowed", validator.findings[0].String()) - require.Equal(t, "Invalid type. Expected: string, given: integer", validator.findings[1].String()) - }) - - t.Run("Template in component import success", func(t *testing.T) { - unmarshalledYaml := readAndUnmarshalYaml[types.ZarfPackage](t, goodZarfPackage) - validator := Validator{typedZarfPackage: unmarshalledYaml} - for _, component := range validator.typedZarfPackage.Components { - lintComponent(&validator, &composer.Node{ZarfComponent: component}) + expected := []types.PackageFinding{ + { + Description: "Invalid type. Expected: array, given: null", + Severity: types.SevErr, + YqPath: ".components", + }, } - require.Empty(t, validator.findings) - }) - - t.Run("Path template in component import failure", func(t *testing.T) { - pathVar := "###ZARF_PKG_TMPL_PATH###" - pathComponent := types.ZarfComponent{Import: types.ZarfComponentImport{Path: pathVar}} - validator := Validator{typedZarfPackage: types.ZarfPackage{Components: []types.ZarfComponent{pathComponent}}} - checkForVarInComponentImport(&validator, &composer.Node{ZarfComponent: pathComponent}) - require.Equal(t, pathVar, validator.findings[0].item) + require.ElementsMatch(t, expected, findings) }) +} - t.Run("OCI template in component import failure", func(t *testing.T) { - ociPathVar := "oci://###ZARF_PKG_TMPL_PATH###" - URLComponent := types.ZarfComponent{Import: types.ZarfComponentImport{URL: ociPathVar}} - validator := Validator{typedZarfPackage: types.ZarfPackage{Components: []types.ZarfComponent{URLComponent}}} - checkForVarInComponentImport(&validator, &composer.Node{ZarfComponent: URLComponent}) - require.Equal(t, ociPathVar, validator.findings[0].item) - }) +func TestValidateComponent(t *testing.T) { + t.Parallel() t.Run("Unpinnned repo warning", func(t *testing.T) { - validator := Validator{} + t.Parallel() unpinnedRepo := "https://github.com/defenseunicorns/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"}} - checkForUnpinnedRepos(&validator, &composer.Node{ZarfComponent: component}) - require.Equal(t, unpinnedRepo, validator.findings[0].item) - require.Len(t, validator.findings, 1) + "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) { - validator := Validator{} + t.Parallel() unpinnedImage := "registry.com:9001/whatever/image:1.0.0" badImage := "badimage:badimage@@sha256:3fbc632167424a6d997e74f5" component := types.ZarfComponent{Images: []string{ unpinnedImage, "busybox:latest@sha256:3fbc632167424a6d997e74f52b878d7cc478225cffac6bc977eedfe51c7f4e79", - badImage}} - checkForUnpinnedImages(&validator, &composer.Node{ZarfComponent: component}) - require.Equal(t, unpinnedImage, validator.findings[0].item) - require.Equal(t, badImage, validator.findings[1].item) - require.Len(t, validator.findings, 2) + badImage, + }} + 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) { - validator := Validator{} + t.Parallel() fileURL := "http://example.com/file.zip" localFile := "local.txt" zarfFiles := []types.ZarfFile{ @@ -154,12 +257,21 @@ func TestValidateSchema(t *testing.T) { }, } component := types.ZarfComponent{Files: zarfFiles} - checkForUnpinnedFiles(&validator, &composer.Node{ZarfComponent: component}) - require.Equal(t, fileURL, validator.findings[0].item) - require.Len(t, validator.findings, 1) + 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" expected := ".components12.[12].import.path" actual := makeFieldPathYqCompat(input) @@ -167,29 +279,26 @@ func TestValidateSchema(t *testing.T) { }) t.Run("root doesn't change", func(t *testing.T) { + t.Parallel() input := "(root)" actual := makeFieldPathYqCompat(input) require.Equal(t, input, actual) }) - t.Run("Test composable components", func(t *testing.T) { - pathVar := "fake-path" - unpinnedImage := "unpinned:latest" - pathComponent := types.ZarfComponent{ - Import: types.ZarfComponentImport{Path: pathVar}, - Images: []string{unpinnedImage}} - validator := Validator{ - typedZarfPackage: types.ZarfPackage{Components: []types.ZarfComponent{pathComponent}, - Metadata: types.ZarfMetadata{Name: "test-zarf-package"}}} + 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: "."} - lintComponents(context.Background(), &validator, &createOpts) - // Require.contains rather than equals since the error message changes from linux to windows - require.Contains(t, validator.findings[0].description, fmt.Sprintf("open %s", filepath.Join("fake-path", "zarf.yaml"))) - require.Equal(t, ".components.[0].import.path", validator.findings[0].yqPath) - require.Equal(t, ".", validator.findings[0].packageRelPath) - require.Equal(t, unpinnedImage, validator.findings[1].item) - require.Equal(t, ".", validator.findings[1].packageRelPath) + _, err := lintComponents(context.Background(), zarfPackage, createOpts) + require.Error(t, err) }) t.Run("isImagePinned", func(t *testing.T) { diff --git a/src/pkg/packager/lint/validator.go b/src/pkg/packager/lint/validator.go deleted file mode 100644 index 532ea28a64..0000000000 --- a/src/pkg/packager/lint/validator.go +++ /dev/null @@ -1,151 +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 ( - "fmt" - "path/filepath" - - "github.com/defenseunicorns/pkg/helpers/v2" - "github.com/defenseunicorns/zarf/src/pkg/message" - "github.com/defenseunicorns/zarf/src/types" - "github.com/fatih/color" -) - -type category int - -const ( - categoryError category = 1 - categoryWarning category = 2 -) - -type validatorMessage struct { - yqPath string - description string - item string - packageRelPath string - packageName string - category category -} - -func (c category) String() string { - if c == categoryError { - return message.ColorWrap("Error", color.FgRed) - } else if c == categoryWarning { - return message.ColorWrap("Warning", color.FgYellow) - } - return "" -} - -func (vm validatorMessage) String() string { - if vm.item != "" { - vm.item = fmt.Sprintf(" - %s", vm.item) - } - return fmt.Sprintf("%s%s", vm.description, vm.item) -} - -// Validator holds the warnings/errors and messaging that we get from validation -type Validator struct { - findings []validatorMessage - jsonSchema []byte - typedZarfPackage types.ZarfPackage - untypedZarfPackage interface{} - baseDir string -} - -// DisplayFormattedMessage message sent to user based on validator results -func (v Validator) DisplayFormattedMessage() { - if !v.hasFindings() { - message.Successf("0 findings for %q", v.typedZarfPackage.Metadata.Name) - } - v.printValidationTable() -} - -// IsSuccess returns true if there are not any errors -func (v Validator) IsSuccess() bool { - for _, finding := range v.findings { - if finding.category == categoryError { - return false - } - } - return true -} - -func (v Validator) packageRelPathToUser(vm validatorMessage) string { - if helpers.IsOCIURL(vm.packageRelPath) { - return vm.packageRelPath - } - return filepath.Join(v.baseDir, vm.packageRelPath) -} - -func (v Validator) printValidationTable() { - if !v.hasFindings() { - return - } - - mapOfFindingsByPath := make(map[string][]validatorMessage) - for _, finding := range v.findings { - mapOfFindingsByPath[finding.packageRelPath] = append(mapOfFindingsByPath[finding.packageRelPath], finding) - } - - header := []string{"Type", "Path", "Message"} - - for packageRelPath, findings := range mapOfFindingsByPath { - lintData := [][]string{} - for _, finding := range findings { - lintData = append(lintData, []string{finding.category.String(), finding.getPath(), finding.String()}) - } - message.Notef("Linting package %q at %s", findings[0].packageName, v.packageRelPathToUser(findings[0])) - message.Table(header, lintData) - message.Info(v.getFormattedFindingCount(packageRelPath, findings[0].packageName)) - } -} - -func (v Validator) getFormattedFindingCount(relPath string, packageName string) string { - warningCount := 0 - errorCount := 0 - for _, finding := range v.findings { - if finding.packageRelPath != relPath { - continue - } - if finding.category == categoryWarning { - warningCount++ - } - if finding.category == categoryError { - errorCount++ - } - } - wordWarning := "warnings" - if warningCount == 1 { - wordWarning = "warning" - } - wordError := "errors" - if errorCount == 1 { - wordError = "error" - } - return fmt.Sprintf("%d %s and %d %s in %q", - warningCount, wordWarning, errorCount, wordError, packageName) -} - -func (vm validatorMessage) getPath() string { - if vm.yqPath == "" { - return "" - } - return message.ColorWrap(vm.yqPath, color.FgCyan) -} - -func (v Validator) hasFindings() bool { - return len(v.findings) > 0 -} - -func (v *Validator) addWarning(vmessage validatorMessage) { - vmessage.category = categoryWarning - v.findings = helpers.Unique(append(v.findings, vmessage)) -} - -func (v *Validator) addError(vMessage validatorMessage) { - vMessage.category = categoryError - v.findings = helpers.Unique(append(v.findings, vMessage)) -} diff --git a/src/test/e2e/12_lint_test.go b/src/test/e2e/12_lint_test.go index cc6e513129..dc31217601 100644 --- a/src/test/e2e/12_lint_test.go +++ b/src/test/e2e/12_lint_test.go @@ -45,11 +45,8 @@ func TestLint(t *testing.T) { require.Contains(t, strippedStderr, ".components.[1].images.[0] | Image not pinned with digest - registry.com:9001/whatever/image:latest") // Testing import / compose + variables are working require.Contains(t, strippedStderr, ".components.[2].images.[3] | Image not pinned with digest - busybox:latest") - require.Contains(t, strippedStderr, ".components.[3].import.path | Zarf does not evaluate variables at component.x.import.path - ###ZARF_PKG_TMPL_PATH###") // Testing OCI imports get linted require.Contains(t, strippedStderr, ".components.[0].images.[0] | Image not pinned with digest - defenseunicorns/zarf-game:multi-tile-dark") - // Testing a bad path leads to a finding in lint - require.Contains(t, strippedStderr, fmt.Sprintf(".components.[3].import.path | open %s", filepath.Join("###ZARF_PKG_TMPL_PATH###", "zarf.yaml"))) // Check flavors require.NotContains(t, strippedStderr, "image-in-bad-flavor-component:unpinned") diff --git a/src/test/packages/12-lint/zarf.yaml b/src/test/packages/12-lint/zarf.yaml index 980fa78eeb..84f5b53c72 100644 --- a/src/test/packages/12-lint/zarf.yaml +++ b/src/test/packages/12-lint/zarf.yaml @@ -30,10 +30,6 @@ components: - source: file-without-shasum.txt target: src/ - - name: import - import: - path: "###ZARF_PKG_TMPL_PATH###" - - name: oci-games-url import: url: oci://🦄/dos-games:1.0.0 diff --git a/src/types/component.go b/src/types/component.go index a9997ac504..958d60e74d 100644 --- a/src/types/component.go +++ b/src/types/component.go @@ -8,6 +8,7 @@ import ( "github.com/defenseunicorns/zarf/src/pkg/utils/exec" "github.com/defenseunicorns/zarf/src/pkg/variables" "github.com/defenseunicorns/zarf/src/types/extensions" + "github.com/invopop/jsonschema" ) // ZarfComponent is the primary functional grouping of assets to deploy by Zarf. @@ -121,7 +122,7 @@ type ZarfChart struct { RepoName string `json:"repoName,omitempty" jsonschema:"description=The name of a chart within a Helm repository (defaults to the Zarf name of the chart)"` GitPath string `json:"gitPath,omitempty" jsonschema:"description=(git repo only) The sub directory to the chart within a git repo,example=charts/your-chart"` LocalPath string `json:"localPath,omitempty" jsonschema:"description=The path to a local chart's folder or .tgz archive"` - Namespace string `json:"namespace" jsonschema:"description=The namespace to deploy the chart to"` + Namespace string `json:"namespace,omitempty" jsonschema:"description=The namespace to deploy the chart to"` ReleaseName string `json:"releaseName,omitempty" jsonschema:"description=The name of the Helm release to create (defaults to the Zarf name of the chart)"` NoWait bool `json:"noWait,omitempty" jsonschema:"description=Whether to not wait for chart resources to be ready before continuing"` ValuesFiles []string `json:"valuesFiles,omitempty" jsonschema:"description=List of local values file paths or remote URLs to include in the package; these will be merged together when deployed"` @@ -240,3 +241,16 @@ type ZarfComponentImport struct { // For further explanation see https://regex101.com/r/nxX8vx/1 URL string `json:"url,omitempty" jsonschema:"description=[beta] The URL to a Zarf package to import via OCI,pattern=^oci://.*$"` } + +// JSONSchemaExtend extends the generated json schema during `zarf internal gen-config-schema` +func (ZarfComponentImport) JSONSchemaExtend(schema *jsonschema.Schema) { + path, _ := schema.Properties.Get("path") + url, _ := schema.Properties.Get("url") + + notSchema := &jsonschema.Schema{ + Pattern: ZarfPackageTemplatePrefix, + } + + path.Not = notSchema + url.Not = notSchema +} diff --git a/src/types/package.go b/src/types/package.go index 5b7e734f3c..cff00da592 100644 --- a/src/types/package.go +++ b/src/types/package.go @@ -21,7 +21,7 @@ type ZarfPackage struct { Kind ZarfPackageKind `json:"kind" jsonschema:"description=The kind of Zarf package,enum=ZarfInitConfig,enum=ZarfPackageConfig,default=ZarfPackageConfig"` Metadata ZarfMetadata `json:"metadata,omitempty" jsonschema:"description=Package metadata"` Build ZarfBuildData `json:"build,omitempty" jsonschema:"description=Zarf-generated package build data"` - Components []ZarfComponent `json:"components" jsonschema:"description=List of components to deploy in this package"` + Components []ZarfComponent `json:"components" jsonschema:"description=List of components to deploy in this package,minItems=1"` Constants []variables.Constant `json:"constants,omitempty" jsonschema:"description=Constant template values applied on deploy for K8s resources"` Variables []variables.InteractiveVariable `json:"variables,omitempty" jsonschema:"description=Variable template values applied on deploy for K8s resources"` } diff --git a/src/types/runtime.go b/src/types/runtime.go index 24a466926e..b4a7d44ad2 100644 --- a/src/types/runtime.go +++ b/src/types/runtime.go @@ -144,3 +144,29 @@ type DifferentialData struct { DifferentialRepos map[string]bool DifferentialPackageVersion string } + +// 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 package error +// Either Err or Warning +type Severity int + +// different severities of package errors +const ( + SevErr Severity = iota + 1 + SevWarn +) diff --git a/zarf.schema.json b/zarf.schema.json index c26bc550ff..3004a424b1 100644 --- a/zarf.schema.json +++ b/zarf.schema.json @@ -382,8 +382,7 @@ "additionalProperties": false, "type": "object", "required": [ - "name", - "namespace" + "name" ], "patternProperties": { "^x-": {} @@ -789,10 +788,16 @@ "description": "The name of the component to import from the referenced zarf.yaml" }, "path": { + "not": { + "pattern": "###ZARF_PKG_TMPL_" + }, "type": "string", "description": "The relative path to a directory containing a zarf.yaml to import from" }, "url": { + "not": { + "pattern": "###ZARF_PKG_TMPL_" + }, "type": "string", "pattern": "^oci://.*$", "description": "[beta] The URL to a Zarf package to import via OCI" @@ -1091,6 +1096,7 @@ "$ref": "#/$defs/ZarfComponent" }, "type": "array", + "minItems": 1, "description": "List of components to deploy in this package" }, "constants": {