From d84cba8bac7639be47a597792a5805079a3c06be Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Mon, 10 Jun 2024 10:36:45 +0200 Subject: [PATCH] refactor: remove use of message.Fatal in tools --- src/cmd/tools/archiver.go | 60 ++++++++++++++++++-------------------- src/cmd/tools/crane.go | 10 ++++--- src/cmd/tools/wait.go | 8 +++-- src/config/lang/english.go | 8 ++--- src/pkg/utils/wait.go | 22 +++++++------- 5 files changed, 54 insertions(+), 54 deletions(-) diff --git a/src/cmd/tools/archiver.go b/src/cmd/tools/archiver.go index 344cc7398c..d62ce32785 100644 --- a/src/cmd/tools/archiver.go +++ b/src/cmd/tools/archiver.go @@ -12,7 +12,6 @@ import ( "github.com/defenseunicorns/zarf/src/config/lang" "github.com/defenseunicorns/zarf/src/pkg/layout" - "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/mholt/archiver/v3" "github.com/spf13/cobra" ) @@ -32,12 +31,13 @@ var archiverCompressCmd = &cobra.Command{ Aliases: []string{"c"}, Short: lang.CmdToolsArchiverCompressShort, Args: cobra.MinimumNArgs(2), - Run: func(_ *cobra.Command, args []string) { + RunE: func(_ *cobra.Command, args []string) error { sourceFiles, destinationArchive := args[:len(args)-1], args[len(args)-1] err := archiver.Archive(sourceFiles, destinationArchive) if err != nil { - message.Fatalf(err, lang.CmdToolsArchiverCompressErr, err.Error()) + return fmt.Errorf("unable to perform compression: %w", err) } + return err }, } @@ -48,39 +48,40 @@ var archiverDecompressCmd = &cobra.Command{ Aliases: []string{"d"}, Short: lang.CmdToolsArchiverDecompressShort, Args: cobra.ExactArgs(2), - Run: func(_ *cobra.Command, args []string) { + RunE: func(_ *cobra.Command, args []string) error { sourceArchive, destinationPath := args[0], args[1] err := archiver.Unarchive(sourceArchive, destinationPath) if err != nil { - message.Fatalf(err, lang.CmdToolsArchiverDecompressErr, err.Error()) + return fmt.Errorf("unable to perform decompression: %w", err) } - - if unarchiveAll { - err := filepath.Walk(destinationPath, func(path string, info os.FileInfo, err error) error { + if !unarchiveAll { + return nil + } + err = filepath.Walk(destinationPath, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if strings.HasSuffix(path, ".tar") { + dst := filepath.Join(strings.TrimSuffix(path, ".tar"), "..") + // Unpack sboms.tar differently since it has a different folder structure than components + if info.Name() == layout.SBOMTar { + dst = strings.TrimSuffix(path, ".tar") + } + err := archiver.Unarchive(path, dst) if err != nil { - return err + return fmt.Errorf(lang.ErrUnarchive, path, err.Error()) } - if strings.HasSuffix(path, ".tar") { - dst := filepath.Join(strings.TrimSuffix(path, ".tar"), "..") - // Unpack sboms.tar differently since it has a different folder structure than components - if info.Name() == layout.SBOMTar { - dst = strings.TrimSuffix(path, ".tar") - } - err := archiver.Unarchive(path, dst) - if err != nil { - return fmt.Errorf(lang.ErrUnarchive, path, err.Error()) - } - err = os.Remove(path) - if err != nil { - return fmt.Errorf(lang.ErrRemoveFile, path, err.Error()) - } + err = os.Remove(path) + if err != nil { + return fmt.Errorf(lang.ErrRemoveFile, path, err.Error()) } - return nil - }) - if err != nil { - message.Fatalf(err, lang.CmdToolsArchiverUnarchiveAllErr, err.Error()) } + return nil + }) + if err != nil { + return fmt.Errorf("unable to unarchive all nested tarballs: %w", err) } + return nil }, } @@ -93,8 +94,5 @@ func init() { archiverDecompressCmd.Flags().BoolVar(&unarchiveAll, "decompress-all", false, "Decompress all tarballs in the archive") archiverDecompressCmd.Flags().BoolVar(&unarchiveAll, "unarchive-all", false, "Unarchive all tarballs in the archive") archiverDecompressCmd.MarkFlagsMutuallyExclusive("decompress-all", "unarchive-all") - err := archiverDecompressCmd.Flags().MarkHidden("decompress-all") - if err != nil { - message.Fatal(err, err.Error()) - } + archiverDecompressCmd.Flags().MarkHidden("decompress-all") } diff --git a/src/cmd/tools/crane.go b/src/cmd/tools/crane.go index a89041bb26..47b4397cdf 100644 --- a/src/cmd/tools/crane.go +++ b/src/cmd/tools/crane.go @@ -5,6 +5,7 @@ package tools import ( + "errors" "fmt" "os" "strings" @@ -37,7 +38,7 @@ func init() { Use: "registry", Aliases: []string{"r", "crane"}, Short: lang.CmdToolsRegistryShort, - PersistentPreRun: func(cmd *cobra.Command, _ []string) { + PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { // The crane options loading here comes from the rootCmd of crane craneOptions = append(craneOptions, crane.WithContext(cmd.Context())) // TODO(jonjohnsonjr): crane.Verbose option? @@ -56,11 +57,12 @@ func init() { if platform != "all" { v1Platform, err = v1.ParsePlatform(platform) if err != nil { - message.Fatalf(err, lang.CmdToolsRegistryInvalidPlatformErr, platform, err.Error()) + return fmt.Errorf("invalid platform %s: %w", platform, err) } } craneOptions = append(craneOptions, crane.WithPlatform(v1Platform)) + return nil }, } @@ -159,7 +161,7 @@ func zarfCraneInternalWrapper(commandToWrap func(*[]crane.Option) *cobra.Command wrappedCommand.RunE = func(cmd *cobra.Command, args []string) error { if len(args) < imageNameArgumentIndex+1 { - message.Fatal(nil, lang.CmdToolsCraneNotEnoughArgumentsErr) + return errors.New("not have enough arguments specified for this command") } // Try to connect to a Zarf initialized cluster otherwise then pass it down to crane. @@ -333,7 +335,7 @@ func doPruneImagesForPackages(zarfState *types.ZarfState, zarfPackages []types.D Message: lang.CmdConfirmContinue, } if err := survey.AskOne(prompt, &confirm); err != nil { - message.Fatalf(nil, lang.ErrConfirmCancel, err) + return fmt.Errorf("confirm selection canceled: %w", err) } } if confirm { diff --git a/src/cmd/tools/wait.go b/src/cmd/tools/wait.go index 9977c58011..bb767ec972 100644 --- a/src/cmd/tools/wait.go +++ b/src/cmd/tools/wait.go @@ -5,6 +5,7 @@ package tools import ( + "fmt" "time" "github.com/defenseunicorns/zarf/src/config/lang" @@ -28,11 +29,11 @@ var waitForCmd = &cobra.Command{ Long: lang.CmdToolsWaitForLong, Example: lang.CmdToolsWaitForExample, Args: cobra.MinimumNArgs(1), - Run: func(_ *cobra.Command, args []string) { + RunE: func(_ *cobra.Command, args []string) error { // Parse the timeout string timeout, err := time.ParseDuration(waitTimeout) if err != nil { - message.Fatalf(err, lang.CmdToolsWaitForErrTimeoutString, waitTimeout) + return fmt.Errorf("invalid timeout duration %s, use a valid duration string e.g. 1s, 2m, 3h: %w", waitTimeout, err) } kind := args[0] @@ -51,8 +52,9 @@ var waitForCmd = &cobra.Command{ // Execute the wait command. if err := utils.ExecuteWait(waitTimeout, waitNamespace, condition, kind, identifier, timeout); err != nil { - message.Fatal(err, err.Error()) + return err } + return err }, } diff --git a/src/config/lang/english.go b/src/config/lang/english.go index 36112f99a9..35017ac758 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -580,12 +580,8 @@ $ zarf tools wait-for https 1.1.1.1 200 # wait $ zarf tools wait-for http google.com # wait for any 2xx response from http://google.com $ zarf tools wait-for http google.com success # wait for any 2xx response from http://google.com ` - CmdToolsWaitForFlagTimeout = "Specify the timeout duration for the wait command." - CmdToolsWaitForErrTimeoutString = "Invalid timeout duration '%s'. Please use a valid duration string (e.g. 1s, 2m, 3h)." - CmdToolsWaitForErrTimeout = "Wait timed out." - CmdToolsWaitForErrConditionString = "Invalid HTTP status code. Please use a valid HTTP status code (e.g. 200, 404, 500)." - CmdToolsWaitForErrZarfPath = "Could not locate the current Zarf binary path." - CmdToolsWaitForFlagNamespace = "Specify the namespace of the resources to wait for." + CmdToolsWaitForFlagTimeout = "Specify the timeout duration for the wait command." + CmdToolsWaitForFlagNamespace = "Specify the namespace of the resources to wait for." CmdToolsKubectlDocs = "Kubectl command. See https://kubernetes.io/docs/reference/kubectl/overview/ for more information." diff --git a/src/pkg/utils/wait.go b/src/pkg/utils/wait.go index a4815e5ea5..4ccc3d2481 100644 --- a/src/pkg/utils/wait.go +++ b/src/pkg/utils/wait.go @@ -5,6 +5,7 @@ package utils import ( + "errors" "fmt" "net" "net/http" @@ -15,7 +16,6 @@ import ( "github.com/defenseunicorns/zarf/src/pkg/utils/exec" - "github.com/defenseunicorns/zarf/src/config/lang" "github.com/defenseunicorns/zarf/src/pkg/message" ) @@ -33,8 +33,7 @@ func ExecuteWait(waitTimeout, waitNamespace, condition, kind, identifier string, // Handle network endpoints. switch kind { case "http", "https", "tcp": - waitForNetworkEndpoint(kind, identifier, condition, timeout) - return nil + return waitForNetworkEndpoint(kind, identifier, condition, timeout) } // Type of wait, condition or JSONPath @@ -50,7 +49,7 @@ func ExecuteWait(waitTimeout, waitNamespace, condition, kind, identifier string, // Get the Zarf command configuration. zarfCommand, err := GetFinalExecutableCommand() if err != nil { - message.Fatal(err, lang.CmdToolsWaitForErrZarfPath) + return fmt.Errorf("could not locate the current Zarf binary path: %w", err) } identifierMsg := identifier @@ -88,7 +87,7 @@ func ExecuteWait(waitTimeout, waitNamespace, condition, kind, identifier string, select { case <-expired: - message.Fatal(nil, lang.CmdToolsWaitForErrTimeout) + return errors.New("wait timed out") default: spinner.Updatef(existMsg) @@ -132,7 +131,7 @@ func ExecuteWait(waitTimeout, waitNamespace, condition, kind, identifier string, } // waitForNetworkEndpoint waits for a network endpoint to respond. -func waitForNetworkEndpoint(resource, name, condition string, timeout time.Duration) { +func waitForNetworkEndpoint(resource, name, condition string, timeout time.Duration) error { // Set the timeout for the wait-for command. expired := time.After(timeout) @@ -153,7 +152,7 @@ func waitForNetworkEndpoint(resource, name, condition string, timeout time.Durat select { case <-expired: - message.Fatal(nil, lang.CmdToolsWaitForErrTimeout) + return errors.New("wait timed out") default: switch resource { @@ -179,8 +178,11 @@ func waitForNetworkEndpoint(resource, name, condition string, timeout time.Durat // Convert the condition to an int and check if it's a valid HTTP status code. code, err := strconv.Atoi(condition) - if err != nil || http.StatusText(code) == "" { - message.Fatal(err, lang.CmdToolsWaitForErrConditionString) + if err != nil { + return fmt.Errorf("http status code %s is not an integer: %w", condition, err) + } + if http.StatusText(code) == "" { + return errors.New("http status code %s is unknown") } // Try to get the URL and check the status code. @@ -202,7 +204,7 @@ func waitForNetworkEndpoint(resource, name, condition string, timeout time.Durat // Yay, we made it! spinner.Success() - return + return nil } } }