From cedd45b653c398dca859623ea5baaebda9f74a4a Mon Sep 17 00:00:00 2001 From: sbldevnet Date: Mon, 28 Sep 2026 13:08:32 +0200 Subject: [PATCH] refactor(cmd): use an injected viper instance instead of the global singleton Thread *viper.Viper through initConfig and logOptions.withConfig instead of calling package-level viper functions, removing the need for viper.Reset() between tests. --- internal/cmd/cmd.go | 21 +++++++++++---------- internal/cmd/cmd_test.go | 15 ++++++--------- internal/cmd/logger.go | 10 +++++----- internal/cmd/logger_test.go | 17 ++++++----------- 4 files changed, 28 insertions(+), 35 deletions(-) diff --git a/internal/cmd/cmd.go b/internal/cmd/cmd.go index 4175a05..8b59d46 100644 --- a/internal/cmd/cmd.go +++ b/internal/cmd/cmd.go @@ -70,7 +70,8 @@ func Run() *cobra.Command { } } else { // If config or default provided - if err := initConfig(log, cfgFile); err != nil { + v := viper.New() + if err := initConfig(v, log, cfgFile); err != nil { return err } @@ -78,21 +79,21 @@ func Run() *cobra.Command { // built, because loading it logs; its log settings therefore apply // from here on and messages logged while loading follow the flag and // environment only. - logOpts, changed, err := logOpts.withConfig() + logOpts, changed, err := logOpts.withConfig(v) if err != nil { return err } if changed { log = logOpts.logger() } - log.Debug("config file loaded", "path", viper.ConfigFileUsed()) + log.Debug("config file loaded", "path", v.ConfigFileUsed()) - if !viper.IsSet("proxies") { + if !v.IsSet("proxies") { return fmt.Errorf("no proxies defined in config file") } var cfg config.Config - if err := viper.Unmarshal(&cfg); err != nil { + if err := v.Unmarshal(&cfg); err != nil { return fmt.Errorf("unable to decode into struct, %v", err) } config.SetDefaults(cfg.Proxies) @@ -193,11 +194,11 @@ func validateListen(proxies []config.ProxyConfig) error { return nil } -func initConfig(log *slog.Logger, cfgFile string) error { +func initConfig(v *viper.Viper, log *slog.Logger, cfgFile string) error { var dir string if cfgFile != "" { log.Debug("using explicit config file", "path", cfgFile) - viper.SetConfigFile(cfgFile) + v.SetConfigFile(cfgFile) } else { home, err := os.UserHomeDir() if err != nil { @@ -205,11 +206,11 @@ func initConfig(log *slog.Logger, cfgFile string) error { } dir = filepath.Join(home, ".config", "cloudflared-proxy") log.Debug("using default config location", "path", dir) - viper.AddConfigPath(dir) - viper.SetConfigName("config") + v.AddConfigPath(dir) + v.SetConfigName("config") } - if err := viper.ReadInConfig(); err != nil { + if err := v.ReadInConfig(); err != nil { if _, ok := err.(viper.ConfigFileNotFoundError); ok { if cfgFile != "" { return fmt.Errorf("config file not found: %s", cfgFile) diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index 74910e9..1f53942 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -52,8 +52,6 @@ func (h *recordingHandler) messages() [][2]string { func TestInitConfigDiagnostics(t *testing.T) { setup := func(t *testing.T) (*slog.Logger, *recordingHandler) { - viper.Reset() - t.Cleanup(viper.Reset) h := &recordingHandler{} return slog.New(h), h } @@ -63,7 +61,7 @@ func TestInitConfigDiagnostics(t *testing.T) { file := filepath.Join(t.TempDir(), "cfg.yaml") require.NoError(t, os.WriteFile(file, []byte("proxies: []\n"), 0o600)) - require.NoError(t, initConfig(log, file)) + require.NoError(t, initConfig(viper.New(), log, file)) assert.Equal(t, [][2]string{{"using explicit config file", file}}, h.messages()) }) @@ -76,7 +74,7 @@ func TestInitConfigDiagnostics(t *testing.T) { require.NoError(t, os.MkdirAll(dir, 0o700)) require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), []byte("proxies: []\n"), 0o600)) - require.NoError(t, initConfig(log, "")) + require.NoError(t, initConfig(viper.New(), log, "")) assert.Equal(t, [][2]string{{"using default config location", dir}}, h.messages()) }) @@ -87,7 +85,7 @@ func TestInitConfigDiagnostics(t *testing.T) { t.Setenv("HOME", home) dir := filepath.Join(home, ".config", "cloudflared-proxy") - err := initConfig(log, "") + err := initConfig(viper.New(), log, "") assert.EqualError(t, err, "no config file or endpoints provided") assert.Equal(t, [][2]string{ @@ -132,8 +130,7 @@ func TestValidateListen(t *testing.T) { } func TestListenConfigKeys(t *testing.T) { - viper.Reset() - t.Cleanup(viper.Reset) + v := viper.New() file := filepath.Join(t.TempDir(), "cfg.yaml") yaml := `listen: 0.0.0.0 proxies: @@ -142,10 +139,10 @@ proxies: listen: 127.0.0.1 ` require.NoError(t, os.WriteFile(file, []byte(yaml), 0o600)) - require.NoError(t, initConfig(slog.New(&recordingHandler{}), file)) + require.NoError(t, initConfig(v, slog.New(&recordingHandler{}), file)) var cfg config.Config - require.NoError(t, viper.Unmarshal(&cfg)) + require.NoError(t, v.Unmarshal(&cfg)) applyListenDefault(cfg.Proxies, cfg.Listen) assert.Equal(t, "0.0.0.0", cfg.Listen) diff --git a/internal/cmd/logger.go b/internal/cmd/logger.go index a1bb1dd..a039b32 100644 --- a/internal/cmd/logger.go +++ b/internal/cmd/logger.go @@ -69,17 +69,17 @@ func resolveLogOptions(flagLevel, flagFormat string, levelFlagSet, formatFlagSet // withConfig fills in the settings that no flag or environment variable set // from the logLevel and logFormat keys of the loaded config file. It reports // whether anything changed. -func (o logOptions) withConfig() (logOptions, bool, error) { +func (o logOptions) withConfig(v *viper.Viper) (logOptions, bool, error) { changed := false - if !o.levelSet && viper.IsSet("logLevel") { - level, err := parseLogLevel("logLevel", viper.GetString("logLevel")) + if !o.levelSet && v.IsSet("logLevel") { + level, err := parseLogLevel("logLevel", v.GetString("logLevel")) if err != nil { return o, false, err } o.level, changed = level, true } - if !o.formatSet && viper.IsSet("logFormat") { - json, err := parseLogFormat("logFormat", viper.GetString("logFormat")) + if !o.formatSet && v.IsSet("logFormat") { + json, err := parseLogFormat("logFormat", v.GetString("logFormat")) if err != nil { return o, false, err } diff --git a/internal/cmd/logger_test.go b/internal/cmd/logger_test.go index 1bf1d7a..90582d0 100644 --- a/internal/cmd/logger_test.go +++ b/internal/cmd/logger_test.go @@ -145,13 +145,12 @@ func logFromEnv() *slog.Logger { // resolveWithConfig runs the full resolution: flag, environment, then the config file. func resolveWithConfig(t *testing.T, flag, env, cfg string, key, envName string) (logOptions, error) { t.Helper() - viper.Reset() - t.Cleanup(viper.Reset) + v := viper.New() t.Setenv("LOG_LEVEL", "") t.Setenv("LOG_FORMAT", "") t.Setenv(envName, env) if cfg != "" { - viper.Set(key, cfg) + v.Set(key, cfg) } var o logOptions var err error @@ -163,7 +162,7 @@ func resolveWithConfig(t *testing.T, flag, env, cfg string, key, envName string) if err != nil { return o, err } - o, _, err = o.withConfig() + o, _, err = o.withConfig(v) return o, err } @@ -238,9 +237,6 @@ func TestInvalidFlagAndConfigValues(t *testing.T) { func TestInvalidEnvLevelWarns(t *testing.T) { t.Setenv("LOG_LEVEL", "bogus") t.Setenv("LOG_FORMAT", "") - viper.Reset() - t.Cleanup(viper.Reset) - viper.Set("logFormat", "json") o, err := resolveLogOptions("", "", false, false) require.NoError(t, err) @@ -251,8 +247,7 @@ func TestInvalidEnvLevelWarns(t *testing.T) { func TestConfigFileLogSettingsApplyAfterLoading(t *testing.T) { t.Setenv("LOG_LEVEL", "") t.Setenv("LOG_FORMAT", "") - viper.Reset() - t.Cleanup(viper.Reset) + v := viper.New() file := filepath.Join(t.TempDir(), "cfg.yaml") require.NoError(t, os.WriteFile(file, []byte("logLevel: debug\nlogFormat: json\nproxies: []\n"), 0o600)) @@ -260,8 +255,8 @@ func TestConfigFileLogSettingsApplyAfterLoading(t *testing.T) { require.NoError(t, err) assert.Equal(t, slog.LevelInfo, o.level) - require.NoError(t, initConfig(slog.New(&recordingHandler{}), file)) - o, changed, err := o.withConfig() + require.NoError(t, initConfig(v, slog.New(&recordingHandler{}), file)) + o, changed, err := o.withConfig(v) require.NoError(t, err) assert.True(t, changed)