Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: move finding table printing to CLI #2960

Merged
merged 1 commit into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions src/cmd/common/table.go
Original file line number Diff line number Diff line change
@@ -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)
}
28 changes: 25 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,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
AustinAbro321 marked this conversation as resolved.
Show resolved Hide resolved
},
}

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
78 changes: 12 additions & 66 deletions src/pkg/lint/findings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
}
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))
})
}
}
35 changes: 27 additions & 8 deletions src/pkg/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
27 changes: 27 additions & 0 deletions src/pkg/lint/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading