Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix configuration via environment variables for optional parameters #3702

Merged
merged 3 commits into from
Jun 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion services/horizon/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
23 changes: 21 additions & 2 deletions support/config/config_option.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"go/types"
stdLog "log"
"net/url"
"os"
"strings"
"time"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -166,19 +168,36 @@ 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 {
*key = nil
}
}

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 {
Expand Down
66 changes: 66 additions & 0 deletions support/config/config_option_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down