From e08049e6e9f1b568de4ecb90b4365278cac8a264 Mon Sep 17 00:00:00 2001 From: Alejandro Do Nascimento Mora Date: Mon, 31 Oct 2022 16:45:32 +0100 Subject: [PATCH] Allow config options in file as yaml mappings Replaces the `ff` package with a custom implementation that uses Viper to parse the config file and sets the values for the `flag.Flag`s. By design the ff package doesn't allow flag subsets or nested maps inside the config yaml and all config options must be explicitely defined as `flag.Flag`. This limits the ability to specify more dynamic config options like lists or maps of user defined rules (Ex: metric rollups). Viper provides the flexibility we require for parsing the config file, and unmarshaling directly into more complex datastructures. To define how to unmarshal a subtree of the configuration into a datastructure via unmarshalRule. An unmarshalRule for the dataset config was added as: ``` withUnmarshalRules( []unmarshalRule{{"startup.dataset", &dataset.Config}} ) ``` Given a config file that contains the subtree: ``` startup: dataset: metrics: ... traces: ... ``` The subtree `startup.dataset` will be unmarshal into `&dataset.Config` by Viper using the github.com/mitchellh/mapstructure package. Viper can access a nested field by passing a `.` delimited path of keys. This change also allows us to format our configuration files to use yaml mappings instead of long key names with `.`, thus maintaining backwards compatibility: ``` web.listen-address: localhost:9201 web.auth.password: my-password web.auth.username: promscale ``` Can be rewritten as: ``` web: listen-address: localhost:9201 auth: password: my-password username: promscale ``` --- CHANGELOG.md | 4 + docs/dataset.md | 41 +-- pkg/dataset/config.go | 16 +- pkg/dataset/duration.go | 28 ++ pkg/runner/client.go | 14 +- pkg/runner/config_parser.go | 292 ++++++++++++++++++ pkg/runner/flags.go | 28 +- pkg/runner/flags_test.go | 114 ++++++- .../end_to_end_tests/config_dataset_test.go | 3 +- 9 files changed, 471 insertions(+), 69 deletions(-) create mode 100644 pkg/runner/config_parser.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e1c3b9ddf..0a4e22912f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ We use the following categories for changes: ### Added - Alerts from promscale monitoring mixin are groupped also by namespace label [#1714] - Added a new family of metrics tracking database maintenance jobs durations and failures [#1745] +- Allow config options in file to be set as yaml mappings [#1737] +- Add `startup.dataset` option in the config file for the dataset + configuration. Supersedes `startup.dataset.config` which accepts a string + instead of a mapping [#1737] ### Changed - Reduced the verbosity of the logs emitted by the vacuum engine [#1715] diff --git a/docs/dataset.md b/docs/dataset.md index b3996894e5..b9ee56413d 100644 --- a/docs/dataset.md +++ b/docs/dataset.md @@ -2,28 +2,19 @@ Promscale stores some configuration information in the Postgres database which it is connected to. We call this configuration the *Promscale dataset configuration*. Promscale accepts an option to set the dataset values. This document describes its format and mechanics. -Setup is done by using the `-startup.dataset.config` flag which should contain this configuration structure in YAML format - -Example usage: - -```bash -promscale -startup.dataset.config=$'metrics:\n default_chunk_interval: 6h' -``` - -The expected format is YAML. - -The YAML values are easier to set when using `config.yaml` instead of the cli flags. This would look as follows: +Setup is done via the `config.yaml` under `startup.dataset`: ```yaml -startup.dataset.config: | - metrics: - default_chunk_interval: 6h - compress_data: true - ha_lease_refresh: 10s - ha_lease_timeout: 1m - default_retention_period: 90d - traces: - default_retention_period: 30d +startup: + dataset: + metrics: + default_chunk_interval: 6h + compress_data: true + ha_lease_refresh: 10s + ha_lease_timeout: 1m + default_retention_period: 90d + traces: + default_retention_period: 30d ``` Note: Any configuration omitted from the configuration structure will be set to its default value. @@ -32,9 +23,9 @@ Note: Any configuration omitted from the configuration structure will be set to | Section | Setting | Type | Default | Description | |:--------|:-------------------------|:--------:|:-------:|:----------------------------------------------------------------------------------------------------------------| -| metric | default_chunk_interval | duration | 8h | Chunk interval used to create hypertable chunks that store the metric data | -| metric | compress_data | bool | true | Boolean setting to turn on or off compression of metric data | -| metric | ha_lease_refresh | duration | 10s | High availability lease refresh duration, period after which the lease will be refreshed | -| metric | ha_lease_timeout | duration | 1m | High availability lease timeout duration, period after which the lease will be lost in case it wasn't refreshed | -| metric | default_retention_period | duration | 90d | Retention period for metric data, all data older than this period will be dropped | +| metrics | default_chunk_interval | duration | 8h | Chunk interval used to create hypertable chunks that store the metric data | +| metrics | compress_data | bool | true | Boolean setting to turn on or off compression of metric data | +| metrics | ha_lease_refresh | duration | 10s | High availability lease refresh duration, period after which the lease will be refreshed | +| metrics | ha_lease_timeout | duration | 1m | High availability lease timeout duration, period after which the lease will be lost in case it wasn't refreshed | +| metrics | default_retention_period | duration | 90d | Retention period for metric data, all data older than this period will be dropped | | traces | default_retention_period | duration | 90d | Retention period for tracing data, all data older than this period will be dropped | diff --git a/pkg/dataset/config.go b/pkg/dataset/config.go index 634ef9f1dd..f61c7f8b06 100644 --- a/pkg/dataset/config.go +++ b/pkg/dataset/config.go @@ -36,22 +36,22 @@ var ( // Config represents a dataset config. type Config struct { - Metrics `yaml:"metrics"` - Traces `yaml:"traces"` + Metrics + Traces } // Metrics contains dataset configuration options for metrics data. type Metrics struct { - ChunkInterval DayDuration `yaml:"default_chunk_interval"` - Compression *bool `yaml:"compress_data"` // Using pointer to check if the the value was set. - HALeaseRefresh DayDuration `yaml:"ha_lease_refresh"` - HALeaseTimeout DayDuration `yaml:"ha_lease_timeout"` - RetentionPeriod DayDuration `yaml:"default_retention_period"` + ChunkInterval DayDuration `mapstructure:"default_chunk_interval" yaml:"default_chunk_interval"` + Compression *bool `mapstructure:"compress_data" yaml:"compress_data"` // Using pointer to check if the the value was set. + HALeaseRefresh DayDuration `mapstructure:"ha_lease_refresh" yaml:"ha_lease_refresh"` + HALeaseTimeout DayDuration `mapstructure:"ha_lease_timeout" yaml:"ha_lease_timeout"` + RetentionPeriod DayDuration `mapstructure:"default_retention_period" yaml:"default_retention_period"` } // Traces contains dataset configuration options for traces data. type Traces struct { - RetentionPeriod DayDuration `yaml:"default_retention_period"` + RetentionPeriod DayDuration `mapstructure:"default_retention_period" yaml:"default_retention_period"` } // NewConfig creates a new dataset config based on the configuration YAML contents. diff --git a/pkg/dataset/duration.go b/pkg/dataset/duration.go index 82c9a5ae04..128e41e4fb 100644 --- a/pkg/dataset/duration.go +++ b/pkg/dataset/duration.go @@ -5,8 +5,11 @@ package dataset import ( "fmt" + "reflect" "strings" "time" + + "github.com/mitchellh/mapstructure" ) const ( @@ -67,3 +70,28 @@ func handleDays(s []byte) (time.Duration, error) { func (d DayDuration) String() string { return time.Duration(d).String() } + +// StringToDayDurationHookFunc returns a mapstructure.DecodeHookFunc that +// converts strings to DayDuration. +func StringToDayDurationHookFunc() mapstructure.DecodeHookFunc { + return func( + f reflect.Type, + t reflect.Type, + data interface{}) (interface{}, error) { + if f.Kind() != reflect.String { + return data, nil + } + + var d DayDuration + + if t != reflect.TypeOf(d) { + return data, nil + } + + err := d.UnmarshalText([]byte(data.(string))) + if err != nil { + return nil, err + } + return DayDuration(d), nil + } +} diff --git a/pkg/runner/client.go b/pkg/runner/client.go index 63dbd49f4c..3cf3f3601b 100644 --- a/pkg/runner/client.go +++ b/pkg/runner/client.go @@ -165,11 +165,13 @@ func CreateClient(r prometheus.Registerer, cfg *Config) (*pgclient.Client, error cfg.APICfg.MultiTenancy = multiTenancy } - if cfg.DatasetConfig != "" { - err = ApplyDatasetConfig(conn, cfg.DatasetConfig) - if err != nil { - return nil, fmt.Errorf("error applying dataset configuration: %w", err) - } + if (cfg.DatasetCfg != dataset.Config{}) { + err = cfg.DatasetCfg.Apply(conn) + } else if cfg.DatasetConfig != "" { + err = applyDatasetConfig(conn, cfg.DatasetConfig) + } + if err != nil { + return nil, fmt.Errorf("error applying dataset configuration: %w", err) } // client has to be initiated after migrate since migrate @@ -226,7 +228,7 @@ func isBGWLessThanDBs(conn *pgx.Conn) (bool, error) { return false, nil } -func ApplyDatasetConfig(conn *pgx.Conn, cfgFilename string) error { +func applyDatasetConfig(conn *pgx.Conn, cfgFilename string) error { cfg, err := dataset.NewConfig(cfgFilename) if err != nil { return err diff --git a/pkg/runner/config_parser.go b/pkg/runner/config_parser.go new file mode 100644 index 0000000000..deecce9fdd --- /dev/null +++ b/pkg/runner/config_parser.go @@ -0,0 +1,292 @@ +// This file and its contents are licensed under the Apache License 2.0. +// Please see the included NOTICE for copyright information and +// LICENSE for a copy of the license. +package runner + +import ( + "errors" + "flag" + "fmt" + "io/fs" + "strings" + + "github.com/mitchellh/mapstructure" + "github.com/spf13/viper" + "github.com/timescale/promscale/pkg/dataset" +) + +// unmarshalRule defines that the subtree located on the `key` of the Viper +// config should be unmarshal into the `target`. +type unmarshalRule struct { + key string + target interface{} +} + +type parserOption func(*parserConfig) + +type parserConfig struct { + ignoreUndefined bool + unmarshalRules []unmarshalRule +} + +// parse the flags in the flag set from the provided (presumably commandline) +// args. +// +// This method replicates the behaviour we had with `ff.Parse`, with the +// difference that this one uses viper to load the yaml configuration file. +// +// By design the ff package doesn't allow flag subsets or nested maps inside +// the config yaml and all config options must be explicitely defined as +// `flag.Flag`. This limits the ability to specify more dynamic config options +// like lists or maps of user defined rules (Ex: metric rollups). +// +// Viper provides the flexibility we require for parsing the config file, and +// unmarshaling directly into more complex datastructures. To define how to +// unmarshal a subtree of the configuration into a datastructure via +// unmarshalRule: +// +// ``` +// withUnmarshalRules( +// []unmarshalRule{{"startup.dataset", &dataset.Config}} +// ) +// ``` +// +// Given a config file that contains the subtree: +// +// ``` +// startup: +// dataset: +// metrics: +// ... +// traces: +// ... +// ``` +// +// Viper can access a nested field by passing a `.` delimited path of keys. +// This change also allows us to format our configuration files to use +// yaml mappings instead of long key names with `.`, thus maintaining +// backwards compatibility: +// +// ``` +// web.listen-address: localhost:9201 +// web.auth.password: my-password +// web.auth.username: promscale +// ``` +// +// Can be rewritten as: +// +// ``` +// web: +// listen-address: localhost:9201 +// auth: +// password: my-password +// username: promscale +// ``` +// +// In case of conflict the variable that matches the delimited key path will +// take precedence (https://github.com/spf13/viper#accessing-nested-keys). In +// the following example the returned value would be `localhost:9201`: +// +// ``` +// web.listen-address: localhost:9201 +// web: +// listen-address: htto://not.going.to.be.used:9201 +// ``` +// +// Migrating our codebase to take advantage of all the features of Viper will +// require some extra effort, and might not bring extra benefits since we +// already consolidate commandline arguments with environment variables. This +// approach gives us the features we need without the need of extensive +// modification. +func parse(fSet *flag.FlagSet, args []string, options ...parserOption) error { + + cfg := getParserConfig(options) + + // First priority: commandline flags (explicit user preference). + if err := fSet.Parse(args); err != nil { + return fmt.Errorf("error parsing commandline args: %w", err) + } + + configFile, err := configFileNameFromFlags(fSet) + + // This error won't be triggered unless the line that defines the config + // flag is deleted. + if err != nil { + return err + } + + v, err := newViperConfig(configFile) + + if err != nil { + // If the file can't be found, we don't return the error. This is the + // same behaviour we had with `ff.WithAllowMissingConfigFile(true)`. + var e *fs.PathError + if errors.As(err, &e) { + return nil + } + return fmt.Errorf("couldn't load config file %s: %w", configFile, err) + } + + if !cfg.ignoreUndefined { + err := checkUndefinedFlags(v, fSet, cfg.unmarshalRules) + if err != nil { + return err + } + } + + err = setFlagsWithConfigFileValues(fSet, v) + + if err != nil { + return fmt.Errorf( + "Couldn't apply the configuration from file %s: %w", + configFile, + err, + ) + } + + err = applyUnmarshalRules(v, cfg.unmarshalRules) + + return err +} + +// withIgnoreUndefined tells parse to ignore undefined flags that it encounters +// in config files. By default, if parse encounters an undefined flag in a +// config file, it will return an error. Note that this setting does not apply +// to undefined flags passed as arguments. +func withIgnoreUndefined(ignore bool) parserOption { + return func(c *parserConfig) { + c.ignoreUndefined = ignore + } +} + +func withUnmarshalRules(r []unmarshalRule) parserOption { + return func(c *parserConfig) { + c.unmarshalRules = r + } +} +func newViperConfig(configFile string) (*viper.Viper, error) { + v := viper.New() + v.SetConfigFile(configFile) + v.SetConfigType("yaml") + err := v.ReadInConfig() + return v, err +} + +func configFileNameFromFlags(fSet *flag.FlagSet) (string, error) { + f := fSet.Lookup(configFileFlagName) + if f == nil { + return "", fmt.Errorf("config file flag not defined") + } + + return f.Value.String(), nil +} + +func getFlagsThatHaveBeenSet(fSet *flag.FlagSet) map[string]struct{} { + provided := map[string]struct{}{} + fSet.Visit(func(f *flag.Flag) { + provided[f.Name] = struct{}{} + }) + return provided +} + +// checkUndefinedFlags returns an error if it founds at least one key in the +// Viper registry that's neither defined in the FlagSet nor is part of an +// unmarshalRule. +func checkUndefinedFlags( + v *viper.Viper, + fSet *flag.FlagSet, + rules []unmarshalRule, +) error { + + for _, k := range v.AllKeys() { + // If it's part of a unmarshalRule we do nothing + isUnmarshalRule := false + for _, r := range rules { + if strings.HasPrefix(k, r.key) { + isUnmarshalRule = true + break + } + } + if isUnmarshalRule { + continue + } + + if f := fSet.Lookup(k); f == nil { + return fmt.Errorf("config file flag %q not defined in flag set", k) + } + } + return nil +} + +// setFlagsWithConfigFileValues sets the value of the unset flags from the +// FlagSet with the matching entries in the Viper registry. +// +// Viper contains the values from the config file. +// +// Only flags that have not been set are changed. This is to respect the +// precedence between env vars, command line args and config file. +func setFlagsWithConfigFileValues( + fSet *flag.FlagSet, + v *viper.Viper, +) error { + + // These are the flags that have matched against the given `args`. These + // have already consolidated commandline arguments (via FlagSet.Parse) and + // environment variables (via `pkg/util/util.go:ParseEnv`). + alreadySetFlags := getFlagsThatHaveBeenSet(fSet) + + var visitErr error + fSet.VisitAll(func(f *flag.Flag) { + if visitErr != nil { + return + } + if _, ok := alreadySetFlags[f.Name]; ok { + return + } + if !v.IsSet(f.Name) { + return + } + val := v.GetString(f.Name) + err := fSet.Set(f.Name, val) + if err != nil { + visitErr = err + } + }) + + return visitErr +} + +func applyUnmarshalRules(v *viper.Viper, unmarshalRules []unmarshalRule) error { + for _, rule := range unmarshalRules { + s := v.Sub(rule.key) + if s == nil { + continue + } + err := s.Unmarshal( + rule.target, + viper.DecodeHook( + mapstructure.ComposeDecodeHookFunc( + dataset.StringToDayDurationHookFunc(), + mapstructure.StringToTimeDurationHookFunc(), + mapstructure.StringToSliceHookFunc(","), + ), + ), + ) + if err != nil { + return fmt.Errorf( + "failed to apply unmarshal rule for config file item %s: %w", + rule.key, + err, + ) + } + } + return nil +} + +func getParserConfig(options []parserOption) parserConfig { + cfg := parserConfig{} + for _, option := range options { + option(&cfg) + } + return cfg +} diff --git a/pkg/runner/flags.go b/pkg/runner/flags.go index c1cb54b56a..34802fc991 100644 --- a/pkg/runner/flags.go +++ b/pkg/runner/flags.go @@ -11,10 +11,9 @@ import ( "strings" "time" - "github.com/peterbourgon/ff/v3" - "github.com/peterbourgon/ff/v3/ffyaml" "github.com/timescale/promscale/pkg/api" "github.com/timescale/promscale/pkg/auth" + "github.com/timescale/promscale/pkg/dataset" jaegerStore "github.com/timescale/promscale/pkg/jaeger/store" "github.com/timescale/promscale/pkg/limits" "github.com/timescale/promscale/pkg/log" @@ -44,6 +43,7 @@ type Config struct { VacuumCfg vacuum.Config ConfigFile string DatasetConfig string + DatasetCfg dataset.Config TLSCertFile string TLSKeyFile string ThroughputInterval time.Duration @@ -119,6 +119,8 @@ var ( } ) +const configFileFlagName = "config" + func ParseFlags(cfg *Config, args []string) (*Config, error) { var ( fs = flag.NewFlagSet(os.Args[0], flag.ContinueOnError) @@ -139,7 +141,7 @@ func ParseFlags(cfg *Config, args []string) (*Config, error) { rules.ParseFlags(fs, &cfg.RulesCfg) vacuum.ParseFlags(fs, &cfg.VacuumCfg) - fs.StringVar(&cfg.ConfigFile, "config", "config.yml", "YAML configuration file path for Promscale.") + fs.StringVar(&cfg.ConfigFile, configFileFlagName, "config.yml", "YAML configuration file path for Promscale.") fs.StringVar(&cfg.ListenAddr, "web.listen-address", ":9201", "Address to listen on for web endpoints.") fs.StringVar(&cfg.ThanosStoreAPIListenAddr, "thanos.store-api.server-address", "", "Address to listen on for Thanos Store API endpoints.") fs.StringVar(&cfg.TracingGRPCListenAddr, "tracing.otlp.server-address", ":9202", "GRPC server address to listen on for Jaeger and OTEL traces(DEPRECATED: use `tracing.grpc.server-address` instead).") //TODO: remove this flag at some point @@ -165,10 +167,14 @@ func ParseFlags(cfg *Config, args []string) (*Config, error) { return nil, fmt.Errorf("error parsing env variables: %w", err) } - if err := ff.Parse(fs, args, - ff.WithConfigFileFlag("config"), - ff.WithConfigFileParser(ffyaml.Parser), - ff.WithAllowMissingConfigFile(true), + unmarshalRules := []unmarshalRule{ + {"startup.dataset", &cfg.DatasetCfg}, + } + + if err := parse( + fs, + args, + withUnmarshalRules(unmarshalRules), ); err != nil { // We might be dealing with old flags whose usage needs to be logged. // TODO: remove handling of old flags in a future version @@ -284,12 +290,7 @@ func checkForRemovedConfigFlags(fs *flag.FlagSet, args []string) error { aliasFS.String("migrate", "true", "") addAliases(aliasFS, flagAliases) - if err := ff.Parse(aliasFS, args, - ff.WithConfigFileFlag("config"), - ff.WithConfigFileParser(ffyaml.Parser), - ff.WithIgnoreUndefined(true), - ff.WithAllowMissingConfigFile(true), - ); err != nil { + if err := parse(aliasFS, args, withIgnoreUndefined(true)); err != nil { // If are having trouble parsing the old flags, just ignore since there isn't much we can do. // Logging this error would just create confusion for the user. return nil @@ -325,6 +326,7 @@ func checkForRemovedConfigFlags(fs *flag.FlagSet, args []string) error { func handleOldFlags(oldFlag, newFlag string) { if oldFlag != "migrate" { log.Warn("msg", fmt.Sprintf("Update removed flag `%s` to new flag `%s`", oldFlag, newFlag)) + return } // Handling the special migrate flag diff --git a/pkg/runner/flags_test.go b/pkg/runner/flags_test.go index 8b525c9a02..f289cfc7ce 100644 --- a/pkg/runner/flags_test.go +++ b/pkg/runner/flags_test.go @@ -13,9 +13,12 @@ import ( "time" "github.com/stretchr/testify/require" + "github.com/timescale/promscale/pkg/dataset" ) func TestParseFlags(t *testing.T) { + // Clearing environment variables so they don't interfere with the test. + os.Clearenv() defaultConfig, err := ParseFlags(&Config{}, []string{}) if err != nil { t.Fatal("error occured on default config with no arguments") @@ -197,12 +200,8 @@ func TestParseFlags(t *testing.T) { for _, c := range testCases { t.Run(c.name, func(t *testing.T) { - // Clearing environment variables so they don't interfere with the test. - os.Clearenv() for name, value := range c.env { - if err := os.Setenv(name, value); err != nil { - t.Fatalf("unexpected error when setting env variable: name %s, value %s, error %s", name, value, err) - } + t.Setenv(name, value) } config, err := ParseFlags(&Config{}, c.args) if c.shouldError { @@ -242,10 +241,101 @@ func TestParseFlagsConfigPrecedence(t *testing.T) { result: func(c Config) Config { return c }, }, { - name: "Config file only", - configFileContents: "web.listen-address: localhost:9201", + name: "Config file only", + configFileContents: ` +web: + listen-address: localhost:9201 + auth: + password: my-password + username: promscale +`, + result: func(c Config) Config { + c.ListenAddr = "localhost:9201" + c.AuthConfig.BasicAuthUsername = "promscale" + c.AuthConfig.BasicAuthPassword = "my-password" + return c + }, + }, + { + name: "Config file with dataset", + configFileContents: ` +web: + listen-address: localhost:9201 + auth: + password: my-password + username: promscale +startup.dataset.config: | + metrics: + default_chunk_interval: 1h +startup: + dataset: + metrics: + default_chunk_interval: 1d + compress_data: false + ha_lease_refresh: 2d + ha_lease_timeout: 3d + default_retention_period: 4d + traces: + default_retention_period: 5d`, + result: func(c Config) Config { + c.ListenAddr = "localhost:9201" + c.AuthConfig.BasicAuthUsername = "promscale" + c.AuthConfig.BasicAuthPassword = "my-password" + c.DatasetCfg.Metrics.ChunkInterval = dataset.DayDuration(24 * time.Hour) + c.DatasetCfg.Metrics.Compression = func(b bool) *bool { return &b }(false) + c.DatasetCfg.Metrics.HALeaseRefresh = dataset.DayDuration(24 * time.Hour * 2) + c.DatasetCfg.Metrics.HALeaseTimeout = dataset.DayDuration(24 * time.Hour * 3) + c.DatasetCfg.Metrics.RetentionPeriod = dataset.DayDuration(24 * time.Hour * 4) + c.DatasetCfg.Traces.RetentionPeriod = dataset.DayDuration(24 * time.Hour * 5) + c.DatasetConfig = "metrics:\n default_chunk_interval: 1h\n" + return c + }, + }, + { + name: "Config file only with flat map", + configFileContents: ` +web.listen-address: localhost:9201 +web.auth.password: my-password +web.auth.username: promscale +`, + result: func(c Config) Config { + c.ListenAddr = "localhost:9201" + c.AuthConfig.BasicAuthUsername = "promscale" + c.AuthConfig.BasicAuthPassword = "my-password" + return c + }, + }, + { + name: "Config file only with semi flat map", + configFileContents: ` +web: + listen-address: localhost:9201 + auth.password: my-password + auth.username: promscale +`, result: func(c Config) Config { c.ListenAddr = "localhost:9201" + c.AuthConfig.BasicAuthUsername = "promscale" + c.AuthConfig.BasicAuthPassword = "my-password" + return c + }, + }, + { + name: "Config file only flat takes precendence over yaml mapping", + configFileContents: ` +web.listen-address: localhost:9242 +web.auth.password: my-password-42 +web.auth.username: promscale-42 +web: + listen-address: localhost:9201 + auth: + password: my-password + username: promscale +`, + result: func(c Config) Config { + c.ListenAddr = "localhost:9242" + c.AuthConfig.BasicAuthUsername = "promscale-42" + c.AuthConfig.BasicAuthPassword = "my-password-42" return c }, }, @@ -287,12 +377,8 @@ func TestParseFlagsConfigPrecedence(t *testing.T) { for _, c := range testCases { t.Run(c.name, func(t *testing.T) { - // Clearing environment variables so they don't interfere with the test. - os.Clearenv() for name, value := range c.env { - if err := os.Setenv(name, value); err != nil { - t.Fatalf("unexpected error when setting env variable: name %s, value %s, error %s", name, value, err) - } + t.Setenv(name, value) } var configFilePath string @@ -404,9 +490,7 @@ func TestRemovedFlagUsage(t *testing.T) { // Clearing environment variables so they don't interfere with the test. os.Clearenv() for name, value := range c.env { - if err := os.Setenv(name, value); err != nil { - t.Fatalf("unexpected error when setting env variable: name %s, value %s, error %s", name, value, err) - } + t.Setenv(name, value) } if c.configFileContents != "" { diff --git a/pkg/tests/end_to_end_tests/config_dataset_test.go b/pkg/tests/end_to_end_tests/config_dataset_test.go index 2468062add..02592ef79c 100644 --- a/pkg/tests/end_to_end_tests/config_dataset_test.go +++ b/pkg/tests/end_to_end_tests/config_dataset_test.go @@ -50,8 +50,7 @@ func TestDatasetConfigApply(t *testing.T) { require.Equal(t, 10*24*time.Hour, getTracesDefaultRetention(t, pgxConn)) // Set to default if chunk interval is not specified. - cfg, err = dataset.NewConfig("") - require.NoError(t, err) + cfg = dataset.Config{} err = cfg.Apply(pgxConn) require.NoError(t, err)