From ef222e24876b36299e246c0b239768d45b2da6d1 Mon Sep 17 00:00:00 2001 From: Anton Nikiforov Date: Tue, 25 Apr 2023 10:04:26 +0300 Subject: [PATCH] [#125] cmd: Refactor `internal/common/viper` Add `opts.WithViper`, set opts struct as private. Signed-off-by: Anton Nikiforov --- cmd/frostfs-node/config/config.go | 24 +++++++---- cmd/frostfs-node/config/configdir_test.go | 3 +- cmd/frostfs-node/config/test/config.go | 9 ++-- cmd/frostfs-node/main.go | 5 +-- cmd/frostfs-node/validate_test.go | 5 +-- cmd/internal/common/config/opts.go | 37 +++++++++++------ cmd/internal/common/config/viper.go | 50 +++++++++++++++-------- 7 files changed, 80 insertions(+), 53 deletions(-) diff --git a/cmd/frostfs-node/config/config.go b/cmd/frostfs-node/config/config.go index 383897fc8..77e34d613 100644 --- a/cmd/frostfs-node/config/config.go +++ b/cmd/frostfs-node/config/config.go @@ -14,7 +14,9 @@ import ( type Config struct { v *viper.Viper - opts configViper.Opts + configFile string + configDir string + envPrefix string defaultPath []string path []string @@ -28,23 +30,31 @@ const ( // New creates a new Config instance. // -// If file option is provided (WithConfigFile), +// If file option is provided, // configuration values are read from it. // Otherwise, Config is a degenerate tree. -func New(opts ...configViper.Option) *Config { - v, o, err := configViper.CreateViper(opts...) +func New(configFile, configDir, envPrefix string) *Config { + v, err := configViper.CreateViper( + configViper.WithConfigFile(configFile), + configViper.WithConfigDir(configDir), + configViper.WithEnvPrefix(envPrefix)) if err != nil { panic(err) } return &Config{ - v: v, - opts: *o, + v: v, + configFile: configFile, + configDir: configDir, + envPrefix: envPrefix, } } // Reload reads configuration path if it was provided to New. func (x *Config) Reload() error { - return configViper.ReloadViper(x.v, x.opts) + return configViper.ReloadViper( + configViper.WithViper(x.v), configViper.WithConfigFile(x.configFile), + configViper.WithConfigDir(x.configDir), + configViper.WithEnvPrefix(x.envPrefix)) } diff --git a/cmd/frostfs-node/config/configdir_test.go b/cmd/frostfs-node/config/configdir_test.go index f41e6edd9..ede15a522 100644 --- a/cmd/frostfs-node/config/configdir_test.go +++ b/cmd/frostfs-node/config/configdir_test.go @@ -5,7 +5,6 @@ import ( "path" "testing" - configViper "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/internal/common/config" "github.com/spf13/cast" "github.com/stretchr/testify/require" ) @@ -19,7 +18,7 @@ func TestConfigDir(t *testing.T) { require.NoError(t, os.WriteFile(cfgFileName0, []byte(`{"storage":{"shard_pool_size":15}}`), 0777)) require.NoError(t, os.WriteFile(cfgFileName1, []byte("logger:\n level: debug"), 0777)) - c := New(configViper.WithConfigDir(dir)) + c := New("", dir, "") require.Equal(t, "debug", cast.ToString(c.Sub("logger").Value("level"))) require.EqualValues(t, 15, cast.ToUint32(c.Sub("storage").Value("shard_pool_size"))) } diff --git a/cmd/frostfs-node/config/test/config.go b/cmd/frostfs-node/config/test/config.go index 2bd5ade21..28ec65291 100644 --- a/cmd/frostfs-node/config/test/config.go +++ b/cmd/frostfs-node/config/test/config.go @@ -7,22 +7,19 @@ import ( "testing" "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-node/config" - configViper "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/internal/common/config" "github.com/stretchr/testify/require" ) func fromFile(path string) *config.Config { os.Clearenv() // ENVs have priority over config files, so we do this in tests - return config.New( - configViper.WithConfigFile(path), - ) + return config.New(path, "", "") } func fromEnvFile(t testing.TB, path string) *config.Config { loadEnv(t, path) // github.com/joho/godotenv can do that as well - return config.New(configViper.WithEnvPrefix(config.EnvPrefix)) + return config.New("", "", config.EnvPrefix) } func forEachFile(paths []string, f func(*config.Config)) { @@ -48,7 +45,7 @@ func ForEnvFileType(t testing.TB, pref string, f func(*config.Config)) { // EmptyConfig returns config without any values and sections. func EmptyConfig() *config.Config { - return config.New(configViper.WithEnvPrefix(config.EnvPrefix)) + return config.New("", "", config.EnvPrefix) } // loadEnv reads .env file, parses `X=Y` records and sets OS ENVs. diff --git a/cmd/frostfs-node/main.go b/cmd/frostfs-node/main.go index 5a66b210b..4fcc38e08 100644 --- a/cmd/frostfs-node/main.go +++ b/cmd/frostfs-node/main.go @@ -8,7 +8,6 @@ import ( "os" "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-node/config" - configViper "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/internal/common/config" "git.frostfs.info/TrueCloudLab/frostfs-node/internal/logs" "git.frostfs.info/TrueCloudLab/frostfs-node/misc" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control" @@ -47,9 +46,7 @@ func main() { os.Exit(SuccessReturnCode) } - appCfg := config.New( - configViper.WithConfigFile(*configFile), configViper.WithConfigDir(*configDir), - configViper.WithEnvPrefix(config.EnvPrefix)) + appCfg := config.New(*configFile, *configDir, config.EnvPrefix) err := validateConfig(appCfg) fatalOnErr(err) diff --git a/cmd/frostfs-node/validate_test.go b/cmd/frostfs-node/validate_test.go index 75b694cec..d9c0f167f 100644 --- a/cmd/frostfs-node/validate_test.go +++ b/cmd/frostfs-node/validate_test.go @@ -7,7 +7,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-node/config" configtest "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/frostfs-node/config/test" - configViper "git.frostfs.info/TrueCloudLab/frostfs-node/cmd/internal/common/config" "github.com/stretchr/testify/require" ) @@ -27,13 +26,13 @@ func TestValidate(t *testing.T) { t.Run("mainnet", func(t *testing.T) { os.Clearenv() // ENVs have priority over config files, so we do this in tests p := filepath.Join(exampleConfigPrefix, "mainnet/config.yml") - c := config.New(configViper.WithConfigFile(p)) + c := config.New(p, "", config.EnvPrefix) require.NoError(t, validateConfig(c)) }) t.Run("testnet", func(t *testing.T) { os.Clearenv() // ENVs have priority over config files, so we do this in tests p := filepath.Join(exampleConfigPrefix, "testnet/config.yml") - c := config.New(configViper.WithConfigFile(p)) + c := config.New(p, "", config.EnvPrefix) require.NoError(t, validateConfig(c)) }) } diff --git a/cmd/internal/common/config/opts.go b/cmd/internal/common/config/opts.go index a28b2b522..46e565639 100644 --- a/cmd/internal/common/config/opts.go +++ b/cmd/internal/common/config/opts.go @@ -1,38 +1,49 @@ package config -type Opts struct { - Path string - ConfigDir string - EnvPrefix string +import "github.com/spf13/viper" + +type opts struct { + path string + configDir string + envPrefix string + v *viper.Viper } -func DefaultOpts() *Opts { - return new(Opts) +func defaultOpts() *opts { + return new(opts) } // Option allows to set an optional parameter of the Config. -type Option func(*Opts) +type Option func(*opts) // WithConfigFile returns an option to set the system path // to the configuration file. func WithConfigFile(path string) Option { - return func(o *Opts) { - o.Path = path + return func(o *opts) { + o.path = path } } // WithConfigDir returns an option to set the system path // to the directory with configuration files. func WithConfigDir(path string) Option { - return func(o *Opts) { - o.ConfigDir = path + return func(o *opts) { + o.configDir = path } } // WithEnvPrefix returns an option to defines // a prefix that ENVIRONMENT variables will use. func WithEnvPrefix(envPrefix string) Option { - return func(o *Opts) { - o.EnvPrefix = envPrefix + return func(o *opts) { + o.envPrefix = envPrefix + } +} + +// WithViper returns an option to defines +// a predefined viper.Viper. +func WithViper(v *viper.Viper) Option { + return func(o *opts) { + o.v = v } } diff --git a/cmd/internal/common/config/viper.go b/cmd/internal/common/config/viper.go index b0af9ae28..41b8831ff 100644 --- a/cmd/internal/common/config/viper.go +++ b/cmd/internal/common/config/viper.go @@ -15,48 +15,62 @@ const ( EnvSeparator = "_" ) -func CreateViper(opts ...Option) (*viper.Viper, *Opts, error) { - v := viper.New() - - o := DefaultOpts() +func CreateViper(opts ...Option) (*viper.Viper, error) { + o := defaultOpts() for i := range opts { opts[i](o) } - if o.EnvPrefix != "" { - v.SetEnvPrefix(o.EnvPrefix) + var v *viper.Viper + if o.v != nil { + v = o.v + } else { + v = viper.New() + } + + if o.envPrefix != "" { + v.SetEnvPrefix(o.envPrefix) v.AutomaticEnv() v.SetEnvKeyReplacer(strings.NewReplacer(Separator, EnvSeparator)) } - if o.Path != "" { - v.SetConfigFile(o.Path) + if o.path != "" { + v.SetConfigFile(o.path) err := v.ReadInConfig() if err != nil { - return nil, nil, fmt.Errorf("failed to read config: %w", err) + return nil, fmt.Errorf("failed to read config: %w", err) } } - if o.ConfigDir != "" { - if err := config.ReadConfigDir(v, o.ConfigDir); err != nil { - return nil, nil, fmt.Errorf("failed to read config dir: %w", err) + if o.configDir != "" { + if err := config.ReadConfigDir(v, o.configDir); err != nil { + return nil, fmt.Errorf("failed to read config dir: %w", err) } } - return v, o, nil + return v, nil } -func ReloadViper(v *viper.Viper, o Opts) error { - if o.Path != "" { - err := v.ReadInConfig() +func ReloadViper(opts ...Option) error { + o := defaultOpts() + for i := range opts { + opts[i](o) + } + + if o.v == nil { + return fmt.Errorf("provide viper in opts") + } + + if o.path != "" { + err := o.v.ReadInConfig() if err != nil { return fmt.Errorf("rereading configuration file: %w", err) } } - if o.ConfigDir != "" { - if err := config.ReadConfigDir(v, o.ConfigDir); err != nil { + if o.configDir != "" { + if err := config.ReadConfigDir(o.v, o.configDir); err != nil { return fmt.Errorf("rereading configuration dir: %w", err) } }