Fix configuration via environment variables for optional parameters (#…

…3702)

This PR fixes a bug where the configuration variables listed below were ignored if they were configured via environment variables instead of command line arguments:

CAPTIVE_CORE_LOG_PATH
CAPTIVE_CORE_PEER_PORT
CAPTIVE_CORE_HTTP_PORT

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
 

support/config/config_option.go

@@ -5,6 +5,8 @@ import (
 	"go/types"
 	stdLog "log"
 	"net/url"
+	"os"
+	"strings"
 	"time"
 
 	"github.com/spf13/cobra"
@@ -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 {

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) {