Skip to content

Commit

Permalink
refactor lint
Browse files Browse the repository at this point in the history
fix e2e tests

check for var in component import

improve tests

changes

add test file I forgot to add earlier

testing

package finding

feedback

validate schema

use built in interface

moving

change where print happens

get package name

pass in package

move badZarfPackage to better spot

better badZarfPackage

if to require

cleanup tests

whoops accidentally committed deletion
  • Loading branch information
AustinAbro321 committed Jun 28, 2024
1 parent 7b8cc2d commit b48f058
Show file tree
Hide file tree
Showing 14 changed files with 628 additions and 422 deletions.
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 @@ import (
"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 @@ var devLintCmd = &cobra.Command{
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 @@ func Table(header []string, data [][]string) {
// 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 @@ package packager

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

"github.com/defenseunicorns/pkg/helpers/v2"
Expand All @@ -16,7 +18,10 @@ import (
"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 @@ func (p *Packager) DevDeploy(ctx context.Context) error {
// 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
}

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
}

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

0 comments on commit b48f058

Please sign in to comment.