diff --git a/services/horizon/CHANGELOG.md b/services/horizon/CHANGELOG.md index c30301f711..2cee3f6dd7 100644 --- a/services/horizon/CHANGELOG.md +++ b/services/horizon/CHANGELOG.md @@ -21,7 +21,7 @@ file. This project adheres to [Semantic Versioning](http://semver.org/). * Fix bug in `horizon db reingest range` command, which would throw a duplicate entry conflict error from the DB. ([3661](https://github.com/stellar/go/pull/3661)). * Fix bug in DB metrics preventing Horizon from starting when read-only replica middleware is enabled. ([3668](https://github.com/stellar/go/pull/3668)). * Fix bug in the value of `route` in the logs for rate-limited requests (previously it was set to `undefined`). ([3658](https://github.com/stellar/go/pull/3658)). - +* Fix bug where the configuration for `CAPTIVE_CORE_LOG_PATH`, `CAPTIVE_CORE_PEER_PORT`, and `CAPTIVE_CORE_HTTP_PORT` were ignored if they were configured via environment variables instead of command line arguments. ([3702](https://github.com/stellar/go/pull/3702)). ## v2.4.0 diff --git a/support/config/config_option.go b/support/config/config_option.go index 7a8300966d..90b2480056 100644 --- a/support/config/config_option.go +++ b/support/config/config_option.go @@ -5,6 +5,8 @@ import ( "go/types" stdLog "log" "net/url" + "os" + "strings" "time" "github.com/spf13/cobra" @@ -166,7 +168,7 @@ func SetURL(co *ConfigOption) { // value indicates the flag was not explicitly set func SetOptionalUint(co *ConfigOption) { key := co.ConfigKey.(**uint) - if co.flag.Changed { + if isExplicitlySet(co) { *key = new(uint) **key = uint(viper.GetInt(co.Name)) } else { @@ -174,11 +176,28 @@ func SetOptionalUint(co *ConfigOption) { } } +func parseEnvVars(entries []string) map[string]bool { + set := map[string]bool{} + for _, entry := range entries { + key := strings.Split(entry, "=")[0] + set[key] = true + } + return set +} + +var envVars = parseEnvVars(os.Environ()) + +func isExplicitlySet(co *ConfigOption) bool { + // co.flag.Changed is only set to true when the configuration is set via command line parameter. + // In the case where a variable is configured via environment variable we need to check envVars. + return co.flag.Changed || envVars[co.EnvVar] +} + // SetOptionalString converts a command line uint to a *string where the nil // value indicates the flag was not explicitly set func SetOptionalString(co *ConfigOption) { key := co.ConfigKey.(**string) - if co.flag.Changed { + if isExplicitlySet(co) { *key = new(string) **key = viper.GetString(co.Name) } else { diff --git a/support/config/config_option_test.go b/support/config/config_option_test.go index a19e718907..30be01424b 100644 --- a/support/config/config_option_test.go +++ b/support/config/config_option_test.go @@ -71,6 +71,72 @@ func TestConfigOption_optionalFlags_set(t *testing.T) { assert.Equal(t, uint(6), *optUint) } +// Test that optional flags are set to non nil values when they are configured explicitly. +func TestConfigOption_optionalFlags_env_set_empty(t *testing.T) { + var optUint *uint + var optString *string + configOpts := ConfigOptions{ + {Name: "uint", OptType: types.Uint, ConfigKey: &optUint, CustomSetValue: SetOptionalUint, FlagDefault: uint(0)}, + {Name: "string", OptType: types.String, ConfigKey: &optString, CustomSetValue: SetOptionalString}, + } + cmd := &cobra.Command{ + Use: "doathing", + Run: func(_ *cobra.Command, _ []string) { + configOpts.Require() + configOpts.SetValues() + }, + } + configOpts.Init(cmd) + + prev := envVars + envVars = map[string]bool{ + "STRING": true, + } + defer func() { + envVars = prev + }() + + cmd.Execute() + assert.Equal(t, "", *optString) + assert.Equal(t, (*uint)(nil), optUint) +} + +// Test that optional flags are set to non nil values when they are configured explicitly. +func TestConfigOption_optionalFlags_env_set(t *testing.T) { + var optUint *uint + var optString *string + configOpts := ConfigOptions{ + {Name: "uint", OptType: types.Uint, ConfigKey: &optUint, CustomSetValue: SetOptionalUint, FlagDefault: uint(0)}, + {Name: "string", OptType: types.String, ConfigKey: &optString, CustomSetValue: SetOptionalString}, + } + cmd := &cobra.Command{ + Use: "doathing", + Run: func(_ *cobra.Command, _ []string) { + configOpts.Require() + configOpts.SetValues() + }, + } + configOpts.Init(cmd) + + prev := envVars + envVars = map[string]bool{ + "STRING": true, + "UINT": true, + } + defer func() { + envVars = prev + }() + + defer os.Setenv("STRING", os.Getenv("STRING")) + defer os.Setenv("UINT", os.Getenv("UINT")) + os.Setenv("STRING", "str") + os.Setenv("UINT", "6") + + cmd.Execute() + assert.Equal(t, "str", *optString) + assert.Equal(t, uint(6), *optUint) +} + // Test that when there are no args the defaults in the config options are // used. func TestConfigOption_getSimpleValue_defaults(t *testing.T) {