Skip to content
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
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
1 change: 1 addition & 0 deletions site/src/content/docs/commands/zarf_dev_lint.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ zarf dev lint [ DIRECTORY ] [flags]
### Options

```
--all-variants Lints all package components, regardless of flavor. NOTE: This will disable checks for component name uniqueness.
-f, --flavor string The flavor of components to include in the resulting package (i.e. have a matching or empty "only.flavor" key)
-h, --help help for lint
--set stringToString Specify package templates to set on the command line (KEY=value) (default [])
Expand Down
9 changes: 7 additions & 2 deletions src/cmd/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -871,8 +871,9 @@ func (o *devGenerateConfigOptions) run(_ *cobra.Command, args []string) error {
}

type devLintOptions struct {
setPkgTmpl map[string]string
flavor string
setPkgTmpl map[string]string
flavor string
allVariants bool
}

func newDevLintCommand(v *viper.Viper) *cobra.Command {
Expand All @@ -889,6 +890,9 @@ func newDevLintCommand(v *viper.Viper) *cobra.Command {

cmd.Flags().StringToStringVar(&o.setPkgTmpl, "set", v.GetStringMapString(VPkgCreateSet), lang.CmdPackageCreateFlagSetPkgTmpl)
cmd.Flags().StringVarP(&o.flavor, "flavor", "f", v.GetString(VPkgCreateFlavor), lang.CmdPackageCreateFlagFlavor)
cmd.Flags().BoolVar(&o.allVariants, "all-variants", false, lang.CmdDevLintFlagAllVariants)

cmd.MarkFlagsMutuallyExclusive("flavor", "all-variants")

return cmd
}
Expand All @@ -908,6 +912,7 @@ func (o *devLintOptions) run(cmd *cobra.Command, args []string) error {
}
err = packager.Lint(ctx, basePath, packager.LintOptions{
Flavor: o.flavor,
AllVariants: o.allVariants,
SetVariables: o.setPkgTmpl,
CachePath: cachePath,
RemoteOptions: defaultRemoteOptions(),
Expand Down
5 changes: 3 additions & 2 deletions src/config/lang/english.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,8 +426,9 @@ $ zarf package pull oci://ghcr.io/zarf-dev/packages/dos-games:1.3.0 -a skeleton`
CmdDevFlagFindImagesSkipCosign = "Skip searching for cosign artifacts related to discovered images"
CmdDevFlagFindImagesUpdate = "Update the images in the zarf.yaml file if needed. Formatting such as comments and newlines may change."

CmdDevLintShort = "Lints the given package for valid schema and recommended practices"
CmdDevLintLong = "Verifies the package schema, checks if any variables won't be evaluated, and checks for unpinned images/repos/files"
CmdDevLintShort = "Lints the given package for valid schema and recommended practices"
CmdDevLintLong = "Verifies the package schema, checks if any variables won't be evaluated, and checks for unpinned images/repos/files"
CmdDevLintFlagAllVariants = "Lints all package components, regardless of flavor. NOTE: This will disable checks for component name uniqueness."

