From 7568cd7e8075dcce59cba54c4849716ff15ca10c Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Tue, 3 Sep 2024 14:12:47 +0200 Subject: [PATCH] refactor: move finding table printing to CLI Signed-off-by: Philip Laine --- src/cmd/common/table.go | 49 ++++++++++++++++++++ src/cmd/dev.go | 24 ++++++++-- src/cmd/package.go | 8 +++- src/pkg/lint/findings.go | 76 ++++--------------------------- src/pkg/lint/findings_test.go | 50 -------------------- src/pkg/lint/lint.go | 22 +++++---- src/pkg/packager/creator/utils.go | 14 +++--- 7 files changed, 108 insertions(+), 135 deletions(-) create mode 100644 src/cmd/common/table.go diff --git a/src/cmd/common/table.go b/src/cmd/common/table.go new file mode 100644 index 0000000000..5f447cfce5 --- /dev/null +++ b/src/cmd/common/table.go @@ -0,0 +1,49 @@ +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" +) + +func PrintFindings(lintErr *lint.LintError) { + mapOfFindingsByPath := lint.GroupFindingsByPath(lintErr.Findings, lintErr.PackageName) + for _, findings := range mapOfFindingsByPath { + lintData := [][]string{} + for _, finding := range findings { + lintData = append(lintData, []string{ + colorWrapSev(finding.Severity), + 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) +} + +func colorWrapSev(s lint.Severity) string { + if s == lint.SevErr { + return ColorWrap("Error", color.FgRed) + } else if s == lint.SevWarn { + return ColorWrap("Warning", color.FgYellow) + } + return "unknown" +} diff --git a/src/cmd/dev.go b/src/cmd/dev.go index 322b82fb84..1e67b8f7d4 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,15 @@ 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) + } + 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..8483b6c6e3 100644 --- a/src/pkg/lint/findings.go +++ b/src/pkg/lint/findings.go @@ -6,11 +6,14 @@ package lint import ( "fmt" - "path/filepath" +) + +// Severity is the type of finding. +type Severity int - "github.com/defenseunicorns/pkg/helpers/v2" - "github.com/fatih/color" - "github.com/zarf-dev/zarf/src/pkg/message" +const ( + SevErr Severity = iota + 1 + SevWarn ) // PackageFinding is a struct that contains a finding about something wrong with a package @@ -26,71 +29,17 @@ 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 { +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 +57,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..4e42b57f88 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,21 @@ 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" ) +type LintError struct { + BaseDir string + PackageName string + Findings []PackageFinding +} + +func (e *LintError) Error() string { + return fmt.Sprintf("found %d lint findings", len(e.Findings)) +} + // Validate lints the given Zarf package func Validate(ctx context.Context, createOpts types.ZarfCreateOptions) error { var findings []PackageFinding @@ -41,16 +49,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/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.