From 3d4896ddcf34f56f83b000eb1af689060f68a1f1 Mon Sep 17 00:00:00 2001 From: Urvi Date: Fri, 18 Aug 2023 12:34:39 -0700 Subject: [PATCH] Add deprecation warning for --db-url commandline parameter. --- services/horizon/internal/flags.go | 17 +++++++++++++- services/horizon/internal/flags_test.go | 30 +++++++++++++++++++++++++ support/config/config_option.go | 21 ++++++++++++----- 3 files changed, 62 insertions(+), 6 deletions(-) diff --git a/services/horizon/internal/flags.go b/services/horizon/internal/flags.go index 34d9314758..24a7c3f3e9 100644 --- a/services/horizon/internal/flags.go +++ b/services/horizon/internal/flags.go @@ -62,6 +62,10 @@ const ( StellarPubnet = "pubnet" // StellarTestnet is a constant representing the Stellar test network StellarTestnet = "testnet" + + dbUrlDeprecationWarning = "DEPRECATION WARNING: --db-url is deprecated and will be removed in a future" + + " Horizon release. Please use the DATABASE_URL environment variable to configure Horizon's" + + " PostgreSQL database connection" ) // validateBothOrNeither ensures that both options are provided, if either is provided. @@ -132,7 +136,18 @@ func Flags() (*Config, support.ConfigOptions) { ConfigKey: &config.DatabaseURL, OptType: types.String, Required: true, - Usage: "horizon postgres database to connect with", + Usage: dbUrlDeprecationWarning, + CustomSetValue: func(co *support.ConfigOption) error { + if val := viper.GetString(DatabaseURLFlagName); val != "" { + if co.IsSetViaCommandline() { + stdLog.Printf(dbUrlDeprecationWarning) + + } + config.DatabaseURL = val + } + return nil + }, + EnvOnly: false, // set this to true when removing this option from the commandline }, &support.ConfigOption{ Name: "ro-database-url", diff --git a/services/horizon/internal/flags_test.go b/services/horizon/internal/flags_test.go index 38e0c16bda..592e9d1ff9 100644 --- a/services/horizon/internal/flags_test.go +++ b/services/horizon/internal/flags_test.go @@ -1,9 +1,13 @@ package horizon import ( + "bytes" "fmt" + "io" + stdLog "log" "testing" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -131,3 +135,29 @@ func Test_createCaptiveCoreConfig(t *testing.T) { }) } } + +func TestDatabaseUrlDeprecationWarning(t *testing.T) { + origWriter := stdLog.Writer() + defer stdLog.SetOutput(origWriter) + + config, flags := Flags() + horizonCmd := &cobra.Command{ + Use: "horizon", + RunE: func(cmd *cobra.Command, args []string) error { + err := ApplyFlags(config, flags, ApplyOptions{RequireCaptiveCoreConfig: true, AlwaysIngest: false}) + return err + }, + } + + var writer io.Writer = &bytes.Buffer{} + stdLog.SetOutput(writer) + horizonCmd.SetArgs([]string{"--" + DatabaseURLFlagName + "=testUrl", + "--" + IngestFlagName + "=false", + "--" + EnableCaptiveCoreIngestionFlagName + "=false", + "--" + StellarCoreURLFlagName + "=testUrl", + }) + _ = flags.Init(horizonCmd) + err := horizonCmd.Execute() + assert.NoError(t, err) + assert.Contains(t, writer.(*bytes.Buffer).String(), dbUrlDeprecationWarning) +} diff --git a/support/config/config_option.go b/support/config/config_option.go index 29fa77af3b..28006d0e5f 100644 --- a/support/config/config_option.go +++ b/support/config/config_option.go @@ -70,6 +70,7 @@ type ConfigOption struct { ConfigKey interface{} // Pointer to the final key in the linked Config struct flag *pflag.Flag // The persistent flag that the config option is attached to Hidden bool // Indicates whether to hide the flag from --help output + EnvOnly bool // Indicates the option is supported via environment variable only (no commandline) } // Init handles initialisation steps, including configuring and binding the env variable name. @@ -79,20 +80,25 @@ func (co *ConfigOption) Init(cmd *cobra.Command) error { if co.EnvVar == "" { co.EnvVar = strutils.KebabToConstantCase(co.Name) } - // Initialise and bind the persistent flags - return co.setFlag(cmd) + if !co.EnvOnly { + // Initialise and bind the persistent flags + return co.setFlag(cmd) + } + return nil } // SetDeprecated Hides the deprecated flag from --help output func (co *ConfigOption) SetDeprecated(cmd *cobra.Command) { - if co.Hidden { + if !co.EnvOnly && co.Hidden { co.flag.Hidden = true } } // Bind binds the config option to viper. func (co *ConfigOption) Bind() { - viper.BindPFlag(co.Name, co.flag) + if !co.EnvOnly { + viper.BindPFlag(co.Name, co.flag) + } viper.BindEnv(co.Name, co.EnvVar) } @@ -237,7 +243,12 @@ 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] + return (co.flag != nil && co.flag.Changed) || envVars[co.EnvVar] +} + +// IsSetViaCommandline returns true if the config option was set via commandline +func (co *ConfigOption) IsSetViaCommandline() bool { + return co.flag != nil && co.flag.Value.String() != "" } // SetOptionalString converts a command line uint to a *string where the nil