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: make lint use more accessible data type #2660

Merged
merged 1 commit into from
Jun 28, 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
15 changes: 7 additions & 8 deletions src/cmd/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
"github.com/defenseunicorns/zarf/src/config/lang"
"github.com/defenseunicorns/zarf/src/pkg/message"
"github.com/defenseunicorns/zarf/src/pkg/packager"
"github.com/defenseunicorns/zarf/src/pkg/packager/lint"
"github.com/defenseunicorns/zarf/src/pkg/transform"
"github.com/defenseunicorns/zarf/src/pkg/utils"
"github.com/defenseunicorns/zarf/src/types"
Expand Down Expand Up @@ -249,19 +248,19 @@
Aliases: []string{"l"},
Short: lang.CmdDevLintShort,
Long: lang.CmdDevLintLong,
Run: func(cmd *cobra.Command, args []string) {
RunE: func(cmd *cobra.Command, args []string) error {

Check warning on line 251 in src/cmd/dev.go

View check run for this annotation

Codecov / codecov/patch

src/cmd/dev.go#L251

Added line #L251 was not covered by tests
pkgConfig.CreateOpts.BaseDir = common.SetBaseDirectory(args)
v := common.GetViper()
pkgConfig.CreateOpts.SetVariables = helpers.TransformAndMergeMap(
v.GetStringMapString(common.VPkgCreateSet), pkgConfig.CreateOpts.SetVariables, strings.ToUpper)
validator, err := lint.Validate(cmd.Context(), pkgConfig.CreateOpts)

pkgClient, err := packager.New(&pkgConfig)

Check warning on line 257 in src/cmd/dev.go

View check run for this annotation

Codecov / codecov/patch

src/cmd/dev.go#L257

Added line #L257 was not covered by tests
if err != nil {
message.Fatal(err, err.Error())
}
validator.DisplayFormattedMessage()
if !validator.IsSuccess() {
os.Exit(1)
return err

Check warning on line 259 in src/cmd/dev.go

View check run for this annotation

Codecov / codecov/patch

src/cmd/dev.go#L259

Added line #L259 was not covered by tests
}
defer pkgClient.ClearTempPaths()

Check warning on line 261 in src/cmd/dev.go

View check run for this annotation

Codecov / codecov/patch

src/cmd/dev.go#L261

Added line #L261 was not covered by tests

return pkgClient.Lint(cmd.Context())

Check warning on line 263 in src/cmd/dev.go

View check run for this annotation

Codecov / codecov/patch

src/cmd/dev.go#L263

Added line #L263 was not covered by tests
},
}

Expand Down
2 changes: 1 addition & 1 deletion src/pkg/message/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@
// 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 config.NoColor {
if config.NoColor || str == "" {

Check warning on line 330 in src/pkg/message/message.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/message/message.go#L330

Added line #L330 was not covered by tests
return str
}
return fmt.Sprintf("\x1b[%dm%s\x1b[0m", attr, str)
Expand Down
71 changes: 71 additions & 0 deletions src/pkg/packager/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@

import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
"runtime"

"github.com/defenseunicorns/pkg/helpers/v2"
Expand All @@ -16,7 +18,10 @@
"github.com/defenseunicorns/zarf/src/pkg/message"
"github.com/defenseunicorns/zarf/src/pkg/packager/creator"
"github.com/defenseunicorns/zarf/src/pkg/packager/filters"
"github.com/defenseunicorns/zarf/src/pkg/packager/lint"
"github.com/defenseunicorns/zarf/src/pkg/utils"
"github.com/defenseunicorns/zarf/src/types"
"github.com/fatih/color"
)

// DevDeploy creates + deploys a package in one shot
Expand Down Expand Up @@ -105,3 +110,69 @@
// cd back
return os.Chdir(cwd)
}

// Lint ensures a package is valid & follows suggested conventions
func (p *Packager) Lint(ctx context.Context) error {
if err := os.Chdir(p.cfg.CreateOpts.BaseDir); err != nil {
return fmt.Errorf("unable to access directory %q: %w", p.cfg.CreateOpts.BaseDir, err)

Check warning on line 117 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L115-L117

Added lines #L115 - L117 were not covered by tests
}

if err := utils.ReadYaml(layout.ZarfYAML, &p.cfg.Pkg); err != nil {
return err

Check warning on line 121 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L120-L121

Added lines #L120 - L121 were not covered by tests
}
lucasrod16 marked this conversation as resolved.
Show resolved Hide resolved

findings, err := lint.Validate(ctx, p.cfg.Pkg, p.cfg.CreateOpts)
if err != nil {
return fmt.Errorf("linting failed: %w", err)

Check warning on line 126 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L124-L126

Added lines #L124 - L126 were not covered by tests
}

if len(findings) == 0 {
message.Successf("0 findings for %q", p.cfg.Pkg.Metadata.Name)
return nil

Check warning on line 131 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L129-L131

Added lines #L129 - L131 were not covered by tests
}
AustinAbro321 marked this conversation as resolved.
Show resolved Hide resolved

mapOfFindingsByPath := lint.GroupFindingsByPath(findings, types.SevWarn, p.cfg.Pkg.Metadata.Name)

Check warning on line 134 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L134

Added line #L134 was not covered by tests

header := []string{"Type", "Path", "Message"}

Check warning on line 136 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L136

Added line #L136 was not covered by tests

for _, findings := range mapOfFindingsByPath {
lintData := [][]string{}
for _, finding := range findings {
lintData = append(lintData, []string{
colorWrapSev(finding.Severity),
message.ColorWrap(finding.YqPath, color.FgCyan),
itemizedDescription(finding.Description, finding.Item),
})

Check warning on line 145 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L138-L145

Added lines #L138 - L145 were not covered by tests
}
var packagePathFromUser string
if helpers.IsOCIURL(findings[0].PackagePathOverride) {
packagePathFromUser = findings[0].PackagePathOverride
} else {
packagePathFromUser = filepath.Join(p.cfg.CreateOpts.BaseDir, findings[0].PackagePathOverride)

Check warning on line 151 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L147-L151

Added lines #L147 - L151 were not covered by tests
}
message.Notef("Linting package %q at %s", findings[0].PackageNameOverride, packagePathFromUser)
message.Table(header, lintData)

Check warning on line 154 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L153-L154

Added lines #L153 - L154 were not covered by tests
}

if lint.HasSeverity(findings, types.SevErr) {
return errors.New("errors during lint")

Check warning on line 158 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L157-L158

Added lines #L157 - L158 were not covered by tests
}

return nil

Check warning on line 161 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L161

Added line #L161 was not covered by tests
}

