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

services/horizon: Add deprecation warning for using command-line flags #5051

Merged
merged 24 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
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
6 changes: 5 additions & 1 deletion services/horizon/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@
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",

Check failure on line 23 in services/horizon/cmd/root.go

View workflow job for this annotation

GitHub Actions / golangci

line is 152 characters (lll)
aditya1702 marked this conversation as resolved.
Show resolved Hide resolved
RunE: func(cmd *cobra.Command, args []string) error {
app, err := horizon.NewAppFromFlags(config, flags)
if err != nil {
Expand Down
65 changes: 65 additions & 0 deletions services/horizon/internal/environment.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
9 changes: 9 additions & 0 deletions services/horizon/internal/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -832,10 +832,19 @@

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

Check failure on line 839 in services/horizon/internal/flags.go

View workflow job for this annotation

GitHub Actions / golangci

line is 178 characters (lll)
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
}
Expand Down
54 changes: 54 additions & 0 deletions services/horizon/internal/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import (
"fmt"
"github.com/spf13/cobra"

Check failure on line 5 in services/horizon/internal/flags_test.go

View workflow job for this annotation

GitHub Actions / golangci

File is not `goimports`-ed with -local github.com/golangci/golangci-lint (goimports)
"os"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -180,3 +182,55 @@
})
}
}

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()
aditya1702 marked this conversation as resolved.
Show resolved Hide resolved
}
120 changes: 84 additions & 36 deletions services/horizon/internal/integration/parameters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,12 +431,6 @@
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",
Expand All @@ -450,40 +444,94 @@
})
}

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 "+
aditya1702 marked this conversation as resolved.
Show resolved Hide resolved
"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)

Check failure on line 488 in services/horizon/internal/integration/parameters_test.go

View workflow job for this annotation

GitHub Actions / golangci

File is not `gofmt`-ed with `-s` (gofmt)
config, flags := horizon.Flags()

horizonCmd := &cobra.Command{
aditya1702 marked this conversation as resolved.
Show resolved Hide resolved
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
},
}

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)
}
}()

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
}
}()

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.")
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 - the use of command-line flags: "+
"[--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")
})
}

func TestHelpOutput(t *testing.T) {
Expand All @@ -505,7 +553,7 @@
}

var writer io.Writer = &bytes.Buffer{}
horizonCmd.SetOutput(writer)
horizonCmd.SetOut(writer)

horizonCmd.SetArgs([]string{"-h"})
if err := flags.Init(horizonCmd); err != nil {
Expand Down
10 changes: 10 additions & 0 deletions support/config/config_option.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ func (cos ConfigOptions) SetValues() error {
return nil
}

func (cos ConfigOptions) GetAllFlagsPassedByUser() []string {
aditya1702 marked this conversation as resolved.
Show resolved Hide resolved
var flagsPassedByUser []string
for _, co := range cos {
if co.flag.Changed {
flagsPassedByUser = append(flagsPassedByUser, "--"+co.flag.Name)
aditya1702 marked this conversation as resolved.
Show resolved Hide resolved
}
}
return flagsPassedByUser
}

// ConfigOption is a complete description of the configuration of a command line option
type ConfigOption struct {
Name string // e.g. "db-url"
Expand Down
Loading