refactor: move finding table printing to CLI (#2960)

Signed-off-by: Philip Laine <philip.laine@gmail.com>

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)
+}

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
 	},
 }
 

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

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
-}

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))
-		})
-	}
-}

src/pkg/lint/lint.go

@@ -6,20 +6,41 @@ package lint
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"os"
 
 	"github.com/zarf-dev/zarf/src/api/v1alpha1"
 	"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) {

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()