From 43b55ca4efa7cdf9c367dfe8bb476aba3830dc58 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Wed, 13 Sep 2023 16:30:37 -0400 Subject: [PATCH 01/18] Add deprecated warning --- services/horizon/cmd/root.go | 6 +++++- services/horizon/internal/flags.go | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/services/horizon/cmd/root.go b/services/horizon/cmd/root.go index af63675d0c..ee8c18db43 100644 --- a/services/horizon/cmd/root.go +++ b/services/horizon/cmd/root.go @@ -16,7 +16,11 @@ var ( Short: "client-facing api server for the Stellar network", SilenceErrors: true, SilenceUsage: true, - Long: "Client-facing API server for the Stellar network. It acts as the interface between Stellar Core and applications that want to access the Stellar network. It allows you to submit transactions to the network, check the status of accounts, subscribe to event streams and more.", + Long: "Client-facing API server for the Stellar network. It acts as the interface between Stellar Core " + + "and applications that want to access the Stellar network. It allows you to submit transactions to the " + + "network, check the status of accounts, subscribe to event streams and more.\n" + + "DEPRECATED - the use of command-line flags has been deprecated in favor of environment variables. Please" + + "consult our Configuring section in the developer documentation on how to use them - https://developers.stellar.org/docs/run-api-server/configuring", RunE: func(cmd *cobra.Command, args []string) error { app, err := horizon.NewAppFromFlags(config, flags) if err != nil { diff --git a/services/horizon/internal/flags.go b/services/horizon/internal/flags.go index d2cc055ef8..b24ef101c0 100644 --- a/services/horizon/internal/flags.go +++ b/services/horizon/internal/flags.go @@ -642,6 +642,8 @@ func Flags() (*Config, support.ConfigOptions) { // NewAppFromFlags constructs a new Horizon App from the given command line flags func NewAppFromFlags(config *Config, flags support.ConfigOptions) (*App, error) { + stdLog.Println("DEPRECATED - the use of command-line flags has been deprecated in favor of environment variables. " + + "Please consult our Configuring section in the developer documentation on how to use them - https://developers.stellar.org/docs/run-api-server/configuring") err := ApplyFlags(config, flags, ApplyOptions{RequireCaptiveCoreConfig: true, AlwaysIngest: false}) if err != nil { return nil, err From 12bf9e7aafb3c8cbd844942fcc0b3781e51b4c85 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Wed, 13 Sep 2023 16:56:26 -0400 Subject: [PATCH 02/18] Add deprecated warning tests --- .../internal/integration/parameters_test.go | 94 +++++++++++++------ 1 file changed, 64 insertions(+), 30 deletions(-) diff --git a/services/horizon/internal/integration/parameters_test.go b/services/horizon/internal/integration/parameters_test.go index c5a990e73c..0958063ae4 100644 --- a/services/horizon/internal/integration/parameters_test.go +++ b/services/horizon/internal/integration/parameters_test.go @@ -450,40 +450,74 @@ func TestDisableTxSub(t *testing.T) { }) } -func TestDeprecatedOutputForIngestionFilteringFlag(t *testing.T) { - originalStderr := os.Stderr - r, w, _ := os.Pipe() - os.Stderr = w - stdLog.SetOutput(os.Stderr) +func TestDeprecatedOutputs(t *testing.T) { + t.Run("deprecated output for ingestion filtering", func(t *testing.T) { + originalStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + stdLog.SetOutput(os.Stderr) - testConfig := integration.GetTestConfig() - testConfig.HorizonIngestParameters = map[string]string{"exp-enable-ingestion-filtering": "false"} - test := integration.NewTest(t, *testConfig) - err := test.StartHorizon() - assert.NoError(t, err) - test.WaitForHorizon() + testConfig := integration.GetTestConfig() + testConfig.HorizonIngestParameters = map[string]string{"exp-enable-ingestion-filtering": "false"} + test := integration.NewTest(t, *testConfig) + err := test.StartHorizon() + assert.NoError(t, err) + test.WaitForHorizon() - // Use a wait group to wait for the goroutine to finish before proceeding - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - if err := w.Close(); err != nil { - t.Errorf("Failed to close Stdout") - return - } - }() + // Use a wait group to wait for the goroutine to finish before proceeding + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + if err := w.Close(); err != nil { + t.Errorf("Failed to close Stdout") + return + } + }() + + outputBytes, _ := io.ReadAll(r) + wg.Wait() // Wait for the goroutine to finish before proceeding + _ = r.Close() + os.Stderr = originalStderr + + assert.Contains(t, string(outputBytes), "DEPRECATED - No ingestion filter rules are defined by default, which equates to "+ + "no filtering of historical data. If you have never added filter rules to this deployment, then nothing further needed. "+ + "If you have defined ingestion filter rules prior but disabled filtering overall by setting this flag disabled with "+ + "--exp-enable-ingestion-filtering=false, then you should now delete the filter rules using the Horizon Admin API to achieve "+ + "the same no-filtering result. Remove usage of this flag in all cases.") + }) + t.Run("deprecated output for command-line flags", func(t *testing.T) { + originalStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + stdLog.SetOutput(os.Stderr) + + testConfig := integration.GetTestConfig() + test := integration.NewTest(t, *testConfig) + err := test.StartHorizon() + assert.NoError(t, err) + test.WaitForHorizon() - outputBytes, _ := io.ReadAll(r) - wg.Wait() // Wait for the goroutine to finish before proceeding - _ = r.Close() - os.Stderr = originalStderr + // Use a wait group to wait for the goroutine to finish before proceeding + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + if err := w.Close(); err != nil { + t.Errorf("Failed to close Stdout") + return + } + }() + + outputBytes, _ := io.ReadAll(r) + wg.Wait() // Wait for the goroutine to finish before proceeding + _ = r.Close() + os.Stderr = originalStderr - assert.Contains(t, string(outputBytes), "DEPRECATED - No ingestion filter rules are defined by default, which equates to "+ - "no filtering of historical data. If you have never added filter rules to this deployment, then nothing further needed. "+ - "If you have defined ingestion filter rules prior but disabled filtering overall by setting this flag disabled with "+ - "--exp-enable-ingestion-filtering=false, then you should now delete the filter rules using the Horizon Admin API to achieve "+ - "the same no-filtering result. Remove usage of this flag in all cases.") + assert.Contains(t, string(outputBytes), "DEPRECATED - the use of command-line flags has been deprecated "+ + "in favor of environment variables. Please consult our Configuring section in the developer documentation on "+ + "how to use them - https://developers.stellar.org/docs/run-api-server/configuring") + }) } func TestHelpOutput(t *testing.T) { From ff8eb7c9aa15c9fc1a524818fa620478b363e381 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Mon, 2 Oct 2023 12:24:22 -0400 Subject: [PATCH 03/18] add test for env vars --- services/horizon/internal/environment.go | 65 +++++++++++++++++++ services/horizon/internal/flags_test.go | 54 +++++++++++++++ .../internal/integration/parameters_test.go | 6 -- 3 files changed, 119 insertions(+), 6 deletions(-) create mode 100644 services/horizon/internal/environment.go diff --git a/services/horizon/internal/environment.go b/services/horizon/internal/environment.go new file mode 100644 index 0000000000..ab1193d680 --- /dev/null +++ b/services/horizon/internal/environment.go @@ -0,0 +1,65 @@ +package horizon + +import ( + "fmt" + "os" + "strings" + + "github.com/stellar/go/support/errors" +) + +type EnvironmentManager struct { + oldEnvironment, newEnvironment map[string]string +} + +func NewEnvironmentManager() *EnvironmentManager { + env := &EnvironmentManager{} + env.oldEnvironment = make(map[string]string) + env.newEnvironment = make(map[string]string) + return env +} + +func (envManager *EnvironmentManager) InitializeEnvironmentVariables(environmentVars map[string]string) error { + var env strings.Builder + for key, value := range environmentVars { + env.WriteString(fmt.Sprintf("%s=%s ", key, value)) + } + + // prepare env + for key, value := range environmentVars { + innerErr := envManager.Add(key, value) + if innerErr != nil { + return errors.Wrap(innerErr, fmt.Sprintf( + "failed to set envvar (%s=%s)", key, value)) + } + } + return nil +} + +// Add sets a new environment variable, saving the original value (if any). +func (envManager *EnvironmentManager) Add(key, value string) error { + // If someone pushes an environmental variable more than once, we don't want + // to lose the *original* value, so we're being careful here. + if _, ok := envManager.newEnvironment[key]; !ok { + if oldValue, ok := os.LookupEnv(key); ok { + envManager.oldEnvironment[key] = oldValue + } + } + + envManager.newEnvironment[key] = value + return os.Setenv(key, value) +} + +// Restore restores the environment prior to any modifications. +// +// You should probably use this alongside `defer` to ensure the global +// environment isn't modified for longer than you intend. +func (envManager *EnvironmentManager) Restore() { + for key := range envManager.newEnvironment { + if oldValue, ok := envManager.oldEnvironment[key]; ok { + os.Setenv(key, oldValue) + } else { + os.Unsetenv(key) + } + } +} diff --git a/services/horizon/internal/flags_test.go b/services/horizon/internal/flags_test.go index 38e0c16bda..5fc42d898f 100644 --- a/services/horizon/internal/flags_test.go +++ b/services/horizon/internal/flags_test.go @@ -2,6 +2,8 @@ package horizon import ( "fmt" + "github.com/spf13/cobra" + "os" "testing" "github.com/stretchr/testify/assert" @@ -131,3 +133,55 @@ func Test_createCaptiveCoreConfig(t *testing.T) { }) } } + +func TestEnvironmentVariables(t *testing.T) { + environmentVars := map[string]string{ + "INGEST": "false", + "HISTORY_ARCHIVE_URLS": "http://localhost:1570", + "DATABASE_URL": "postgres://postgres@localhost/test_332cb65e6b00?sslmode=disable&timezone=UTC", + "STELLAR_CORE_URL": "http://localhost:11626", + "NETWORK_PASSPHRASE": "Standalone Network ; February 2017", + "APPLY_MIGRATIONS": "true", + "ENABLE_CAPTIVE_CORE_INGESTION": "false", + "CHECKPOINT_FREQUENCY": "8", + "MAX_DB_CONNECTIONS": "50", + "ADMIN_PORT": "6060", + "PORT": "8001", + "CAPTIVE_CORE_BINARY_PATH": os.Getenv("HORIZON_INTEGRATION_TESTS_CAPTIVE_CORE_BIN"), + "CAPTIVE_CORE_CONFIG_PATH": "../docker/captive-core-classic-integration-tests.cfg", + "CAPTIVE_CORE_USE_DB": "true", + } + envManager := NewEnvironmentManager() + if err := envManager.InitializeEnvironmentVariables(environmentVars); err != nil { + fmt.Println(err) + } + config, flags := Flags() + horizonCmd := &cobra.Command{ + Use: "horizon", + Short: "Client-facing api server for the Stellar network", + SilenceErrors: true, + SilenceUsage: true, + Long: "Client-facing API server for the Stellar network.", + } + if err := flags.Init(horizonCmd); err != nil { + fmt.Println(err) + } + if err := ApplyFlags(config, flags, ApplyOptions{RequireCaptiveCoreConfig: true, AlwaysIngest: false}); err != nil { + fmt.Println(err) + } + assert.Equal(t, config.Ingest, false) + assert.Equal(t, config.HistoryArchiveURLs, []string{"http://localhost:1570"}) + assert.Equal(t, config.DatabaseURL, "postgres://postgres@localhost/test_332cb65e6b00?sslmode=disable&timezone=UTC") + assert.Equal(t, config.StellarCoreURL, "http://localhost:11626") + assert.Equal(t, config.NetworkPassphrase, "Standalone Network ; February 2017") + assert.Equal(t, config.ApplyMigrations, true) + assert.Equal(t, config.EnableCaptiveCoreIngestion, false) + assert.Equal(t, config.CheckpointFrequency, uint32(8)) + assert.Equal(t, config.MaxDBConnections, 50) + assert.Equal(t, config.AdminPort, uint(6060)) + assert.Equal(t, config.Port, uint(8001)) + assert.Equal(t, config.CaptiveCoreBinaryPath, os.Getenv("HORIZON_INTEGRATION_TESTS_CAPTIVE_CORE_BIN")) + assert.Equal(t, config.CaptiveCoreConfigPath, "../docker/captive-core-classic-integration-tests.cfg") + assert.Equal(t, config.CaptiveCoreConfigUseDB, true) + envManager.Restore() +} diff --git a/services/horizon/internal/integration/parameters_test.go b/services/horizon/internal/integration/parameters_test.go index 0958063ae4..5917320000 100644 --- a/services/horizon/internal/integration/parameters_test.go +++ b/services/horizon/internal/integration/parameters_test.go @@ -431,12 +431,6 @@ func TestDisableTxSub(t *testing.T) { test.Shutdown() }) t.Run("horizon starts successfully when DISABLE_TX_SUB=true and INGEST=true", func(t *testing.T) { - //localParams := integration.MergeMaps(networkParamArgs, map[string]string{ - // //horizon.NetworkFlagName: "testnet", - // horizon.IngestFlagName: "true", - // horizon.DisableTxSubFlagName: "true", - // horizon.StellarCoreBinaryPathName: "/usr/bin/stellar-core", - //}) testConfig := integration.GetTestConfig() testConfig.HorizonIngestParameters = map[string]string{ "disable-tx-sub": "true", From 53d0f76c405c1064d54fa676c9144ffce8325f3b Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Tue, 3 Oct 2023 13:28:42 -0400 Subject: [PATCH 04/18] Dynamically print deprecated message --- services/horizon/internal/flags.go | 11 +++++++++-- .../horizon/internal/integration/parameters_test.go | 7 ++++--- support/config/config_option.go | 10 ++++++++++ 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/services/horizon/internal/flags.go b/services/horizon/internal/flags.go index b24ef101c0..3bfd342b61 100644 --- a/services/horizon/internal/flags.go +++ b/services/horizon/internal/flags.go @@ -642,8 +642,6 @@ func Flags() (*Config, support.ConfigOptions) { // NewAppFromFlags constructs a new Horizon App from the given command line flags func NewAppFromFlags(config *Config, flags support.ConfigOptions) (*App, error) { - stdLog.Println("DEPRECATED - the use of command-line flags has been deprecated in favor of environment variables. " + - "Please consult our Configuring section in the developer documentation on how to use them - https://developers.stellar.org/docs/run-api-server/configuring") err := ApplyFlags(config, flags, ApplyOptions{RequireCaptiveCoreConfig: true, AlwaysIngest: false}) if err != nil { return nil, err @@ -827,10 +825,19 @@ func setCaptiveCoreConfiguration(config *Config) error { // ApplyFlags applies the command line flags on the given Config instance func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOptions) error { + // Check if the user has passed any flags and if so, print a DEPRECATED warning message. + flagsPassedByUser := flags.GetAllFlagsPassedByUser() + if flagsPassedByUser != nil && len(flagsPassedByUser) > 0 { + result := fmt.Sprintf("DEPRECATED - the use of command-line flags: %s, has been deprecated in favor of environment variables. "+ + "Please consult our Configuring section in the developer documentation on how to use them - https://developers.stellar.org/docs/run-api-server/configuring", flagsPassedByUser) + stdLog.Println(result) + } + // Verify required options and load the config struct if err := flags.RequireE(); err != nil { return err } + if err := flags.SetValues(); err != nil { return err } diff --git a/services/horizon/internal/integration/parameters_test.go b/services/horizon/internal/integration/parameters_test.go index 5917320000..3c4d557616 100644 --- a/services/horizon/internal/integration/parameters_test.go +++ b/services/horizon/internal/integration/parameters_test.go @@ -508,9 +508,10 @@ func TestDeprecatedOutputs(t *testing.T) { _ = r.Close() os.Stderr = originalStderr - assert.Contains(t, string(outputBytes), "DEPRECATED - the use of command-line flags has been deprecated "+ - "in favor of environment variables. Please consult our Configuring section in the developer documentation on "+ - "how to use them - https://developers.stellar.org/docs/run-api-server/configuring") + assert.Contains(t, string(outputBytes), "DEPRECATED - the use of command-line flags: "+ + "[--db-url --captive-core-config-path --captive-core-use-db --enable-captive-core-ingestion --captive-core-http-port --captive-core-storage-path --stellar-core-db-url --stellar-core-url --history-archive-urls --port --admin-port --max-db-connections --per-hour-rate-limit --network-passphrase --ingest --apply-migrations --checkpoint-frequency], "+ + "has been deprecated in favor of environment variables. Please consult our Configuring section in the developer "+ + "documentation on how to use them - https://developers.stellar.org/docs/run-api-server/configuring") }) } diff --git a/support/config/config_option.go b/support/config/config_option.go index 29fa77af3b..c1acc825c9 100644 --- a/support/config/config_option.go +++ b/support/config/config_option.go @@ -58,6 +58,16 @@ func (cos ConfigOptions) SetValues() error { return nil } +func (cos ConfigOptions) GetAllFlagsPassedByUser() []string { + var flagsPassedByUser []string + for _, co := range cos { + if co.flag.Changed { + flagsPassedByUser = append(flagsPassedByUser, "--"+co.flag.Name) + } + } + return flagsPassedByUser +} + // ConfigOption is a complete description of the configuration of a command line option type ConfigOption struct { Name string // e.g. "db-url" From 2a58c41977d52d41d0af7ac0dff6ca48710906c6 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Tue, 3 Oct 2023 13:36:41 -0400 Subject: [PATCH 05/18] Update flags.go --- services/horizon/internal/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/horizon/internal/flags.go b/services/horizon/internal/flags.go index 3bfd342b61..2da8910203 100644 --- a/services/horizon/internal/flags.go +++ b/services/horizon/internal/flags.go @@ -827,7 +827,7 @@ func setCaptiveCoreConfiguration(config *Config) error { func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOptions) error { // Check if the user has passed any flags and if so, print a DEPRECATED warning message. flagsPassedByUser := flags.GetAllFlagsPassedByUser() - if flagsPassedByUser != nil && len(flagsPassedByUser) > 0 { + if len(flagsPassedByUser) > 0 { result := fmt.Sprintf("DEPRECATED - the use of command-line flags: %s, has been deprecated in favor of environment variables. "+ "Please consult our Configuring section in the developer documentation on how to use them - https://developers.stellar.org/docs/run-api-server/configuring", flagsPassedByUser) stdLog.Println(result) From 6425ca9427b9a382fe4adadd0dccdecc89c5725e Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Tue, 3 Oct 2023 16:12:27 -0400 Subject: [PATCH 06/18] Update parameters_test.go --- services/horizon/internal/integration/parameters_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/services/horizon/internal/integration/parameters_test.go b/services/horizon/internal/integration/parameters_test.go index 3c4d557616..07f9e7f080 100644 --- a/services/horizon/internal/integration/parameters_test.go +++ b/services/horizon/internal/integration/parameters_test.go @@ -509,9 +509,8 @@ func TestDeprecatedOutputs(t *testing.T) { os.Stderr = originalStderr assert.Contains(t, string(outputBytes), "DEPRECATED - the use of command-line flags: "+ - "[--db-url --captive-core-config-path --captive-core-use-db --enable-captive-core-ingestion --captive-core-http-port --captive-core-storage-path --stellar-core-db-url --stellar-core-url --history-archive-urls --port --admin-port --max-db-connections --per-hour-rate-limit --network-passphrase --ingest --apply-migrations --checkpoint-frequency], "+ - "has been deprecated in favor of environment variables. Please consult our Configuring section in the developer "+ - "documentation on how to use them - https://developers.stellar.org/docs/run-api-server/configuring") + "[--db-url --captive-core-config-path --captive-core-use-db --enable-captive-core-ingestion --exp-enable-ingestion-filtering --captive-core-http-port --captive-core-storage-path --stellar-core-db-url --stellar-core-url --history-archive-urls --port --admin-port --max-db-connections --per-hour-rate-limit --network-passphrase --ingest --apply-migrations --checkpoint-frequency], "+ + "has been deprecated in favor of environment variables. Please consult our Configuring section in the developer documentation on how to use them - https://developers.stellar.org/docs/run-api-server/configuring") }) } @@ -534,7 +533,7 @@ func TestHelpOutput(t *testing.T) { } var writer io.Writer = &bytes.Buffer{} - horizonCmd.SetOutput(writer) + horizonCmd.SetOut(writer) horizonCmd.SetArgs([]string{"-h"}) if err := flags.Init(horizonCmd); err != nil { From 35bad2e8c9a7e0eaa33d2eb87e9567d6f90f7e51 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Tue, 3 Oct 2023 16:50:16 -0400 Subject: [PATCH 07/18] Update parameters_test.go --- services/horizon/internal/integration/parameters_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/horizon/internal/integration/parameters_test.go b/services/horizon/internal/integration/parameters_test.go index 07f9e7f080..f04d0c9899 100644 --- a/services/horizon/internal/integration/parameters_test.go +++ b/services/horizon/internal/integration/parameters_test.go @@ -509,7 +509,7 @@ func TestDeprecatedOutputs(t *testing.T) { os.Stderr = originalStderr assert.Contains(t, string(outputBytes), "DEPRECATED - the use of command-line flags: "+ - "[--db-url --captive-core-config-path --captive-core-use-db --enable-captive-core-ingestion --exp-enable-ingestion-filtering --captive-core-http-port --captive-core-storage-path --stellar-core-db-url --stellar-core-url --history-archive-urls --port --admin-port --max-db-connections --per-hour-rate-limit --network-passphrase --ingest --apply-migrations --checkpoint-frequency], "+ + "[--db-url --stellar-core-binary-path --captive-core-config-path --captive-core-use-db --enable-captive-core-ingestion --captive-core-http-port --captive-core-storage-path --stellar-core-db-url --stellar-core-url --history-archive-urls --port --admin-port --max-db-connections --per-hour-rate-limit --network-passphrase --ingest --apply-migrations --checkpoint-frequency], "+ "has been deprecated in favor of environment variables. Please consult our Configuring section in the developer documentation on how to use them - https://developers.stellar.org/docs/run-api-server/configuring") }) } From d536cba11c028fa5358a69aad8a81a462baa9129 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Tue, 3 Oct 2023 18:09:23 -0400 Subject: [PATCH 08/18] Update parameters_test.go --- .../internal/integration/parameters_test.go | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/services/horizon/internal/integration/parameters_test.go b/services/horizon/internal/integration/parameters_test.go index f04d0c9899..2a314a8f9b 100644 --- a/services/horizon/internal/integration/parameters_test.go +++ b/services/horizon/internal/integration/parameters_test.go @@ -485,12 +485,31 @@ func TestDeprecatedOutputs(t *testing.T) { r, w, _ := os.Pipe() os.Stderr = w stdLog.SetOutput(os.Stderr) + + config, flags := horizon.Flags() + + horizonCmd := &cobra.Command{ + Use: "horizon", + Short: "Client-facing api server for the Stellar network", + SilenceErrors: true, + SilenceUsage: true, + Long: "Client-facing API server for the Stellar network.", + RunE: func(cmd *cobra.Command, args []string) error { + _, err := horizon.NewAppFromFlags(config, flags) + if err != nil { + return err + } + return nil + }, + } - testConfig := integration.GetTestConfig() - test := integration.NewTest(t, *testConfig) - err := test.StartHorizon() - assert.NoError(t, err) - test.WaitForHorizon() + horizonCmd.SetArgs([]string{"--disable-tx-sub=true"}) + if err := flags.Init(horizonCmd); err != nil { + fmt.Println(err) + } + if err := horizonCmd.Execute(); err != nil { + fmt.Println(err) + } // Use a wait group to wait for the goroutine to finish before proceeding var wg sync.WaitGroup @@ -509,8 +528,9 @@ func TestDeprecatedOutputs(t *testing.T) { os.Stderr = originalStderr assert.Contains(t, string(outputBytes), "DEPRECATED - the use of command-line flags: "+ - "[--db-url --stellar-core-binary-path --captive-core-config-path --captive-core-use-db --enable-captive-core-ingestion --captive-core-http-port --captive-core-storage-path --stellar-core-db-url --stellar-core-url --history-archive-urls --port --admin-port --max-db-connections --per-hour-rate-limit --network-passphrase --ingest --apply-migrations --checkpoint-frequency], "+ - "has been deprecated in favor of environment variables. Please consult our Configuring section in the developer documentation on how to use them - https://developers.stellar.org/docs/run-api-server/configuring") + "[--disable-tx-sub], has been deprecated in favor of environment variables. Please consult our "+ + "Configuring section in the developer documentation on how to use them - "+ + "https://developers.stellar.org/docs/run-api-server/configuring") }) } From cca510d64680b30d5dcf4c66d3b87eac86e6f99d Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Wed, 4 Oct 2023 09:56:54 -0400 Subject: [PATCH 09/18] Update parameters_test.go --- services/horizon/internal/integration/parameters_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/horizon/internal/integration/parameters_test.go b/services/horizon/internal/integration/parameters_test.go index 2a314a8f9b..dae45d9ffd 100644 --- a/services/horizon/internal/integration/parameters_test.go +++ b/services/horizon/internal/integration/parameters_test.go @@ -485,7 +485,7 @@ func TestDeprecatedOutputs(t *testing.T) { r, w, _ := os.Pipe() os.Stderr = w stdLog.SetOutput(os.Stderr) - + config, flags := horizon.Flags() horizonCmd := &cobra.Command{ From 546bb3ca14e0148713bde12f4bf65569bcbdd2ee Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Wed, 4 Oct 2023 12:30:39 -0400 Subject: [PATCH 10/18] Shift environment.go to horizon package --- .../internal/test/integration/environment.go | 45 ------------------- .../internal/test/integration/integration.go | 6 +-- 2 files changed, 3 insertions(+), 48 deletions(-) delete mode 100644 services/horizon/internal/test/integration/environment.go diff --git a/services/horizon/internal/test/integration/environment.go b/services/horizon/internal/test/integration/environment.go deleted file mode 100644 index 9a3d7c09d2..0000000000 --- a/services/horizon/internal/test/integration/environment.go +++ /dev/null @@ -1,45 +0,0 @@ -//lint:file-ignore U1001 Ignore all unused code, this is only used in tests. -package integration - -import ( - "os" -) - -type EnvironmentManager struct { - oldEnvironment, newEnvironment map[string]string -} - -func NewEnvironmentManager() *EnvironmentManager { - env := &EnvironmentManager{} - env.oldEnvironment = make(map[string]string) - env.newEnvironment = make(map[string]string) - return env -} - -// Add sets a new environment variable, saving the original value (if any). -func (envmgr *EnvironmentManager) Add(key, value string) error { - // If someone pushes an environmental variable more than once, we don't want - // to lose the *original* value, so we're being careful here. - if _, ok := envmgr.newEnvironment[key]; !ok { - if oldValue, ok := os.LookupEnv(key); ok { - envmgr.oldEnvironment[key] = oldValue - } - } - - envmgr.newEnvironment[key] = value - return os.Setenv(key, value) -} - -// Restore restores the environment prior to any modifications. -// -// You should probably use this alongside `defer` to ensure the global -// environment isn't modified for longer than you intend. -func (envmgr *EnvironmentManager) Restore() { - for key := range envmgr.newEnvironment { - if oldValue, ok := envmgr.oldEnvironment[key]; ok { - os.Setenv(key, oldValue) - } else { - os.Unsetenv(key) - } - } -} diff --git a/services/horizon/internal/test/integration/integration.go b/services/horizon/internal/test/integration/integration.go index e83b061b3e..df5e15aae3 100644 --- a/services/horizon/internal/test/integration/integration.go +++ b/services/horizon/internal/test/integration/integration.go @@ -96,7 +96,7 @@ type Test struct { config Config coreConfig CaptiveConfig horizonIngestConfig horizon.Config - environment *EnvironmentManager + environment *horizon.EnvironmentManager horizonClient *sdk.Client horizonAdminClient *sdk.AdminClient @@ -151,7 +151,7 @@ func NewTest(t *testing.T, config Config) *Test { config: config, composePath: composePath, passPhrase: StandaloneNetworkPassphrase, - environment: NewEnvironmentManager(), + environment: horizon.NewEnvironmentManager(), } i.configureCaptiveCore() // Only run Stellar Core container and its dependencies. @@ -160,7 +160,7 @@ func NewTest(t *testing.T, config Config) *Test { i = &Test{ t: t, config: config, - environment: NewEnvironmentManager(), + environment: horizon.NewEnvironmentManager(), } } From e63356b9d8f01585d27ec648154d02284890e52c Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Wed, 4 Oct 2023 17:21:17 -0400 Subject: [PATCH 11/18] Move environment.go to test dir --- services/horizon/internal/flags_test.go | 3 ++- services/horizon/internal/{ => test}/environment.go | 2 +- services/horizon/internal/test/integration/integration.go | 7 ++++--- 3 files changed, 7 insertions(+), 5 deletions(-) rename services/horizon/internal/{ => test}/environment.go (99%) diff --git a/services/horizon/internal/flags_test.go b/services/horizon/internal/flags_test.go index a51f56ee4e..d8b8ca9be1 100644 --- a/services/horizon/internal/flags_test.go +++ b/services/horizon/internal/flags_test.go @@ -3,6 +3,7 @@ package horizon import ( "fmt" "github.com/spf13/cobra" + "github.com/stellar/go/services/horizon/internal/test" "os" "testing" @@ -200,7 +201,7 @@ func TestEnvironmentVariables(t *testing.T) { "CAPTIVE_CORE_CONFIG_PATH": "../docker/captive-core-classic-integration-tests.cfg", "CAPTIVE_CORE_USE_DB": "true", } - envManager := NewEnvironmentManager() + envManager := test.NewEnvironmentManager() if err := envManager.InitializeEnvironmentVariables(environmentVars); err != nil { fmt.Println(err) } diff --git a/services/horizon/internal/environment.go b/services/horizon/internal/test/environment.go similarity index 99% rename from services/horizon/internal/environment.go rename to services/horizon/internal/test/environment.go index ab1193d680..8019c92208 100644 --- a/services/horizon/internal/environment.go +++ b/services/horizon/internal/test/environment.go @@ -1,4 +1,4 @@ -package horizon +package test import ( "fmt" diff --git a/services/horizon/internal/test/integration/integration.go b/services/horizon/internal/test/integration/integration.go index df5e15aae3..298d8a4d92 100644 --- a/services/horizon/internal/test/integration/integration.go +++ b/services/horizon/internal/test/integration/integration.go @@ -4,6 +4,7 @@ package integration import ( "context" "fmt" + "github.com/stellar/go/services/horizon/internal/test" "io/ioutil" "os" "os/exec" @@ -96,7 +97,7 @@ type Test struct { config Config coreConfig CaptiveConfig horizonIngestConfig horizon.Config - environment *horizon.EnvironmentManager + environment *test.EnvironmentManager horizonClient *sdk.Client horizonAdminClient *sdk.AdminClient @@ -151,7 +152,7 @@ func NewTest(t *testing.T, config Config) *Test { config: config, composePath: composePath, passPhrase: StandaloneNetworkPassphrase, - environment: horizon.NewEnvironmentManager(), + environment: test.NewEnvironmentManager(), } i.configureCaptiveCore() // Only run Stellar Core container and its dependencies. @@ -160,7 +161,7 @@ func NewTest(t *testing.T, config Config) *Test { i = &Test{ t: t, config: config, - environment: horizon.NewEnvironmentManager(), + environment: test.NewEnvironmentManager(), } } From 6fa5852b8a0ac54a2f6d760f57274c95e6a291b0 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Thu, 5 Oct 2023 12:13:39 -0400 Subject: [PATCH 12/18] Update flags_test.go --- services/horizon/internal/flags_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/services/horizon/internal/flags_test.go b/services/horizon/internal/flags_test.go index d8b8ca9be1..35852207ae 100644 --- a/services/horizon/internal/flags_test.go +++ b/services/horizon/internal/flags_test.go @@ -201,10 +201,15 @@ func TestEnvironmentVariables(t *testing.T) { "CAPTIVE_CORE_CONFIG_PATH": "../docker/captive-core-classic-integration-tests.cfg", "CAPTIVE_CORE_USE_DB": "true", } + envManager := test.NewEnvironmentManager() + defer func() { + envManager.Restore() + }() if err := envManager.InitializeEnvironmentVariables(environmentVars); err != nil { fmt.Println(err) } + config, flags := Flags() horizonCmd := &cobra.Command{ Use: "horizon", @@ -233,5 +238,4 @@ func TestEnvironmentVariables(t *testing.T) { assert.Equal(t, config.CaptiveCoreBinaryPath, os.Getenv("HORIZON_INTEGRATION_TESTS_CAPTIVE_CORE_BIN")) assert.Equal(t, config.CaptiveCoreConfigPath, "../docker/captive-core-classic-integration-tests.cfg") assert.Equal(t, config.CaptiveCoreConfigUseDB, true) - envManager.Restore() } From 519d0319289e68fb2ed2666c5ff357d7c5c64002 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Thu, 5 Oct 2023 15:15:56 -0400 Subject: [PATCH 13/18] Update flags_test.go --- services/horizon/internal/flags_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/services/horizon/internal/flags_test.go b/services/horizon/internal/flags_test.go index 35852207ae..6e71c010a7 100644 --- a/services/horizon/internal/flags_test.go +++ b/services/horizon/internal/flags_test.go @@ -2,11 +2,12 @@ package horizon import ( "fmt" - "github.com/spf13/cobra" - "github.com/stellar/go/services/horizon/internal/test" "os" "testing" + "github.com/spf13/cobra" + "github.com/stellar/go/services/horizon/internal/test" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) From b92c673b384f598dca436eb25ac336d1e87c879e Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Thu, 5 Oct 2023 15:35:33 -0400 Subject: [PATCH 14/18] Update integration.go --- services/horizon/internal/test/integration/integration.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/horizon/internal/test/integration/integration.go b/services/horizon/internal/test/integration/integration.go index 298d8a4d92..e0f2e13864 100644 --- a/services/horizon/internal/test/integration/integration.go +++ b/services/horizon/internal/test/integration/integration.go @@ -4,7 +4,6 @@ package integration import ( "context" "fmt" - "github.com/stellar/go/services/horizon/internal/test" "io/ioutil" "os" "os/exec" @@ -17,6 +16,8 @@ import ( "testing" "time" + "github.com/stellar/go/services/horizon/internal/test" + "github.com/2opremio/pretty" "github.com/creachadair/jrpc2" "github.com/creachadair/jrpc2/jhttp" From 758bf93800f9746427f716c921395bb77fa64baf Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Fri, 6 Oct 2023 13:00:35 -0400 Subject: [PATCH 15/18] Nit changes - 1 --- services/horizon/internal/flags.go | 11 +++++------ .../horizon/internal/integration/parameters_test.go | 7 +++---- support/config/config_option.go | 3 ++- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/services/horizon/internal/flags.go b/services/horizon/internal/flags.go index 03585cca1b..d36c7c07dd 100644 --- a/services/horizon/internal/flags.go +++ b/services/horizon/internal/flags.go @@ -238,11 +238,10 @@ func Flags() (*Config, support.ConfigOptions) { if val := viper.GetString(opt.Name); val != "" { stdLog.Printf( - "DEPRECATED - No ingestion filter rules are defined by default, which equates to no filtering " + - "of historical data. If you have never added filter rules to this deployment, then nothing further needed. " + - "If you have defined ingestion filter rules prior but disabled filtering overall by setting this flag " + - "disabled with --exp-enable-ingestion-filtering=false, then you should now delete the filter rules using " + - "the Horizon Admin API to achieve the same no-filtering result. Remove usage of this flag in all cases.", + "DEPRECATED - No ingestion filter rules are defined by default, which equates to " + + "no filtering of historical data. If you have never added filter rules to this deployment, then no further action is needed. " + + "If you have defined ingestion filter rules previously but disabled filtering overall by setting the env variable EXP_ENABLE_INGESTION_FILTERING=false, " + + "then you should now delete the filter rules using the Horizon Admin API to achieve the same no-filtering result. Remove usage of this variable in all cases.", ) } return nil @@ -833,7 +832,7 @@ func setCaptiveCoreConfiguration(config *Config, options ApplyOptions) error { // ApplyFlags applies the command line flags on the given Config instance func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOptions) error { // Check if the user has passed any flags and if so, print a DEPRECATED warning message. - flagsPassedByUser := flags.GetAllFlagsPassedByUser() + flagsPassedByUser := flags.GetCommandLineFlagsPassedByUser() if len(flagsPassedByUser) > 0 { result := fmt.Sprintf("DEPRECATED - the use of command-line flags: %s, has been deprecated in favor of environment variables. "+ "Please consult our Configuring section in the developer documentation on how to use them - https://developers.stellar.org/docs/run-api-server/configuring", flagsPassedByUser) diff --git a/services/horizon/internal/integration/parameters_test.go b/services/horizon/internal/integration/parameters_test.go index dae45d9ffd..69b56e99bd 100644 --- a/services/horizon/internal/integration/parameters_test.go +++ b/services/horizon/internal/integration/parameters_test.go @@ -475,10 +475,9 @@ func TestDeprecatedOutputs(t *testing.T) { os.Stderr = originalStderr assert.Contains(t, string(outputBytes), "DEPRECATED - No ingestion filter rules are defined by default, which equates to "+ - "no filtering of historical data. If you have never added filter rules to this deployment, then nothing further needed. "+ - "If you have defined ingestion filter rules prior but disabled filtering overall by setting this flag disabled with "+ - "--exp-enable-ingestion-filtering=false, then you should now delete the filter rules using the Horizon Admin API to achieve "+ - "the same no-filtering result. Remove usage of this flag in all cases.") + "no filtering of historical data. If you have never added filter rules to this deployment, then no further action is needed. "+ + "If you have defined ingestion filter rules previously but disabled filtering overall by setting the env variable EXP_ENABLE_INGESTION_FILTERING=false, "+ + "then you should now delete the filter rules using the Horizon Admin API to achieve the same no-filtering result. Remove usage of this variable in all cases.") }) t.Run("deprecated output for command-line flags", func(t *testing.T) { originalStderr := os.Stderr diff --git a/support/config/config_option.go b/support/config/config_option.go index c1acc825c9..79f504d8f3 100644 --- a/support/config/config_option.go +++ b/support/config/config_option.go @@ -58,7 +58,8 @@ func (cos ConfigOptions) SetValues() error { return nil } -func (cos ConfigOptions) GetAllFlagsPassedByUser() []string { +// GetCommandLineFlagsPassedByUser returns a list of command-line flags that were passed by the user when running Horizon. +func (cos ConfigOptions) GetCommandLineFlagsPassedByUser() []string { var flagsPassedByUser []string for _, co := range cos { if co.flag.Changed { From 52bb69e8b349887a02da82eb215bb25844dc6fb5 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Fri, 6 Oct 2023 13:10:20 -0400 Subject: [PATCH 16/18] Update CHANGELOG.md --- services/horizon/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/services/horizon/CHANGELOG.md b/services/horizon/CHANGELOG.md index 97c819677e..0a08373678 100644 --- a/services/horizon/CHANGELOG.md +++ b/services/horizon/CHANGELOG.md @@ -15,6 +15,7 @@ file. This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - The same slippage calculation from the [`v2.26.1`](#2261) hotfix now properly excludes spikes for smoother trade aggregation plots ([4999](https://github.com/stellar/go/pull/4999)). +- Add a deprecation warning for using command-line flags when running Horizon ([5051](https://github.com/stellar/go/pull/5051)) ## 2.26.1 From 915011a0c843cacba048ba33ed2a0330080a1b0e Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Fri, 6 Oct 2023 15:09:49 -0400 Subject: [PATCH 17/18] Nit changes - 2 --- services/horizon/internal/flags.go | 4 ++-- support/config/config_option.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/horizon/internal/flags.go b/services/horizon/internal/flags.go index d36c7c07dd..496eeec5a6 100644 --- a/services/horizon/internal/flags.go +++ b/services/horizon/internal/flags.go @@ -834,8 +834,8 @@ func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOption // Check if the user has passed any flags and if so, print a DEPRECATED warning message. flagsPassedByUser := flags.GetCommandLineFlagsPassedByUser() if len(flagsPassedByUser) > 0 { - result := fmt.Sprintf("DEPRECATED - the use of command-line flags: %s, has been deprecated in favor of environment variables. "+ - "Please consult our Configuring section in the developer documentation on how to use them - https://developers.stellar.org/docs/run-api-server/configuring", flagsPassedByUser) + result := fmt.Sprintf("DEPRECATED - the use of command-line flags: [%s], has been deprecated in favor of environment variables. "+ + "Please consult our Configuring section in the developer documentation on how to use them - https://developers.stellar.org/docs/run-api-server/configuring", "--"+strings.Join(flagsPassedByUser, ",--")) stdLog.Println(result) } diff --git a/support/config/config_option.go b/support/config/config_option.go index 79f504d8f3..9a22a48888 100644 --- a/support/config/config_option.go +++ b/support/config/config_option.go @@ -63,7 +63,7 @@ func (cos ConfigOptions) GetCommandLineFlagsPassedByUser() []string { var flagsPassedByUser []string for _, co := range cos { if co.flag.Changed { - flagsPassedByUser = append(flagsPassedByUser, "--"+co.flag.Name) + flagsPassedByUser = append(flagsPassedByUser, co.flag.Name) } } return flagsPassedByUser From 31ce796e824b193bdf7b954ceb90824541ed2001 Mon Sep 17 00:00:00 2001 From: Aditya Vyas Date: Fri, 6 Oct 2023 15:34:45 -0400 Subject: [PATCH 18/18] Update flags_test.go --- services/horizon/internal/flags_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/horizon/internal/flags_test.go b/services/horizon/internal/flags_test.go index a0c63860b3..eab97b1beb 100644 --- a/services/horizon/internal/flags_test.go +++ b/services/horizon/internal/flags_test.go @@ -243,7 +243,7 @@ func TestEnvironmentVariables(t *testing.T) { if err := flags.Init(horizonCmd); err != nil { fmt.Println(err) } - if err := ApplyFlags(config, flags, ApplyOptions{RequireCaptiveCoreConfig: true, AlwaysIngest: false}); err != nil { + if err := ApplyFlags(config, flags, ApplyOptions{RequireCaptiveCoreFullConfig: true, AlwaysIngest: false}); err != nil { fmt.Println(err) } assert.Equal(t, config.Ingest, false)