Skip to content

Commit

Permalink
fix: propagate or log unhandled errors in src/cmd
Browse files Browse the repository at this point in the history
Signed-off-by: Kit Patella <[email protected]>
  • Loading branch information
mkcp committed Sep 25, 2024
1 parent a508aa7 commit 6f53a29
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 26 deletions.
15 changes: 10 additions & 5 deletions src/cmd/destroy.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"context"
"errors"
"fmt"
"log/slog"
"os"
"regexp"

Expand Down Expand Up @@ -58,8 +59,10 @@ var destroyCmd = &cobra.Command{

// Run all the scripts!
pattern := regexp.MustCompile(`(?mi)zarf-clean-.+\.sh$`)
// TODO(mkcp): Handle this error
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
Expand All @@ -73,9 +76,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
// TODO(mkcp): Should we be ignoring this error? Setup handling or retry, or lintignore
_ = os.Remove(script)
// Try to remove the script, but ignore any errors and debug log them
err = os.Remove(script)
if err != nil {
slog.Debug("Unable to remove script", "script", script, "error", err)
}
}
} else {
// Perform chart uninstallation
Expand Down
29 changes: 22 additions & 7 deletions src/cmd/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"errors"
"fmt"
"io"
"log/slog"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -148,14 +149,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)
Expand All @@ -182,7 +182,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 != "" {
Expand All @@ -191,7 +194,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)
Expand All @@ -208,7 +214,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 {
Expand Down Expand Up @@ -323,8 +332,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 {
slog.Debug("Unable to mark dev-find-images flag as set", "error", err)
}
err = devFindImagesCmd.Flags().MarkHidden("set")
if err != nil {
slog.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)
Expand Down
10 changes: 7 additions & 3 deletions src/cmd/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"encoding/json"
"errors"
"fmt"
"log/slog"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -348,8 +349,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
},
Expand Down Expand Up @@ -388,6 +389,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 {
slog.Debug("Unable to add hidden dummy flag", "error", err)
}
}
}
46 changes: 38 additions & 8 deletions src/cmd/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"context"
"errors"
"fmt"
"log/slog"
"os"
"path/filepath"
"regexp"
Expand Down Expand Up @@ -381,9 +382,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 {
slog.Debug("Unable to glob", "tarPath", tarPath, "error", err)
}

zstPath := config.ZarfPackagePrefix + toComplete + "*.tar.zst"
zstFiles, err := filepath.Glob(zstPath)
if err != nil {
slog.Debug("Unable to glob", "zstPath", zstPath, "error", err)
}

splitPath := config.ZarfPackagePrefix + toComplete + "*.part000"
splitFiles, err := filepath.Glob(splitPath)
if err != nil {
slog.Debug("Unable to glob", "splitPath", splitPath, "error", err)
}

files = append(files, zstFiles...)
files = append(files, splitFiles...)
Expand All @@ -408,7 +423,10 @@ func getPackageCompletionArgs(cmd *cobra.Command, _ []string, _ string) ([]strin

ctx := cmd.Context()

deployedZarfPackages, _ := c.GetDeployedZarfPackages(ctx)
deployedZarfPackages, err := c.GetDeployedZarfPackages(ctx)
if err != nil {
slog.Error("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)
Expand Down Expand Up @@ -477,9 +495,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 {
slog.Debug("Unable to mark flag output-directory", "error", errOD)
}
errKey := createFlags.MarkHidden("key")
if errKey != nil {
slog.Debug("Unable to mark flag key", "error", errKey)
}
errKP := createFlags.MarkHidden("key-pass")
if errKP != nil {
slog.Debug("Unable to mark flag key-pass", "error", errKP)
}
}

func bindDeployFlags(v *viper.Viper) {
Expand All @@ -500,7 +527,10 @@ func bindDeployFlags(v *viper.Viper) {
deployFlags.StringVar(&pkgConfig.PkgOpts.SGetKeyPath, "sget", v.GetString(common.VPkgDeploySget), lang.CmdPackageDeployFlagSget)
deployFlags.BoolVar(&pkgConfig.PkgOpts.SkipSignatureValidation, "skip-signature-validation", false, lang.CmdPackageFlagSkipSignatureValidation)

deployFlags.MarkHidden("sget")
err := deployFlags.MarkHidden("sget")
if err != nil {
slog.Debug("Unable to mark flag sget", "error", err)
}
}

func bindMirrorFlags(v *viper.Viper) {
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ var rootCmd = &cobra.Command{
SilenceErrors: true,
Run: func(cmd *cobra.Command, args []string) {
zarfLogo := message.GetLogo()
_, _ = fmt.Fprintln(os.Stderr, zarfLogo) //nolint:errcheck
_, _ = fmt.Fprintln(os.Stderr, zarfLogo)
err := cmd.Help()
if err != nil {
_, _ = fmt.Fprintln(os.Stderr, err) //nolint:errcheck
_, _ = fmt.Fprintln(os.Stderr, err)
}

if len(args) > 0 {
Expand Down
6 changes: 5 additions & 1 deletion src/cmd/tools/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package tools

import (
"log/slog"
"os"

"github.com/zarf-dev/zarf/src/cmd/tools/helm"
Expand All @@ -24,7 +25,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 {
slog.Error("Failed to initialize helm command", "error", err)
}
helmCmd.Short = lang.CmdToolsHelmShort
helmCmd.Long = lang.CmdToolsHelmLong
helmCmd.AddCommand(newVersionCmd("helm", helmVersion))
Expand Down

0 comments on commit 6f53a29

Please sign in to comment.