From ef55f5232ff33d6731e7ead8d64b08dcd1d8b408 Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Tue, 23 Jul 2024 18:15:09 +0200 Subject: [PATCH] refactor: move setup CLI to only run once in root command (#2722) Signed-off-by: Philip Laine Signed-off-by: Austin Abro --- src/cmd/common/setup.go | 50 ++++++++++++++++---------------------- src/cmd/root.go | 41 ++++++++++++++++++++++++------- src/cmd/tools/common.go | 15 ++---------- src/cmd/version.go | 7 ++---- src/config/config.go | 6 ----- src/config/lang/english.go | 2 -- src/pkg/message/message.go | 8 ++++-- src/pkg/utils/yaml.go | 6 ++--- 8 files changed, 66 insertions(+), 69 deletions(-) diff --git a/src/cmd/common/setup.go b/src/cmd/common/setup.go index 703d9355bf..ac47d0c95d 100644 --- a/src/cmd/common/setup.go +++ b/src/cmd/common/setup.go @@ -5,43 +5,38 @@ package common import ( + "errors" "fmt" "io" "os" "time" "github.com/pterm/pterm" - "github.com/zarf-dev/zarf/src/config" - "github.com/zarf-dev/zarf/src/config/lang" + "github.com/zarf-dev/zarf/src/pkg/message" ) -// LogLevelCLI holds the log level as input from a command -var LogLevelCLI string - -// SetupCLI sets up the CLI logging, interrupt functions, and more -func SetupCLI() { - match := map[string]message.LogLevel{ - "warn": message.WarnLevel, - "info": message.InfoLevel, - "debug": message.DebugLevel, - "trace": message.TraceLevel, - } - - if config.NoColor { +// SetupCLI sets up the CLI logging +func SetupCLI(logLevel string, skipLogFile, noColor bool) error { + if noColor { message.DisableColor() } printViperConfigUsed() - // No log level set, so use the default - if LogLevelCLI != "" { - if lvl, ok := match[LogLevelCLI]; ok { - message.SetLogLevel(lvl) - message.Debug("Log level set to " + LogLevelCLI) - } else { - message.Warn(lang.RootCmdErrInvalidLogLevel) + if logLevel != "" { + match := map[string]message.LogLevel{ + "warn": message.WarnLevel, + "info": message.InfoLevel, + "debug": message.DebugLevel, + "trace": message.TraceLevel, } + lvl, ok := match[logLevel] + if !ok { + return errors.New("invalid log level, valid options are warn, info, debug, and trace") + } + message.SetLogLevel(lvl) + message.Debug("Log level set to " + logLevel) } // Disable progress bars for CI envs @@ -50,21 +45,18 @@ func SetupCLI() { message.NoProgress = true } - if !config.SkipLogFile { + if !skipLogFile { ts := time.Now().Format("2006-01-02-15-04-05") - f, err := os.CreateTemp("", fmt.Sprintf("zarf-%s-*.log", ts)) if err != nil { - message.WarnErr(err, "Error creating a log file in a temporary directory") - return + return fmt.Errorf("could not create a log file in a the temporary directory: %w", err) } logFile, err := message.UseLogFile(f) if err != nil { - message.WarnErr(err, "Error saving a log file to a temporary directory") - return + return fmt.Errorf("could not save a log file to the temporary directory: %w", err) } - pterm.SetDefaultOutput(io.MultiWriter(os.Stderr, logFile)) message.Notef("Saving log file to %s", f.Name()) } + return nil } diff --git a/src/cmd/root.go b/src/cmd/root.go index 086a1cab6c..876efdf20e 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -26,20 +26,43 @@ import ( var ( // Default global config for the packager pkgConfig = types.PackagerConfig{} + // LogLevelCLI holds the log level as input from a command + LogLevelCLI string + // SkipLogFile is a flag to skip logging to a file + SkipLogFile bool + // NoColor is a flag to disable colors in output + NoColor bool ) var rootCmd = &cobra.Command{ Use: "zarf COMMAND", - PersistentPreRun: func(cmd *cobra.Command, _ []string) { - // Skip for vendor-only commands + PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { + // Skip for vendor only commands if common.CheckVendorOnlyFromPath(cmd) { - return + return nil } - // Don't log the help command + + skipLogFile := SkipLogFile + + // Dont write tool commands to file. + comps := strings.Split(cmd.CommandPath(), " ") + if len(comps) > 1 && comps[1] == "tools" { + skipLogFile = true + } + if len(comps) > 1 && comps[1] == "version" { + skipLogFile = true + } + + // Dont write help command to file. if cmd.Parent() == nil { - config.SkipLogFile = true + skipLogFile = true + } + + err := common.SetupCLI(LogLevelCLI, skipLogFile, NoColor) + if err != nil { + return err } - common.SetupCLI() + return nil }, Short: lang.RootCmdShort, Long: lang.RootCmdLong, @@ -89,11 +112,11 @@ func init() { v := common.InitViper() - rootCmd.PersistentFlags().StringVarP(&common.LogLevelCLI, "log-level", "l", v.GetString(common.VLogLevel), lang.RootCmdFlagLogLevel) + rootCmd.PersistentFlags().StringVarP(&LogLevelCLI, "log-level", "l", v.GetString(common.VLogLevel), lang.RootCmdFlagLogLevel) rootCmd.PersistentFlags().StringVarP(&config.CLIArch, "architecture", "a", v.GetString(common.VArchitecture), lang.RootCmdFlagArch) - rootCmd.PersistentFlags().BoolVar(&config.SkipLogFile, "no-log-file", v.GetBool(common.VNoLogFile), lang.RootCmdFlagSkipLogFile) + rootCmd.PersistentFlags().BoolVar(&SkipLogFile, "no-log-file", v.GetBool(common.VNoLogFile), lang.RootCmdFlagSkipLogFile) rootCmd.PersistentFlags().BoolVar(&message.NoProgress, "no-progress", v.GetBool(common.VNoProgress), lang.RootCmdFlagNoProgress) - rootCmd.PersistentFlags().BoolVar(&config.NoColor, "no-color", v.GetBool(common.VNoColor), lang.RootCmdFlagNoColor) + rootCmd.PersistentFlags().BoolVar(&NoColor, "no-color", v.GetBool(common.VNoColor), lang.RootCmdFlagNoColor) rootCmd.PersistentFlags().StringVar(&config.CommonOptions.CachePath, "zarf-cache", v.GetString(common.VZarfCache), lang.RootCmdFlagCachePath) rootCmd.PersistentFlags().StringVar(&config.CommonOptions.TempDirectory, "tmpdir", v.GetString(common.VTmpDir), lang.RootCmdFlagTempDir) rootCmd.PersistentFlags().BoolVar(&config.CommonOptions.Insecure, "insecure", v.GetBool(common.VInsecure), lang.RootCmdFlagInsecure) diff --git a/src/cmd/tools/common.go b/src/cmd/tools/common.go index 7f8f375be0..0aef6e95c5 100644 --- a/src/cmd/tools/common.go +++ b/src/cmd/tools/common.go @@ -8,25 +8,14 @@ import ( "fmt" "github.com/spf13/cobra" - "github.com/zarf-dev/zarf/src/cmd/common" - "github.com/zarf-dev/zarf/src/config" + "github.com/zarf-dev/zarf/src/config/lang" ) var toolsCmd = &cobra.Command{ Use: "tools", Aliases: []string{"t"}, - PersistentPreRun: func(cmd *cobra.Command, _ []string) { - config.SkipLogFile = true - - // Skip for vendor-only commands - if common.CheckVendorOnlyFromPath(cmd) { - return - } - - common.SetupCLI() - }, - Short: lang.CmdToolsShort, + Short: lang.CmdToolsShort, } // Include adds the tools command to the root command. diff --git a/src/cmd/version.go b/src/cmd/version.go index 26299c7db5..a08af2aaab 100644 --- a/src/cmd/version.go +++ b/src/cmd/version.go @@ -24,11 +24,8 @@ var outputFormat string var versionCmd = &cobra.Command{ Use: "version", Aliases: []string{"v"}, - PersistentPreRun: func(_ *cobra.Command, _ []string) { - config.SkipLogFile = true - }, - Short: lang.CmdVersionShort, - Long: lang.CmdVersionLong, + Short: lang.CmdVersionShort, + Long: lang.CmdVersionLong, RunE: func(_ *cobra.Command, _ []string) error { if outputFormat == "" { fmt.Println(config.CLIVersion) diff --git a/src/config/config.go b/src/config/config.go index 4f55f48e61..dc60ea2fd8 100644 --- a/src/config/config.go +++ b/src/config/config.go @@ -60,12 +60,6 @@ var ( // ZarfSeedPort is the NodePort Zarf uses for the 'seed registry' ZarfSeedPort string - // SkipLogFile is a flag to skip logging to a file - SkipLogFile bool - - // NoColor is a flag to disable colors in output - NoColor bool - CosignPublicKey string // Timestamp of when the CLI was started diff --git a/src/config/lang/english.go b/src/config/lang/english.go index df39428f48..9b90c23b22 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -58,8 +58,6 @@ const ( RootCmdDeprecatedDeploy = "Deprecated: Please use \"zarf package deploy %s\" to deploy this package. This warning will be removed in Zarf v1.0.0." RootCmdDeprecatedCreate = "Deprecated: Please use \"zarf package create\" to create this package. This warning will be removed in Zarf v1.0.0." - RootCmdErrInvalidLogLevel = "Invalid log level. Valid options are: warn, info, debug, trace." - // zarf connect CmdConnectShort = "Accesses services or pods deployed in the cluster" CmdConnectLong = "Uses a k8s port-forward to connect to resources within the cluster referenced by your kube-context.\n" + diff --git a/src/pkg/message/message.go b/src/pkg/message/message.go index f9d7d48c3c..e271cbf9d4 100644 --- a/src/pkg/message/message.go +++ b/src/pkg/message/message.go @@ -14,7 +14,6 @@ import ( "github.com/defenseunicorns/pkg/helpers/v2" "github.com/fatih/color" "github.com/pterm/pterm" - "github.com/zarf-dev/zarf/src/config" ) // LogLevel is the level of logging to display. @@ -99,6 +98,11 @@ func DisableColor() { pterm.DisableColor() } +// ColorEnabled returns true if color printing is enabled. +func ColorEnabled() bool { + return pterm.PrintColor +} + // ZarfCommand prints a zarf terminal command. func ZarfCommand(format string, a ...any) { Command("zarf "+format, a...) @@ -276,7 +280,7 @@ func Table(header []string, data [][]string) { // preventing future characters from taking on the given color // returns string as normal if color is disabled func ColorWrap(str string, attr color.Attribute) string { - if config.NoColor || str == "" { + if !ColorEnabled() || str == "" { return str } return fmt.Sprintf("\x1b[%dm%s\x1b[0m", attr, str) diff --git a/src/pkg/utils/yaml.go b/src/pkg/utils/yaml.go index e61083bc51..5f86202970 100644 --- a/src/pkg/utils/yaml.go +++ b/src/pkg/utils/yaml.go @@ -20,12 +20,12 @@ import ( "github.com/goccy/go-yaml/lexer" "github.com/goccy/go-yaml/printer" "github.com/pterm/pterm" - "github.com/zarf-dev/zarf/src/config" - "github.com/zarf-dev/zarf/src/pkg/message" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" kubeyaml "k8s.io/apimachinery/pkg/util/yaml" k8syaml "sigs.k8s.io/yaml" + + "github.com/zarf-dev/zarf/src/pkg/message" ) const yamlEscape = "\x1b" @@ -93,7 +93,7 @@ func ColorPrintYAML(data any, hints map[string]string, spaceRootLists bool) { outputYAML = strings.Replace(outputYAML, key, value, 1) } - if config.NoColor { + if !message.ColorEnabled() { // If no color is specified strip any color codes from the output - https://regex101.com/r/YFyIwC/2 ansiRegex := regexp.MustCompile(`\x1b\[(.*?)m`) outputYAML = ansiRegex.ReplaceAllString(outputYAML, "")