From 34d110a9cdf7a4d33e45511ebe1bc21c95181f60 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Wed, 2 Oct 2024 11:01:40 -0700 Subject: [PATCH] chore: directly handle ignored errs and nolint intentionally ignored errs (#2993) Signed-off-by: Kit Patella --- src/cmd/destroy.go | 13 +++++-- src/cmd/dev.go | 28 +++++++++++---- src/cmd/initialize.go | 35 +++++++++++-------- src/cmd/internal.go | 9 +++-- src/cmd/package.go | 50 ++++++++++++++++++++++----- src/cmd/root.go | 5 ++- src/cmd/tools/helm.go | 7 +++- src/cmd/tools/zarf.go | 12 ++++--- src/config/config.go | 13 ++++--- src/internal/git/repository_test.go | 3 +- src/internal/gitea/gitea.go | 9 +++-- src/internal/packager/images/pull.go | 6 +++- src/internal/packager/sbom/catalog.go | 6 +++- src/pkg/interactive/components.go | 7 ++-- src/pkg/layout/component.go | 5 ++- src/pkg/layout/package.go | 14 ++++++-- src/pkg/layout/split.go | 37 +++++++++++++++----- src/pkg/layout/split_test.go | 3 +- src/pkg/message/progress.go | 16 +++++++-- src/pkg/message/spinner.go | 12 +++++-- src/pkg/packager/composer/oci.go | 6 +++- src/pkg/packager/creator/normal.go | 6 +++- src/pkg/packager/deploy.go | 9 +++-- src/pkg/packager/inspect.go | 5 ++- src/pkg/packager/interactive.go | 5 ++- src/pkg/packager/publish.go | 5 ++- src/pkg/utils/auth.go | 20 ++++++++--- src/pkg/utils/cosign.go | 19 +++++----- src/pkg/utils/exec/exec.go | 10 ++++-- src/pkg/utils/network.go | 16 ++++++--- src/pkg/utils/wait.go | 6 +++- src/pkg/utils/yaml.go | 8 +++-- src/pkg/variables/templates.go | 15 ++++++-- src/pkg/variables/templates_test.go | 21 +++++++---- src/pkg/zoci/copier.go | 8 +++-- src/pkg/zoci/pull.go | 8 +++-- src/pkg/zoci/push.go | 13 +++++-- 37 files changed, 349 insertions(+), 121 deletions(-) diff --git a/src/cmd/destroy.go b/src/cmd/destroy.go index 1b71740aa3..6951efd96b 100644 --- a/src/cmd/destroy.go +++ b/src/cmd/destroy.go @@ -30,6 +30,7 @@ var destroyCmd = &cobra.Command{ Aliases: []string{"d"}, Short: lang.CmdDestroyShort, Long: lang.CmdDestroyLong, + // TODO(mkcp): refactor and de-nest this function. RunE: func(cmd *cobra.Command, _ []string) error { ctx := cmd.Context() timeoutCtx, cancel := context.WithTimeout(cmd.Context(), cluster.DefaultTimeout) @@ -57,7 +58,10 @@ var destroyCmd = &cobra.Command{ // Run all the scripts! pattern := regexp.MustCompile(`(?mi)zarf-clean-.+\.sh$`) - scripts, _ := helpers.RecursiveFileList(config.ZarfCleanupScriptsPath, pattern, true) + scripts, err := helpers.RecursiveFileList(config.ZarfCleanupScriptsPath, pattern, true) + if err != nil { + return err + } // Iterate over all matching zarf-clean scripts and exec them for _, script := range scripts { // Run the matched script @@ -71,8 +75,11 @@ var destroyCmd = &cobra.Command{ return fmt.Errorf("received an error when executing the script %s: %w", script, err) } - // Try to remove the script, but ignore any errors - _ = os.Remove(script) + // Try to remove the script, but ignore any errors and debug log them + err = os.Remove(script) + if err != nil { + message.WarnErr(err, fmt.Sprintf("Unable to remove script. script=%s", script)) + } } } else { // Perform chart uninstallation diff --git a/src/cmd/dev.go b/src/cmd/dev.go index 85f08e9c11..ab0a262589 100644 --- a/src/cmd/dev.go +++ b/src/cmd/dev.go @@ -148,14 +148,13 @@ var devSha256SumCmd = &cobra.Command{ Aliases: []string{"s"}, Short: lang.CmdDevSha256sumShort, Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) (err error) { hashErr := errors.New("unable to compute the SHA256SUM hash") fileName := args[0] var tmp string var data io.ReadCloser - var err error if helpers.IsURL(fileName) { message.Warn(lang.CmdDevSha256sumRemoteWarning) @@ -182,7 +181,10 @@ var devSha256SumCmd = &cobra.Command{ fileName = downloadPath - defer os.RemoveAll(tmp) + defer func(path string) { + errRemove := os.RemoveAll(path) + err = errors.Join(err, errRemove) + }(tmp) } if extractPath != "" { @@ -191,7 +193,10 @@ var devSha256SumCmd = &cobra.Command{ if err != nil { return errors.Join(hashErr, err) } - defer os.RemoveAll(tmp) + defer func(path string) { + errRemove := os.RemoveAll(path) + err = errors.Join(err, errRemove) + }(tmp) } extractedFile := filepath.Join(tmp, extractPath) @@ -208,7 +213,10 @@ var devSha256SumCmd = &cobra.Command{ if err != nil { return errors.Join(hashErr, err) } - defer data.Close() + defer func(data io.ReadCloser) { + errClose := data.Close() + err = errors.Join(err, errClose) + }(data) hash, err := helpers.GetSHA256Hash(data) if err != nil { @@ -323,8 +331,14 @@ func init() { // use the package create config for this and reset it here to avoid overwriting the config.CreateOptions.SetVariables devFindImagesCmd.Flags().StringToStringVar(&pkgConfig.CreateOpts.SetVariables, "set", v.GetStringMapString(common.VPkgCreateSet), lang.CmdDevFlagSet) - devFindImagesCmd.Flags().MarkDeprecated("set", "this field is replaced by create-set") - devFindImagesCmd.Flags().MarkHidden("set") + err := devFindImagesCmd.Flags().MarkDeprecated("set", "this field is replaced by create-set") + if err != nil { + message.Debug("Unable to mark dev-find-images flag as set", "error", err) + } + err = devFindImagesCmd.Flags().MarkHidden("set") + if err != nil { + message.Debug("Unable to mark dev-find-images flag as hidden", "error", err) + } devFindImagesCmd.Flags().StringVarP(&pkgConfig.CreateOpts.Flavor, "flavor", "f", v.GetString(common.VPkgCreateFlavor), lang.CmdPackageCreateFlagFlavor) devFindImagesCmd.Flags().StringToStringVar(&pkgConfig.CreateOpts.SetVariables, "create-set", v.GetStringMapString(common.VPkgCreateSet), lang.CmdDevFlagSet) devFindImagesCmd.Flags().StringToStringVar(&pkgConfig.PkgOpts.SetVariables, "deploy-set", v.GetStringMapString(common.VPkgDeploySet), lang.CmdPackageDeployFlagSet) diff --git a/src/cmd/initialize.go b/src/cmd/initialize.go index 376db85da9..7316183e5d 100644 --- a/src/cmd/initialize.go +++ b/src/cmd/initialize.go @@ -94,19 +94,27 @@ func findInitPackage(ctx context.Context, initPackageName string) (string, error } // Create the cache directory if it doesn't exist - if helpers.InvalidPath(config.GetAbsCachePath()) { - if err := helpers.CreateDirectory(config.GetAbsCachePath(), helpers.ReadExecuteAllWriteUser); err != nil { - return "", fmt.Errorf("unable to create the cache directory %s: %w", config.GetAbsCachePath(), err) + absCachePath, err := config.GetAbsCachePath() + if err != nil { + return "", err + } + // Verify that we can write to the path + if helpers.InvalidPath(absCachePath) { + // Create the directory if the path is invalid + err = helpers.CreateDirectory(absCachePath, helpers.ReadExecuteAllWriteUser) + if err != nil { + return "", fmt.Errorf("unable to create the cache directory %s: %w", absCachePath, err) } } // Next, look in the cache directory - if !helpers.InvalidPath(filepath.Join(config.GetAbsCachePath(), initPackageName)) { - return filepath.Join(config.GetAbsCachePath(), initPackageName), nil + if !helpers.InvalidPath(filepath.Join(absCachePath, initPackageName)) { + // join and return + return filepath.Join(absCachePath, initPackageName), nil } // Finally, if the init-package doesn't exist in the cache directory, suggest downloading it - downloadCacheTarget, err := downloadInitPackage(ctx, config.GetAbsCachePath()) + downloadCacheTarget, err := downloadInitPackage(ctx, absCachePath) if err != nil { if errors.Is(err, lang.ErrInitNotFound) { return "", err @@ -121,7 +129,6 @@ func downloadInitPackage(ctx context.Context, cacheDirectory string) (string, er return "", lang.ErrInitNotFound } - var confirmDownload bool url := zoci.GetInitPackageURL(config.CLIVersion) // Give the user the choice to download the init-package and note that this does require an internet connection @@ -129,14 +136,12 @@ func downloadInitPackage(ctx context.Context, cacheDirectory string) (string, er message.Note(lang.CmdInitPullNote) - // Prompt the user if --confirm not specified - if !confirmDownload { - prompt := &survey.Confirm{ - Message: lang.CmdInitPullConfirm, - } - if err := survey.AskOne(prompt, &confirmDownload); err != nil { - return "", fmt.Errorf("confirm download canceled: %w", err) - } + var confirmDownload bool + prompt := &survey.Confirm{ + Message: lang.CmdInitPullConfirm, + } + if err := survey.AskOne(prompt, &confirmDownload); err != nil { + return "", fmt.Errorf("confirm download canceled: %w", err) } // If the user wants to download the init-package, download it diff --git a/src/cmd/internal.go b/src/cmd/internal.go index 4a393d01e6..b3bc2c4592 100644 --- a/src/cmd/internal.go +++ b/src/cmd/internal.go @@ -348,8 +348,8 @@ var isValidHostname = &cobra.Command{ Short: lang.CmdInternalIsValidHostnameShort, RunE: func(_ *cobra.Command, _ []string) error { if valid := helpers.IsValidHostName(); !valid { - hostname, _ := os.Hostname() - return fmt.Errorf("the hostname %s is not valid. Ensure the hostname meets RFC1123 requirements https://www.rfc-editor.org/rfc/rfc1123.html", hostname) + hostname, err := os.Hostname() + return fmt.Errorf("the hostname %s is not valid. Ensure the hostname meets RFC1123 requirements https://www.rfc-editor.org/rfc/rfc1123.html, error=%w", hostname, err) } return nil }, @@ -388,6 +388,9 @@ func addHiddenDummyFlag(cmd *cobra.Command, flagDummy string) { if cmd.PersistentFlags().Lookup(flagDummy) == nil { var dummyStr string cmd.PersistentFlags().StringVar(&dummyStr, flagDummy, "", "") - cmd.PersistentFlags().MarkHidden(flagDummy) + err := cmd.PersistentFlags().MarkHidden(flagDummy) + if err != nil { + message.Debug("Unable to add hidden dummy flag", "error", err) + } } } diff --git a/src/cmd/package.go b/src/cmd/package.go index b0d812fd80..20e72de9b2 100644 --- a/src/cmd/package.go +++ b/src/cmd/package.go @@ -220,7 +220,10 @@ var packageInspectCmd = &cobra.Command{ if err != nil { return fmt.Errorf("failed to inspect package: %w", err) } - utils.ColorPrintYAML(output, nil, false) + err = utils.ColorPrintYAML(output, nil, false) + if err != nil { + return err + } return nil }, } @@ -383,9 +386,23 @@ func choosePackage(args []string) (string, error) { prompt := &survey.Input{ Message: lang.CmdPackageChoose, Suggest: func(toComplete string) []string { - files, _ := filepath.Glob(config.ZarfPackagePrefix + toComplete + "*.tar") - zstFiles, _ := filepath.Glob(config.ZarfPackagePrefix + toComplete + "*.tar.zst") - splitFiles, _ := filepath.Glob(config.ZarfPackagePrefix + toComplete + "*.part000") + tarPath := config.ZarfPackagePrefix + toComplete + "*.tar" + files, err := filepath.Glob(tarPath) + if err != nil { + message.Debug("Unable to glob", "tarPath", tarPath, "error", err) + } + + zstPath := config.ZarfPackagePrefix + toComplete + "*.tar.zst" + zstFiles, err := filepath.Glob(zstPath) + if err != nil { + message.Debug("Unable to glob", "zstPath", zstPath, "error", err) + } + + splitPath := config.ZarfPackagePrefix + toComplete + "*.part000" + splitFiles, err := filepath.Glob(splitPath) + if err != nil { + message.Debug("Unable to glob", "splitPath", splitPath, "error", err) + } files = append(files, zstFiles...) files = append(files, splitFiles...) @@ -410,7 +427,10 @@ func getPackageCompletionArgs(cmd *cobra.Command, _ []string, _ string) ([]strin ctx := cmd.Context() - deployedZarfPackages, _ := c.GetDeployedZarfPackages(ctx) + deployedZarfPackages, err := c.GetDeployedZarfPackages(ctx) + if err != nil { + message.Debug("Unable to get deployed zarf packages for package completion args", "error", err) + } // Populate list of package names for _, pkg := range deployedZarfPackages { pkgCandidates = append(pkgCandidates, pkg.Name) @@ -479,9 +499,18 @@ func bindCreateFlags(v *viper.Viper) { createFlags.IntVar(&pkgConfig.PkgOpts.Retries, "retries", v.GetInt(common.VPkgRetries), lang.CmdPackageFlagRetries) - createFlags.MarkHidden("output-directory") - createFlags.MarkHidden("key") - createFlags.MarkHidden("key-pass") + errOD := createFlags.MarkHidden("output-directory") + if errOD != nil { + message.Debug("Unable to mark flag output-directory", "error", errOD) + } + errKey := createFlags.MarkHidden("key") + if errKey != nil { + message.Debug("Unable to mark flag key", "error", errKey) + } + errKP := createFlags.MarkHidden("key-pass") + if errKP != nil { + message.Debug("Unable to mark flag key-pass", "error", errKP) + } } func bindDeployFlags(v *viper.Viper) { @@ -503,7 +532,10 @@ func bindDeployFlags(v *viper.Viper) { deployFlags.BoolVar(&pkgConfig.PkgOpts.SkipSignatureValidation, "skip-signature-validation", false, lang.CmdPackageFlagSkipSignatureValidation) deployFlags.BoolVar(&pkgConfig.DeployOpts.ForcePushRepos, "force-push-repos", false, lang.CmdPackageForcePushRepos) - deployFlags.MarkHidden("sget") + err := deployFlags.MarkHidden("sget") + if err != nil { + message.Debug("Unable to mark flag sget", "error", err) + } } func bindMirrorFlags(v *viper.Viper) { diff --git a/src/cmd/root.go b/src/cmd/root.go index 188f91e8cc..bf695e3760 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -78,7 +78,10 @@ var rootCmd = &cobra.Command{ Run: func(cmd *cobra.Command, args []string) { zarfLogo := message.GetLogo() _, _ = fmt.Fprintln(os.Stderr, zarfLogo) - cmd.Help() + err := cmd.Help() + if err != nil { + _, _ = fmt.Fprintln(os.Stderr, err) + } if len(args) > 0 { if strings.Contains(args[0], config.ZarfPackagePrefix) || strings.Contains(args[0], "zarf-init") { diff --git a/src/cmd/tools/helm.go b/src/cmd/tools/helm.go index 0d37f8d017..9f367b694e 100644 --- a/src/cmd/tools/helm.go +++ b/src/cmd/tools/helm.go @@ -7,6 +7,8 @@ package tools import ( "os" + "github.com/zarf-dev/zarf/src/pkg/message" + "github.com/zarf-dev/zarf/src/cmd/tools/helm" "github.com/zarf-dev/zarf/src/config/lang" "helm.sh/helm/v3/pkg/action" @@ -24,7 +26,10 @@ func init() { helmArgs = os.Args[3:] } // The inclusion of Helm in this manner should be changed once https://github.com/helm/helm/pull/12725 is merged - helmCmd, _ := helm.NewRootCmd(actionConfig, os.Stdout, helmArgs) + helmCmd, err := helm.NewRootCmd(actionConfig, os.Stdout, helmArgs) + if err != nil { + message.Debug("Failed to initialize helm command", "error", err) + } helmCmd.Short = lang.CmdToolsHelmShort helmCmd.Long = lang.CmdToolsHelmLong helmCmd.AddCommand(newVersionCmd("helm", helmVersion)) diff --git a/src/cmd/tools/zarf.go b/src/cmd/tools/zarf.go index 5a9cd11718..df516f9506 100644 --- a/src/cmd/tools/zarf.go +++ b/src/cmd/tools/zarf.go @@ -205,11 +205,15 @@ var clearCacheCmd = &cobra.Command{ Aliases: []string{"c"}, Short: lang.CmdToolsClearCacheShort, RunE: func(_ *cobra.Command, _ []string) error { - message.Notef(lang.CmdToolsClearCacheDir, config.GetAbsCachePath()) - if err := os.RemoveAll(config.GetAbsCachePath()); err != nil { - return fmt.Errorf("unable to clear the cache directory %s: %w", config.GetAbsCachePath(), err) + cachePath, err := config.GetAbsCachePath() + if err != nil { + return err + } + message.Notef(lang.CmdToolsClearCacheDir, cachePath) + if err := os.RemoveAll(cachePath); err != nil { + return fmt.Errorf("unable to clear the cache directory %s: %w", cachePath, err) } - message.Successf(lang.CmdToolsClearCacheSuccess, config.GetAbsCachePath()) + message.Successf(lang.CmdToolsClearCacheSuccess, cachePath) return nil }, } diff --git a/src/config/config.go b/src/config/config.go index 4a4001dce6..ca80f619b6 100644 --- a/src/config/config.go +++ b/src/config/config.go @@ -97,16 +97,19 @@ func GetDataInjectionMarker() string { } // GetAbsCachePath gets the absolute cache path for images and git repos. -func GetAbsCachePath() string { +func GetAbsCachePath() (string, error) { return GetAbsHomePath(CommonOptions.CachePath) } // GetAbsHomePath replaces ~ with the absolute path to a user's home dir -func GetAbsHomePath(path string) string { - homePath, _ := os.UserHomeDir() +func GetAbsHomePath(path string) (string, error) { + homePath, err := os.UserHomeDir() + if err != nil { + return "", err + } if strings.HasPrefix(path, "~") { - return strings.Replace(path, "~", homePath, 1) + return strings.Replace(path, "~", homePath, 1), nil } - return path + return path, nil } diff --git a/src/internal/git/repository_test.go b/src/internal/git/repository_test.go index 7e75319eee..e833e6e2f7 100644 --- a/src/internal/git/repository_test.go +++ b/src/internal/git/repository_test.go @@ -56,7 +56,8 @@ func TestRepository(t *testing.T) { require.NoError(t, err) _, err = newFile.Write([]byte("Hello World")) require.NoError(t, err) - newFile.Close() + err = newFile.Close() + require.NoError(t, err) _, err = w.Add(filePath) require.NoError(t, err) _, err = w.Commit("Initial commit", &git.CommitOptions{ diff --git a/src/internal/gitea/gitea.go b/src/internal/gitea/gitea.go index 94244d03f1..48bfdfaae1 100644 --- a/src/internal/gitea/gitea.go +++ b/src/internal/gitea/gitea.go @@ -8,6 +8,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -47,7 +48,7 @@ func NewClient(endpoint, username, password string) (*Client, error) { } // DoRequest performs a request to the Gitea API at the given path. -func (g *Client) DoRequest(ctx context.Context, method string, path string, body []byte) ([]byte, int, error) { +func (g *Client) DoRequest(ctx context.Context, method string, path string, body []byte) (_ []byte, _ int, err error) { u, err := g.endpoint.Parse(path) if err != nil { return nil, 0, err @@ -63,7 +64,11 @@ func (g *Client) DoRequest(ctx context.Context, method string, path string, body if err != nil { return nil, 0, err } - defer resp.Body.Close() + defer func() { + errClose := resp.Body.Close() + err = errors.Join(err, errClose) + }() + b, err := io.ReadAll(resp.Body) if err != nil { return nil, 0, err diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index 2e206742c6..a11b6097d7 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -300,7 +300,11 @@ func CleanupInProgressLayers(ctx context.Context, img v1.Image) error { if err != nil { return err } - cacheDir := filepath.Join(config.GetAbsCachePath(), layout.ImagesDir) + absPath, err := config.GetAbsCachePath() + if err != nil { + return err + } + cacheDir := filepath.Join(absPath, layout.ImagesDir) location := filepath.Join(cacheDir, digest.String()) info, err := os.Stat(location) if errors.Is(err, fs.ErrNotExist) { diff --git a/src/internal/packager/sbom/catalog.go b/src/internal/packager/sbom/catalog.go index 58aa23870a..65c32c039b 100755 --- a/src/internal/packager/sbom/catalog.go +++ b/src/internal/packager/sbom/catalog.go @@ -54,9 +54,13 @@ var componentPrefix = "zarf-component-" func Catalog(ctx context.Context, componentSBOMs map[string]*layout.ComponentSBOM, imageList []transform.Image, paths *layout.PackagePaths) error { imageCount := len(imageList) componentCount := len(componentSBOMs) + cachePath, err := config.GetAbsCachePath() + if err != nil { + return err + } builder := Builder{ spinner: message.NewProgressSpinner("Creating SBOMs for %d images and %d components with files.", imageCount, componentCount), - cachePath: config.GetAbsCachePath(), + cachePath: cachePath, imagesPath: paths.Images.Base, outputDir: paths.SBOMs.Path, } diff --git a/src/pkg/interactive/components.go b/src/pkg/interactive/components.go index b742aeed4c..9b5cee436c 100644 --- a/src/pkg/interactive/components.go +++ b/src/pkg/interactive/components.go @@ -20,7 +20,10 @@ func SelectOptionalComponent(component v1alpha1.ZarfComponent) (bool, error) { displayComponent := component displayComponent.Description = "" - utils.ColorPrintYAML(displayComponent, nil, false) + err := utils.ColorPrintYAML(displayComponent, nil, false) + if err != nil { + return false, err + } if component.Description != "" { message.Question(component.Description) } @@ -31,7 +34,7 @@ func SelectOptionalComponent(component v1alpha1.ZarfComponent) (bool, error) { } var confirm bool - err := survey.AskOne(prompt, &confirm) + err = survey.AskOne(prompt, &confirm) if err != nil { return false, err } diff --git a/src/pkg/layout/component.go b/src/pkg/layout/component.go index c3ce6ae930..9a3735b54b 100644 --- a/src/pkg/layout/component.go +++ b/src/pkg/layout/component.go @@ -50,7 +50,10 @@ func (c *Components) Archive(component v1alpha1.ZarfComponent, cleanupTemp bool) } base := c.Dirs[name].Base if cleanupTemp { - _ = os.RemoveAll(c.Dirs[name].Temp) + err := os.RemoveAll(c.Dirs[name].Temp) + if err != nil { + return err + } } size, err := helpers.GetDirSize(base) if err != nil { diff --git a/src/pkg/layout/package.go b/src/pkg/layout/package.go index 532f46653d..7824d34ba9 100644 --- a/src/pkg/layout/package.go +++ b/src/pkg/layout/package.go @@ -5,6 +5,7 @@ package layout import ( + "errors" "fmt" "os" "path/filepath" @@ -77,11 +78,12 @@ func (pp *PackagePaths) ReadZarfYAML() (v1alpha1.ZarfPackage, []string, error) { } // MigrateLegacy migrates a legacy package layout to the new layout. -func (pp *PackagePaths) MigrateLegacy() error { +func (pp *PackagePaths) MigrateLegacy() (err error) { var pkg v1alpha1.ZarfPackage base := pp.Base // legacy layout does not contain a checksums file, nor a signature + // TODO(mkcp): This can be un-nested as an early return if helpers.InvalidPath(pp.Checksums) && pp.Signature == "" { if err := utils.ReadYaml(pp.ZarfYAML, &pkg); err != nil { return err @@ -113,7 +115,10 @@ func (pp *PackagePaths) MigrateLegacy() error { if !helpers.InvalidPath(legacyImagesTar) { pp = pp.AddImages() message.Debugf("Migrating %q to %q", legacyImagesTar, pp.Images.Base) - defer os.Remove(legacyImagesTar) + defer func(name string) { + err2 := os.Remove(name) + err = errors.Join(err, err2) + }(legacyImagesTar) imgTags := []string{} for _, component := range pkg.Components { imgTags = append(imgTags, component.Images...) @@ -323,7 +328,10 @@ func (pp *PackagePaths) Files() map[string]string { pathMap := make(map[string]string) stripBase := func(path string) string { - rel, _ := filepath.Rel(pp.Base, path) + rel, err := filepath.Rel(pp.Base, path) + if err != nil { + message.Debug("unable to strip base from path", "error", err) + } // Convert from the OS path separator to the standard '/' for Windows support return filepath.ToSlash(rel) } diff --git a/src/pkg/layout/split.go b/src/pkg/layout/split.go index 4668107405..3354a66038 100644 --- a/src/pkg/layout/split.go +++ b/src/pkg/layout/split.go @@ -18,12 +18,20 @@ import ( ) // splitFile will split the file into chunks and remove the original file. -func splitFile(srcPath string, chunkSize int) error { +func splitFile(srcPath string, chunkSize int) (err error) { srcFile, err := os.Open(srcPath) if err != nil { return err } - defer srcFile.Close() + // Ensure we close our sourcefile, even if we error out. + defer func() { + err2 := srcFile.Close() + // Ignore if file is already closed + if !errors.Is(err2, os.ErrClosed) { + err = errors.Join(err, err2) + } + }() + fi, err := srcFile.Stat() if err != nil { return err @@ -31,17 +39,28 @@ func splitFile(srcPath string, chunkSize int) error { title := fmt.Sprintf("[0/%d] MB bytes written", fi.Size()/1000/1000) progressBar := message.NewProgressBar(fi.Size(), title) - defer progressBar.Close() + defer func(progressBar *message.ProgressBar) { + err2 := progressBar.Close() + err = errors.Join(err, err2) + }(progressBar) hash := sha256.New() fileCount := 0 + // TODO(mkcp): The inside of this loop should be wrapped in a closure so we can close the destination file each + // iteration as soon as we're done writing. for { path := fmt.Sprintf("%s.part%03d", srcPath, fileCount+1) dstFile, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, helpers.ReadAllWriteUser) if err != nil { return err } - defer dstFile.Close() + defer func(dstFile *os.File) { + err2 := dstFile.Close() + // Ignore if file is already closed + if !errors.Is(err2, os.ErrClosed) { + err = errors.Join(err, err2) + } + }(dstFile) written, copyErr := io.CopyN(dstFile, srcFile, int64(chunkSize)) if copyErr != nil && !errors.Is(copyErr, io.EOF) { @@ -59,13 +78,14 @@ func splitFile(srcPath string, chunkSize int) error { if err != nil { return err } - err = dstFile.Close() - if err != nil { - return err - } // EOF error could be returned on 0 bytes written. if written == 0 { + // NOTE(mkcp): We have to close the file before removing it or windows will break with a file-in-use err. + err = dstFile.Close() + if err != nil { + return err + } err = os.Remove(path) if err != nil { return err @@ -80,6 +100,7 @@ func splitFile(srcPath string, chunkSize int) error { } // Remove original file + // NOTE(mkcp): We have to close the file before removing or windows can break with a file-in-use err. err = srcFile.Close() if err != nil { return err diff --git a/src/pkg/layout/split_test.go b/src/pkg/layout/split_test.go index 718dc7bfee..51866374ec 100644 --- a/src/pkg/layout/split_test.go +++ b/src/pkg/layout/split_test.go @@ -61,7 +61,8 @@ func TestSplitFile(t *testing.T) { require.NoError(t, err) _, err = f.Write(b) require.NoError(t, err) - f.Close() + err = f.Close() + require.NoError(t, err) err = splitFile(p, tt.chunkSize) require.NoError(t, err) diff --git a/src/pkg/message/progress.go b/src/pkg/message/progress.go index 7c050b0ba2..e756ae0715 100644 --- a/src/pkg/message/progress.go +++ b/src/pkg/message/progress.go @@ -22,10 +22,11 @@ type ProgressBar struct { // NewProgressBar creates a new ProgressBar instance from a total value and a format. func NewProgressBar(total int64, text string) *ProgressBar { var progress *pterm.ProgressbarPrinter + var err error if NoProgress { Info(text) } else { - progress, _ = pterm.DefaultProgressbar. + progress, err = pterm.DefaultProgressbar. WithTotal(int(total)). WithShowCount(false). WithTitle(padding + text). @@ -33,6 +34,9 @@ func NewProgressBar(total int64, text string) *ProgressBar { WithMaxWidth(TermWidth). WithWriter(os.Stderr). Start() + if err != nil { + WarnErr(err, "Unable to create default progressbar") + } } return &ProgressBar{ @@ -53,7 +57,10 @@ func (p *ProgressBar) Updatef(format string, a ...any) { // Failf marks the ProgressBar as failed in the CLI. func (p *ProgressBar) Failf(format string, a ...any) { - p.Close() + err := p.Close() + if err != nil { + Debug("unable to close failed progressbar", "error", err) + } Warnf(format, a...) } @@ -103,7 +110,10 @@ func (p *ProgressBar) Write(data []byte) (int, error) { // Successf marks the ProgressBar as successful in the CLI. func (p *ProgressBar) Successf(format string, a ...any) { - p.Close() + err := p.Close() + if err != nil { + Debug("unable to close successful progressbar", "error", err) + } pterm.Success.Printfln(format, a...) } diff --git a/src/pkg/message/spinner.go b/src/pkg/message/spinner.go index 93511a705c..76d2d81bb5 100644 --- a/src/pkg/message/spinner.go +++ b/src/pkg/message/spinner.go @@ -8,6 +8,7 @@ import ( "bufio" "bytes" "fmt" + "log/slog" "strings" "github.com/pterm/pterm" @@ -34,15 +35,19 @@ func NewProgressSpinner(format string, a ...any) *Spinner { } var spinner *pterm.SpinnerPrinter + var err error text := pterm.Sprintf(format, a...) if NoProgress { Info(text) } else { - spinner, _ = pterm.DefaultSpinner. + spinner, err = pterm.DefaultSpinner. WithRemoveWhenDone(false). // Src: https://github.com/gernest/wow/blob/master/spin/spinners.go#L335 WithSequence(sequence...). Start(text) + if err != nil { + slog.Debug("unable to create default spinner", "error", err) + } } activeSpinner = &Spinner{ @@ -108,7 +113,10 @@ func (p *Spinner) Updatef(format string, a ...any) { // Stop the spinner. func (p *Spinner) Stop() { if p.spinner != nil && p.spinner.IsActive { - _ = p.spinner.Stop() + err := p.spinner.Stop() + if err != nil { + slog.Debug("unable to stop spinner", "error", err) + } } activeSpinner = nil } diff --git a/src/pkg/packager/composer/oci.go b/src/pkg/packager/composer/oci.go index 2541e45f32..4d2589f717 100644 --- a/src/pkg/packager/composer/oci.go +++ b/src/pkg/packager/composer/oci.go @@ -64,7 +64,11 @@ func (ic *ImportChain) fetchOCISkeleton(ctx context.Context) error { componentDesc := manifest.Locate(filepath.Join(layout.ComponentsDir, fmt.Sprintf("%s.tar", name))) - cache := filepath.Join(config.GetAbsCachePath(), "oci") + absCachePath, err := config.GetAbsCachePath() + if err != nil { + return err + } + cache := filepath.Join(absCachePath, "oci") if err := helpers.CreateDirectory(cache, helpers.ReadWriteExecuteUser); err != nil { return err } diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index 9b2c8ab372..099fda5e2b 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -177,12 +177,16 @@ func (pc *PackageCreator) Assemble(ctx context.Context, dst *layout.PackagePaths dst.AddImages() + cachePath, err := config.GetAbsCachePath() + if err != nil { + return err + } pullCfg := images.PullConfig{ DestinationDirectory: dst.Images.Base, ImageList: imageList, Arch: arch, RegistryOverrides: pc.createOpts.RegistryOverrides, - CacheDirectory: filepath.Join(config.GetAbsCachePath(), layout.ImagesDir), + CacheDirectory: filepath.Join(cachePath, layout.ImagesDir), } pulled, err := images.Pull(ctx, pullCfg) diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index a47cf816d3..c32296ba62 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -438,8 +438,11 @@ func (p *Packager) processComponentFiles(component v1alpha1.ZarfComponent, pkgLo } // Replace temp target directory and home directory - file.Target = strings.Replace(file.Target, "###ZARF_TEMP###", p.layout.Base, 1) - file.Target = config.GetAbsHomePath(file.Target) + target, err := config.GetAbsHomePath(strings.Replace(file.Target, "###ZARF_TEMP###", p.layout.Base, 1)) + if err != nil { + return err + } + file.Target = target fileList := []string{} if helpers.IsDir(fileLocation) { @@ -467,7 +470,7 @@ func (p *Packager) processComponentFiles(component v1alpha1.ZarfComponent, pkgLo // Copy the file to the destination spinner.Updatef("Saving %s", file.Target) - err := helpers.CreatePathAndCopy(fileLocation, file.Target) + err = helpers.CreatePathAndCopy(fileLocation, file.Target) if err != nil { return fmt.Errorf("unable to copy file %s to %s: %w", fileLocation, file.Target, err) } diff --git a/src/pkg/packager/inspect.go b/src/pkg/packager/inspect.go index ae850498b8..03dee54883 100644 --- a/src/pkg/packager/inspect.go +++ b/src/pkg/packager/inspect.go @@ -34,7 +34,10 @@ func (p *Packager) Inspect(ctx context.Context) error { fmt.Fprintln(os.Stdout, "-", image) } } else { - utils.ColorPrintYAML(p.cfg.Pkg, nil, false) + err := utils.ColorPrintYAML(p.cfg.Pkg, nil, false) + if err != nil { + return err + } } sbomDir := p.layout.SBOMs.Path diff --git a/src/pkg/packager/interactive.go b/src/pkg/packager/interactive.go index bd44342e7f..bc29c8e0f8 100644 --- a/src/pkg/packager/interactive.go +++ b/src/pkg/packager/interactive.go @@ -21,7 +21,10 @@ import ( func (p *Packager) confirmAction(stage string, warnings []string, sbomViewFiles []string) bool { pterm.Println() message.HeaderInfof("📦 PACKAGE DEFINITION") - utils.ColorPrintYAML(p.cfg.Pkg, p.getPackageYAMLHints(stage), true) + err := utils.ColorPrintYAML(p.cfg.Pkg, p.getPackageYAMLHints(stage), true) + if err != nil { + message.WarnErr(err, "unable to print yaml") + } // Print any potential breaking changes (if this is a Deploy confirm) between this CLI version and the deployed init package if stage == config.ZarfDeployStage { diff --git a/src/pkg/packager/publish.go b/src/pkg/packager/publish.go index 94fdee63c2..6e421efd1f 100644 --- a/src/pkg/packager/publish.go +++ b/src/pkg/packager/publish.go @@ -117,7 +117,10 @@ func (p *Packager) Publish(ctx context.Context) (err error) { }, }) } - utils.ColorPrintYAML(ex, nil, true) + err := utils.ColorPrintYAML(ex, nil, true) + if err != nil { + return err + } } return nil } diff --git a/src/pkg/utils/auth.go b/src/pkg/utils/auth.go index 0bc5cf501a..a6285ec35a 100644 --- a/src/pkg/utils/auth.go +++ b/src/pkg/utils/auth.go @@ -23,7 +23,10 @@ type Credential struct { // FindAuthForHost finds the authentication scheme for a given host using .git-credentials then .netrc. func FindAuthForHost(baseURL string) (*Credential, error) { - homePath, _ := os.UserHomeDir() + homePath, err := os.UserHomeDir() + if err != nil { + return nil, err + } // Read the ~/.git-credentials file credentialsPath := filepath.Join(homePath, ".git-credentials") @@ -52,7 +55,7 @@ func FindAuthForHost(baseURL string) (*Credential, error) { } // credentialParser parses a user's .git-credentials file to find git creds for hosts. -func credentialParser(path string) ([]Credential, error) { +func credentialParser(path string) (_ []Credential, err error) { file, err := os.Open(path) if errors.Is(err, os.ErrNotExist) { return nil, nil @@ -60,7 +63,10 @@ func credentialParser(path string) ([]Credential, error) { if err != nil { return nil, err } - defer file.Close() + defer func() { + err2 := file.Close() + err = errors.Join(err, err2) + }() var credentials []Credential scanner := bufio.NewScanner(file) @@ -83,7 +89,7 @@ func credentialParser(path string) ([]Credential, error) { } // netrcParser parses a user's .netrc file using the method curl did pre 7.84.0: https://daniel.haxx.se/blog/2022/05/31/netrc-pains/. -func netrcParser(path string) ([]Credential, error) { +func netrcParser(path string) (_ []Credential, _ error) { file, err := os.Open(path) if errors.Is(err, os.ErrNotExist) { return nil, nil @@ -91,7 +97,10 @@ func netrcParser(path string) ([]Credential, error) { if err != nil { return nil, err } - defer file.Close() + defer func() { + err2 := file.Close() + err = errors.Join(err, err2) + }() var credentials []Credential scanner := bufio.NewScanner(file) @@ -151,6 +160,7 @@ func netrcParser(path string) ([]Credential, error) { } } } + // Append the last machine (if exists) at the end of the file if activeMachine != nil { credentials = appendNetrcMachine(activeMachine, credentials) diff --git a/src/pkg/utils/cosign.go b/src/pkg/utils/cosign.go index f81183741a..fe6ac9279a 100644 --- a/src/pkg/utils/cosign.go +++ b/src/pkg/utils/cosign.go @@ -228,26 +228,27 @@ func GetCosignArtifacts(image string) ([]string, error) { ref, err := name.ParseReference(image, nameOpts...) if err != nil { - return []string{}, err + return nil, err } + // Return empty if we don't have a signature on the image var remoteOpts []ociremote.Option - simg, _ := ociremote.SignedEntity(ref, remoteOpts...) + simg, _ := ociremote.SignedEntity(ref, remoteOpts...) // TODO(mkcp): //nolint:errcheck if simg == nil { - return []string{}, nil + return nil, 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...) + sigRef, _ := ociremote.SignatureTag(ref, remoteOpts...) // TODO(mkcp): //nolint:errcheck + attRef, _ := ociremote.AttestationTag(ref, remoteOpts...) // TODO(mkcp): //nolint:errcheck ss, err := simg.Signatures() if err != nil { - return []string{}, err + return nil, err } ssLayers, err := ss.Layers() if err != nil { - return []string{}, err + return nil, err } var cosignArtifactList = make([]string, 0) @@ -257,11 +258,11 @@ func GetCosignArtifacts(image string) ([]string, error) { atts, err := simg.Attestations() if err != nil { - return []string{}, err + return nil, err } aLayers, err := atts.Layers() if err != nil { - return []string{}, err + return nil, err } if 0 < len(aLayers) { cosignArtifactList = append(cosignArtifactList, attRef.String()) diff --git a/src/pkg/utils/exec/exec.go b/src/pkg/utils/exec/exec.go index 3f76e3f23c..f9ac5bbed4 100644 --- a/src/pkg/utils/exec/exec.go +++ b/src/pkg/utils/exec/exec.go @@ -65,8 +65,14 @@ func CmdWithContext(ctx context.Context, config Config, command string, args ... cmd.Env = append(os.Environ(), config.Env...) // Capture the command outputs. - cmdStdout, _ := cmd.StdoutPipe() - cmdStderr, _ := cmd.StderrPipe() + cmdStdout, err := cmd.StdoutPipe() + if err != nil { + return "", "", fmt.Errorf("failed to capture stdout, error=%w", err) + } + cmdStderr, err := cmd.StderrPipe() + if err != nil { + return "", "", fmt.Errorf("failed to capture stderr, error=%w", err) + } var ( stdoutBuf, stderrBuf bytes.Buffer diff --git a/src/pkg/utils/network.go b/src/pkg/utils/network.go index ffe5490600..17aa9a6ac3 100644 --- a/src/pkg/utils/network.go +++ b/src/pkg/utils/network.go @@ -6,6 +6,7 @@ package utils import ( "context" + "errors" "fmt" "io" "net/http" @@ -39,7 +40,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, dst, cosignKeyPath string) error { +func DownloadToFile(ctx context.Context, src, dst, cosignKeyPath string) (err 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) @@ -57,7 +58,11 @@ func DownloadToFile(ctx context.Context, src, dst, cosignKeyPath string) error { if err != nil { return fmt.Errorf(lang.ErrWritingFile, dst, err.Error()) } - defer file.Close() + // Ensure our file closes and any error propagate out on error branches + defer func(file *os.File) { + err2 := file.Close() + err = errors.Join(err, err2) + }(file) parsed, err := url.Parse(src) if err != nil { @@ -90,13 +95,16 @@ func DownloadToFile(ctx context.Context, src, dst, cosignKeyPath string) error { return nil } -func httpGetFile(url string, destinationFile *os.File) error { +func httpGetFile(url string, destinationFile *os.File) (err error) { // Get the data resp, err := http.Get(url) if err != nil { return fmt.Errorf("unable to download the file %s", url) } - defer resp.Body.Close() + defer func() { + err2 := resp.Body.Close() + err = errors.Join(err, err2) + }() // Check server response if resp.StatusCode != http.StatusOK { diff --git a/src/pkg/utils/wait.go b/src/pkg/utils/wait.go index dbcbbf0267..8fecaf8e10 100644 --- a/src/pkg/utils/wait.go +++ b/src/pkg/utils/wait.go @@ -197,7 +197,11 @@ func waitForNetworkEndpoint(resource, name, condition string, timeout time.Durat message.Debug(err) continue } - defer conn.Close() + err = conn.Close() + if err != nil { + message.Debug(err) + continue + } } // Yay, we made it! diff --git a/src/pkg/utils/yaml.go b/src/pkg/utils/yaml.go index 641c219977..e0b8a3e953 100644 --- a/src/pkg/utils/yaml.go +++ b/src/pkg/utils/yaml.go @@ -36,8 +36,11 @@ func yamlFormat(attr color.Attribute) string { } // ColorPrintYAML pretty prints a yaml file to the console. -func ColorPrintYAML(data any, hints map[string]string, spaceRootLists bool) { - text, _ := goyaml.Marshal(data) +func ColorPrintYAML(data any, hints map[string]string, spaceRootLists bool) error { + text, err := goyaml.Marshal(data) + if err != nil { + return err + } tokens := lexer.Tokenize(string(text)) if spaceRootLists { @@ -102,6 +105,7 @@ func ColorPrintYAML(data any, hints map[string]string, spaceRootLists bool) { pterm.Println() pterm.Println(outputYAML) + return nil } // AddRootListHint adds a hint string for a given root list key and value. diff --git a/src/pkg/variables/templates.go b/src/pkg/variables/templates.go index f0153d0baa..5f8c9b940c 100644 --- a/src/pkg/variables/templates.go +++ b/src/pkg/variables/templates.go @@ -6,6 +6,7 @@ package variables import ( "bufio" + "errors" "fmt" "os" "regexp" @@ -49,7 +50,7 @@ func (vc *VariableConfig) GetAllTemplates() map[string]*TextTemplate { } // ReplaceTextTemplate loads a file from a given path, replaces text in it and writes it back in place. -func (vc *VariableConfig) ReplaceTextTemplate(path string) error { +func (vc *VariableConfig) ReplaceTextTemplate(path string) (err error) { templateRegex := fmt.Sprintf("###%s_[A-Z0-9_]+###", strings.ToUpper(vc.templatePrefix)) templateMap := vc.GetAllTemplates() @@ -57,7 +58,10 @@ func (vc *VariableConfig) ReplaceTextTemplate(path string) error { if err != nil { return err } - defer textFile.Close() + defer func() { + err2 := textFile.Close() + err = errors.Join(err, err2) + }() // This regex takes a line and parses the text before and after a discovered template: https://regex101.com/r/ilUxAz/1 regexTemplateLine := regexp.MustCompile(fmt.Sprintf("(?P.*?)(?P