func itemizedDescription(description string, item string) string {
if item == "" {
return description

Check warning on line 166 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L164-L166

Added lines #L164 - L166 were not covered by tests
}
return fmt.Sprintf("%s - %s", description, item)

Check warning on line 168 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L168

Added line #L168 was not covered by tests
}

func colorWrapSev(s types.Severity) string {
if s == types.SevErr {
return message.ColorWrap("Error", color.FgRed)
} else if s == types.SevWarn {
return message.ColorWrap("Warning", color.FgYellow)

Check warning on line 175 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L171-L175

Added lines #L171 - L175 were not covered by tests
}
return "unknown"

Check warning on line 177 in src/pkg/packager/dev.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/dev.go#L177

Added line #L177 was not covered by tests
}
41 changes: 41 additions & 0 deletions src/pkg/packager/lint/findings.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2021-Present The Zarf Authors

// Package lint contains functions for verifying zarf yaml files are valid
package lint

import (
"github.com/defenseunicorns/pkg/helpers/v2"
"github.com/defenseunicorns/zarf/src/types"
)

// GroupFindingsByPath groups findings by their package path
func GroupFindingsByPath(findings []types.PackageFinding, severity types.Severity, packageName string) map[string][]types.PackageFinding {
findings = helpers.RemoveMatches(findings, func(finding types.PackageFinding) bool {
return finding.Severity > severity
})
for i := range findings {
if findings[i].PackageNameOverride == "" {
findings[i].PackageNameOverride = packageName
}
if findings[i].PackagePathOverride == "" {
findings[i].PackagePathOverride = "."
}
}

mapOfFindingsByPath := make(map[string][]types.PackageFinding)
for _, finding := range findings {
mapOfFindingsByPath[finding.PackagePathOverride] = append(mapOfFindingsByPath[finding.PackagePathOverride], finding)
}
return mapOfFindingsByPath
}

// HasSeverity returns true if the findings contain a severity equal to or greater than the given severity
func HasSeverity(findings []types.PackageFinding, severity types.Severity) bool {
for _, finding := range findings {
if finding.Severity <= severity {
return true
}
}
return false
}
122 changes: 122 additions & 0 deletions src/pkg/packager/lint/findings_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2021-Present The Zarf Authors

// Package lint contains functions for verifying zarf yaml files are valid
package lint

import (
"testing"

"github.com/defenseunicorns/zarf/src/types"
"github.com/stretchr/testify/require"
)

func TestGroupFindingsByPath(t *testing.T) {
t.Parallel()
tests := []struct {
name string
findings []types.PackageFinding
severity types.Severity
packageName string
want map[string][]types.PackageFinding
}{
{
name: "same package multiple findings",
findings: []types.PackageFinding{
{Severity: types.SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"},
{Severity: types.SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"},
},
severity: types.SevWarn,
packageName: "testPackage",
want: map[string][]types.PackageFinding{
"path": {
{Severity: types.SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"},
{Severity: types.SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"},
},
},
},
{
name: "different packages single finding",
findings: []types.PackageFinding{
{Severity: types.SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"},
{Severity: types.SevErr, PackageNameOverride: "", PackagePathOverride: ""},
},
severity: types.SevWarn,
packageName: "testPackage",
want: map[string][]types.PackageFinding{
"path": {{Severity: types.SevWarn, PackageNameOverride: "import", PackagePathOverride: "path"}},
".": {{Severity: types.SevErr, PackageNameOverride: "testPackage", PackagePathOverride: "."}},
},
},
{
name: "Multiple findings, mixed severity",
findings: []types.PackageFinding{
{Severity: types.SevWarn, PackageNameOverride: "", PackagePathOverride: ""},
{Severity: types.SevErr, PackageNameOverride: "", PackagePathOverride: ""},
},
severity: types.SevErr,
packageName: "testPackage",
want: map[string][]types.PackageFinding{
".": {{Severity: types.SevErr, PackageNameOverride: "testPackage", PackagePathOverride: "."}},
},
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tt.want, GroupFindingsByPath(tt.findings, tt.severity, tt.packageName))
})
}
}

func TestHasSeverity(t *testing.T) {
t.Parallel()
tests := []struct {
name string
severity types.Severity
expected bool
findings []types.PackageFinding
}{
{
name: "error severity present",
findings: []types.PackageFinding{
{
Severity: types.SevErr,
},
},
severity: types.SevErr,
expected: true,
},
{
name: "error severity not present",
findings: []types.PackageFinding{
{
Severity: types.SevWarn,
},
},
severity: types.SevErr,
expected: false,
},
{
name: "err and warning severity present",
findings: []types.PackageFinding{
{
Severity: types.SevWarn,
},
{
Severity: types.SevErr,
},
},
severity: types.SevErr,
expected: true,
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tt.expected, HasSeverity(tt.findings, tt.severity))
})
}
}
Loading
Loading