// zarf tools
CmdToolsShort = "Collection of additional tools to make airgap easier"
Expand Down
25 changes: 21 additions & 4 deletions src/internal/api/v1alpha1/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ package v1alpha1
import (
"errors"
"fmt"
"reflect"
"slices"
"strings"

"github.com/zarf-dev/zarf/src/api/v1alpha1"
Expand Down Expand Up @@ -50,8 +52,13 @@ const (
PkgValidateErrActionTemplateOnCreate = "templating is not supported in onCreate actions"
)

// ValidateOpts governs what validation checks are run in ValidatePackage
type ValidateOpts struct {
SkipComponentNameUniquenessValidation bool
}

// ValidatePackage runs all validation checks on the package.
func ValidatePackage(pkg v1alpha1.ZarfPackage) error {
func ValidatePackage(pkg v1alpha1.ZarfPackage, opts ValidateOpts) error {
var err error
if len(pkg.Components) == 0 {
err = errors.Join(err, errors.New(PkgValidateErrNoComponents))
Expand All @@ -64,7 +71,7 @@ func ValidatePackage(pkg v1alpha1.ZarfPackage) error {
err = errors.Join(err, fmt.Errorf(PkgValidateErrConstant, varErr))
}
}
uniqueComponentNames := make(map[string]bool)
uniqueComponentNames := make(map[string][]v1alpha1.ZarfComponentOnlyTarget)
groupDefault := make(map[string]string)
groupedComponents := make(map[string][]string)
if pkg.Metadata.YOLO {
Expand All @@ -84,11 +91,21 @@ func ValidatePackage(pkg v1alpha1.ZarfPackage) error {
}
}
for _, component := range pkg.Components {
var duplicateOnly bool
// ensure component name is unique
if _, ok := uniqueComponentNames[component.Name]; ok {
err = errors.Join(err, fmt.Errorf(PkgValidateErrComponentNameNotUnique, component.Name))
// only check if only block is duplicated if we're skipping name uniqueness checks
if opts.SkipComponentNameUniquenessValidation {
duplicateOnly = slices.ContainsFunc(uniqueComponentNames[component.Name], func(o v1alpha1.ZarfComponentOnlyTarget) bool {
return reflect.DeepEqual(o, component.Only)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally avoid reflect, I'd do something like this instead

func onlyTargetsEqual(a, b v1alpha1.ZarfComponentOnlyTarget) bool {
    return a.LocalOS == b.LocalOS && a.Flavor == b.Flavor &&
        a.Cluster.Architecture == b.Cluster.Architecture &&
        slices.Equal(a.Cluster.Distros, b.Cluster.Distros)
}

})
}
if !opts.SkipComponentNameUniquenessValidation || duplicateOnly {
err = errors.Join(err, fmt.Errorf(PkgValidateErrComponentNameNotUnique, component.Name))
}
}
uniqueComponentNames[component.Name] = true

uniqueComponentNames[component.Name] = append(uniqueComponentNames[component.Name], component.Only)
if component.IsRequired() {
if component.Default {
err = errors.Join(err, fmt.Errorf(PkgValidateErrComponentReqDefault, component.Name))
Expand Down
131 changes: 127 additions & 4 deletions src/internal/api/v1alpha1/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ import (
func TestZarfPackageValidate(t *testing.T) {
t.Parallel()
tests := []struct {
name string
pkg v1alpha1.ZarfPackage
expectedErrs []string
name string
pkg v1alpha1.ZarfPackage
expectedErrs []string
validateOptions ValidateOpts
}{
{
name: "valid package",
Expand Down Expand Up @@ -138,12 +139,134 @@ func TestZarfPackageValidate(t *testing.T) {
PkgValidateErrYOLONoDistro,
},
},
{
name: "duplicate component names",
pkg: v1alpha1.ZarfPackage{
Kind: v1alpha1.ZarfPackageConfig,
Metadata: v1alpha1.ZarfMetadata{
Name: "duplicate-component-name-pacakage",
},
Components: []v1alpha1.ZarfComponent{
{
Name: "component1",
},
{
Name: "component1",
},
},
},
expectedErrs: []string{
fmt.Sprintf(PkgValidateErrComponentNameNotUnique, "component1"),
},
},
{
name: "duplicate component names, skip component name uniqueness validation",
pkg: v1alpha1.ZarfPackage{
Kind: v1alpha1.ZarfPackageConfig,
Metadata: v1alpha1.ZarfMetadata{
Name: "duplicate-component-name-pacakage",
},
Components: []v1alpha1.ZarfComponent{
{
Name: "component1",
Only: v1alpha1.ZarfComponentOnlyTarget{
Flavor: "strawberry",
LocalOS: "darwin",
Cluster: v1alpha1.ZarfComponentOnlyCluster{
Architecture: "arm64",
Distros: []string{
"eks",
"rke2",
},
},
},
},
{
Name: "component1",
Only: v1alpha1.ZarfComponentOnlyTarget{
Flavor: "blueberry",
LocalOS: "darwin",
Cluster: v1alpha1.ZarfComponentOnlyCluster{
Architecture: "arm64",
Distros: []string{
"eks",
"rke2",
},
},
},
},
},
},
expectedErrs: nil,
validateOptions: ValidateOpts{
SkipComponentNameUniquenessValidation: true,
},
},
{
name: "duplicate component names, skip component name uniqueness validation, only blocks are the same",
pkg: v1alpha1.ZarfPackage{
Kind: v1alpha1.ZarfPackageConfig,
Metadata: v1alpha1.ZarfMetadata{
Name: "duplicate-component-name-pacakage-with-duplicate-only",
},
Components: []v1alpha1.ZarfComponent{
{
Name: "component2",
Only: v1alpha1.ZarfComponentOnlyTarget{
Flavor: "test",
LocalOS: "darwin",
Cluster: v1alpha1.ZarfComponentOnlyCluster{
Architecture: "arm64",
Distros: []string{
"eks",
"rke2",
},
},
},
},
{
Name: "component2",
Only: v1alpha1.ZarfComponentOnlyTarget{
Flavor: "different",
LocalOS: "darwin",
Cluster: v1alpha1.ZarfComponentOnlyCluster{
Architecture: "arm64",
Distros: []string{
"eks",
"rke2",
},
},
},
},
{
Name: "component2",
Only: v1alpha1.ZarfComponentOnlyTarget{
Flavor: "test",
LocalOS: "darwin",
Cluster: v1alpha1.ZarfComponentOnlyCluster{
Architecture: "arm64",
Distros: []string{
"eks",
"rke2",
},
},
},
},
},
},
expectedErrs: []string{
fmt.Sprintf(PkgValidateErrComponentNameNotUnique, "component2"),
},
validateOptions: ValidateOpts{
SkipComponentNameUniquenessValidation: true,
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
err := ValidatePackage(tt.pkg)
err := ValidatePackage(tt.pkg, tt.validateOptions)
if tt.expectedErrs == nil {
require.NoError(t, err)
return
Expand Down
4 changes: 3 additions & 1 deletion src/pkg/packager/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ func Generate(ctx context.Context, packageName, url, version string, opts Genera
pkg.Components[i].Images = append(pkg.Components[i].Images, imageScan.CosignArtifacts...)
}

if err := internalv1alpha1.ValidatePackage(pkg); err != nil {
validateOpts := internalv1alpha1.ValidateOpts{}

if err := internalv1alpha1.ValidatePackage(pkg, validateOpts); err != nil {
return v1alpha1.ZarfPackage{}, err
}
return pkg, nil
Expand Down
2 changes: 2 additions & 0 deletions src/pkg/packager/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
type LintOptions struct {
SetVariables map[string]string
Flavor string
AllVariants bool
CachePath string
types.RemoteOptions
}
Expand All @@ -35,6 +36,7 @@ func Lint(ctx context.Context, packagePath string, opts LintOptions) error {

loadOpts := load.DefinitionOptions{
Flavor: opts.Flavor,
AllVariants: opts.AllVariants,
SetVariables: opts.SetVariables,
CachePath: opts.CachePath,
IsInteractive: false,
Expand Down
27 changes: 27 additions & 0 deletions src/pkg/packager/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,33 @@ func TestLintPackageWithImports(t *testing.T) {
},
},
},
{
name: "all-variants test",
path: filepath.Join("testdata", "lint-with-imports", "all-variants"),
opts: LintOptions{
AllVariants: true,
},
findings: []lint.PackageFinding{
{
YqPath: ".components.[0].images.[0]",
Description: "Image not pinned with digest",
Item: "image-in-first-flavor-component:1.0.0",
Severity: lint.SevWarn,
},
{
YqPath: ".components.[1].images.[0]",
Description: "Image not pinned with digest",
Item: "image-in-second-flavor-component:1.0.0",
Severity: lint.SevWarn,
},
{
YqPath: ".components.[2].images.[0]",
Description: "Image not pinned with digest",
Item: "busybox:0.0.1",
Severity: lint.SevWarn,
},
},
},
}

for _, tc := range testCases {
Expand Down
14 changes: 7 additions & 7 deletions src/pkg/packager/load/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func getComponentToImportName(component v1alpha1.ZarfComponent) string {
return component.Name
}

func resolveImports(ctx context.Context, pkg v1alpha1.ZarfPackage, packagePath, arch, flavor string, importStack []string, cachePath string, skipVersionCheck bool, remoteOptions types.RemoteOptions) (v1alpha1.ZarfPackage, error) {
func resolveImports(ctx context.Context, pkg v1alpha1.ZarfPackage, packagePath, arch, flavor string, importStack []string, cachePath string, allVariants, skipVersionCheck bool, remoteOptions types.RemoteOptions) (v1alpha1.ZarfPackage, error) {
l := logger.From(ctx)
start := time.Now()

Expand Down Expand Up @@ -67,7 +67,7 @@ func resolveImports(ctx context.Context, pkg v1alpha1.ZarfPackage, packagePath,
components := []v1alpha1.ZarfComponent{}

for _, component := range pkg.Components {
if !compatibleComponent(component, arch, flavor) {
if !compatibleComponent(component, arch, flavor, allVariants) {
continue
}

Expand Down Expand Up @@ -110,7 +110,7 @@ func resolveImports(ctx context.Context, pkg v1alpha1.ZarfPackage, packagePath,
}
}
importedPkg.Components = relevantComponents
importedPkg, err = resolveImports(ctx, importedPkg, importPkgPath.ManifestFile, arch, flavor, importStack, cachePath, skipVersionCheck, remoteOptions)
importedPkg, err = resolveImports(ctx, importedPkg, importPkgPath.ManifestFile, arch, flavor, importStack, cachePath, allVariants, skipVersionCheck, remoteOptions)
if err != nil {
return v1alpha1.ZarfPackage{}, err
}
Expand Down Expand Up @@ -151,7 +151,7 @@ func resolveImports(ctx context.Context, pkg v1alpha1.ZarfPackage, packagePath,
name := getComponentToImportName(component)
found := []v1alpha1.ZarfComponent{}
for _, component := range importedPkg.Components {
if component.Name == name && compatibleComponent(component, arch, flavor) {
if component.Name == name && compatibleComponent(component, arch, flavor, allVariants) {
found = append(found, component)
}
}
Expand Down Expand Up @@ -259,9 +259,9 @@ func validateComponentCompose(c v1alpha1.ZarfComponent) error {
return errors.Join(errs...)
}

func compatibleComponent(c v1alpha1.ZarfComponent, arch, flavor string) bool {
satisfiesArch := c.Only.Cluster.Architecture == "" || c.Only.Cluster.Architecture == arch
satisfiesFlavor := c.Only.Flavor == "" || c.Only.Flavor == flavor
func compatibleComponent(c v1alpha1.ZarfComponent, arch, flavor string, allVariants bool) bool {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we'll also want all variants of architectures.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I interpreted this to mean we want allVariants to b evaluated within compatibleComponent for both flavor and arch checks, but I also see how it might be better to leave the function unchanged and instead do something like if !compatibleComponent && !allVariants.

satisfiesArch := c.Only.Cluster.Architecture == "" || c.Only.Cluster.Architecture == arch || allVariants
satisfiesFlavor := c.Only.Flavor == "" || c.Only.Flavor == flavor || allVariants
return satisfiesArch && satisfiesFlavor
}

Expand Down
Loading
Loading