diff --git a/src/cmd/common/table.go b/src/cmd/common/table.go new file mode 100644 index 0000000000..ee5f9ee190 --- /dev/null +++ b/src/cmd/common/table.go @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +package common + +import ( + "fmt" + "path/filepath" + + "github.com/defenseunicorns/pkg/helpers/v2" + "github.com/fatih/color" + + "github.com/zarf-dev/zarf/src/pkg/lint" + "github.com/zarf-dev/zarf/src/pkg/message" +) + +// PrintFindings prints the findings in the LintError as a table. +func PrintFindings(lintErr *lint.LintError) { + mapOfFindingsByPath := lint.GroupFindingsByPath(lintErr.Findings, lintErr.PackageName) + for _, findings := range mapOfFindingsByPath { + lintData := [][]string{} + for _, finding := range findings { + sevColor := color.FgWhite + switch finding.Severity { + case lint.SevErr: + sevColor = color.FgRed + case lint.SevWarn: + sevColor = color.FgYellow + } + + lintData = append(lintData, []string{ + colorWrap(string(finding.Severity), sevColor), + colorWrap(finding.YqPath, color.FgCyan), + finding.ItemizedDescription(), + }) + } + var packagePathFromUser string + if helpers.IsOCIURL(findings[0].PackagePathOverride) { + packagePathFromUser = findings[0].PackagePathOverride + } else { + packagePathFromUser = filepath.Join(lintErr.BaseDir, findings[0].PackagePathOverride) + } + message.Notef("Linting package %q at %s", findings[0].PackageNameOverride, packagePathFromUser) + message.Table([]string{"Type", "Path", "Message"}, lintData) + } +} + +func colorWrap(str string, attr color.Attribute) string { + if !message.ColorEnabled() || str == "" { + return str + } + return fmt.Sprintf("\x1b[%dm%s\x1b[0m", attr, str) +} diff --git a/src/cmd/dev.go b/src/cmd/dev.go index 322b82fb84..7077f5dccd 100644 --- a/src/cmd/dev.go +++ b/src/cmd/dev.go @@ -59,7 +59,12 @@ var devDeployCmd = &cobra.Command{ } defer pkgClient.ClearTempPaths() - if err := pkgClient.DevDeploy(cmd.Context()); err != nil { + err = pkgClient.DevDeploy(cmd.Context()) + var lintErr *lint.LintError + if errors.As(err, &lintErr) { + common.PrintFindings(lintErr) + } + if err != nil { return fmt.Errorf("failed to dev deploy: %w", err) } return nil @@ -235,7 +240,12 @@ var devFindImagesCmd = &cobra.Command{ } defer pkgClient.ClearTempPaths() - if _, err := pkgClient.FindImages(cmd.Context()); err != nil { + _, err = pkgClient.FindImages(cmd.Context()) + var lintErr *lint.LintError + if errors.As(err, &lintErr) { + common.PrintFindings(lintErr) + } + if err != nil { return fmt.Errorf("unable to find images: %w", err) } return nil @@ -282,7 +292,19 @@ var devLintCmd = &cobra.Command{ } defer pkgClient.ClearTempPaths() - return lint.Validate(cmd.Context(), pkgConfig.CreateOpts) + err = lint.Validate(cmd.Context(), pkgConfig.CreateOpts) + var lintErr *lint.LintError + if errors.As(err, &lintErr) { + common.PrintFindings(lintErr) + // Do not return an error if the findings are all warnings. + if lintErr.OnlyWarnings() { + return nil + } + } + if err != nil { + return err + } + return nil }, } diff --git a/src/cmd/package.go b/src/cmd/package.go index 8b1405e25c..a40439d53f 100644 --- a/src/cmd/package.go +++ b/src/cmd/package.go @@ -14,6 +14,7 @@ import ( "github.com/zarf-dev/zarf/src/cmd/common" "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/sources" "github.com/zarf-dev/zarf/src/types" @@ -60,7 +61,12 @@ var packageCreateCmd = &cobra.Command{ } defer pkgClient.ClearTempPaths() - if err := pkgClient.Create(cmd.Context()); err != nil { + err = pkgClient.Create(cmd.Context()) + var lintErr *lint.LintError + if errors.As(err, &lintErr) { + common.PrintFindings(lintErr) + } + if err != nil { return fmt.Errorf("failed to create package: %w", err) } return nil diff --git a/src/pkg/lint/findings.go b/src/pkg/lint/findings.go index a8ad9b5eac..92486f77d5 100644 --- a/src/pkg/lint/findings.go +++ b/src/pkg/lint/findings.go @@ -6,11 +6,15 @@ package lint import ( "fmt" - "path/filepath" +) + +// Severity is the type of finding. +type Severity string - "github.com/defenseunicorns/pkg/helpers/v2" - "github.com/fatih/color" - "github.com/zarf-dev/zarf/src/pkg/message" +// Severity definitions. +const ( + SevErr = "Error" + SevWarn = "Warning" ) // PackageFinding is a struct that contains a finding about something wrong with a package @@ -26,71 +30,18 @@ type PackageFinding struct { // 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 of finding. + 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 { +// ItemizedDescription returns a string with the description and item if finding contains one. +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 { @@ -108,8 +59,3 @@ func GroupFindingsByPath(findings []PackageFinding, packageName string) map[stri } 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 index f3c09673c8..b922927b1a 100644 --- a/src/pkg/lint/findings_test.go +++ b/src/pkg/lint/findings_test.go @@ -53,53 +53,3 @@ func TestGroupFindingsByPath(t *testing.T) { }) } } - -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 index 3c4be87eaf..344f3b9db0 100644 --- a/src/pkg/lint/lint.go +++ b/src/pkg/lint/lint.go @@ -6,7 +6,6 @@ package lint import ( "context" - "errors" "fmt" "os" @@ -14,12 +13,34 @@ import ( "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" ) +// LintError represents an error containing lint findings. +// +//nolint:revive // ignore name +type LintError struct { + BaseDir string + PackageName string + Findings []PackageFinding +} + +func (e *LintError) Error() string { + return fmt.Sprintf("linting error found %d instance(s)", len(e.Findings)) +} + +// OnlyWarnings returns true if all findings have severity warning. +func (e *LintError) OnlyWarnings() bool { + for _, f := range e.Findings { + if f.Severity == SevErr { + return false + } + } + return true +} + // Validate lints the given Zarf package func Validate(ctx context.Context, createOpts types.ZarfCreateOptions) error { var findings []PackageFinding @@ -41,16 +62,14 @@ func Validate(ctx context.Context, createOpts types.ZarfCreateOptions) error { 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 &LintError{ + BaseDir: createOpts.BaseDir, + PackageName: pkg.Metadata.Name, + Findings: findings, } - return nil } func lintComponents(ctx context.Context, pkg v1alpha1.ZarfPackage, createOpts types.ZarfCreateOptions) ([]PackageFinding, error) { diff --git a/src/pkg/lint/lint_test.go b/src/pkg/lint/lint_test.go index d499ba6e45..84ea5e4cd2 100644 --- a/src/pkg/lint/lint_test.go +++ b/src/pkg/lint/lint_test.go @@ -15,6 +15,33 @@ import ( "github.com/zarf-dev/zarf/src/types" ) +func TestLintError(t *testing.T) { + t.Parallel() + + lintErr := &LintError{ + Findings: []PackageFinding{ + { + Severity: SevWarn, + }, + }, + } + require.Equal(t, "linting error found 1 instance(s)", lintErr.Error()) + require.True(t, lintErr.OnlyWarnings()) + + lintErr = &LintError{ + Findings: []PackageFinding{ + { + Severity: SevWarn, + }, + { + Severity: SevErr, + }, + }, + } + require.Equal(t, "linting error found 2 instance(s)", lintErr.Error()) + require.False(t, lintErr.OnlyWarnings()) +} + func TestLintComponents(t *testing.T) { t.Run("Test composable components with bad path", func(t *testing.T) { t.Parallel() diff --git a/src/pkg/message/message.go b/src/pkg/message/message.go index e271cbf9d4..0eea8b326c 100644 --- a/src/pkg/message/message.go +++ b/src/pkg/message/message.go @@ -12,7 +12,6 @@ import ( "time" "github.com/defenseunicorns/pkg/helpers/v2" - "github.com/fatih/color" "github.com/pterm/pterm" ) @@ -276,16 +275,6 @@ func Table(header []string, data [][]string) { pterm.DefaultTable.WithHasHeader().WithData(table).Render() } -// ColorWrap changes a string to an ansi color code and appends the default color to the end -// 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 !ColorEnabled() || str == "" { - return str - } - return fmt.Sprintf("\x1b[%dm%s\x1b[0m", attr, str) -} - func debugPrinter(offset int, a ...any) { printer := pterm.Debug.WithShowLineNumber(logLevel > 2).WithLineNumberOffset(offset) now := time.Now().Format(time.RFC3339) diff --git a/src/pkg/packager/creator/creator_test.go b/src/pkg/packager/creator/creator_test.go index 95803cc0f4..6d87d09aac 100644 --- a/src/pkg/packager/creator/creator_test.go +++ b/src/pkg/packager/creator/creator_test.go @@ -35,7 +35,7 @@ func TestLoadPackageDefinition(t *testing.T) { { name: "invalid package definition", testDir: "invalid", - expectedErr: "found errors in schema", + expectedErr: "linting error found 1 instance(s)", creator: NewPackageCreator(types.ZarfCreateOptions{}, ""), }, { @@ -47,7 +47,7 @@ func TestLoadPackageDefinition(t *testing.T) { { name: "invalid package definition", testDir: "invalid", - expectedErr: "found errors in schema", + expectedErr: "linting error found 1 instance(s)", creator: NewSkeletonCreator(types.ZarfCreateOptions{}, types.ZarfPublishOptions{}), }, } diff --git a/src/pkg/packager/creator/utils.go b/src/pkg/packager/creator/utils.go index 1bb88750e0..3d5c5ef016 100644 --- a/src/pkg/packager/creator/utils.go +++ b/src/pkg/packager/creator/utils.go @@ -23,18 +23,18 @@ func Validate(pkg v1alpha1.ZarfPackage, baseDir string, setVariables map[string] if err := lint.ValidatePackage(pkg); err != nil { return fmt.Errorf("package validation failed: %w", err) } - findings, err := lint.ValidatePackageSchema(setVariables) if err != nil { return fmt.Errorf("unable to check schema: %w", err) } - - if lint.HasSevOrHigher(findings, lint.SevErr) { - lint.PrintFindings(findings, lint.SevErr, baseDir, pkg.Metadata.Name) - return fmt.Errorf("found errors in schema") + if len(findings) == 0 { + return nil + } + return &lint.LintError{ + BaseDir: baseDir, + PackageName: pkg.Metadata.Name, + Findings: findings, } - - return nil } // recordPackageMetadata records various package metadata during package create.