From bc8cc35d81a4e5f3557dc041579624e88018e4e5 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Wed, 16 Oct 2024 14:28:00 -0700 Subject: [PATCH 01/10] refactor cmd, setup and add logger to root cmd context, add format flag, and adjust log levels Signed-off-by: Kit Patella --- src/cmd/cmd.go | 13 +++ src/cmd/common/setup.go | 62 ------------ src/cmd/common/utils.go | 13 --- src/cmd/common/viper.go | 14 ++- src/cmd/dev.go | 6 +- src/cmd/package.go | 2 +- src/cmd/root.go | 196 +++++++++++++++++++++++++++---------- src/config/lang/english.go | 3 +- 8 files changed, 173 insertions(+), 136 deletions(-) create mode 100644 src/cmd/cmd.go delete mode 100644 src/cmd/common/setup.go delete mode 100644 src/cmd/common/utils.go diff --git a/src/cmd/cmd.go b/src/cmd/cmd.go new file mode 100644 index 0000000000..169c0747dc --- /dev/null +++ b/src/cmd/cmd.go @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package cmd contains the CLI commands for Zarf. +package cmd + +// setBaseDirectory sets the base directory. This is a directory with a zarf.yaml. +func setBaseDirectory(args []string) string { + if len(args) > 0 { + return args[0] + } + return "." +} diff --git a/src/cmd/common/setup.go b/src/cmd/common/setup.go deleted file mode 100644 index ac47d0c95d..0000000000 --- a/src/cmd/common/setup.go +++ /dev/null @@ -1,62 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: 2021-Present The Zarf Authors - -// Package common handles command configuration across all commands -package common - -import ( - "errors" - "fmt" - "io" - "os" - "time" - - "github.com/pterm/pterm" - - "github.com/zarf-dev/zarf/src/pkg/message" -) - -// SetupCLI sets up the CLI logging -func SetupCLI(logLevel string, skipLogFile, noColor bool) error { - if noColor { - message.DisableColor() - } - - printViperConfigUsed() - - 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 - if os.Getenv("CI") == "true" { - message.Debug("CI environment detected, disabling progress bars") - message.NoProgress = true - } - - 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 { - return fmt.Errorf("could not create a log file in a the temporary directory: %w", err) - } - logFile, err := message.UseLogFile(f) - if err != nil { - 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/common/utils.go b/src/cmd/common/utils.go deleted file mode 100644 index 1da7e456ee..0000000000 --- a/src/cmd/common/utils.go +++ /dev/null @@ -1,13 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: 2021-Present The Zarf Authors - -// Package common handles command configuration across all commands -package common - -// SetBaseDirectory sets the base directory. This is a directory with a zarf.yaml. -func SetBaseDirectory(args []string) string { - if len(args) > 0 { - return args[0] - } - return "." -} diff --git a/src/cmd/common/viper.go b/src/cmd/common/viper.go index fa4f548582..36b037efef 100644 --- a/src/cmd/common/viper.go +++ b/src/cmd/common/viper.go @@ -20,17 +20,21 @@ const ( // Root config keys - VLogLevel = "log_level" VArchitecture = "architecture" - VNoLogFile = "no_log_file" - VNoProgress = "no_progress" - VNoColor = "no_color" VZarfCache = "zarf_cache" VTmpDir = "tmp_dir" VInsecure = "insecure" VPlainHTTP = "plain_http" VInsecureSkipTLSVerify = "insecure_skip_tls_verify" + // Root config, Logging + + VLogLevel = "log_level" + VLogFormat = "log_format" + VNoLogFile = "no_log_file" + VNoProgress = "no_progress" + VNoColor = "no_color" + // Init config keys VInitComponents = "init.components" @@ -162,7 +166,7 @@ func isVersionCmd() bool { return len(args) > 1 && (args[1] == "version" || args[1] == "v") } -func printViperConfigUsed() { +func PrintViperConfigUsed() { // Only print config info if viper is initialized. vInitialized := v != nil if !vInitialized { diff --git a/src/cmd/dev.go b/src/cmd/dev.go index 34e0f76776..09cb67e53e 100644 --- a/src/cmd/dev.go +++ b/src/cmd/dev.go @@ -44,7 +44,7 @@ var devDeployCmd = &cobra.Command{ Short: lang.CmdDevDeployShort, Long: lang.CmdDevDeployLong, RunE: func(cmd *cobra.Command, args []string) error { - pkgConfig.CreateOpts.BaseDir = common.SetBaseDirectory(args) + pkgConfig.CreateOpts.BaseDir = setBaseDirectory(args) v := common.GetViper() pkgConfig.CreateOpts.SetVariables = helpers.TransformAndMergeMap( @@ -234,7 +234,7 @@ var devFindImagesCmd = &cobra.Command{ Short: lang.CmdDevFindImagesShort, Long: lang.CmdDevFindImagesLong, RunE: func(cmd *cobra.Command, args []string) error { - pkgConfig.CreateOpts.BaseDir = common.SetBaseDirectory(args) + pkgConfig.CreateOpts.BaseDir = setBaseDirectory(args) v := common.GetViper() @@ -289,7 +289,7 @@ var devLintCmd = &cobra.Command{ Long: lang.CmdDevLintLong, RunE: func(cmd *cobra.Command, args []string) error { config.CommonOptions.Confirm = true - pkgConfig.CreateOpts.BaseDir = common.SetBaseDirectory(args) + pkgConfig.CreateOpts.BaseDir = setBaseDirectory(args) v := common.GetViper() pkgConfig.CreateOpts.SetVariables = helpers.TransformAndMergeMap( v.GetStringMapString(common.VPkgCreateSet), pkgConfig.CreateOpts.SetVariables, strings.ToUpper) diff --git a/src/cmd/package.go b/src/cmd/package.go index 30b80c612a..58e714a543 100644 --- a/src/cmd/package.go +++ b/src/cmd/package.go @@ -47,7 +47,7 @@ var packageCreateCmd = &cobra.Command{ Short: lang.CmdPackageCreateShort, Long: lang.CmdPackageCreateLong, RunE: func(cmd *cobra.Command, args []string) error { - pkgConfig.CreateOpts.BaseDir = common.SetBaseDirectory(args) + pkgConfig.CreateOpts.BaseDir = setBaseDirectory(args) var isCleanPathRegex = regexp.MustCompile(`^[a-zA-Z0-9\_\-\/\.\~\\:]+$`) if !isCleanPathRegex.MatchString(config.CommonOptions.CachePath) { diff --git a/src/cmd/root.go b/src/cmd/root.go index bf695e3760..ac773607a9 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -6,10 +6,15 @@ package cmd import ( "context" + "errors" "fmt" + "github.com/zarf-dev/zarf/src/pkg/logger" + "io" + "log/slog" "os" "slices" "strings" + "time" "github.com/pterm/pterm" "github.com/spf13/cobra" @@ -28,6 +33,8 @@ var ( pkgConfig = types.PackagerConfig{} // LogLevelCLI holds the log level as input from a command LogLevelCLI string + // LogFormat holds the log format as input from a command + LogFormat string // SkipLogFile is a flag to skip logging to a file SkipLogFile bool // NoColor is a flag to disable colors in output @@ -35,63 +42,80 @@ var ( ) var rootCmd = &cobra.Command{ - Use: "zarf COMMAND", - PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { - // If --insecure was provided, set --insecure-skip-tls-verify and --plain-http to match - if config.CommonOptions.Insecure { - config.CommonOptions.InsecureSkipTLSVerify = true - config.CommonOptions.PlainHTTP = true - } + Use: "zarf COMMAND", + Short: lang.RootCmdShort, + Long: lang.RootCmdLong, + Args: cobra.MaximumNArgs(1), + SilenceUsage: true, + // TODO(mkcp): Do we actually want to silence errors here? + SilenceErrors: true, + PersistentPreRunE: preRun, + Run: run, +} - // Skip for vendor only commands - if common.CheckVendorOnlyFromPath(cmd) { - return nil - } +func preRun(cmd *cobra.Command, args []string) error { + // If --insecure was provided, set --insecure-skip-tls-verify and --plain-http to match + if config.CommonOptions.Insecure { + config.CommonOptions.InsecureSkipTLSVerify = true + config.CommonOptions.PlainHTTP = true + } - skipLogFile := SkipLogFile + // Skip for vendor only commands + if common.CheckVendorOnlyFromPath(cmd) { + return nil + } - // 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 - } + // TODO(mkcp): Skipping log writes here. We'll want to remove these when we remove the flag. + skipLogFile := SkipLogFile - // Dont write help command to file. - if cmd.Parent() == nil { - skipLogFile = true - } + // Don't 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 + } - err := common.SetupCLI(LogLevelCLI, skipLogFile, NoColor) - if err != nil { - return err - } - return nil - }, - Short: lang.RootCmdShort, - Long: lang.RootCmdLong, - Args: cobra.MaximumNArgs(1), - SilenceUsage: true, - SilenceErrors: true, - Run: func(cmd *cobra.Command, args []string) { - zarfLogo := message.GetLogo() - _, _ = fmt.Fprintln(os.Stderr, zarfLogo) - err := cmd.Help() - if err != nil { - _, _ = fmt.Fprintln(os.Stderr, err) - } + // Dont write help command to file. + if cmd.Parent() == nil { + skipLogFile = true + } + // TODO End skip log writes. + + // Configure logger and add it to cmd context. + // FIXME(mkcp): Where is the best place to read these flags? + l, err := setupLogger(LogLevelCLI, LogFormat) + if err != nil { + return err + } + ctx := context.WithValue(cmd.Context(), "logger", l) + cmd.SetContext(ctx) - if len(args) > 0 { - if strings.Contains(args[0], config.ZarfPackagePrefix) || strings.Contains(args[0], "zarf-init") { - message.Warnf(lang.RootCmdDeprecatedDeploy, args[0]) - } - if args[0] == layout.ZarfYAML { - message.Warn(lang.RootCmdDeprecatedCreate) - } + err = setupMessage(LogLevelCLI, skipLogFile, NoColor) + if err != nil { + return err + } + return nil +} + +func run(cmd *cobra.Command, args []string) { + zarfLogo := message.GetLogo() + _, _ = fmt.Fprintln(os.Stderr, zarfLogo) + err := cmd.Help() + if err != nil { + _, _ = fmt.Fprintln(os.Stderr, err) //nolint:errcheck + } + + // FIXME(mkcp): These we can probably complete deprecation on? + if len(args) > 0 { + if strings.Contains(args[0], config.ZarfPackagePrefix) || strings.Contains(args[0], "zarf-init") { + message.Warnf(lang.RootCmdDeprecatedDeploy, args[0]) + } + if args[0] == layout.ZarfYAML { + message.Warn(lang.RootCmdDeprecatedCreate) } - }, + } } // Execute is the entrypoint for the CLI. @@ -122,15 +146,85 @@ func init() { v := common.InitViper() + // Logs 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().StringVarP(&LogFormat, "log-format", "f", v.GetString(common.VLogFormat), lang.RootCmdFlagLogFormat) 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(&NoColor, "no-color", v.GetBool(common.VNoColor), lang.RootCmdFlagNoColor) + + rootCmd.PersistentFlags().StringVarP(&config.CLIArch, "architecture", "a", v.GetString(common.VArchitecture), lang.RootCmdFlagArch) 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) + + // Security rootCmd.PersistentFlags().BoolVar(&config.CommonOptions.Insecure, "insecure", v.GetBool(common.VInsecure), lang.RootCmdFlagInsecure) rootCmd.PersistentFlags().MarkDeprecated("insecure", "please use --plain-http, --insecure-skip-tls-verify, or --skip-signature-validation instead.") rootCmd.PersistentFlags().BoolVar(&config.CommonOptions.PlainHTTP, "plain-http", v.GetBool(common.VPlainHTTP), lang.RootCmdFlagPlainHTTP) rootCmd.PersistentFlags().BoolVar(&config.CommonOptions.InsecureSkipTLSVerify, "insecure-skip-tls-verify", v.GetBool(common.VInsecureSkipTLSVerify), lang.RootCmdFlagInsecureSkipTLSVerify) } + +// setup Logger handles creating a logger and setting it as the global default. +func setupLogger(level, format string) (*slog.Logger, error) { + sLevel, err := logger.ParseLevel(level) + if err != nil { + return nil, err + } + l, err := logger.New(logger.Config{ + Level: sLevel, + Format: logger.Format(format), + Destination: logger.DestinationDefault, + }) + if err != nil { + return nil, err + } + logger.SetDefault(l) + l.Debug("Logger successfully initialized", "logger", l) + return l, nil +} + +// setupMessage configures message while we migrate over to logger. +func setupMessage(logLevel string, skipLogFile, noColor bool) error { + // TODO(mkcp): Delete no-color + if noColor { + message.DisableColor() + } + + common.PrintViperConfigUsed() + + 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 + if os.Getenv("CI") == "true" { + message.Debug("CI environment detected, disabling progress bars") + message.NoProgress = true + } + + 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 { + return fmt.Errorf("could not create a log file in a the temporary directory: %w", err) + } + logFile, err := message.UseLogFile(f) + if err != nil { + 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/config/lang/english.go b/src/config/lang/english.go index c78e57a443..7e520f24cf 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -45,7 +45,8 @@ const ( RootCmdLong = "Zarf eliminates the complexity of air gap software delivery for Kubernetes clusters and cloud native workloads\n" + "using a declarative packaging strategy to support DevSecOps in offline and semi-connected environments." - RootCmdFlagLogLevel = "Log level when running Zarf. Valid options are: warn, info, debug, trace" + RootCmdFlagLogLevel = "Log level when running Zarf. Defaults to info. Valid options are: debug, info, warn, error" + RootCmdFlagLogFormat = "Select a logging format. Defaults to 'text'. Valid options are: 'text', 'json'" RootCmdFlagArch = "Architecture for OCI images and Zarf packages" RootCmdFlagSkipLogFile = "Disable log file creation" RootCmdFlagNoProgress = "Disable fancy UI progress bars, spinners, logos, etc" From 949cc5a0d31801569cf43165d722b29bf714b5fe Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Wed, 16 Oct 2024 15:12:06 -0700 Subject: [PATCH 02/10] -f shorthand flag is used elsewhere, remove from root Signed-off-by: Kit Patella --- src/cmd/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/root.go b/src/cmd/root.go index ac773607a9..2927f8344e 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -148,7 +148,7 @@ func init() { // Logs rootCmd.PersistentFlags().StringVarP(&LogLevelCLI, "log-level", "l", v.GetString(common.VLogLevel), lang.RootCmdFlagLogLevel) - rootCmd.PersistentFlags().StringVarP(&LogFormat, "log-format", "f", v.GetString(common.VLogFormat), lang.RootCmdFlagLogFormat) + rootCmd.PersistentFlags().StringVar(&LogFormat, "log-format", v.GetString(common.VLogFormat), lang.RootCmdFlagLogFormat) 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(&NoColor, "no-color", v.GetBool(common.VNoColor), lang.RootCmdFlagNoColor) From 5f1213105b2c425eb09a6d33be536739fc23b500 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Wed, 16 Oct 2024 15:37:59 -0700 Subject: [PATCH 03/10] migrate logo to 'zarf say' and migrate common.viper to logger Signed-off-by: Kit Patella --- src/cmd/common/viper.go | 12 ++++--- src/cmd/initialize.go | 4 --- src/cmd/root.go | 37 +++++++------------- src/cmd/say/say.go | 71 ++++++++++++++++++++++++++++++++++++++ src/config/lang/english.go | 4 --- 5 files changed, 91 insertions(+), 37 deletions(-) create mode 100644 src/cmd/say/say.go diff --git a/src/cmd/common/viper.go b/src/cmd/common/viper.go index 36b037efef..697bbb57d0 100644 --- a/src/cmd/common/viper.go +++ b/src/cmd/common/viper.go @@ -5,14 +5,14 @@ package common import ( + "context" "errors" + "log/slog" "os" "strings" "github.com/spf13/viper" "github.com/zarf-dev/zarf/src/config" - "github.com/zarf-dev/zarf/src/config/lang" - "github.com/zarf-dev/zarf/src/pkg/message" ) // Constants for use when loading configurations from viper config files @@ -166,7 +166,9 @@ func isVersionCmd() bool { return len(args) > 1 && (args[1] == "version" || args[1] == "v") } -func PrintViperConfigUsed() { +func PrintViperConfigUsed(ctx context.Context) { + log := ctx.Value("logger").(*slog.Logger) + // Only print config info if viper is initialized. vInitialized := v != nil if !vInitialized { @@ -177,11 +179,11 @@ func PrintViperConfigUsed() { return } if vConfigError != nil { - message.WarnErrf(vConfigError, lang.CmdViperErrLoadingConfigFile, vConfigError.Error()) + log.Error("unable to load config file", "error", vConfigError) return } if cfgFile := v.ConfigFileUsed(); cfgFile != "" { - message.Notef(lang.CmdViperInfoUsingConfigFile, cfgFile) + log.Info("Using config file", "location", cfgFile) } } diff --git a/src/cmd/initialize.go b/src/cmd/initialize.go index 01470e817d..70e73c1223 100644 --- a/src/cmd/initialize.go +++ b/src/cmd/initialize.go @@ -8,7 +8,6 @@ import ( "context" "errors" "fmt" - "os" "path" "path/filepath" "strings" @@ -37,9 +36,6 @@ var initCmd = &cobra.Command{ Long: lang.CmdInitLong, Example: lang.CmdInitExample, RunE: func(cmd *cobra.Command, _ []string) error { - zarfLogo := message.GetLogo() - _, _ = fmt.Fprintln(os.Stderr, zarfLogo) - if err := validateInitFlags(); err != nil { return fmt.Errorf("invalid command flags were provided: %w", err) } diff --git a/src/cmd/root.go b/src/cmd/root.go index 2927f8344e..e7fbe2b919 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -8,6 +8,7 @@ import ( "context" "errors" "fmt" + "github.com/zarf-dev/zarf/src/cmd/say" "github.com/zarf-dev/zarf/src/pkg/logger" "io" "log/slog" @@ -23,7 +24,6 @@ import ( "github.com/zarf-dev/zarf/src/cmd/tools" "github.com/zarf-dev/zarf/src/config" "github.com/zarf-dev/zarf/src/config/lang" - "github.com/zarf-dev/zarf/src/pkg/layout" "github.com/zarf-dev/zarf/src/pkg/message" "github.com/zarf-dev/zarf/src/types" ) @@ -53,7 +53,7 @@ var rootCmd = &cobra.Command{ Run: run, } -func preRun(cmd *cobra.Command, args []string) error { +func preRun(cmd *cobra.Command, _ []string) error { // If --insecure was provided, set --insecure-skip-tls-verify and --plain-http to match if config.CommonOptions.Insecure { config.CommonOptions.InsecureSkipTLSVerify = true @@ -65,7 +65,7 @@ func preRun(cmd *cobra.Command, args []string) error { return nil } - // TODO(mkcp): Skipping log writes here. We'll want to remove these when we remove the flag. + // Setup message skipLogFile := SkipLogFile // Don't write tool commands to file. @@ -81,10 +81,12 @@ func preRun(cmd *cobra.Command, args []string) error { if cmd.Parent() == nil { skipLogFile = true } - // TODO End skip log writes. + err := setupMessage(LogLevelCLI, skipLogFile, NoColor) + if err != nil { + return err + } // Configure logger and add it to cmd context. - // FIXME(mkcp): Where is the best place to read these flags? l, err := setupLogger(LogLevelCLI, LogFormat) if err != nil { return err @@ -92,34 +94,23 @@ func preRun(cmd *cobra.Command, args []string) error { ctx := context.WithValue(cmd.Context(), "logger", l) cmd.SetContext(ctx) - err = setupMessage(LogLevelCLI, skipLogFile, NoColor) - if err != nil { - return err - } + // Print out config location + common.PrintViperConfigUsed(cmd.Context()) return nil } -func run(cmd *cobra.Command, args []string) { - zarfLogo := message.GetLogo() - _, _ = fmt.Fprintln(os.Stderr, zarfLogo) +func run(cmd *cobra.Command, _ []string) { err := cmd.Help() if err != nil { _, _ = fmt.Fprintln(os.Stderr, err) //nolint:errcheck } - - // FIXME(mkcp): These we can probably complete deprecation on? - if len(args) > 0 { - if strings.Contains(args[0], config.ZarfPackagePrefix) || strings.Contains(args[0], "zarf-init") { - message.Warnf(lang.RootCmdDeprecatedDeploy, args[0]) - } - if args[0] == layout.ZarfYAML { - message.Warn(lang.RootCmdDeprecatedCreate) - } - } } // Execute is the entrypoint for the CLI. func Execute(ctx context.Context) { + // Add `zarf say` + rootCmd.AddCommand(say.Command()) + cmd, err := rootCmd.ExecuteContextC(ctx) if err == nil { return @@ -190,8 +181,6 @@ func setupMessage(logLevel string, skipLogFile, noColor bool) error { message.DisableColor() } - common.PrintViperConfigUsed() - if logLevel != "" { match := map[string]message.LogLevel{ "warn": message.WarnLevel, diff --git a/src/cmd/say/say.go b/src/cmd/say/say.go new file mode 100644 index 0000000000..2ac63bb43a --- /dev/null +++ b/src/cmd/say/say.go @@ -0,0 +1,71 @@ +package say + +import ( + "fmt" + "github.com/spf13/cobra" + "os" +) + +func Command() *cobra.Command { + return &cobra.Command{ + Use: "say", + Short: "Print Zarf logo", + Long: "Print out the adorable Zarf logo", + RunE: func(cmd *cobra.Command, args []string) error { + _, err := fmt.Fprintln(os.Stderr, logo()) + return err + }, + } +} + +func logo() string { + return ` +         *,                                                                               +         *(((&&&&&/*.                                                                     +          *(((((%&&&&&&&*,                                                                +           *(((((((&&&&&&&&&*              ,,*****,.                      **%&&&&&((((((  +            *(((((((((&&&&&&&@*    **@@@@@@&&&&&&&&&&@@@@@**         */&&&&&&((((((((((   +              *((((((///(&&&&&&@@@@&&&&@@@@@@@@@&&&&&&&&&&&&&&@/* *%&&&&&&/////((((((*    +                *(((///////&&&&&&&&&&&&&@@@@@@@@@&&&&&&&&&&&&&&&&&(%&&&/**///////(/*      +       */&&&&&&&&&&&&&&&&*/***&%&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&/*******///*          +   *&%&&&&&&&&&&&&&&&&&&&&&&&***&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&****/&&&&&&&&&&&&(/* + */((((((((((((((///////******%%&&&&&&&&//&@@*&&&&&&&&&&&&&&&&&&&#%&&&&*/####(/////((((/. +     */((((((((((///////******%%%%%%%%%(##@@@//%&%%%%%%%&%&%%&&/(@@(/&&(***///////(((*    +          ***(((((/////********%%%%%%%/&%***/((%%%%%%%%%%%%%%%%(#&@*%/%%***/////**        +            *&&%%%%%%//*******%%%%%%%%@@****%%/%%%%%%%%%%%%%%%%%%***@@%%**(%%%&&*         +          *&&%%%%%(////******/(%%%%%%%@@@**@@&%%%%%%%%%%%%%%%%%(@@*%@@%%*****////%&*      +        *&%%%%%#////////***/////%%%%%%*@@@@@/%%%%%%%%%%%%%%%%%%%%@@@@%%*****///////((*    +       *%%%%((((///////*    *////(%%%%%%##%%%%%%%%%%%(%%%%*%%%%%%%%%%%*                   +      *(((((((/***            */////#%%%%%%%%%%#%%%%%%%%%%%%%%%%%%%%#*                    +                   %%(           ,*///((%%%%%%%%(**/#%%%##**/%%%%%*                       +                 %%%&&&&           *///*/(((((########//######**                          +                 %&&&&&*          *#######(((((((//////((((*                              +                                  ###%##############(((#####*                             +                   %@&&          *&#(%######*#########(#####/                             +                   /&&* ..       ,&#(/%####(*#########/#####/             #%@%&&&         +             **         &&     ./%##((*&####/(#######(#####*(*            %&&&&&&         +           *@%%@*             *&#####((((####*(#####(*###(*(##*              ,  %@&       +          *@%%%%*            *%######((((*%####/*((*%####/*(###*  *                       +         *@%%%%%%*      *##* **#(###((((///#*#*(((((/#**#((*(##**#,*/##*,    %@&&         +         *@%%%*%%%*  ****,*##/*#*##(((((((/(((((((((/(((*(((((###########*,  #&&#         +         *@%%%*(%%%/*   **######(#((..((((((((((((((((((*  ,*(#####(((((*,                +         *@%%%#(*%%%%*   ,**/####(* */(((((((((((((((((*     ,**,                         +          *@%%%*(/(%%%%/*     ******(((((((((((*(((((*                                    +           *@%%%#(((*/%%%%%%##%%*((((((((((((**((((*                                      +            *@%%%%*(((((((((((((((((((((((*/%*((*.             (&&&(                      +             ,*%%%%%%*((((((((((((((((**%%%**,                (&                          +                *%%%%%%%%%(/*****(#%%%%%**                      &%                        +                   ,**%%%%%%%%%%%%%***                                                    +                                                                                          +             ,((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((,           +                         .....(((((((/////////////////((((((((.....                       +                                                                                          +            ///////////////      ///////      *****************  ***************,         +                    ////.       ///  ////     *///          ***  ****                     +                 ////,         ///    ////    *///////////////.  /////**/******           +              /////          //////////////   *///      *///     ///*                     +           ./////////////// ////         ///  *///        ////   ///*                     +                                                                                          + +` +} diff --git a/src/config/lang/english.go b/src/config/lang/english.go index 7e520f24cf..f755bfd984 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -590,10 +590,6 @@ $ zarf tools update-creds artifact --artifact-push-username={USERNAME} --artifac // tools version CmdToolsVersionShort = "Print the version" - - // cmd viper setup - CmdViperErrLoadingConfigFile = "failed to load config file: %s" - CmdViperInfoUsingConfigFile = "Using config file %s" ) // Zarf Agent messages From c313063b648eeec8b014f7ede446b12624b3dbd3 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Fri, 18 Oct 2024 16:04:07 -0700 Subject: [PATCH 04/10] introduce logger.From() and make the linter happy Signed-off-by: Kit Patella --- src/cmd/common/viper.go | 10 ++++++---- src/cmd/root.go | 9 +++++---- src/cmd/say/say.go | 10 ++++++++-- src/pkg/logger/logger.go | 37 +++++++++++++++++++++++++++++++++++ src/pkg/logger/logger_test.go | 27 +++++++++++++++++++++++++ 5 files changed, 83 insertions(+), 10 deletions(-) diff --git a/src/cmd/common/viper.go b/src/cmd/common/viper.go index 697bbb57d0..93187e63fa 100644 --- a/src/cmd/common/viper.go +++ b/src/cmd/common/viper.go @@ -7,10 +7,11 @@ package common import ( "context" "errors" - "log/slog" "os" "strings" + "github.com/zarf-dev/zarf/src/pkg/logger" + "github.com/spf13/viper" "github.com/zarf-dev/zarf/src/config" ) @@ -166,8 +167,9 @@ func isVersionCmd() bool { return len(args) > 1 && (args[1] == "version" || args[1] == "v") } +// PrintViperConfigUsed informs users when Zarf has detected a config file. func PrintViperConfigUsed(ctx context.Context) { - log := ctx.Value("logger").(*slog.Logger) + l := logger.From(ctx) // Only print config info if viper is initialized. vInitialized := v != nil @@ -179,11 +181,11 @@ func PrintViperConfigUsed(ctx context.Context) { return } if vConfigError != nil { - log.Error("unable to load config file", "error", vConfigError) + l.Error("unable to load config file", "error", vConfigError) return } if cfgFile := v.ConfigFileUsed(); cfgFile != "" { - log.Info("Using config file", "location", cfgFile) + l.Info("Using config file", "location", cfgFile) } } diff --git a/src/cmd/root.go b/src/cmd/root.go index e7fbe2b919..27ecdf4a44 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -8,8 +8,6 @@ import ( "context" "errors" "fmt" - "github.com/zarf-dev/zarf/src/cmd/say" - "github.com/zarf-dev/zarf/src/pkg/logger" "io" "log/slog" "os" @@ -17,6 +15,9 @@ import ( "strings" "time" + "github.com/zarf-dev/zarf/src/cmd/say" + "github.com/zarf-dev/zarf/src/pkg/logger" + "github.com/pterm/pterm" "github.com/spf13/cobra" @@ -91,7 +92,7 @@ func preRun(cmd *cobra.Command, _ []string) error { if err != nil { return err } - ctx := context.WithValue(cmd.Context(), "logger", l) + ctx := context.WithValue(cmd.Context(), logger.DefaultCtxKey, l) cmd.SetContext(ctx) // Print out config location @@ -102,7 +103,7 @@ func preRun(cmd *cobra.Command, _ []string) error { func run(cmd *cobra.Command, _ []string) { err := cmd.Help() if err != nil { - _, _ = fmt.Fprintln(os.Stderr, err) //nolint:errcheck + _, _ = fmt.Fprintln(os.Stderr, err) } } diff --git a/src/cmd/say/say.go b/src/cmd/say/say.go index 2ac63bb43a..25032f9195 100644 --- a/src/cmd/say/say.go +++ b/src/cmd/say/say.go @@ -1,17 +1,23 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package say prints out the adorable creature we all know and love. package say import ( "fmt" - "github.com/spf13/cobra" "os" + + "github.com/spf13/cobra" ) +// Command prints out the Zarf logo. func Command() *cobra.Command { return &cobra.Command{ Use: "say", Short: "Print Zarf logo", Long: "Print out the adorable Zarf logo", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, _ []string) error { _, err := fmt.Fprintln(os.Stderr, logo()) return err }, diff --git a/src/pkg/logger/logger.go b/src/pkg/logger/logger.go index 41400666d8..d31cc1739e 100644 --- a/src/pkg/logger/logger.go +++ b/src/pkg/logger/logger.go @@ -5,6 +5,7 @@ package logger import ( + "context" "fmt" "io" "log/slog" @@ -21,6 +22,12 @@ func init() { SetDefault(l) } +// CtxKey limits access to context values by type. This encourages consumers to not store loggers in random strings. +type CtxKey string + +// DefaultCtxKey declares the standard location to store a *slog.Logger on context. +var DefaultCtxKey = CtxKey("logger") + // Level declares each supported log level. These are 1:1 what log/slog supports by default. Info is the default level. type Level int @@ -145,6 +152,36 @@ func New(cfg Config) (*slog.Logger, error) { return log, nil } +// defaultCtxKey provides a default key if one is not passed into From. +var defaultCtxKey = CtxKey("logger") + +// From takes a context and reads out a "logger" value, optionally taking a key string. If multiple keys are provided, +// any after the first will be ignored. Note that if From does not find a value, or that value is not a *slog.Logger, +// it will return return nil. +// +// Usage: +// +// l := From(ctx) +// l := From(ctx, "logger2") +func From(ctx context.Context, key ...CtxKey) *slog.Logger { + k := defaultCtxKey + // Grab optional key. + if len(key) > 0 { + k = key[0] + } + // Grab value from key + log := ctx.Value(k) + + // Ensure our value is a *slog.Logger before we cast. + switch l := log.(type) { + case *slog.Logger: + return l + default: + // Not a *slog.Logger, pass back nil. + return nil + } +} + // Default retrieves a logger from the package default. This is intended as a fallback when a logger cannot easily be // passed in as a dependency, like when developing a new function. Use it like you would use context.TODO(). func Default() *slog.Logger { diff --git a/src/pkg/logger/logger_test.go b/src/pkg/logger/logger_test.go index db8851e25c..0bab660c69 100644 --- a/src/pkg/logger/logger_test.go +++ b/src/pkg/logger/logger_test.go @@ -5,6 +5,7 @@ package logger import ( + "context" "os" "testing" @@ -214,3 +215,29 @@ func Test_ParseLevelErrors(t *testing.T) { }) } } + +func Test_From(t *testing.T) { + t.Parallel() + + t.Run("can load a logger from the default key", func(t *testing.T) { + key := CtxKey("logger") + ctx := context.WithValue(context.Background(), key, Default()) + res := From(ctx) + require.NotNil(t, res) + }) + + t.Run("can load a logger from a specific key", func(t *testing.T) { + key := CtxKey("logger-2-the-sequel-to-logger") + ctx := context.WithValue(context.Background(), key, Default()) + res := From(ctx, key) + require.NotNil(t, res) + }) + + t.Run("can load a logger from a specific key and ignores extra keys", func(t *testing.T) { + key := CtxKey("logger-2-the-sequel-to-logger") + extraKey := CtxKey("oaphijdsf") + ctx := context.WithValue(context.Background(), key, Default()) + res := From(ctx, key, extraKey) + require.NotNil(t, res) + }) +} From 06cacb0a6887d8ca96763b97d1661967abb13af2 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Mon, 21 Oct 2024 09:37:28 -0700 Subject: [PATCH 05/10] fix bug with trace no longer supported, make trace the same as debug Signed-off-by: Kit Patella --- src/pkg/logger/logger.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/pkg/logger/logger.go b/src/pkg/logger/logger.go index d31cc1739e..28362c22be 100644 --- a/src/pkg/logger/logger.go +++ b/src/pkg/logger/logger.go @@ -49,6 +49,8 @@ var validLevels = map[Level]bool{ // strLevels maps a string to its Level. var strLevels = map[string]Level{ + // NOTE(mkcp): We're mapping trace to debug for backwards compatibility. + "trace": Debug, "debug": Debug, "info": Info, "warn": Warn, From 870253d49c42d4230448a414eb28b60456fce303 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Mon, 21 Oct 2024 11:41:51 -0700 Subject: [PATCH 06/10] ensure message setup can passthru error to warn Signed-off-by: Kit Patella --- src/cmd/root.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cmd/root.go b/src/cmd/root.go index 27ecdf4a44..68e14d37d5 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -184,6 +184,8 @@ func setupMessage(logLevel string, skipLogFile, noColor bool) error { if logLevel != "" { match := map[string]message.LogLevel{ + // NOTE(mkcp): Add error for forwards compatibility with logger + "error": message.WarnLevel, "warn": message.WarnLevel, "info": message.InfoLevel, "debug": message.DebugLevel, @@ -191,7 +193,7 @@ func setupMessage(logLevel string, skipLogFile, noColor bool) error { } lvl, ok := match[logLevel] if !ok { - return errors.New("invalid log level, valid options are warn, info, debug, and trace") + return errors.New("invalid log level, valid options are warn, info, debug, error, and trace") } message.SetLogLevel(lvl) message.Debug("Log level set to " + logLevel) From ab0898532a93580233cbc5174e1d3542a6ce4dfa Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Mon, 21 Oct 2024 12:09:35 -0700 Subject: [PATCH 07/10] work around the change detectors for CLI help text Signed-off-by: Kit Patella --- src/cmd/root.go | 11 ++++++++--- src/config/lang/english.go | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/cmd/root.go b/src/cmd/root.go index 68e14d37d5..04a3ec6ff0 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -154,6 +154,10 @@ func init() { rootCmd.PersistentFlags().MarkDeprecated("insecure", "please use --plain-http, --insecure-skip-tls-verify, or --skip-signature-validation instead.") rootCmd.PersistentFlags().BoolVar(&config.CommonOptions.PlainHTTP, "plain-http", v.GetBool(common.VPlainHTTP), lang.RootCmdFlagPlainHTTP) rootCmd.PersistentFlags().BoolVar(&config.CommonOptions.InsecureSkipTLSVerify, "insecure-skip-tls-verify", v.GetBool(common.VInsecureSkipTLSVerify), lang.RootCmdFlagInsecureSkipTLSVerify) + + // HACK(mkcp): This is a workaround for us testing that help output matches to the byte. Undo this and update tests + // before release. + rootCmd.PersistentFlags().MarkHidden("log-format") } // setup Logger handles creating a logger and setting it as the global default. @@ -162,16 +166,17 @@ func setupLogger(level, format string) (*slog.Logger, error) { if err != nil { return nil, err } - l, err := logger.New(logger.Config{ + cfg := logger.Config{ Level: sLevel, Format: logger.Format(format), Destination: logger.DestinationDefault, - }) + } + l, err := logger.New(cfg) if err != nil { return nil, err } logger.SetDefault(l) - l.Debug("Logger successfully initialized", "logger", l) + l.Debug("Logger successfully initialized", "cfg", cfg) return l, nil } diff --git a/src/config/lang/english.go b/src/config/lang/english.go index f755bfd984..5007ad9a1f 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -45,7 +45,7 @@ const ( RootCmdLong = "Zarf eliminates the complexity of air gap software delivery for Kubernetes clusters and cloud native workloads\n" + "using a declarative packaging strategy to support DevSecOps in offline and semi-connected environments." - RootCmdFlagLogLevel = "Log level when running Zarf. Defaults to info. Valid options are: debug, info, warn, error" + RootCmdFlagLogLevel = "Log level when running Zarf. Valid options are: warn, info, debug, trace" RootCmdFlagLogFormat = "Select a logging format. Defaults to 'text'. Valid options are: 'text', 'json'" RootCmdFlagArch = "Architecture for OCI images and Zarf packages" RootCmdFlagSkipLogFile = "Disable log file creation" From 7bb59c587f36c1482db1e312db178f4bc46d9204 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Mon, 21 Oct 2024 12:28:56 -0700 Subject: [PATCH 08/10] hide zarf say to make test-docs-and-schema happy Signed-off-by: Kit Patella --- src/cmd/say/say.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cmd/say/say.go b/src/cmd/say/say.go index 25032f9195..94daed409f 100644 --- a/src/cmd/say/say.go +++ b/src/cmd/say/say.go @@ -17,6 +17,8 @@ func Command() *cobra.Command { Use: "say", Short: "Print Zarf logo", Long: "Print out the adorable Zarf logo", + // HACK(mkcp): Hidden is a workaround until we update `test-docs-and-schema` for the new command and flags. + Hidden: true, RunE: func(_ *cobra.Command, _ []string) error { _, err := fmt.Fprintln(os.Stderr, logo()) return err From b0c378b044a4ff7a57323d1a4f037f69c792b36b Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Mon, 21 Oct 2024 14:25:54 -0700 Subject: [PATCH 09/10] handle empty level in setupLogger Signed-off-by: Kit Patella --- src/cmd/root.go | 4 ++++ src/pkg/logger/logger.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cmd/root.go b/src/cmd/root.go index 04a3ec6ff0..e4070cab73 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -162,6 +162,10 @@ func init() { // setup Logger handles creating a logger and setting it as the global default. func setupLogger(level, format string) (*slog.Logger, error) { + // If we didn't get a level from config, fallback to "info" + if level == "" { + level = "info" + } sLevel, err := logger.ParseLevel(level) if err != nil { return nil, err diff --git a/src/pkg/logger/logger.go b/src/pkg/logger/logger.go index 28362c22be..37923345f5 100644 --- a/src/pkg/logger/logger.go +++ b/src/pkg/logger/logger.go @@ -49,7 +49,7 @@ var validLevels = map[Level]bool{ // strLevels maps a string to its Level. var strLevels = map[string]Level{ - // NOTE(mkcp): We're mapping trace to debug for backwards compatibility. + // NOTE(mkcp): Map trace to debug for backwards compatibility. "trace": Debug, "debug": Debug, "info": Info, From 00bde4cc891b986d1b8ba037ada112fe9c544f37 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Tue, 22 Oct 2024 10:17:54 -0700 Subject: [PATCH 10/10] review items and change From and add WithContext after chattin Signed-off-by: Kit Patella --- src/cmd/common/viper.go | 2 +- src/cmd/root.go | 6 ++--- src/config/lang/english.go | 1 - src/pkg/logger/logger.go | 42 ++++++++++++++--------------------- src/pkg/logger/logger_test.go | 20 ++--------------- 5 files changed, 23 insertions(+), 48 deletions(-) diff --git a/src/cmd/common/viper.go b/src/cmd/common/viper.go index 93187e63fa..cef9456562 100644 --- a/src/cmd/common/viper.go +++ b/src/cmd/common/viper.go @@ -185,7 +185,7 @@ func PrintViperConfigUsed(ctx context.Context) { return } if cfgFile := v.ConfigFileUsed(); cfgFile != "" { - l.Info("Using config file", "location", cfgFile) + l.Info("using config file", "location", cfgFile) } } diff --git a/src/cmd/root.go b/src/cmd/root.go index e4070cab73..f72358e288 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -92,7 +92,7 @@ func preRun(cmd *cobra.Command, _ []string) error { if err != nil { return err } - ctx := context.WithValue(cmd.Context(), logger.DefaultCtxKey, l) + ctx := logger.WithContext(cmd.Context(), l) cmd.SetContext(ctx) // Print out config location @@ -140,7 +140,7 @@ func init() { // Logs rootCmd.PersistentFlags().StringVarP(&LogLevelCLI, "log-level", "l", v.GetString(common.VLogLevel), lang.RootCmdFlagLogLevel) - rootCmd.PersistentFlags().StringVar(&LogFormat, "log-format", v.GetString(common.VLogFormat), lang.RootCmdFlagLogFormat) + rootCmd.PersistentFlags().StringVar(&LogFormat, "log-format", v.GetString(common.VLogFormat), "Select a logging format. Defaults to 'text'. Valid options are: 'text', 'json'") 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(&NoColor, "no-color", v.GetBool(common.VNoColor), lang.RootCmdFlagNoColor) @@ -180,7 +180,7 @@ func setupLogger(level, format string) (*slog.Logger, error) { return nil, err } logger.SetDefault(l) - l.Debug("Logger successfully initialized", "cfg", cfg) + l.Debug("logger successfully initialized", "cfg", cfg) return l, nil } diff --git a/src/config/lang/english.go b/src/config/lang/english.go index 5007ad9a1f..18889ab0a1 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -46,7 +46,6 @@ const ( "using a declarative packaging strategy to support DevSecOps in offline and semi-connected environments." RootCmdFlagLogLevel = "Log level when running Zarf. Valid options are: warn, info, debug, trace" - RootCmdFlagLogFormat = "Select a logging format. Defaults to 'text'. Valid options are: 'text', 'json'" RootCmdFlagArch = "Architecture for OCI images and Zarf packages" RootCmdFlagSkipLogFile = "Disable log file creation" RootCmdFlagNoProgress = "Disable fancy UI progress bars, spinners, logos, etc" diff --git a/src/pkg/logger/logger.go b/src/pkg/logger/logger.go index 37923345f5..aa0d4b7f07 100644 --- a/src/pkg/logger/logger.go +++ b/src/pkg/logger/logger.go @@ -22,12 +22,6 @@ func init() { SetDefault(l) } -// CtxKey limits access to context values by type. This encourages consumers to not store loggers in random strings. -type CtxKey string - -// DefaultCtxKey declares the standard location to store a *slog.Logger on context. -var DefaultCtxKey = CtxKey("logger") - // Level declares each supported log level. These are 1:1 what log/slog supports by default. Info is the default level. type Level int @@ -154,33 +148,31 @@ func New(cfg Config) (*slog.Logger, error) { return log, nil } +// ctxKey provides a location to store a logger in a context. +type ctxKey struct{} + // defaultCtxKey provides a default key if one is not passed into From. -var defaultCtxKey = CtxKey("logger") - -// From takes a context and reads out a "logger" value, optionally taking a key string. If multiple keys are provided, -// any after the first will be ignored. Note that if From does not find a value, or that value is not a *slog.Logger, -// it will return return nil. -// -// Usage: -// -// l := From(ctx) -// l := From(ctx, "logger2") -func From(ctx context.Context, key ...CtxKey) *slog.Logger { - k := defaultCtxKey - // Grab optional key. - if len(key) > 0 { - k = key[0] - } +var defaultCtxKey = ctxKey{} + +// WithContext takes a context.Context and a *slog.Logger, storing it on the key +func WithContext(ctx context.Context, logger *slog.Logger) context.Context { + return context.WithValue(ctx, defaultCtxKey, logger) +} + +// From takes a context and reads out a *slog.Logger. If From does not find a value it will return a discarding logger +// similar to log-format "none". +func From(ctx context.Context) *slog.Logger { // Grab value from key - log := ctx.Value(k) + log := ctx.Value(defaultCtxKey) // Ensure our value is a *slog.Logger before we cast. switch l := log.(type) { case *slog.Logger: return l default: - // Not a *slog.Logger, pass back nil. - return nil + // Value is empty or not a *slog.Logger, pass back a Discard logger. + h := slog.NewTextHandler(DestinationNone, &slog.HandlerOptions{}) + return slog.New(h) } } diff --git a/src/pkg/logger/logger_test.go b/src/pkg/logger/logger_test.go index 0bab660c69..409922c7b2 100644 --- a/src/pkg/logger/logger_test.go +++ b/src/pkg/logger/logger_test.go @@ -216,28 +216,12 @@ func Test_ParseLevelErrors(t *testing.T) { } } -func Test_From(t *testing.T) { +func TestContext(t *testing.T) { t.Parallel() t.Run("can load a logger from the default key", func(t *testing.T) { - key := CtxKey("logger") - ctx := context.WithValue(context.Background(), key, Default()) + ctx := WithContext(context.Background(), Default()) res := From(ctx) require.NotNil(t, res) }) - - t.Run("can load a logger from a specific key", func(t *testing.T) { - key := CtxKey("logger-2-the-sequel-to-logger") - ctx := context.WithValue(context.Background(), key, Default()) - res := From(ctx, key) - require.NotNil(t, res) - }) - - t.Run("can load a logger from a specific key and ignores extra keys", func(t *testing.T) { - key := CtxKey("logger-2-the-sequel-to-logger") - extraKey := CtxKey("oaphijdsf") - ctx := context.WithValue(context.Background(), key, Default()) - res := From(ctx, key, extraKey) - require.NotNil(t, res) - }) }