Skip to content

Commit

Permalink
refactor: remove use of message.Fatal in tools
Browse files Browse the repository at this point in the history
  • Loading branch information
phillebaba committed Jun 10, 2024
1 parent b2b504b commit 4299205
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 72 deletions.
4 changes: 3 additions & 1 deletion src/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"context"
"fmt"
"os"
"slices"
"strings"

"github.com/pterm/pterm"
Expand Down Expand Up @@ -67,8 +68,9 @@ func Execute(ctx context.Context) {
if err == nil {
return
}
defaultPrintCmds := []string{"helm", "yq", "kubectl"}
comps := strings.Split(cmd.CommandPath(), " ")
if len(comps) > 1 && comps[1] == "tools" {
if len(comps) > 1 && comps[1] == "tools" && slices.Contains(defaultPrintCmds, comps[2]) {
cmd.PrintErrln(cmd.ErrPrefix(), err.Error())
} else {
pterm.Error.Println(err.Error())
Expand Down
60 changes: 29 additions & 31 deletions src/cmd/tools/archiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
},
}

Expand All @@ -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
},
}

Expand All @@ -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")
}
10 changes: 6 additions & 4 deletions src/cmd/tools/crane.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package tools

import (
"errors"
"fmt"
"os"
"strings"
Expand Down Expand Up @@ -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?
Expand All @@ -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
},
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
11 changes: 2 additions & 9 deletions src/cmd/tools/helm/load_plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"strings"
"syscall"

"github.com/defenseunicorns/zarf/src/pkg/message"
"github.com/pkg/errors"
"github.com/spf13/cobra"
"sigs.k8s.io/yaml"
Expand Down Expand Up @@ -217,10 +216,7 @@ func loadCompletionForPlugin(pluginCmd *cobra.Command, plugin *plugin.Plugin) {
if err != nil {
// The file could be missing or invalid. No static completion for this plugin.
if settings.Debug {
err := log.Output(2, fmt.Sprintf("[info] %s\n", err.Error()))
if err != nil {
message.Fatal(err, err.Error())
}
log.Output(2, fmt.Sprintf("[info] %s\n", err.Error()))
}
// Continue to setup dynamic completion.
cmds = &pluginCommand{}
Expand All @@ -242,10 +238,7 @@ func addPluginCommands(plugin *plugin.Plugin, baseCmd *cobra.Command, cmds *plug
if len(cmds.Name) == 0 {
// Missing name for a command
if settings.Debug {
err := log.Output(2, fmt.Sprintf("[info] sub-command name field missing for %s", baseCmd.CommandPath()))
if err != nil {
message.Fatal(err, err.Error())
}
log.Output(2, fmt.Sprintf("[info] sub-command name field missing for %s", baseCmd.CommandPath()))
}
return
}
Expand Down
7 changes: 1 addition & 6 deletions src/cmd/tools/helm/repo_add.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (

"github.com/defenseunicorns/pkg/helpers/v2"
"github.com/defenseunicorns/zarf/src/pkg/cluster"
"github.com/defenseunicorns/zarf/src/pkg/message"
"github.com/gofrs/flock"
"github.com/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -134,11 +133,7 @@ func (o *repoAddOptions) run(ctx context.Context, out io.Writer) error {
defer cancel()
locked, err := fileLock.TryLockContext(lockCtx, time.Second)
if err == nil && locked {
defer func() {
if err := fileLock.Unlock(); err != nil {
message.Fatal(err, err.Error())
}
}()
defer fileLock.Unlock()
}
if err != nil {
return err
Expand Down
3 changes: 1 addition & 2 deletions src/cmd/tools/helm/repo_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"path/filepath"

"github.com/defenseunicorns/pkg/helpers/v2"
"github.com/defenseunicorns/zarf/src/pkg/message"
"github.com/pkg/errors"
"github.com/spf13/cobra"

Expand Down Expand Up @@ -104,7 +103,7 @@ func index(dir, url, mergeTo string) error {
i2 = repo.NewIndexFile()
err := i2.WriteFile(mergeTo, helpers.ReadAllWriteUser)
if err != nil {
message.Fatal(err, err.Error())
return err
}
} else {
i2, err = repo.LoadIndexFile(mergeTo)
Expand Down
8 changes: 5 additions & 3 deletions src/cmd/tools/wait.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package tools

import (
"fmt"
"time"

"github.com/defenseunicorns/zarf/src/config/lang"
Expand All @@ -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]
Expand All @@ -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
},
}

Expand Down
8 changes: 2 additions & 6 deletions src/config/lang/english.go
Original file line number Diff line number Diff line change
Expand Up @@ -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."

Expand Down
22 changes: 12 additions & 10 deletions src/pkg/utils/wait.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package utils

import (
"errors"
"fmt"
"net"
"net/http"
Expand All @@ -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"
)

Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)

Expand All @@ -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 {
Expand All @@ -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.
Expand All @@ -202,7 +204,7 @@ func waitForNetworkEndpoint(resource, name, condition string, timeout time.Durat

// Yay, we made it!
spinner.Success()
return
return nil
}
}
}

0 comments on commit 4299205

Please sign in to comment.