Skip to content

Commit

Permalink
refactor: move finding table printing to CLI
Browse files Browse the repository at this point in the history
Signed-off-by: Philip Laine <philip.laine@gmail.com>
  • Loading branch information
phillebaba committed Sep 3, 2024
1 parent e239857 commit 73cf9de
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 148 deletions.
49 changes: 49 additions & 0 deletions src/cmd/common/table.go
Original file line number Diff line number Diff line change
@@ -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 {
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)
}
24 changes: 21 additions & 3 deletions src/cmd/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
},
}

Expand Down
8 changes: 7 additions & 1 deletion src/cmd/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
76 changes: 10 additions & 66 deletions src/pkg/lint/findings.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ 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"
const (
SevErr = "Error"
SevWarn = "Warning"
)

// PackageFinding is a struct that contains a finding about something wrong with a package
Expand All @@ -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 {
Expand All @@ -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
}
50 changes: 0 additions & 50 deletions src/pkg/lint/findings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
}
}
22 changes: 14 additions & 8 deletions src/pkg/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,28 @@ 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"
)

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

// Validate lints the given Zarf package
func Validate(ctx context.Context, createOpts types.ZarfCreateOptions) error {
var findings []PackageFinding
Expand All @@ -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) {
Expand Down
11 changes: 0 additions & 11 deletions src/pkg/message/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"time"

"github.com/defenseunicorns/pkg/helpers/v2"
"github.com/fatih/color"
"github.com/pterm/pterm"
)

Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions src/pkg/packager/creator/creator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}, ""),
},
{
Expand All @@ -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{}),
},
}
Expand Down
14 changes: 7 additions & 7 deletions src/pkg/packager/creator/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 73cf9de

Please sign in to comment.