Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
AustinAbro321 committed Jun 25, 2024
1 parent d7fedae commit 11eb685
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 85 deletions.
90 changes: 45 additions & 45 deletions src/pkg/packager/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ var ZarfSchema FileLoader
// Validate validates a zarf file
func Validate(ctx context.Context, createOpts types.ZarfCreateOptions) ([]types.PackageFinding, error) {
var pkg types.ZarfPackage
var pkgErrs []types.PackageFinding
var findings []types.PackageFinding
if err := utils.ReadYaml(layout.ZarfYAML, &pkg); err != nil {

Check warning on line 37 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L34-L37

Added lines #L34 - L37 were not covered by tests
return nil, err
}
compFindings, err := lintComponents(ctx, pkg, createOpts)
if err != nil {

Check warning on line 41 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L40-L41

Added lines #L40 - L41 were not covered by tests
return nil, err
}
pkgErrs = append(pkgErrs, compFindings...)
findings = append(findings, compFindings...)

Check warning on line 44 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L44

Added line #L44 was not covered by tests

jsonSchema, err := ZarfSchema.ReadFile("zarf.schema.json")
if err != nil {
Expand All @@ -57,13 +57,13 @@ func Validate(ctx context.Context, createOpts types.ZarfCreateOptions) ([]types.
if err != nil {

Check warning on line 57 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L56-L57

Added lines #L56 - L57 were not covered by tests
return nil, err
}
pkgErrs = append(pkgErrs, schemaFindings...)
findings = append(findings, schemaFindings...)

Check warning on line 60 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L60

Added line #L60 was not covered by tests

return pkgErrs, nil
return findings, nil

Check warning on line 62 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L62

Added line #L62 was not covered by tests
}

func lintComponents(ctx context.Context, pkg types.ZarfPackage, createOpts types.ZarfCreateOptions) ([]types.PackageFinding, error) {
var pkgErrs []types.PackageFinding
var findings []types.PackageFinding

for i, component := range pkg.Components {
arch := config.GetArch(pkg.Metadata.Architecture)
Expand All @@ -80,51 +80,51 @@ func lintComponents(ctx context.Context, pkg types.ZarfPackage, createOpts types
node := chain.Head()

Check warning on line 80 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L80

Added line #L80 was not covered by tests
for node != nil {
component := node.ZarfComponent
nodeErrs := fillComponentTemplate(&component, &createOpts)
nodeErrs = append(nodeErrs, checkComponent(component, node.Index())...)
for i := range nodeErrs {
nodeErrs[i].PackagePathOverride = node.ImportLocation()
nodeErrs[i].PackageNameOverride = node.OriginalPackageName()
compFindings := fillComponentTemplate(&component, &createOpts)
compFindings = append(compFindings, checkComponent(component, node.Index())...)
for i := range compFindings {
compFindings[i].PackagePathOverride = node.ImportLocation()
compFindings[i].PackageNameOverride = node.OriginalPackageName()

Check warning on line 87 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L82-L87

Added lines #L82 - L87 were not covered by tests
}
pkgErrs = append(pkgErrs, nodeErrs...)
findings = append(findings, compFindings...)

Check warning on line 89 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L89

Added line #L89 was not covered by tests
node = node.Next()
}
}
return pkgErrs, nil
return findings, nil

Check warning on line 93 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L93

Added line #L93 was not covered by tests
}

func fillComponentTemplate(c *types.ZarfComponent, createOpts *types.ZarfCreateOptions) []types.PackageFinding {
err := creator.ReloadComponentTemplate(c)
var nodeErrs []types.PackageFinding
var findings []types.PackageFinding

Check warning on line 98 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L96-L98

Added lines #L96 - L98 were not covered by tests
if err != nil {
nodeErrs = append(nodeErrs, types.PackageFinding{
findings = append(findings, types.PackageFinding{
Description: err.Error(),
Category: types.SevWarn,
Severity: types.SevWarn,

Check warning on line 102 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L100-L102

Added lines #L100 - L102 were not covered by tests
})
}
templateMap := map[string]string{}

setVarsAndWarn := func(templatePrefix string, deprecated bool) {
yamlTemplates, err := utils.FindYamlTemplates(c, templatePrefix, "###")

Check warning on line 108 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L108

Added line #L108 was not covered by tests
if err != nil {
nodeErrs = append(nodeErrs, types.PackageFinding{
findings = append(findings, types.PackageFinding{
Description: err.Error(),
Category: types.SevWarn,
Severity: types.SevWarn,

Check warning on line 112 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L110-L112

Added lines #L110 - L112 were not covered by tests
})
}

for key := range yamlTemplates {
if deprecated {
nodeErrs = append(nodeErrs, types.PackageFinding{
findings = append(findings, types.PackageFinding{
Description: fmt.Sprintf(lang.PkgValidateTemplateDeprecation, key, key, key),
Category: types.SevWarn,
Severity: types.SevWarn,

Check warning on line 120 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L118-L120

Added lines #L118 - L120 were not covered by tests
})
}
_, present := createOpts.SetVariables[key]
if !present {
nodeErrs = append(nodeErrs, types.PackageFinding{
findings = append(findings, types.PackageFinding{
Description: lang.UnsetVarLintWarning,
Category: types.SevWarn,
Severity: types.SevWarn,

Check warning on line 127 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L125-L127

Added lines #L125 - L127 were not covered by tests
})
}
}
Expand All @@ -140,7 +140,7 @@ func fillComponentTemplate(c *types.ZarfComponent, createOpts *types.ZarfCreateO

//nolint: errcheck // This error should bubble up
utils.ReloadYamlTemplate(c, templateMap)
return nodeErrs
return findings

Check warning on line 143 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L142-L143

Added lines #L142 - L143 were not covered by tests
}

func isPinnedImage(image string) (bool, error) {
Expand All @@ -161,69 +161,69 @@ func isPinnedRepo(repo string) bool {

// checkComponent runs lint rules against a component
func checkComponent(c types.ZarfComponent, i int) []types.PackageFinding {
var pkgErrs []types.PackageFinding
pkgErrs = append(pkgErrs, checkForUnpinnedRepos(c, i)...)
pkgErrs = append(pkgErrs, checkForUnpinnedImages(c, i)...)
pkgErrs = append(pkgErrs, checkForUnpinnedFiles(c, i)...)
return pkgErrs
var findings []types.PackageFinding
findings = append(findings, checkForUnpinnedRepos(c, i)...)
findings = append(findings, checkForUnpinnedImages(c, i)...)
findings = append(findings, checkForUnpinnedFiles(c, i)...)
return findings

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

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L163-L168

Added lines #L163 - L168 were not covered by tests
}

func checkForUnpinnedRepos(c types.ZarfComponent, i int) []types.PackageFinding {
var pkgErrs []types.PackageFinding
var findings []types.PackageFinding
for j, repo := range c.Repos {
repoYqPath := fmt.Sprintf(".components.[%d].repos.[%d]", i, j)
if !isPinnedRepo(repo) {
pkgErrs = append(pkgErrs, types.PackageFinding{
findings = append(findings, types.PackageFinding{
YqPath: repoYqPath,
Description: "Unpinned repository",
Item: repo,
Category: types.SevWarn,
Severity: types.SevWarn,
})
}
}
return pkgErrs
return findings
}

func checkForUnpinnedImages(c types.ZarfComponent, i int) []types.PackageFinding {
var pkgErrs []types.PackageFinding
var findings []types.PackageFinding
for j, image := range c.Images {
imageYqPath := fmt.Sprintf(".components.[%d].images.[%d]", i, j)
pinnedImage, err := isPinnedImage(image)
if err != nil {
pkgErrs = append(pkgErrs, types.PackageFinding{
findings = append(findings, types.PackageFinding{
YqPath: imageYqPath,
Description: "Failed to parse image reference",
Item: image,
Category: types.SevWarn,
Severity: types.SevWarn,
})
continue
}
if !pinnedImage {
pkgErrs = append(pkgErrs, types.PackageFinding{
findings = append(findings, types.PackageFinding{
YqPath: imageYqPath,
Description: "Image not pinned with digest",
Item: image,
Category: types.SevWarn,
Severity: types.SevWarn,
})
}
}
return pkgErrs
return findings
}

func checkForUnpinnedFiles(c types.ZarfComponent, i int) []types.PackageFinding {
var pkgErrs []types.PackageFinding
var findings []types.PackageFinding
for j, file := range c.Files {
fileYqPath := fmt.Sprintf(".components.[%d].files.[%d]", i, j)
if file.Shasum == "" && helpers.IsURL(file.Source) {
pkgErrs = append(pkgErrs, types.PackageFinding{
findings = append(findings, types.PackageFinding{
YqPath: fileYqPath,
Description: "No shasum for remote file",
Item: file.Source,
Category: types.SevWarn,
Severity: types.SevWarn,
})
}
}
return pkgErrs
return findings
}

func makeFieldPathYqCompat(field string) string {
Expand All @@ -240,7 +240,7 @@ func makeFieldPathYqCompat(field string) string {
}

func validateSchema(jsonSchema []byte, untypedZarfPackage interface{}) ([]types.PackageFinding, error) {
var pkgErrs []types.PackageFinding
var findings []types.PackageFinding

Check warning on line 243 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L242-L243

Added lines #L242 - L243 were not covered by tests

schemaErrors, err := runSchema(jsonSchema, untypedZarfPackage)

Check warning on line 245 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L245

Added line #L245 was not covered by tests
if err != nil {
Expand All @@ -249,15 +249,15 @@ func validateSchema(jsonSchema []byte, untypedZarfPackage interface{}) ([]types.

if len(schemaErrors) != 0 {
for _, schemaErr := range schemaErrors {
pkgErrs = append(pkgErrs, types.PackageFinding{
findings = append(findings, types.PackageFinding{
YqPath: makeFieldPathYqCompat(schemaErr.Field()),
Description: schemaErr.Description(),
Category: types.SevErr,
Severity: types.SevErr,

Check warning on line 255 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L250-L255

Added lines #L250 - L255 were not covered by tests
})
}
}

return pkgErrs, err
return findings, err

Check warning on line 260 in src/pkg/packager/lint/lint.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/lint.go#L260

Added line #L260 was not covered by tests
}

func runSchema(jsonSchema []byte, pkg interface{}) ([]gojsonschema.ResultError, error) {
Expand Down
30 changes: 15 additions & 15 deletions src/pkg/packager/lint/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,14 @@ func TestValidateSchema(t *testing.T) {
},
},
}
for _, tc := range tests {
tt := tc
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
schemaErrs, err := runSchema(getZarfSchema(t), tt.pkg)
findings, err := runSchema(getZarfSchema(t), tt.pkg)
require.NoError(t, err)
var schemaStrings []string
for _, schemaErr := range schemaErrs {
for _, schemaErr := range findings {
schemaStrings = append(schemaStrings, schemaErr.String())
}
require.ElementsMatch(t, tt.expectedSchemaStrings, schemaStrings)
Expand Down Expand Up @@ -195,16 +195,16 @@ func TestValidateComponent(t *testing.T) {
unpinnedRepo,
"https://dev.azure.com/defenseunicorns/zarf-public-test/_git/[email protected]",
}}
pkgErrs := checkForUnpinnedRepos(component, 0)
findings := checkForUnpinnedRepos(component, 0)
expected := []types.PackageFinding{
{
Item: unpinnedRepo,
Description: "Unpinned repository",
Category: types.SevWarn,
Severity: types.SevWarn,
YqPath: ".components.[0].repos.[0]",
},
}
require.Equal(t, expected, pkgErrs)
require.Equal(t, expected, findings)
})

t.Run("Unpinnned image warning", func(t *testing.T) {
Expand All @@ -216,22 +216,22 @@ func TestValidateComponent(t *testing.T) {
"busybox:latest@sha256:3fbc632167424a6d997e74f52b878d7cc478225cffac6bc977eedfe51c7f4e79",
badImage,
}}
pkgErrs := checkForUnpinnedImages(component, 0)
findings := checkForUnpinnedImages(component, 0)
expected := []types.PackageFinding{
{
Item: unpinnedImage,
Description: "Image not pinned with digest",
Category: types.SevWarn,
Severity: types.SevWarn,
YqPath: ".components.[0].images.[0]",
},
{
Item: badImage,
Description: "Failed to parse image reference",
Category: types.SevWarn,
Severity: types.SevWarn,
YqPath: ".components.[0].images.[2]",
},
}
require.Equal(t, expected, pkgErrs)
require.Equal(t, expected, findings)
})

t.Run("Unpinnned file warning", func(t *testing.T) {
Expand All @@ -251,17 +251,17 @@ func TestValidateComponent(t *testing.T) {
},
}
component := types.ZarfComponent{Files: zarfFiles}
pkgErrs := checkForUnpinnedFiles(component, 0)
findings := checkForUnpinnedFiles(component, 0)
expectedErr := []types.PackageFinding{
{
Item: fileURL,
Description: "No shasum for remote file",
Category: types.SevWarn,
Severity: types.SevWarn,
YqPath: ".components.[0].files.[0]",
},
}
require.Equal(t, expectedErr, pkgErrs)
require.Len(t, pkgErrs, 1)
require.Equal(t, expectedErr, findings)
require.Len(t, findings, 1)
})

t.Run("Wrap standalone numbers in bracket", func(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions src/pkg/packager/lint/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func PrintFindings(findings []types.PackageFinding, severity types.Severity, bas
lintData := [][]string{}
for _, finding := range findings {
lintData = append(lintData, []string{
colorWrapSev(finding.Category),
colorWrapSev(finding.Severity),
pathColorWrap(finding.YqPath),
itemizedDescription(finding.Description, finding.Item),
})

Check warning on line 57 in src/pkg/packager/lint/validator.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/packager/lint/validator.go#L53-L57

Added lines #L53 - L57 were not covered by tests
Expand All @@ -64,7 +64,7 @@ func PrintFindings(findings []types.PackageFinding, severity types.Severity, bas

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.Category > severity
return finding.Severity > severity
})
for i := range findings {
if findings[i].PackageNameOverride == "" {
Expand All @@ -91,7 +91,7 @@ func pathColorWrap(path string) string {

func hasSeverity(findings []types.PackageFinding, category types.Severity) bool {
for _, finding := range findings {
if finding.Category <= category {
if finding.Severity <= category {
return true
}
}
Expand Down
Loading

0 comments on commit 11eb685

Please sign in to comment.