From bc8702b73acfc1442d0feb45936da364992c6f15 Mon Sep 17 00:00:00 2001 From: Clint Date: Tue, 10 Sep 2024 11:37:11 -0500 Subject: [PATCH 1/4] chore: only show config file if there is one (#2985) Signed-off-by: catsby --- src/cmd/common/viper.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cmd/common/viper.go b/src/cmd/common/viper.go index e161c22d00..1077b654a0 100644 --- a/src/cmd/common/viper.go +++ b/src/cmd/common/viper.go @@ -175,7 +175,9 @@ func printViperConfigUsed() { message.WarnErrf(vConfigError, lang.CmdViperErrLoadingConfigFile, vConfigError.Error()) return } - message.Notef(lang.CmdViperInfoUsingConfigFile, v.ConfigFileUsed()) + if cfgFile := v.ConfigFileUsed(); cfgFile != "" { + message.Notef(lang.CmdViperInfoUsingConfigFile, cfgFile) + } } func setDefaults() { From 6eeabf95e26feff623bdd54cdba979946e7d6ee2 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Tue, 10 Sep 2024 11:10:48 -0700 Subject: [PATCH 2/4] refactor: trim named returns in pkg #2950 (#2979) Signed-off-by: Kit Patella --- src/pkg/cluster/state.go | 4 +- src/pkg/cluster/zarf.go | 11 ++-- src/pkg/interactive/components.go | 9 +++- src/pkg/interactive/prompt.go | 15 ++++-- src/pkg/layout/component.go | 26 +++++----- src/pkg/layout/package.go | 7 ++- src/pkg/layout/sbom.go | 17 ++++--- src/pkg/lint/validate.go | 9 ++-- src/pkg/message/pausable.go | 2 +- src/pkg/transform/image.go | 26 ++++++---- src/pkg/utils/bytes.go | 72 +++++++++++++++++--------- src/pkg/utils/bytes_test.go | 78 +++++++++++++++++++++++++++++ src/pkg/utils/cosign.go | 75 ++++++++++++++++----------- src/pkg/utils/network.go | 7 +-- src/pkg/variables/variables.go | 4 +- src/pkg/variables/variables_test.go | 2 +- src/pkg/zoci/fetch.go | 18 +++++-- src/pkg/zoci/pull.go | 7 ++- 18 files changed, 271 insertions(+), 118 deletions(-) create mode 100644 src/pkg/utils/bytes_test.go diff --git a/src/pkg/cluster/state.go b/src/pkg/cluster/state.go index 3c31ccf128..f2279b0221 100644 --- a/src/pkg/cluster/state.go +++ b/src/pkg/cluster/state.go @@ -193,12 +193,14 @@ func (c *Cluster) InitZarfState(ctx context.Context, initOptions types.ZarfInitO } // LoadZarfState returns the current zarf/zarf-state secret data or an empty ZarfState. -func (c *Cluster) LoadZarfState(ctx context.Context) (state *types.ZarfState, err error) { +func (c *Cluster) LoadZarfState(ctx context.Context) (*types.ZarfState, error) { stateErr := errors.New("failed to load the Zarf State from the cluster, has Zarf been initiated?") secret, err := c.Clientset.CoreV1().Secrets(ZarfNamespaceName).Get(ctx, ZarfStateSecretName, metav1.GetOptions{}) if err != nil { return nil, fmt.Errorf("%w: %w", stateErr, err) } + + state := &types.ZarfState{} err = json.Unmarshal(secret.Data[ZarfStateDataKey], &state) if err != nil { return nil, fmt.Errorf("%w: %w", stateErr, err) diff --git a/src/pkg/cluster/zarf.go b/src/pkg/cluster/zarf.go index 3557544e14..eebbd6d295 100644 --- a/src/pkg/cluster/zarf.go +++ b/src/pkg/cluster/zarf.go @@ -174,7 +174,7 @@ func (c *Cluster) RecordPackageDeploymentAndWait(ctx context.Context, pkg v1alph } // RecordPackageDeployment saves metadata about a package that has been deployed to the cluster. -func (c *Cluster) RecordPackageDeployment(ctx context.Context, pkg v1alpha1.ZarfPackage, components []types.DeployedComponent, generation int) (deployedPackage *types.DeployedPackage, err error) { +func (c *Cluster) RecordPackageDeployment(ctx context.Context, pkg v1alpha1.ZarfPackage, components []types.DeployedComponent, generation int) (*types.DeployedPackage, error) { packageName := pkg.Metadata.Name // Attempt to load information about webhooks for the package @@ -187,7 +187,7 @@ func (c *Cluster) RecordPackageDeployment(ctx context.Context, pkg v1alpha1.Zarf componentWebhooks = existingPackageSecret.ComponentWebhooks } - // TODO: This is done for backwards compartibility and could be removed in the future. + // TODO: This is done for backwards compatibility and could be removed in the future. connectStrings := types.ConnectStrings{} for _, comp := range components { for _, chart := range comp.InstalledCharts { @@ -197,7 +197,7 @@ func (c *Cluster) RecordPackageDeployment(ctx context.Context, pkg v1alpha1.Zarf } } - deployedPackage = &types.DeployedPackage{ + deployedPackage := &types.DeployedPackage{ Name: packageName, CLIVersion: config.CLIVersion, Data: pkg, @@ -285,12 +285,13 @@ func (c *Cluster) DisableRegHPAScaleDown(ctx context.Context) error { } // GetInstalledChartsForComponent returns any installed Helm Charts for the provided package component. -func (c *Cluster) GetInstalledChartsForComponent(ctx context.Context, packageName string, component v1alpha1.ZarfComponent) (installedCharts []types.InstalledChart, err error) { +func (c *Cluster) GetInstalledChartsForComponent(ctx context.Context, packageName string, component v1alpha1.ZarfComponent) ([]types.InstalledChart, error) { deployedPackage, err := c.GetDeployedPackage(ctx, packageName) if err != nil { - return installedCharts, err + return nil, err } + installedCharts := make([]types.InstalledChart, 0) for _, deployedComponent := range deployedPackage.DeployedComponents { if deployedComponent.Name == component.Name { installedCharts = append(installedCharts, deployedComponent.InstalledCharts...) diff --git a/src/pkg/interactive/components.go b/src/pkg/interactive/components.go index 719228cb5c..b742aeed4c 100644 --- a/src/pkg/interactive/components.go +++ b/src/pkg/interactive/components.go @@ -15,7 +15,7 @@ import ( ) // SelectOptionalComponent prompts to confirm optional components -func SelectOptionalComponent(component v1alpha1.ZarfComponent) (confirm bool, err error) { +func SelectOptionalComponent(component v1alpha1.ZarfComponent) (bool, error) { message.HorizontalRule() displayComponent := component @@ -30,7 +30,12 @@ func SelectOptionalComponent(component v1alpha1.ZarfComponent) (confirm bool, er Default: component.Default, } - return confirm, survey.AskOne(prompt, &confirm) + var confirm bool + err := survey.AskOne(prompt, &confirm) + if err != nil { + return false, err + } + return confirm, nil } // SelectChoiceGroup prompts to select component groups diff --git a/src/pkg/interactive/prompt.go b/src/pkg/interactive/prompt.go index 5af5f9c451..b6b6e69c94 100644 --- a/src/pkg/interactive/prompt.go +++ b/src/pkg/interactive/prompt.go @@ -19,11 +19,15 @@ func PromptSigPassword() ([]byte, error) { prompt := &survey.Password{ Message: "Private key password (empty for no password): ", } - return []byte(password), survey.AskOne(prompt, &password) + err := survey.AskOne(prompt, &password) + if err != nil { + return []byte{}, err + } + return []byte(password), nil } // PromptVariable prompts the user for a value for a variable -func PromptVariable(variable v1alpha1.InteractiveVariable) (value string, err error) { +func PromptVariable(variable v1alpha1.InteractiveVariable) (string, error) { if variable.Description != "" { message.Question(variable.Description) } @@ -33,5 +37,10 @@ func PromptVariable(variable v1alpha1.InteractiveVariable) (value string, err er Default: variable.Default, } - return value, survey.AskOne(prompt, &value) + var value string + err := survey.AskOne(prompt, &value) + if err != nil { + return "", err + } + return value, nil } diff --git a/src/pkg/layout/component.go b/src/pkg/layout/component.go index fee2d90082..c3ce6ae930 100644 --- a/src/pkg/layout/component.go +++ b/src/pkg/layout/component.go @@ -39,7 +39,7 @@ type Components struct { var ErrNotLoaded = fmt.Errorf("not loaded") // Archive archives a component. -func (c *Components) Archive(component v1alpha1.ZarfComponent, cleanupTemp bool) (err error) { +func (c *Components) Archive(component v1alpha1.ZarfComponent, cleanupTemp bool) error { name := component.Name if _, ok := c.Dirs[name]; !ok { return &fs.PathError{ @@ -75,7 +75,7 @@ func (c *Components) Archive(component v1alpha1.ZarfComponent, cleanupTemp bool) } // Unarchive unarchives a component. -func (c *Components) Unarchive(component v1alpha1.ZarfComponent) (err error) { +func (c *Components) Unarchive(component v1alpha1.ZarfComponent) error { name := component.Name tb, ok := c.Tarballs[name] if !ok { @@ -138,7 +138,7 @@ func (c *Components) Unarchive(component v1alpha1.ZarfComponent) (err error) { } // Create creates a new component directory structure. -func (c *Components) Create(component v1alpha1.ZarfComponent) (cp *ComponentPaths, err error) { +func (c *Components) Create(component v1alpha1.ZarfComponent) (*ComponentPaths, error) { name := component.Name _, ok := c.Tarballs[name] @@ -150,41 +150,41 @@ func (c *Components) Create(component v1alpha1.ZarfComponent) (cp *ComponentPath } } - if err = helpers.CreateDirectory(c.Base, helpers.ReadWriteExecuteUser); err != nil { + if err := helpers.CreateDirectory(c.Base, helpers.ReadWriteExecuteUser); err != nil { return nil, err } base := filepath.Join(c.Base, name) - if err = helpers.CreateDirectory(base, helpers.ReadWriteExecuteUser); err != nil { + if err := helpers.CreateDirectory(base, helpers.ReadWriteExecuteUser); err != nil { return nil, err } - cp = &ComponentPaths{ + cp := &ComponentPaths{ Base: base, } cp.Temp = filepath.Join(base, TempDir) - if err = helpers.CreateDirectory(cp.Temp, helpers.ReadWriteExecuteUser); err != nil { + if err := helpers.CreateDirectory(cp.Temp, helpers.ReadWriteExecuteUser); err != nil { return nil, err } if len(component.Files) > 0 { cp.Files = filepath.Join(base, FilesDir) - if err = helpers.CreateDirectory(cp.Files, helpers.ReadWriteExecuteUser); err != nil { + if err := helpers.CreateDirectory(cp.Files, helpers.ReadWriteExecuteUser); err != nil { return nil, err } } if len(component.Charts) > 0 { cp.Charts = filepath.Join(base, ChartsDir) - if err = helpers.CreateDirectory(cp.Charts, helpers.ReadWriteExecuteUser); err != nil { + if err := helpers.CreateDirectory(cp.Charts, helpers.ReadWriteExecuteUser); err != nil { return nil, err } for _, chart := range component.Charts { cp.Values = filepath.Join(base, ValuesDir) if len(chart.ValuesFiles) > 0 { - if err = helpers.CreateDirectory(cp.Values, helpers.ReadWriteExecuteUser); err != nil { + if err := helpers.CreateDirectory(cp.Values, helpers.ReadWriteExecuteUser); err != nil { return nil, err } break @@ -194,21 +194,21 @@ func (c *Components) Create(component v1alpha1.ZarfComponent) (cp *ComponentPath if len(component.Repos) > 0 { cp.Repos = filepath.Join(base, ReposDir) - if err = helpers.CreateDirectory(cp.Repos, helpers.ReadWriteExecuteUser); err != nil { + if err := helpers.CreateDirectory(cp.Repos, helpers.ReadWriteExecuteUser); err != nil { return nil, err } } if len(component.Manifests) > 0 { cp.Manifests = filepath.Join(base, ManifestsDir) - if err = helpers.CreateDirectory(cp.Manifests, helpers.ReadWriteExecuteUser); err != nil { + if err := helpers.CreateDirectory(cp.Manifests, helpers.ReadWriteExecuteUser); err != nil { return nil, err } } if len(component.DataInjections) > 0 { cp.DataInjections = filepath.Join(base, DataInjectionsDir) - if err = helpers.CreateDirectory(cp.DataInjections, helpers.ReadWriteExecuteUser); err != nil { + if err := helpers.CreateDirectory(cp.DataInjections, helpers.ReadWriteExecuteUser); err != nil { return nil, err } } diff --git a/src/pkg/layout/package.go b/src/pkg/layout/package.go index a38ec599a1..532f46653d 100644 --- a/src/pkg/layout/package.go +++ b/src/pkg/layout/package.go @@ -52,11 +52,14 @@ func New(baseDir string) *PackagePaths { // ReadZarfYAML reads a zarf.yaml file into memory, // checks if it's using the legacy layout, and migrates deprecated component configs. -func (pp *PackagePaths) ReadZarfYAML() (pkg v1alpha1.ZarfPackage, warnings []string, err error) { +func (pp *PackagePaths) ReadZarfYAML() (v1alpha1.ZarfPackage, []string, error) { + var pkg v1alpha1.ZarfPackage + if err := utils.ReadYaml(pp.ZarfYAML, &pkg); err != nil { return v1alpha1.ZarfPackage{}, nil, fmt.Errorf("unable to read zarf.yaml: %w", err) } + warnings := make([]string, 0) if pp.IsLegacyLayout() { warnings = append(warnings, "Detected deprecated package layout, migrating to new layout - support for this package will be dropped in v1.0.0") } @@ -74,7 +77,7 @@ func (pp *PackagePaths) ReadZarfYAML() (pkg v1alpha1.ZarfPackage, warnings []str } // MigrateLegacy migrates a legacy package layout to the new layout. -func (pp *PackagePaths) MigrateLegacy() (err error) { +func (pp *PackagePaths) MigrateLegacy() error { var pkg v1alpha1.ZarfPackage base := pp.Base diff --git a/src/pkg/layout/sbom.go b/src/pkg/layout/sbom.go index 7ac39c02a7..fcfb300be6 100644 --- a/src/pkg/layout/sbom.go +++ b/src/pkg/layout/sbom.go @@ -26,7 +26,7 @@ type SBOMs struct { } // Unarchive unarchives the package's SBOMs. -func (s *SBOMs) Unarchive() (err error) { +func (s *SBOMs) Unarchive() error { if s.Path == "" || helpers.InvalidPath(s.Path) { return &fs.PathError{ Op: "stat", @@ -47,7 +47,7 @@ func (s *SBOMs) Unarchive() (err error) { } // Archive archives the package's SBOMs. -func (s *SBOMs) Archive() (err error) { +func (s *SBOMs) Archive() error { if s.Path == "" || helpers.InvalidPath(s.Path) { return &fs.PathError{ Op: "stat", @@ -68,18 +68,23 @@ func (s *SBOMs) Archive() (err error) { return os.RemoveAll(dir) } -// StageSBOMViewFiles copies SBOM viewer HTML files to the Zarf SBOM directory. -func (s *SBOMs) StageSBOMViewFiles() (sbomViewFiles, warnings []string, err error) { +// StageSBOMViewFiles copies SBOM viewer HTML files to the Zarf SBOM directory. Returns sbomViewFiles, warnings, and an +// error. +func (s *SBOMs) StageSBOMViewFiles() ([]string, []string, error) { + sbomViewFiles := make([]string, 0) + warnings := make([]string, 0) + if s.IsTarball() { return nil, nil, fmt.Errorf("unable to process the SBOM files for this package: %s is a tarball", s.Path) } // If SBOMs were loaded, temporarily place them in the deploy directory if !helpers.InvalidPath(s.Path) { - sbomViewFiles, err = filepath.Glob(filepath.Join(s.Path, "sbom-viewer-*")) + files, err := filepath.Glob(filepath.Join(s.Path, "sbom-viewer-*")) if err != nil { return nil, nil, err } + sbomViewFiles = files if _, err := s.OutputSBOMFiles(SBOMDir, ""); err != nil { // Don't stop the deployment, let the user decide if they want to continue the deployment @@ -107,6 +112,6 @@ func (s *SBOMs) OutputSBOMFiles(outputDir, packageName string) (string, error) { } // IsTarball returns true if the SBOMs are a tarball. -func (s SBOMs) IsTarball() bool { +func (s *SBOMs) IsTarball() bool { return !helpers.IsDir(s.Path) && filepath.Ext(s.Path) == ".tar" } diff --git a/src/pkg/lint/validate.go b/src/pkg/lint/validate.go index 2de0ba8e91..0083c8a6b7 100644 --- a/src/pkg/lint/validate.go +++ b/src/pkg/lint/validate.go @@ -234,7 +234,7 @@ func validateAction(action v1alpha1.ZarfComponentAction) error { // validateReleaseName validates a release name against DNS 1035 spec, using chartName as fallback. // https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#rfc-1035-label-names -func validateReleaseName(chartName, releaseName string) (err error) { +func validateReleaseName(chartName, releaseName string) error { // Fallback to chartName if releaseName is empty // NOTE: Similar fallback mechanism happens in src/internal/packager/helm/chart.go:InstallOrUpgradeChart if releaseName == "" { @@ -243,16 +243,15 @@ func validateReleaseName(chartName, releaseName string) (err error) { // Check if the final releaseName is empty and return an error if so if releaseName == "" { - err = errors.New(errChartReleaseNameEmpty) - return + return errors.New(errChartReleaseNameEmpty) } // Validate the releaseName against DNS 1035 label spec if errs := validation.IsDNS1035Label(releaseName); len(errs) > 0 { - err = fmt.Errorf("invalid release name '%s': %s", releaseName, strings.Join(errs, "; ")) + return fmt.Errorf("invalid release name '%s': %s", releaseName, strings.Join(errs, "; ")) } - return + return nil } // validateChart runs all validation checks on a chart. diff --git a/src/pkg/message/pausable.go b/src/pkg/message/pausable.go index b9e8fae1c7..3a61f3cb59 100644 --- a/src/pkg/message/pausable.go +++ b/src/pkg/message/pausable.go @@ -29,6 +29,6 @@ func (pw *PausableWriter) Resume() { } // Write writes the data to the underlying output writer -func (pw *PausableWriter) Write(p []byte) (n int, err error) { +func (pw *PausableWriter) Write(p []byte) (int, error) { return pw.out.Write(p) } diff --git a/src/pkg/transform/image.go b/src/pkg/transform/image.go index ca6fcdc820..c12bb1d232 100644 --- a/src/pkg/transform/image.go +++ b/src/pkg/transform/image.go @@ -62,32 +62,36 @@ func ImageTransformHostWithoutChecksum(targetHost, srcReference string) (string, } // ParseImageRef parses a source reference into an Image struct -func ParseImageRef(srcReference string) (out Image, err error) { +func ParseImageRef(srcReference string) (Image, error) { srcReference = strings.TrimPrefix(srcReference, helpers.OCIURLPrefix) ref, err := reference.ParseAnyReference(srcReference) if err != nil { - return out, err + return Image{}, err } // Parse the reference into its components - if named, ok := ref.(reference.Named); ok { - out.Name = named.Name() - out.Path = reference.Path(named) - out.Host = reference.Domain(named) - out.Reference = ref.String() - } else { - return out, fmt.Errorf("unable to parse image name from %s", srcReference) + named, ok := ref.(reference.Named) + if !ok { + return Image{}, fmt.Errorf("unable to parse image name from %s", srcReference) } + out := Image{ + Name: named.Name(), + Path: reference.Path(named), + Host: reference.Domain(named), + Reference: ref.String(), + } + + // TODO(mkcp): This rewriting tag and digest code could probably be consolidated with types // Parse the tag and add it to digestOrReference - if tagged, ok := ref.(reference.Tagged); ok { + if tagged, tagOK := ref.(reference.Tagged); tagOK { out.Tag = tagged.Tag() out.TagOrDigest = fmt.Sprintf(":%s", tagged.Tag()) } // Parse the digest and override digestOrReference - if digested, ok := ref.(reference.Digested); ok { + if digested, digOK := ref.(reference.Digested); digOK { out.Digest = digested.Digest().String() out.TagOrDigest = fmt.Sprintf("@%s", digested.Digest().String()) } diff --git a/src/pkg/utils/bytes.go b/src/pkg/utils/bytes.go index 7dd159b91f..22b3322614 100644 --- a/src/pkg/utils/bytes.go +++ b/src/pkg/utils/bytes.go @@ -16,45 +16,69 @@ import ( "github.com/zarf-dev/zarf/src/pkg/message" ) +type unit struct { + name string + size float64 +} + +var ( + gigabyte = unit{ + name: "GB", + size: 1000000000, + } + megabyte = unit{ + name: "MB", + size: 1000000, + } + kilobyte = unit{ + name: "KB", + size: 1000, + } + unitByte = unit{ + name: "Byte", + } +) + // RoundUp rounds a float64 to the given number of decimal places. -func RoundUp(input float64, places int) (newVal float64) { - var round float64 +func RoundUp(input float64, places int) float64 { pow := math.Pow(10, float64(places)) digit := pow * input - round = math.Ceil(digit) - newVal = round / pow - return + round := math.Ceil(digit) + return round / pow } -// ByteFormat formats a number of bytes into a human readable string. -func ByteFormat(inputNum float64, precision int) string { +// ByteFormat formats a number of bytes into a human-readable string. +func ByteFormat(in float64, precision int) string { if precision <= 0 { precision = 1 } - var unit string - var returnVal float64 + var v float64 + var u string // https://www.techtarget.com/searchstorage/definition/mebibyte-MiB - if inputNum >= 1000000000 { - returnVal = RoundUp(inputNum/1000000000, precision) - unit = " GB" // gigabyte - } else if inputNum >= 1000000 { - returnVal = RoundUp(inputNum/1000000, precision) - unit = " MB" // megabyte - } else if inputNum >= 1000 { - returnVal = RoundUp(inputNum/1000, precision) - unit = " KB" // kilobyte - } else { - returnVal = inputNum - unit = " Byte" // byte + switch { + case gigabyte.size <= in: + v = RoundUp(in/gigabyte.size, precision) + u = gigabyte.name + case megabyte.size <= in: + v = RoundUp(in/megabyte.size, precision) + u = megabyte.name + case kilobyte.size <= in: + v = RoundUp(in/kilobyte.size, precision) + u = kilobyte.name + default: + v = in + u = unitByte.name } - if returnVal > 1 { - unit += "s" + // NOTE(mkcp): Negative bytes are nonsense, but it's more robust for inputs without erroring. + if v < -1 || 1 < v { + u += "s" } - return strconv.FormatFloat(returnVal, 'f', precision, 64) + unit + vFmt := strconv.FormatFloat(v, 'f', precision, 64) + return vFmt + " " + u } // RenderProgressBarForLocalDirWrite creates a progress bar that continuously tracks the progress of writing files to a local directory and all of its subdirectories. diff --git a/src/pkg/utils/bytes_test.go b/src/pkg/utils/bytes_test.go new file mode 100644 index 0000000000..492048f788 --- /dev/null +++ b/src/pkg/utils/bytes_test.go @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package utils provides generic utility functions. +package utils + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestByteFormat(t *testing.T) { + t.Parallel() + tt := []struct { + name string + in float64 + precision int + expect string + }{ + { + name: "accepts empty", + expect: "0.0 Byte", + }, + { + name: "accepts empty bytes with precision", + precision: 1, + expect: "0.0 Byte", + }, + { + name: "accepts empty bytes with meaningful precision", + precision: 3, + expect: "0.000 Byte", + }, + { + name: "formats negative byte with empty precision", + in: -1, + expect: "-1.0 Byte", + }, + { + name: "formats negative bytes with empty precision", + in: -2, + expect: "-2.0 Bytes", + }, + { + name: "formats kilobyte", + in: 1000, + expect: "1.0 KB", + }, + { + name: "formats kilobytes", + in: 1100, + expect: "1.1 KBs", + }, + { + name: "formats megabytes", + in: 10000000, + expect: "10.0 MBs", + }, + { + name: "formats gigabytes", + in: 100000000000, + expect: "100.0 GBs", + }, + { + name: "formats arbitrary in", + in: 4238970784923, + precision: 99, + expect: "4238.970784922999882837757468223571777343750000000000000000000000000000000000000000000000000000000000000 GBs", + }, + } + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + actual := ByteFormat(tc.in, tc.precision) + require.Equal(t, tc.expect, actual) + }) + } +} diff --git a/src/pkg/utils/cosign.go b/src/pkg/utils/cosign.go index 21bfc282bd..f81183741a 100644 --- a/src/pkg/utils/cosign.go +++ b/src/pkg/utils/cosign.go @@ -33,6 +33,12 @@ import ( "github.com/zarf-dev/zarf/src/pkg/message" ) +const ( + cosignB64Enabled = true + cosignOutputCertificate = "" + cosignTLogUpload = false +) + // Sget performs a cosign signature verification on a given image using the specified public key. // // Forked from https://github.com/sigstore/cosign/blob/v1.7.1/pkg/sget/sget.go @@ -171,7 +177,7 @@ func Sget(ctx context.Context, image, key string, out io.Writer) error { } // CosignVerifyBlob verifies the zarf.yaml.sig was signed with the key provided by the flag -func CosignVerifyBlob(ctx context.Context, blobRef string, sigRef string, keyPath string) error { +func CosignVerifyBlob(ctx context.Context, blobRef, sigRef, keyPath string) error { keyOptions := options.KeyOpts{KeyRef: keyPath} cmd := &verify.VerifyBlobCmd{ KeyOpts: keyOptions, @@ -181,74 +187,83 @@ func CosignVerifyBlob(ctx context.Context, blobRef string, sigRef string, keyPat IgnoreTlog: true, } err := cmd.Exec(ctx, blobRef) - if err == nil { - message.Successf("Package signature validated!") + if err != nil { + return err } - return err + message.Successf("Package signature validated!") + return nil } // CosignSignBlob signs the provide binary and returns the signature -func CosignSignBlob(blobPath string, outputSigPath string, keyPath string, passwordFunc func(bool) ([]byte, error)) ([]byte, error) { - rootOptions := &options.RootOptions{Verbose: false, Timeout: options.DefaultTimeout} +func CosignSignBlob(blobPath, outputSigPath, keyPath string, passFn cosign.PassFunc) ([]byte, error) { + rootOptions := &options.RootOptions{ + Verbose: false, + Timeout: options.DefaultTimeout, + } - keyOptions := options.KeyOpts{KeyRef: keyPath, - PassFunc: passwordFunc} - b64 := true - outputCertificate := "" - tlogUpload := false + keyOptions := options.KeyOpts{ + KeyRef: keyPath, + PassFunc: passFn, + } - sig, err := sign.SignBlobCmd(rootOptions, + sig, err := sign.SignBlobCmd( + rootOptions, keyOptions, blobPath, - b64, + cosignB64Enabled, outputSigPath, - outputCertificate, - tlogUpload) + cosignOutputCertificate, + cosignTLogUpload) + if err != nil { + return []byte{}, err + } - return sig, err + return sig, nil } // GetCosignArtifacts returns signatures and attestations for the given image -func GetCosignArtifacts(image string) (cosignList []string, err error) { - var cosignArtifactList []string +func GetCosignArtifacts(image string) ([]string, error) { var nameOpts []name.Option - ref, err := name.ParseReference(image, nameOpts...) + ref, err := name.ParseReference(image, nameOpts...) if err != nil { - return cosignArtifactList, err + return []string{}, err } var remoteOpts []ociremote.Option simg, _ := ociremote.SignedEntity(ref, remoteOpts...) if simg == nil { - return cosignArtifactList, nil + return []string{}, nil } + // Errors are dogsled because these functions always return a name.Tag which we can check for layers sigRef, _ := ociremote.SignatureTag(ref, remoteOpts...) attRef, _ := ociremote.AttestationTag(ref, remoteOpts...) - sigs, err := simg.Signatures() + ss, err := simg.Signatures() if err != nil { - return cosignArtifactList, err + return []string{}, err } - layers, err := sigs.Layers() + ssLayers, err := ss.Layers() if err != nil { - return cosignArtifactList, err + return []string{}, err } - if len(layers) > 0 { + + var cosignArtifactList = make([]string, 0) + if 0 < len(ssLayers) { cosignArtifactList = append(cosignArtifactList, sigRef.String()) } atts, err := simg.Attestations() if err != nil { - return cosignArtifactList, err + return []string{}, err } - layers, err = atts.Layers() + aLayers, err := atts.Layers() if err != nil { - return cosignArtifactList, err + return []string{}, err } - if len(layers) > 0 { + if 0 < len(aLayers) { cosignArtifactList = append(cosignArtifactList, attRef.String()) } return cosignArtifactList, nil diff --git a/src/pkg/utils/network.go b/src/pkg/utils/network.go index be0b80a2ed..ffe5490600 100644 --- a/src/pkg/utils/network.go +++ b/src/pkg/utils/network.go @@ -39,7 +39,7 @@ func parseChecksum(src string) (string, string, error) { } // DownloadToFile downloads a given URL to the target filepath (including the cosign key if necessary). -func DownloadToFile(ctx context.Context, src string, dst string, cosignKeyPath string) (err error) { +func DownloadToFile(ctx context.Context, src, dst, cosignKeyPath string) error { // check if the parsed URL has a checksum // if so, remove it and use the checksum to validate the file src, checksum, err := parseChecksum(src) @@ -69,9 +69,6 @@ func DownloadToFile(ctx context.Context, src string, dst string, cosignKeyPath s if err != nil { return fmt.Errorf("unable to download file with sget: %s: %w", src, err) } - if err != nil { - return err - } } else { err = httpGetFile(src, file) if err != nil { @@ -80,7 +77,7 @@ func DownloadToFile(ctx context.Context, src string, dst string, cosignKeyPath s } // If the file has a checksum, validate it - if len(checksum) > 0 { + if 0 < len(checksum) { received, err := helpers.GetSHA256OfFile(dst) if err != nil { return err diff --git a/src/pkg/variables/variables.go b/src/pkg/variables/variables.go index 929cb13182..353040eab4 100644 --- a/src/pkg/variables/variables.go +++ b/src/pkg/variables/variables.go @@ -15,8 +15,8 @@ import ( type SetVariableMap map[string]*v1alpha1.SetVariable // GetSetVariable gets a variable set within a VariableConfig by its name -func (vc *VariableConfig) GetSetVariable(name string) (variable *v1alpha1.SetVariable, ok bool) { - variable, ok = vc.setVariableMap[name] +func (vc *VariableConfig) GetSetVariable(name string) (*v1alpha1.SetVariable, bool) { + variable, ok := vc.setVariableMap[name] return variable, ok } diff --git a/src/pkg/variables/variables_test.go b/src/pkg/variables/variables_test.go index 07442e97f5..f0aeea78c5 100644 --- a/src/pkg/variables/variables_test.go +++ b/src/pkg/variables/variables_test.go @@ -20,7 +20,7 @@ func TestPopulateVariables(t *testing.T) { wantVars SetVariableMap } - prompt := func(_ v1alpha1.InteractiveVariable) (value string, err error) { return "Prompt", nil } + prompt := func(_ v1alpha1.InteractiveVariable) (string, error) { return "Prompt", nil } tests := []test{ { diff --git a/src/pkg/zoci/fetch.go b/src/pkg/zoci/fetch.go index 923e3d7c24..ca46d8e996 100644 --- a/src/pkg/zoci/fetch.go +++ b/src/pkg/zoci/fetch.go @@ -14,19 +14,27 @@ import ( ) // FetchZarfYAML fetches the zarf.yaml file from the remote repository. -func (r *Remote) FetchZarfYAML(ctx context.Context) (pkg v1alpha1.ZarfPackage, err error) { +func (r *Remote) FetchZarfYAML(ctx context.Context) (v1alpha1.ZarfPackage, error) { manifest, err := r.FetchRoot(ctx) if err != nil { - return pkg, err + return v1alpha1.ZarfPackage{}, err } - return oci.FetchYAMLFile[v1alpha1.ZarfPackage](ctx, r.FetchLayer, manifest, layout.ZarfYAML) + result, err := oci.FetchYAMLFile[v1alpha1.ZarfPackage](ctx, r.FetchLayer, manifest, layout.ZarfYAML) + if err != nil { + return v1alpha1.ZarfPackage{}, err + } + return result, nil } // FetchImagesIndex fetches the images/index.json file from the remote repository. -func (r *Remote) FetchImagesIndex(ctx context.Context) (index *ocispec.Index, err error) { +func (r *Remote) FetchImagesIndex(ctx context.Context) (*ocispec.Index, error) { manifest, err := r.FetchRoot(ctx) if err != nil { return nil, err } - return oci.FetchJSONFile[*ocispec.Index](ctx, r.FetchLayer, manifest, layout.IndexPath) + result, err := oci.FetchJSONFile[*ocispec.Index](ctx, r.FetchLayer, manifest, layout.IndexPath) + if err != nil { + return nil, err + } + return result, nil } diff --git a/src/pkg/zoci/pull.go b/src/pkg/zoci/pull.go index 9fd76e9ccc..44c16b5646 100644 --- a/src/pkg/zoci/pull.go +++ b/src/pkg/zoci/pull.go @@ -76,7 +76,9 @@ func (r *Remote) PullPackage(ctx context.Context, destinationDir string, concurr // LayersFromRequestedComponents returns the descriptors for the given components from the root manifest. // // It also retrieves the descriptors for all image layers that are required by the components. -func (r *Remote) LayersFromRequestedComponents(ctx context.Context, requestedComponents []v1alpha1.ZarfComponent) (layers []ocispec.Descriptor, err error) { +func (r *Remote) LayersFromRequestedComponents(ctx context.Context, requestedComponents []v1alpha1.ZarfComponent) ([]ocispec.Descriptor, error) { + layers := make([]ocispec.Descriptor, 0) + root, err := r.FetchRoot(ctx) if err != nil { return nil, err @@ -98,7 +100,8 @@ func (r *Remote) LayersFromRequestedComponents(ctx context.Context, requestedCom for _, image := range component.Images { images[image] = true } - layers = append(layers, root.Locate(filepath.Join(layout.ComponentsDir, fmt.Sprintf(tarballFormat, component.Name)))) + desc := root.Locate(filepath.Join(layout.ComponentsDir, fmt.Sprintf(tarballFormat, component.Name))) + layers = append(layers, desc) } // Append the sboms.tar layer if it exists // From cf4e9891d531b792849394f80830363411242fd9 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Tue, 10 Sep 2024 17:38:01 -0700 Subject: [PATCH 3/4] chore: finish removing named returns outside of package and extensions #2950 (#2987) Signed-off-by: Kit Patella --- src/internal/agent/hooks/argocd-application.go | 5 ++--- src/internal/agent/hooks/argocd-repository.go | 2 +- src/internal/agent/hooks/flux-gitrepo.go | 2 +- src/test/e2e/28_wait_test.go | 3 ++- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/internal/agent/hooks/argocd-application.go b/src/internal/agent/hooks/argocd-application.go index b234f29e84..e7351c89fd 100644 --- a/src/internal/agent/hooks/argocd-application.go +++ b/src/internal/agent/hooks/argocd-application.go @@ -59,7 +59,7 @@ func NewApplicationMutationHook(ctx context.Context, cluster *cluster.Cluster) o } // mutateApplication mutates the git repository url to point to the repository URL defined in the ZarfState. -func mutateApplication(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Cluster) (result *operations.Result, err error) { +func mutateApplication(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Cluster) (*operations.Result, error) { state, err := cluster.LoadZarfState(ctx) if err != nil { return nil, err @@ -72,8 +72,7 @@ func mutateApplication(ctx context.Context, r *v1.AdmissionRequest, cluster *clu return nil, fmt.Errorf(lang.ErrUnmarshal, err) } - patches := []operations.PatchOperation{} - + patches := make([]operations.PatchOperation, 0) if app.Spec.Source != nil { patchedURL, err := getPatchedRepoURL(app.Spec.Source.RepoURL, state.GitServer, r) if err != nil { diff --git a/src/internal/agent/hooks/argocd-repository.go b/src/internal/agent/hooks/argocd-repository.go index 1875772d05..cf2e9d895e 100644 --- a/src/internal/agent/hooks/argocd-repository.go +++ b/src/internal/agent/hooks/argocd-repository.go @@ -47,7 +47,7 @@ func NewRepositorySecretMutationHook(ctx context.Context, cluster *cluster.Clust } // mutateRepositorySecret mutates the git URL in the ArgoCD repository secret to point to the repository URL defined in the ZarfState. -func mutateRepositorySecret(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Cluster) (result *operations.Result, err error) { +func mutateRepositorySecret(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Cluster) (*operations.Result, error) { isCreate := r.Operation == v1.Create isUpdate := r.Operation == v1.Update var isPatched bool diff --git a/src/internal/agent/hooks/flux-gitrepo.go b/src/internal/agent/hooks/flux-gitrepo.go index 2fda2969bb..77447b7c34 100644 --- a/src/internal/agent/hooks/flux-gitrepo.go +++ b/src/internal/agent/hooks/flux-gitrepo.go @@ -37,7 +37,7 @@ func NewGitRepositoryMutationHook(ctx context.Context, cluster *cluster.Cluster) } // mutateGitRepoCreate mutates the git repository url to point to the repository URL defined in the ZarfState. -func mutateGitRepo(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Cluster) (result *operations.Result, err error) { +func mutateGitRepo(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Cluster) (*operations.Result, error) { var ( patches []operations.PatchOperation isPatched bool diff --git a/src/test/e2e/28_wait_test.go b/src/test/e2e/28_wait_test.go index e150d163fd..e67b60d178 100644 --- a/src/test/e2e/28_wait_test.go +++ b/src/test/e2e/28_wait_test.go @@ -20,7 +20,8 @@ type zarfCommandResult struct { err error } -func zarfCommandWStruct(t *testing.T, e2e test.ZarfE2ETest, path string) (result zarfCommandResult) { +func zarfCommandWStruct(t *testing.T, e2e test.ZarfE2ETest, path string) zarfCommandResult { + result := zarfCommandResult{} result.stdOut, result.stdErr, result.err = e2e.Zarf(t, "package", "deploy", path, "--confirm") return result } From e72a2736bf475d0d3879133ea653326e747f74d2 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Wed, 11 Sep 2024 15:14:48 -0700 Subject: [PATCH 4/4] chore: ensure we return zeroed value when returning errors (#2988) Signed-off-by: Kit Patella --- src/cmd/tools/helm/load_plugins.go | 7 +++++-- src/pkg/cluster/pvc.go | 2 ++ src/pkg/cluster/tunnel.go | 7 ++++--- src/pkg/cluster/zarf.go | 11 +++++++++-- src/pkg/transform/artifact.go | 10 +++++----- src/pkg/utils/io.go | 9 ++++++--- src/pkg/utils/yaml.go | 6 +++--- src/pkg/zoci/pull.go | 5 ++++- 8 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/cmd/tools/helm/load_plugins.go b/src/cmd/tools/helm/load_plugins.go index 28ea155030..df8b7cad67 100644 --- a/src/cmd/tools/helm/load_plugins.go +++ b/src/cmd/tools/helm/load_plugins.go @@ -318,11 +318,14 @@ func loadFile(path string) (*pluginCommand, error) { cmds := new(pluginCommand) b, err := os.ReadFile(path) if err != nil { - return cmds, fmt.Errorf("file (%s) not provided by plugin. No plugin auto-completion possible", path) + return nil, fmt.Errorf("file (%s) not provided by plugin. No plugin auto-completion possible", path) } err = yaml.Unmarshal(b, cmds) - return cmds, err + if err != nil { + return nil, err + } + return cmds, nil } // pluginDynamicComp call the plugin.complete script of the plugin (if available) diff --git a/src/pkg/cluster/pvc.go b/src/pkg/cluster/pvc.go index 21a0a45ecf..6bef179623 100644 --- a/src/pkg/cluster/pvc.go +++ b/src/pkg/cluster/pvc.go @@ -10,6 +10,8 @@ import ( ) // UpdateGiteaPVC updates the existing Gitea persistent volume claim and tells Gitea whether to create or not. +// TODO(mkcp): We return both string true/false and errors here so our callers get a string. This should be returning an +// empty val if we error, but we'll have to refactor upstream beforehand. func (c *Cluster) UpdateGiteaPVC(ctx context.Context, pvcName string, shouldRollBack bool) (string, error) { if shouldRollBack { pvc, err := c.Clientset.CoreV1().PersistentVolumeClaims(ZarfNamespaceName).Get(ctx, pvcName, metav1.GetOptions{}) diff --git a/src/pkg/cluster/tunnel.go b/src/pkg/cluster/tunnel.go index 764ae9a6eb..9798fd3272 100644 --- a/src/pkg/cluster/tunnel.go +++ b/src/pkg/cluster/tunnel.go @@ -83,7 +83,6 @@ func (c *Cluster) ListConnections(ctx context.Context) (types.ConnectStrings, er // NewTargetTunnelInfo returns a new TunnelInfo object for the specified target. func (c *Cluster) NewTargetTunnelInfo(ctx context.Context, target string) (TunnelInfo, error) { - var err error zt := TunnelInfo{ Namespace: ZarfNamespaceName, ResourceType: SvcResource, @@ -102,9 +101,11 @@ func (c *Cluster) NewTargetTunnelInfo(ctx context.Context, target string) (Tunne zt.RemotePort = ZarfInjectorPort default: if target != "" { - if zt, err = c.checkForZarfConnectLabel(ctx, target); err != nil { + ztNew, err := c.checkForZarfConnectLabel(ctx, target) + if err != nil { return TunnelInfo{}, fmt.Errorf("problem looking for a zarf connect label in the cluster: %s", err.Error()) } + zt = ztNew } if zt.ResourceName == "" { return TunnelInfo{}, fmt.Errorf("missing resource name") @@ -113,7 +114,7 @@ func (c *Cluster) NewTargetTunnelInfo(ctx context.Context, target string) (Tunne return TunnelInfo{}, fmt.Errorf("missing remote port") } } - return zt, err + return zt, nil } // Connect will establish a tunnel to the specified target. diff --git a/src/pkg/cluster/zarf.go b/src/pkg/cluster/zarf.go index eebbd6d295..b38b55d783 100644 --- a/src/pkg/cluster/zarf.go +++ b/src/pkg/cluster/zarf.go @@ -52,7 +52,11 @@ func (c *Cluster) GetDeployedZarfPackages(ctx context.Context) ([]types.Deployed deployedPackages = append(deployedPackages, deployedPackage) } - return deployedPackages, errors.Join(errs...) + err = errors.Join(errs...) + if err != nil { + return nil, err + } + return deployedPackages, nil } // GetDeployedPackage gets the metadata information about the package name provided (if it exists in the cluster). @@ -325,7 +329,10 @@ func (c *Cluster) UpdateInternalArtifactServerToken(ctx context.Context, oldGitS } return nil }) - return newToken, err + if err != nil { + return "", err + } + return newToken, nil } // UpdateInternalGitServerSecret updates the internal gitea server secrets with the new git server info diff --git a/src/pkg/transform/artifact.go b/src/pkg/transform/artifact.go index 0aed7a46ea..2c78dac233 100644 --- a/src/pkg/transform/artifact.go +++ b/src/pkg/transform/artifact.go @@ -87,16 +87,16 @@ func GenTransformURL(targetBaseURL string, sourceURL string) (*url.URL, error) { // Rebuild the generic URL transformedURL := fmt.Sprintf("%s/generic/%s/%s/%s", targetBaseURL, packageNameGlobal, version, fileName) - url, err := url.Parse(transformedURL) + parsedURL, err := url.Parse(transformedURL) if err != nil { - return url, err + return &url.URL{}, err } // Drop the RawQuery and Fragment to avoid them being interpreted for generic packages - url.RawQuery = "" - url.Fragment = "" + parsedURL.RawQuery = "" + parsedURL.Fragment = "" - return url, err + return parsedURL, nil } // transformRegistryPath transforms a given request path using a new base URL and regex. diff --git a/src/pkg/utils/io.go b/src/pkg/utils/io.go index 7edee56422..f4ec9b07ab 100755 --- a/src/pkg/utils/io.go +++ b/src/pkg/utils/io.go @@ -40,7 +40,10 @@ func GetFinalExecutablePath() (string, error) { // In case the binary is symlinked somewhere else, get the final destination linkedPath, err := filepath.EvalSymlinks(binaryPath) - return linkedPath, err + if err != nil { + return "", err + } + return linkedPath, nil } // GetFinalExecutableCommand returns the final path to the Zarf executable including and library prefixes and overrides. @@ -48,7 +51,7 @@ func GetFinalExecutableCommand() (string, error) { // In case the binary is symlinked somewhere else, get the final destination zarfCommand, err := GetFinalExecutablePath() if err != nil { - return zarfCommand, err + return "", err } if config.ActionsCommandZarfPrefix != "" { @@ -60,5 +63,5 @@ func GetFinalExecutableCommand() (string, error) { zarfCommand = "zarf" } - return zarfCommand, err + return zarfCommand, nil } diff --git a/src/pkg/utils/yaml.go b/src/pkg/utils/yaml.go index f3fdaa53b7..641c219977 100644 --- a/src/pkg/utils/yaml.go +++ b/src/pkg/utils/yaml.go @@ -192,12 +192,12 @@ func SplitYAML(yamlData []byte) ([]*unstructured.Unstructured, error) { var objs []*unstructured.Unstructured ymls, err := SplitYAMLToString(yamlData) if err != nil { - return nil, err + return []*unstructured.Unstructured{}, err } for _, yml := range ymls { u := &unstructured.Unstructured{} if err := k8syaml.Unmarshal([]byte(yml), u); err != nil { - return objs, fmt.Errorf("failed to unmarshal manifest: %w", err) + return []*unstructured.Unstructured{}, fmt.Errorf("failed to unmarshal manifest: %w", err) } objs = append(objs, u) } @@ -220,7 +220,7 @@ func SplitYAMLToString(yamlData []byte) ([]string, error) { if errors.Is(err, io.EOF) { break } - return objs, fmt.Errorf("failed to unmarshal manifest: %w", err) + return []string{}, fmt.Errorf("failed to unmarshal manifest: %w", err) } ext.Raw = bytes.TrimSpace(ext.Raw) if len(ext.Raw) == 0 || bytes.Equal(ext.Raw, []byte("null")) { diff --git a/src/pkg/zoci/pull.go b/src/pkg/zoci/pull.go index 44c16b5646..d8ba73775e 100644 --- a/src/pkg/zoci/pull.go +++ b/src/pkg/zoci/pull.go @@ -70,7 +70,10 @@ func (r *Remote) PullPackage(ctx context.Context, destinationDir string, concurr err = r.CopyToTarget(ctx, layersToPull, dst, copyOpts) doneSaving <- err <-doneSaving - return layersToPull, err + if err != nil { + return nil, err + } + return layersToPull, nil } // LayersFromRequestedComponents returns the descriptors for the given components from the root manifest.