From 74116b36eb69d555a45b96ee3bc0d7acc336f1ca Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Jul 2020 12:21:40 -0700 Subject: [PATCH 01/15] use URL & duration types from go-configtypes --- config/config.go | 35 ++--- config/config_field_types.go | 166 +-------------------- config/config_field_types_test.go | 146 ------------------ config/config_from_env_test.go | 9 +- config/config_from_file_test.go | 7 +- config/config_validation.go | 3 +- config/test_data_test.go | 35 ++--- go.mod | 1 + go.sum | 3 + internal/events/event-relay.go | 12 +- internal/events/event-summarizing-relay.go | 6 +- internal/relayenv/env_context_impl.go | 6 +- relay_endpoints_events_test.go | 5 +- relay_endpoints_stream_test.go | 5 +- relay_other_test.go | 3 +- testutils_values_test.go | 3 +- 16 files changed, 84 insertions(+), 361 deletions(-) diff --git a/config/config.go b/config/config.go index 6c59c461..a98e6816 100644 --- a/config/config.go +++ b/config/config.go @@ -3,6 +3,7 @@ package config import ( "time" + ct "github.com/launchdarkly/go-configtypes" "github.com/launchdarkly/ld-relay/v6/internal/logging" ) @@ -36,7 +37,7 @@ const ( ) var ( - defaultRedisURL = newOptAbsoluteURLMustBeValid("redis://localhost:6379") + defaultRedisURL = newOptURLAbsoluteMustBeValid("redis://localhost:6379") ) // DefaultLoggers is the default logging configuration used by Relay. @@ -66,11 +67,11 @@ type MainConfig struct { ExitOnError bool ExitAlways bool IgnoreConnectionErrors bool - StreamURI OptAbsoluteURL - BaseURI OptAbsoluteURL + StreamURI ct.OptURLAbsolute + BaseURI ct.OptURLAbsolute Port int - HeartbeatInterval OptDuration - MaxClientConnectionTime OptDuration + HeartbeatInterval ct.OptDuration + MaxClientConnectionTime ct.OptDuration TLSEnabled bool TLSCert string TLSKey string @@ -79,9 +80,9 @@ type MainConfig struct { // EventsConfig contains configuration parameters for proxying events. type EventsConfig struct { - EventsURI OptAbsoluteURL + EventsURI ct.OptURLAbsolute SendEvents bool - FlushInterval OptDuration + FlushInterval ct.OptDuration SamplingInterval int32 Capacity int InlineUsers bool @@ -97,8 +98,8 @@ type EventsConfig struct { type RedisConfig struct { Host string Port int - URL OptAbsoluteURL - LocalTTL OptDuration + URL ct.OptURLAbsolute + LocalTTL ct.OptDuration TLS bool Password string } @@ -110,7 +111,7 @@ type RedisConfig struct { // This corresponds to the [Consul] section in the configuration file. type ConsulConfig struct { Host string - LocalTTL OptDuration + LocalTTL ct.OptDuration } // DynamoDBConfig configures the optional DynamoDB integration, which is used only if Enabled is true. @@ -119,8 +120,8 @@ type ConsulConfig struct { type DynamoDBConfig struct { Enabled bool TableName string - URL OptAbsoluteURL - LocalTTL OptDuration + URL ct.OptURLAbsolute + LocalTTL ct.OptDuration } // EnvConfig describes an environment to be relayed. There may be any number of these. @@ -138,12 +139,12 @@ type EnvConfig struct { SecureMode bool InsecureSkipVerify bool LogLevel OptLogLevel - TTL OptDuration + TTL ct.OptDuration } // ProxyConfig represents all the supported proxy options. type ProxyConfig struct { - URL OptAbsoluteURL + URL ct.OptURLAbsolute NTLMAuth bool User string Password string @@ -198,13 +199,13 @@ type PrometheusConfig struct { // start by copying relay.DefaultConfig and then changing only the fields you need to change. var DefaultConfig = Config{ Main: MainConfig{ - BaseURI: newOptAbsoluteURLMustBeValid(DefaultBaseURI), - StreamURI: newOptAbsoluteURLMustBeValid(DefaultStreamURI), + BaseURI: newOptURLAbsoluteMustBeValid(DefaultBaseURI), + StreamURI: newOptURLAbsoluteMustBeValid(DefaultStreamURI), Port: defaultPort, }, Events: EventsConfig{ Capacity: defaultEventCapacity, - EventsURI: newOptAbsoluteURLMustBeValid(DefaultEventsURI), + EventsURI: newOptURLAbsoluteMustBeValid(DefaultEventsURI), }, MetricsConfig: MetricsConfig{ Prometheus: PrometheusConfig{ diff --git a/config/config_field_types.go b/config/config_field_types.go index 46e1d1ac..cb7ac162 100644 --- a/config/config_field_types.go +++ b/config/config_field_types.go @@ -1,12 +1,10 @@ package config import ( - "errors" "fmt" - "net/url" "strings" - "time" + ct "github.com/launchdarkly/go-configtypes" "gopkg.in/launchdarkly/go-sdk-common.v2/ldlog" ) @@ -47,172 +45,14 @@ func (k EnvironmentID) GetAuthorizationHeaderValue() string { return "" } -// OptAbsoluteURL represents an optional URL parameter which, if present, must be a valid URL. This is enforced -// by its representation of encoding.TextUnmarshaler, which is called by both the gcfg config file parser -// and our environment variable logic. -// -// The zero value OptAbsoluteURL{} is valid and undefined (IsDefined() is false, Get() is nil). -type OptAbsoluteURL struct { - url *url.URL -} - -// NewOptAbsoluteURLFromURL creates an OptAbsoluteURL from an already-parsed URL. It returns an error if the -// URL is not an absolute URL. If the parameter is nil, it returns an empty OptAbsoluteURL{}. -func NewOptAbsoluteURLFromURL(url *url.URL) (OptAbsoluteURL, error) { - if url == nil { - return OptAbsoluteURL{}, nil - } - if !url.IsAbs() { - return OptAbsoluteURL{}, errNotAbsoluteURL() - } - u := *url - return OptAbsoluteURL{url: &u}, nil -} - -// NewOptAbsoluteURLFromString creates an OptAbsoluteURL from a string. It returns an error if the string is not -// a URL or is a relative URL. If the string is empty, it returns an empty OptAbsoluteURL{}. -func NewOptAbsoluteURLFromString(urlString string) (OptAbsoluteURL, error) { - if urlString == "" { - return OptAbsoluteURL{}, nil - } - u, err := url.Parse(urlString) - if err == nil { - return NewOptAbsoluteURLFromURL(u) - } - return OptAbsoluteURL{}, errBadURLString() -} - -func newOptAbsoluteURLMustBeValid(urlString string) OptAbsoluteURL { - o, err := NewOptAbsoluteURLFromString(urlString) +func newOptURLAbsoluteMustBeValid(urlString string) ct.OptURLAbsolute { + o, err := ct.NewOptURLAbsoluteFromString(urlString) if err != nil { panic(err) } return o } -// IsDefined is true if this instance has a value (Get() is not nil). -func (o OptAbsoluteURL) IsDefined() bool { - return o.url != nil -} - -// Get returns the wrapped URL if any, or nil. -func (o OptAbsoluteURL) Get() *url.URL { - if o.url == nil { - return nil - } - u := *o.url // copy the value so we're not exposing anything mutable - return &u -} - -// String returns the URL converted to a string, or "" if undefined. -func (o OptAbsoluteURL) String() string { - return o.StringOrElse("") -} - -// StringOrElse returns the URL converted to a string, or the alternative value if undefined. -func (o OptAbsoluteURL) StringOrElse(orElseValue string) string { - if o.url == nil { - return orElseValue - } - return o.url.String() -} - -// UnmarshalText attempts to parse the value from a byte string, using the same logic as -// NewOptAbsoluteURLFromString. -func (o *OptAbsoluteURL) UnmarshalText(data []byte) error { - parsed, err := NewOptAbsoluteURLFromString(string(data)) - if err == nil { - *o = parsed - } - return err -} - -func errBadURLString() error { - return errors.New("not a valid URL/URI") -} - -func errNotAbsoluteURL() error { - return errors.New("must be an absolute URL/URI") -} - -// OptDuration represents an optional time.Duration parameter. It can be an integer followed by "ms", "s", -// "m", or "h" (milliseconds/seconds/minutes/hours), or use a format of "hh:mm:ss" or "mm:ss". -// -// The zero value OptDuration{} is valid and undefined (IsDefined() is false). -type OptDuration struct { - hasValue bool - value time.Duration -} - -// NewOptDuration creates an OptDuration containing the given value. -func NewOptDuration(value time.Duration) OptDuration { - return OptDuration{hasValue: true, value: value} -} - -// NewOptDurationFromString creates an OptDuration from a string. It returns an error if the string is not -// in a supported format. If the string is empty, it returns an empty OptDuration{}. -func NewOptDurationFromString(s string) (OptDuration, error) { - if s == "" { - return OptDuration{}, nil - } - var n, hh, mm, ss int - // note that the newlines in these format strings mean there should be no other characters after the format - if count, err := fmt.Sscanf(s, "%dms\n", &n); err == nil && count == 1 { - return NewOptDuration(time.Duration(n) * time.Millisecond), nil - } - if count, err := fmt.Sscanf(s, "%ds\n", &n); err == nil && count == 1 { - return NewOptDuration(time.Duration(n) * time.Second), nil - } - if count, err := fmt.Sscanf(s, "%dm\n", &n); err == nil && count == 1 { - return NewOptDuration(time.Duration(n) * time.Minute), nil - } - if count, err := fmt.Sscanf(s, "%dh\n", &n); err == nil && count == 1 { - return NewOptDuration(time.Duration(n) * time.Hour), nil - } - if count, err := fmt.Sscanf(s, ":%d\n", &ss); err == nil && count == 1 { - return NewOptDuration(time.Duration(ss) * time.Second), nil - } - if count, err := fmt.Sscanf(s, "%d:%d\n", &mm, &ss); err == nil && count == 2 { - secs := mm*60 + ss - return NewOptDuration(time.Duration(secs) * time.Second), nil - } - if count, err := fmt.Sscanf(s, "%d:%d:%d\n", &hh, &mm, &ss); err == nil && count == 3 { - secs := (hh*60+mm)*60 + ss - return NewOptDuration(time.Duration(secs) * time.Second), nil - } - return OptDuration{}, errBadDuration(s) -} - -// IsDefined is true if this instance has a value (Get() is not nil). -func (o OptDuration) IsDefined() bool { - return o.hasValue -} - -// GetOrElse returns the wrapped value, or the alternative value if there is no value. -func (o OptDuration) GetOrElse(orElseValue time.Duration) time.Duration { - if !o.hasValue { - return orElseValue - } - return o.value -} - -// UnmarshalText attempts to parse the value from a byte string, using the same logic as -// NewOptDurationFromString. -func (o *OptDuration) UnmarshalText(data []byte) error { - opt, err := NewOptDurationFromString(string(data)) - if err == nil { - *o = opt - } - return err -} - -func errBadDuration(s string) error { - return fmt.Errorf( - `%q is not a valid duration (must use format "1ms", "1s", "1m", "1h", ":ss", "mm:ss", or "hh:mm:ss"`, - s, - ) -} - // OptLogLevel represents an optional log level parameter. It must match one of the level names "debug", // "info", "warn", or "error" (case-insensitive). // diff --git a/config/config_field_types_test.go b/config/config_field_types_test.go index 464bc31d..54aecc80 100644 --- a/config/config_field_types_test.go +++ b/config/config_field_types_test.go @@ -1,9 +1,7 @@ package config import ( - "net/url" "testing" - "time" "github.com/stretchr/testify/assert" @@ -16,150 +14,6 @@ func TestSDKCredential(t *testing.T) { assert.Equal(t, "", EnvironmentID("123").GetAuthorizationHeaderValue()) } -func TestOptAbsoluteURL(t *testing.T) { - absoluteURLString := "http://absolute/url" - absoluteURL, _ := url.Parse(absoluteURLString) - relativeURLString := "relative/url" - relativeURL, _ := url.Parse(relativeURLString) - malformedURLString := "::" - - t.Run("zero value", func(t *testing.T) { - o := OptAbsoluteURL{} - assert.False(t, o.IsDefined()) - assert.Nil(t, o.Get()) - assert.Equal(t, "", o.String()) - assert.Equal(t, "x", o.StringOrElse("x")) - }) - - t.Run("new from valid URL", func(t *testing.T) { - o, err := NewOptAbsoluteURLFromURL(absoluteURL) - assert.NoError(t, err) - assert.True(t, o.IsDefined()) - assert.Equal(t, absoluteURL, o.Get()) - assert.Equal(t, absoluteURLString, o.String()) - assert.Equal(t, absoluteURLString, o.StringOrElse("x")) - }) - - t.Run("new from valid URL string", func(t *testing.T) { - o, err := NewOptAbsoluteURLFromString(absoluteURLString) - assert.NoError(t, err) - assert.True(t, o.IsDefined()) - assert.Equal(t, absoluteURL, o.Get()) - assert.Equal(t, absoluteURLString, o.String()) - assert.Equal(t, absoluteURLString, o.StringOrElse("x")) - }) - - t.Run("new from nil URL", func(t *testing.T) { - o, err := NewOptAbsoluteURLFromURL(nil) - assert.NoError(t, err) - assert.Equal(t, OptAbsoluteURL{}, o) - }) - - t.Run("new from empty string", func(t *testing.T) { - o, err := NewOptAbsoluteURLFromString("") - assert.NoError(t, err) - assert.Equal(t, OptAbsoluteURL{}, o) - }) - - t.Run("new from relative URL", func(t *testing.T) { - o, err := NewOptAbsoluteURLFromURL(relativeURL) - assert.Equal(t, errNotAbsoluteURL(), err) - assert.Equal(t, OptAbsoluteURL{}, o) - }) - - t.Run("new from relative URL string", func(t *testing.T) { - o, err := NewOptAbsoluteURLFromString(relativeURLString) - assert.Equal(t, errNotAbsoluteURL(), err) - assert.Equal(t, OptAbsoluteURL{}, o) - }) - - t.Run("new from malformed URL string", func(t *testing.T) { - o, err := NewOptAbsoluteURLFromString(malformedURLString) - assert.Equal(t, errBadURLString(), err) - assert.Equal(t, OptAbsoluteURL{}, o) - }) - - t.Run("parse from valid URL string", func(t *testing.T) { - var o OptAbsoluteURL - assert.NoError(t, o.UnmarshalText([]byte(absoluteURLString))) - assert.Equal(t, absoluteURL, o.Get()) - }) - - t.Run("parse from empty string", func(t *testing.T) { - var o OptAbsoluteURL - assert.NoError(t, o.UnmarshalText([]byte(""))) - assert.Equal(t, OptAbsoluteURL{}, o) - }) - - t.Run("parse from relative URL string", func(t *testing.T) { - var o OptAbsoluteURL - assert.Equal(t, errNotAbsoluteURL(), o.UnmarshalText([]byte(relativeURLString))) - assert.Equal(t, OptAbsoluteURL{}, o) - }) - - t.Run("parse from malformed URL string", func(t *testing.T) { - var o OptAbsoluteURL - assert.Equal(t, errBadURLString(), o.UnmarshalText([]byte(malformedURLString))) - assert.Equal(t, OptAbsoluteURL{}, o) - }) -} - -func TestOptLogDuration(t *testing.T) { - t.Run("zero value", func(t *testing.T) { - o1 := OptDuration{} - assert.False(t, o1.IsDefined()) - assert.Equal(t, time.Hour, o1.GetOrElse(time.Hour)) - - o2, err := NewOptDurationFromString("") - assert.NoError(t, err) - assert.Equal(t, o1, o2) - - var o3 OptDuration - assert.NoError(t, o3.UnmarshalText([]byte(""))) - assert.Equal(t, o1, o3) - }) - - t.Run("valid strings", func(t *testing.T) { - testValue := func(input string, result time.Duration) { - t.Run(input, func(t *testing.T) { - o1, err := NewOptDurationFromString(input) - if assert.NoError(t, err) { - assert.Equal(t, result, o1.GetOrElse(0)) - } - - var o2 OptDuration - err = o2.UnmarshalText([]byte(input)) - if assert.NoError(t, err) { - assert.Equal(t, result, o2.GetOrElse(0)) - } - }) - } - testValue("3ms", 3*time.Millisecond) - testValue("3s", 3*time.Second) - testValue("3m", 3*time.Minute) - testValue("3h", 3*time.Hour) - testValue(":30", 30*time.Second) - testValue("1:30", time.Minute+30*time.Second) - testValue("1:10:30", time.Hour+10*time.Minute+30*time.Second) - }) - - t.Run("invalid strings", func(t *testing.T) { - testValue := func(input string) { - t.Run(input, func(t *testing.T) { - _, err := NewOptDurationFromString(input) - assert.Equal(t, errBadDuration(input), err) - - var o2 OptDuration - err = o2.UnmarshalText([]byte(input)) - assert.Equal(t, errBadDuration(input), err) - }) - } - testValue("1") - testValue("1x") - testValue("00:30:") - }) -} - func TestOptLogLevel(t *testing.T) { validLevel := ldlog.Warn validString := "wArN" diff --git a/config/config_from_env_test.go b/config/config_from_env_test.go index 6f05637a..c9aa343d 100644 --- a/config/config_from_env_test.go +++ b/config/config_from_env_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + ct "github.com/launchdarkly/go-configtypes" "gopkg.in/launchdarkly/go-sdk-common.v2/ldlog" ) @@ -94,7 +95,7 @@ func TestConfigFromEnvironmentFieldValidation(t *testing.T) { t.Run("parses valid URI", func(t *testing.T) { testValidConfigVars(t, - func(c *Config) { c.Main.BaseURI = newOptAbsoluteURLMustBeValid("http://some/uri") }, + func(c *Config) { c.Main.BaseURI = newOptURLAbsoluteMustBeValid("http://some/uri") }, map[string]string{"BASE_URI": "http://some/uri"}, ) }) @@ -112,7 +113,7 @@ func TestConfigFromEnvironmentFieldValidation(t *testing.T) { t.Run("parses valid duration", func(t *testing.T) { testValidConfigVars(t, - func(c *Config) { c.Main.HeartbeatInterval = NewOptDuration(3 * time.Second) }, + func(c *Config) { c.Main.HeartbeatInterval = ct.NewOptDuration(3 * time.Second) }, map[string]string{"HEARTBEAT_INTERVAL": "3s"}, ) }) @@ -120,7 +121,7 @@ func TestConfigFromEnvironmentFieldValidation(t *testing.T) { t.Run("rejects invalid duration", func(t *testing.T) { testInvalidConfigVars(t, map[string]string{"HEARTBEAT_INTERVAL": "x"}, - "HEARTBEAT_INTERVAL: "+errBadDuration("x").Error(), + "HEARTBEAT_INTERVAL: not a valid duration", ) }) @@ -161,7 +162,7 @@ func testInvalidConfigVars(t *testing.T, vars map[string]string, errMessage stri c := DefaultConfig err := LoadConfigFromEnvironment(&c, ldlog.NewDisabledLoggers()) require.Error(t, err) - assert.Equal(t, errMessage, err.Error()) + assert.Contains(t, err.Error(), errMessage) }) } diff --git a/config/config_from_file_test.go b/config/config_from_file_test.go index 66ac3c69..47639160 100644 --- a/config/config_from_file_test.go +++ b/config/config_from_file_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + ct "github.com/launchdarkly/go-configtypes" "gopkg.in/launchdarkly/go-sdk-common.v2/ldlog" helpers "github.com/launchdarkly/go-test-helpers/v2" @@ -120,7 +121,7 @@ Port = "x"`, t.Run("parses valid URI", func(t *testing.T) { testFileWithValidConfig(t, - func(c *Config) { c.Main.BaseURI = newOptAbsoluteURLMustBeValid("http://some/uri") }, + func(c *Config) { c.Main.BaseURI = newOptURLAbsoluteMustBeValid("http://some/uri") }, `[Main] BaseUri = "http://some/uri"`, ) @@ -141,7 +142,7 @@ BaseUri = "not/absolute"`, t.Run("parses valid duration", func(t *testing.T) { testFileWithValidConfig(t, - func(c *Config) { c.Main.HeartbeatInterval = NewOptDuration(3 * time.Second) }, + func(c *Config) { c.Main.HeartbeatInterval = ct.NewOptDuration(3 * time.Second) }, `[Main] HeartbeatInterval = 3s`, ) @@ -151,7 +152,7 @@ HeartbeatInterval = 3s`, testFileWithInvalidConfig(t, `[Main] HeartbeatInterval = "x"`, - errBadDuration("x").Error(), + "not a valid duration", ) }) diff --git a/config/config_validation.go b/config/config_validation.go index 3414f9dc..b22e858f 100644 --- a/config/config_validation.go +++ b/config/config_validation.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + ct "github.com/launchdarkly/go-configtypes" "gopkg.in/launchdarkly/go-sdk-common.v2/ldlog" ) @@ -49,7 +50,7 @@ func ValidateConfig(c *Config, loggers ldlog.Loggers) error { if port <= 0 { port = defaultRedisPort } - url, err := NewOptAbsoluteURLFromString(fmt.Sprintf("redis://%s:%d", host, port)) + url, err := ct.NewOptURLAbsoluteFromString(fmt.Sprintf("redis://%s:%d", host, port)) if err != nil { return errors.New("invalid Redis hostname") } diff --git a/config/test_data_test.go b/config/test_data_test.go index dd416ef5..241202e1 100644 --- a/config/test_data_test.go +++ b/config/test_data_test.go @@ -3,6 +3,7 @@ package config import ( "time" + ct "github.com/launchdarkly/go-configtypes" "gopkg.in/launchdarkly/go-sdk-common.v2/ldlog" ) @@ -59,13 +60,13 @@ func makeValidConfigAllBaseProperties() testDataValidConfig { c.makeConfig = func(c *Config) { c.Main = MainConfig{ Port: 8333, - BaseURI: newOptAbsoluteURLMustBeValid("http://base"), - StreamURI: newOptAbsoluteURLMustBeValid("http://stream"), + BaseURI: newOptURLAbsoluteMustBeValid("http://base"), + StreamURI: newOptURLAbsoluteMustBeValid("http://stream"), ExitOnError: true, ExitAlways: true, IgnoreConnectionErrors: true, - HeartbeatInterval: NewOptDuration(90 * time.Second), - MaxClientConnectionTime: NewOptDuration(30 * time.Minute), + HeartbeatInterval: ct.NewOptDuration(90 * time.Second), + MaxClientConnectionTime: ct.NewOptDuration(30 * time.Minute), TLSEnabled: true, TLSCert: "cert", TLSKey: "key", @@ -73,8 +74,8 @@ func makeValidConfigAllBaseProperties() testDataValidConfig { } c.Events = EventsConfig{ SendEvents: true, - EventsURI: newOptAbsoluteURLMustBeValid("http://events"), - FlushInterval: NewOptDuration(120 * time.Second), + EventsURI: newOptURLAbsoluteMustBeValid("http://events"), + FlushInterval: ct.NewOptDuration(120 * time.Second), SamplingInterval: 3, Capacity: 500, InlineUsers: true, @@ -96,7 +97,7 @@ func makeValidConfigAllBaseProperties() testDataValidConfig { Prefix: "krypton-", TableName: "krypton-table", AllowedOrigin: []string{"https://oa", "https://rann"}, - TTL: NewOptDuration(5 * time.Minute), + TTL: ct.NewOptDuration(5 * time.Minute), }, } } @@ -183,7 +184,7 @@ func makeValidConfigRedisMinimal() testDataValidConfig { c := testDataValidConfig{name: "Redis - minimal parameters"} c.makeConfig = func(c *Config) { c.Redis = RedisConfig{ - URL: newOptAbsoluteURLMustBeValid("redis://localhost:6379"), + URL: newOptURLAbsoluteMustBeValid("redis://localhost:6379"), } } c.envVars = map[string]string{ @@ -201,8 +202,8 @@ func makeValidConfigRedisAll() testDataValidConfig { c := testDataValidConfig{name: "Redis - all parameters"} c.makeConfig = func(c *Config) { c.Redis = RedisConfig{ - URL: newOptAbsoluteURLMustBeValid("redis://redishost:6400"), - LocalTTL: NewOptDuration(3 * time.Second), + URL: newOptURLAbsoluteMustBeValid("redis://redishost:6400"), + LocalTTL: ct.NewOptDuration(3 * time.Second), TLS: true, Password: "pass", } @@ -230,7 +231,7 @@ func makeValidConfigRedisURL() testDataValidConfig { c := testDataValidConfig{name: "Redis - URL instead of host/port"} c.makeConfig = func(c *Config) { c.Redis = RedisConfig{ - URL: newOptAbsoluteURLMustBeValid("rediss://redishost:6400"), + URL: newOptURLAbsoluteMustBeValid("rediss://redishost:6400"), } } c.envVars = map[string]string{ @@ -248,7 +249,7 @@ func makeValidConfigRedisPortOnly() testDataValidConfig { c := testDataValidConfig{name: "Redis - URL instead of host/port"} c.makeConfig = func(c *Config) { c.Redis = RedisConfig{ - URL: newOptAbsoluteURLMustBeValid("redis://localhost:9999"), + URL: newOptURLAbsoluteMustBeValid("redis://localhost:9999"), } } c.envVars = map[string]string{ @@ -266,7 +267,7 @@ func makeValidConfigRedisDockerPort() testDataValidConfig { c := testDataValidConfig{name: "Redis - special Docker port syntax"} c.makeConfig = func(c *Config) { c.Redis = RedisConfig{ - URL: newOptAbsoluteURLMustBeValid("redis://redishost:6400"), + URL: newOptURLAbsoluteMustBeValid("redis://redishost:6400"), } } c.envVars = map[string]string{ @@ -300,7 +301,7 @@ func makeValidConfigConsulAll() testDataValidConfig { func(c *Config) { c.Consul = ConsulConfig{ Host: "consulhost", - LocalTTL: NewOptDuration(3 * time.Second), + LocalTTL: ct.NewOptDuration(3 * time.Second), } } c.envVars = map[string]string{ @@ -339,8 +340,8 @@ func makeValidConfigDynamoDBAll() testDataValidConfig { c.DynamoDB = DynamoDBConfig{ Enabled: true, TableName: "table", - URL: newOptAbsoluteURLMustBeValid("http://localhost:8000"), - LocalTTL: NewOptDuration(3 * time.Second), + URL: newOptURLAbsoluteMustBeValid("http://localhost:8000"), + LocalTTL: ct.NewOptDuration(3 * time.Second), } } c.envVars = map[string]string{ @@ -489,7 +490,7 @@ func makeValidConfigProxy() testDataValidConfig { c := testDataValidConfig{name: "proxy"} c.makeConfig = func(c *Config) { c.Proxy = ProxyConfig{ - URL: newOptAbsoluteURLMustBeValid("http://proxy"), + URL: newOptURLAbsoluteMustBeValid("http://proxy"), User: "user", Password: "pass", Domain: "domain", diff --git a/go.mod b/go.mod index 32166bea..c05eb166 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( github.com/kardianos/minwinsvc v0.0.0-20151122163309-cad6b2b879b0 github.com/launchdarkly/eventsource v1.5.0 github.com/launchdarkly/gcfg v0.0.0-20160218190638-83c3f001aeeb + github.com/launchdarkly/go-configtypes v1.0.0 github.com/launchdarkly/go-server-sdk-consul v1.0.0-beta.1 github.com/launchdarkly/go-server-sdk-dynamodb v1.0.0-beta.1 github.com/launchdarkly/go-server-sdk-redis v1.0.0-beta.1 diff --git a/go.sum b/go.sum index b86ae133..fe990560 100644 --- a/go.sum +++ b/go.sum @@ -92,12 +92,15 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/launchdarkly-labs/go-coverage-enforcer v0.0.0-20200610003341-8bc77dea293c/go.mod h1:fJXIR7UJYgdXVmLM40NGl/dbs2Smv+iESsaT9uC4v/s= +github.com/launchdarkly-labs/go-coverage-enforcer v1.1.0/go.mod h1:fJXIR7UJYgdXVmLM40NGl/dbs2Smv+iESsaT9uC4v/s= github.com/launchdarkly/eventsource v1.4.3 h1:G0s/nlctuI6Vz/BKpNLE2L/S4U0lQ/ghYqecPjUuQIA= github.com/launchdarkly/eventsource v1.4.3/go.mod h1:LHxSeb4OnqznNZxCSXbFghxS/CjIQfzHovNoAqbO/Wk= github.com/launchdarkly/eventsource v1.5.0 h1:2XEm/6CLedcdXnHP1Dyy7o+baiw4fZ2+1KOvWSZYg/A= github.com/launchdarkly/eventsource v1.5.0/go.mod h1:LHxSeb4OnqznNZxCSXbFghxS/CjIQfzHovNoAqbO/Wk= github.com/launchdarkly/gcfg v0.0.0-20160218190638-83c3f001aeeb h1:a+uuDT3nURKt4laWk3PF7gxUuQfZ3XNE51AiOKjj5GA= github.com/launchdarkly/gcfg v0.0.0-20160218190638-83c3f001aeeb/go.mod h1:Y0RjGJ9V/bMra5xAA/Z0bkwdkwi/41b+/Krem9nIYs8= +github.com/launchdarkly/go-configtypes v1.0.0 h1:GB558x5leXoTvtv0mNE5aceCihKLI2SJ5ZVdDKWf2BM= +github.com/launchdarkly/go-configtypes v1.0.0/go.mod h1:wtIQpouDz1srzXiid2EGdv5iEolh6KQPxfVvgd1W0c4= github.com/launchdarkly/go-ntlm-proxy-auth v1.0.1 h1:Iz5cg9mB/0vt5llZE+J0iGQ5+O/U8CWuRUUR7Ou8eNM= github.com/launchdarkly/go-ntlm-proxy-auth v1.0.1/go.mod h1:hKWfH/hga5oslM2mRkDZi+14u2h1dFsmgbvSM9qF8pk= github.com/launchdarkly/go-ntlmssp v1.0.1 h1:snB77118TQvf9tfHrkSyrIop/UX5e5VD2D2mv7Kh3wE= diff --git a/internal/events/event-relay.go b/internal/events/event-relay.go index 528cbcad..2f02b969 100644 --- a/internal/events/event-relay.go +++ b/internal/events/event-relay.go @@ -270,9 +270,13 @@ func NewEventDispatcher( } func newDiagnosticEventEndpointDispatcher(config c.EventsConfig, httpClient *http.Client, loggers ldlog.Loggers, remotePath string) *diagnosticEventEndpointDispatcher { + eventsURI := config.EventsURI.String() + if eventsURI == "" { + eventsURI = c.DefaultEventsURI + } return &diagnosticEventEndpointDispatcher{ httpClient: httpClient, - remoteEndpointURI: strings.TrimRight(config.EventsURI.StringOrElse(c.DefaultEventsURI), "/") + remotePath, + remoteEndpointURI: strings.TrimRight(eventsURI, "/") + remotePath, loggers: loggers, } } @@ -291,9 +295,13 @@ func newAnalyticsEventEndpointDispatcher(authKey c.SDKCredential, config c.Event } func newEventVerbatimRelay(authKey c.SDKCredential, config c.EventsConfig, httpClient *http.Client, loggers ldlog.Loggers, remotePath string) *eventVerbatimRelay { + eventsURI := config.EventsURI.String() + if eventsURI == "" { + eventsURI = c.DefaultEventsURI + } opts := []OptionType{ OptionCapacity(config.Capacity), - OptionEndpointURI(strings.TrimRight(config.EventsURI.StringOrElse(c.DefaultEventsURI), "/") + remotePath), + OptionEndpointURI(strings.TrimRight(eventsURI, "/") + remotePath), OptionClient{Client: httpClient}, } diff --git a/internal/events/event-summarizing-relay.go b/internal/events/event-summarizing-relay.go index 96f7a181..81eb6bac 100644 --- a/internal/events/event-summarizing-relay.go +++ b/internal/events/event-summarizing-relay.go @@ -65,7 +65,11 @@ func newEventSummarizingRelay(authKey c.SDKCredential, config c.EventsConfig, ht loggers ldlog.Loggers, remotePath string) *eventSummarizingRelay { httpClient := httpConfig.SDKHTTPConfig.CreateHTTPClient() headers := httpConfig.SDKHTTPConfig.GetDefaultHeaders() - eventsURI := strings.TrimRight(config.EventsURI.StringOrElse(c.DefaultEventsURI), "/") + remotePath + eventsURI := config.EventsURI.String() + if eventsURI == "" { + eventsURI = c.DefaultEventsURI + } + eventsURI = strings.TrimRight(eventsURI, "/") + remotePath eventSender := ldevents.NewDefaultEventSender(httpClient, eventsURI, "", headers, loggers) eventsConfig := ldevents.EventsConfiguration{ diff --git a/internal/relayenv/env_context_impl.go b/internal/relayenv/env_context_impl.go index e1762968..8ebe3f9b 100644 --- a/internal/relayenv/env_context_impl.go +++ b/internal/relayenv/env_context_impl.go @@ -91,8 +91,12 @@ func NewEnvContext( envLoggers, allConfig.Events, httpConfig, storeAdapter) } + eventsURI := allConfig.Events.EventsURI.String() + if eventsURI == "" { + eventsURI = config.DefaultEventsURI + } eventsPublisher, err := events.NewHttpEventPublisher(envConfig.SDKKey, envLoggers, - events.OptionUri(allConfig.Events.EventsURI.StringOrElse(config.DefaultEventsURI)), + events.OptionUri(eventsURI), events.OptionClient{Client: httpConfig.Client()}) if err != nil { return nil, fmt.Errorf("unable to create publisher: %s", err) diff --git a/relay_endpoints_events_test.go b/relay_endpoints_events_test.go index fa18f9dc..bd7a28d7 100644 --- a/relay_endpoints_events_test.go +++ b/relay_endpoints_events_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + ct "github.com/launchdarkly/go-configtypes" "github.com/launchdarkly/ld-relay/v6/config" c "github.com/launchdarkly/ld-relay/v6/config" "github.com/launchdarkly/ld-relay/v6/internal/events" @@ -54,8 +55,8 @@ func relayEventsTest(config config.Config, action func(relayEventsTestParams)) { defer eventsServer.Close() config.Events.SendEvents = true - config.Events.EventsURI, _ = c.NewOptAbsoluteURLFromString(eventsServer.URL) - config.Events.FlushInterval = c.NewOptDuration(time.Second) + config.Events.EventsURI, _ = ct.NewOptURLAbsoluteFromString(eventsServer.URL) + config.Events.FlushInterval = ct.NewOptDuration(time.Second) config.Events.Capacity = c.DefaultConfig.Events.Capacity relayTest(config, func(pBase relayTestParams) { diff --git a/relay_endpoints_stream_test.go b/relay_endpoints_stream_test.go index ba42e860..955cc87d 100644 --- a/relay_endpoints_stream_test.go +++ b/relay_endpoints_stream_test.go @@ -11,6 +11,7 @@ import ( "gopkg.in/launchdarkly/go-sdk-common.v2/lduser" "github.com/launchdarkly/eventsource" + ct "github.com/launchdarkly/go-configtypes" c "github.com/launchdarkly/ld-relay/v6/config" ) @@ -27,7 +28,7 @@ func (s streamEndpointTestParams) runBasicStreamTests( invalidCredentialExpectedStatus int, ) { configWithoutTimeLimit := baseConfig - configWithoutTimeLimit.Main.MaxClientConnectionTime = c.OptDuration{} + configWithoutTimeLimit.Main.MaxClientConnectionTime = ct.OptDuration{} relayTest(configWithoutTimeLimit, func(p relayTestParams) { t.Run("success", func(t *testing.T) { @@ -45,7 +46,7 @@ func (s streamEndpointTestParams) runBasicStreamTests( maxConnTime := 100 * time.Millisecond configWithTimeLimit := baseConfig - configWithTimeLimit.Main.MaxClientConnectionTime = c.NewOptDuration(maxConnTime) + configWithTimeLimit.Main.MaxClientConnectionTime = ct.NewOptDuration(maxConnTime) relayTest(configWithTimeLimit, func(p relayTestParams) { t.Run("connection time limit", func(t *testing.T) { diff --git a/relay_other_test.go b/relay_other_test.go index 596f2a5b..84c07dd1 100644 --- a/relay_other_test.go +++ b/relay_other_test.go @@ -10,6 +10,7 @@ import ( "github.com/gorilla/mux" "github.com/stretchr/testify/assert" + ct "github.com/launchdarkly/go-configtypes" c "github.com/launchdarkly/ld-relay/v6/config" ) @@ -75,7 +76,7 @@ func TestRelayJSClientGoalsRoute(t *testing.T) { defer fakeServerWithGoalsEndpoint.Close() config := c.DefaultConfig - config.Main.BaseURI, _ = c.NewOptAbsoluteURLFromString(fakeServerWithGoalsEndpoint.URL) + config.Main.BaseURI, _ = ct.NewOptURLAbsoluteFromString(fakeServerWithGoalsEndpoint.URL) config.Environment = makeEnvConfigs(env) relayTest(config, func(p relayTestParams) { diff --git a/testutils_values_test.go b/testutils_values_test.go index ef6f7198..136cc8b1 100644 --- a/testutils_values_test.go +++ b/testutils_values_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "time" + ct "github.com/launchdarkly/go-configtypes" "github.com/launchdarkly/ld-relay/v6/config" c "github.com/launchdarkly/ld-relay/v6/config" "github.com/launchdarkly/ld-relay/v6/internal/sharedtest" @@ -59,7 +60,7 @@ var testEnvWithTTL = testEnv{ name: "sdk test with TTL", config: config.EnvConfig{ SDKKey: c.SDKKey("sdk-98e2b0b4-2688-4a59-9810-1e0e3d7e42d5"), - TTL: config.NewOptDuration(10 * time.Minute), + TTL: ct.NewOptDuration(10 * time.Minute), }, } From 552c5bf629d1ae6e5754eaad3255dbe50275658d Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Jul 2020 12:50:09 -0700 Subject: [PATCH 02/15] use configtypes.VarReader --- config/config.go | 88 ++++++------- config/config_from_env.go | 200 +++++++----------------------- config/config_from_env_test.go | 13 +- config/test_data_test.go | 4 +- internal/httpconfig/httpconfig.go | 3 +- relay.go | 5 +- 6 files changed, 102 insertions(+), 211 deletions(-) diff --git a/config/config.go b/config/config.go index a98e6816..80798469 100644 --- a/config/config.go +++ b/config/config.go @@ -64,28 +64,28 @@ type Config struct { // // This corresponds to the [Main] section in the configuration file. type MainConfig struct { - ExitOnError bool - ExitAlways bool - IgnoreConnectionErrors bool - StreamURI ct.OptURLAbsolute - BaseURI ct.OptURLAbsolute - Port int - HeartbeatInterval ct.OptDuration - MaxClientConnectionTime ct.OptDuration - TLSEnabled bool - TLSCert string - TLSKey string - LogLevel OptLogLevel + ExitOnError bool `conf:"EXIT_ON_ERROR"` + ExitAlways bool `conf:"EXIT_ALWAYS"` + IgnoreConnectionErrors bool `conf:"IGNORE_CONNECTION_ERRORS"` + StreamURI ct.OptURLAbsolute `conf:"STREAM_URI"` + BaseURI ct.OptURLAbsolute `conf:"BASE_URI"` + Port int `conf:"PORT"` + HeartbeatInterval ct.OptDuration `conf:"HEARTBEAT_INTERVAL"` + MaxClientConnectionTime ct.OptDuration `conf:"MAX_CLIENT_CONNECTION_TIME"` + TLSEnabled bool `conf:"TLS_ENABLED"` + TLSCert string `conf:"TLS_CERT"` + TLSKey string `conf:"TLS_KEY"` + LogLevel OptLogLevel `conf:"LOG_LEVEL"` } // EventsConfig contains configuration parameters for proxying events. type EventsConfig struct { - EventsURI ct.OptURLAbsolute - SendEvents bool - FlushInterval ct.OptDuration + EventsURI ct.OptURLAbsolute `conf:"EVENTS_HOST"` + SendEvents bool `conf:"USE_EVENTS"` + FlushInterval ct.OptDuration `conf:"EVENTS_FLUSH_INTERVAL"` SamplingInterval int32 - Capacity int - InlineUsers bool + Capacity int `conf:"EVENTS_CAPACITY"` + InlineUsers bool `conf:"EVENTS_INLINE_USERS"` } // RedisConfig configures the optional Redis integration. @@ -96,12 +96,12 @@ type EventsConfig struct { // // This corresponds to the [Redis] section in the configuration file. type RedisConfig struct { - Host string + Host string `conf:"REDIS_HOST"` Port int - URL ct.OptURLAbsolute - LocalTTL ct.OptDuration - TLS bool - Password string + URL ct.OptURLAbsolute `conf:"REDIS_URL"` + LocalTTL ct.OptDuration `conf:"CACHE_TTL"` + TLS bool `conf:"REDIS_TLS"` + Password string `conf:"REDIS_PASSWORD"` } // ConsulConfig configures the optional Consul integration. @@ -110,18 +110,18 @@ type RedisConfig struct { // // This corresponds to the [Consul] section in the configuration file. type ConsulConfig struct { - Host string - LocalTTL ct.OptDuration + Host string `conf:"CONSUL_HOST"` + LocalTTL ct.OptDuration `conf:"CACHE_TTL"` } // DynamoDBConfig configures the optional DynamoDB integration, which is used only if Enabled is true. // // This corresponds to the [DynamoDB] section in the configuration file. type DynamoDBConfig struct { - Enabled bool - TableName string - URL ct.OptURLAbsolute - LocalTTL ct.OptDuration + Enabled bool `conf:"USE_DYNAMODB"` + TableName string `conf:"DYNAMODB_TABLE"` + URL ct.OptURLAbsolute `conf:"DYNAMODB_URL"` + LocalTTL ct.OptDuration `conf:"CACHE_TTL"` } // EnvConfig describes an environment to be relayed. There may be any number of these. @@ -133,23 +133,23 @@ type EnvConfig struct { APIKey string // deprecated, equivalent to SdkKey MobileKey MobileKey EnvID EnvironmentID - Prefix string // used only if Redis, Consul, or DynamoDB is enabled - TableName string // used only if DynamoDB is enabled - AllowedOrigin []string - SecureMode bool + Prefix string `conf:"LD_PREFIX_"` // used only if Redis, Consul, or DynamoDB is enabled + TableName string `conf:"LD_TABLE_NAME_"` // used only if DynamoDB is enabled + AllowedOrigin ct.OptStringList `conf:"LD_ALLOWED_ORIGIN_"` + SecureMode bool `conf:"LD_SECURE_MODE_"` InsecureSkipVerify bool - LogLevel OptLogLevel - TTL ct.OptDuration + LogLevel OptLogLevel `conf:"LD_LOG_LEVEL_"` + TTL ct.OptDuration `conf:"LD_TTL_"` } // ProxyConfig represents all the supported proxy options. type ProxyConfig struct { - URL ct.OptURLAbsolute - NTLMAuth bool - User string - Password string - Domain string - CACertFiles string + URL ct.OptURLAbsolute `conf:"PROXY_URL"` + NTLMAuth bool `conf:"PROXY_AUTH_NTLM"` + User string `conf:"PROXY_AUTH_USER"` + Password string `conf:"PROXY_AUTH_PASSWORD"` + Domain string `conf:"PROXY_AUTH_DOMAIN"` + CACertFiles ct.OptStringList `conf:"PROXY_CA_CERTS"` } // MetricsConfig contains configurations for optional metrics integrations. @@ -171,8 +171,8 @@ type CommonMetricsConfig struct { // // This corresponds to the [Datadog] section in the configuration file. type DatadogConfig struct { - TraceAddr string - StatsAddr string + TraceAddr string `conf:"DATADOG_TRACE_ADDR"` + StatsAddr string `conf:"DATADOG_STATS_ADDR"` Tag []string CommonMetricsConfig } @@ -181,7 +181,7 @@ type DatadogConfig struct { // // This corresponds to the [StackdriverConfig] section in the configuration file. type StackdriverConfig struct { - ProjectID string + ProjectID string `conf:"STACKDRIVER_PROJECT_ID"` CommonMetricsConfig } @@ -189,7 +189,7 @@ type StackdriverConfig struct { // // This corresponds to the [PrometheusConfig] section in the configuration file. type PrometheusConfig struct { - Port int + Port int `conf:"PROMETHEUS_PORT"` CommonMetricsConfig } diff --git a/config/config_from_env.go b/config/config_from_env.go index 97c21668..80bfce77 100644 --- a/config/config_from_env.go +++ b/config/config_from_env.go @@ -1,14 +1,13 @@ package config import ( - "encoding" "errors" "fmt" "os" - "sort" "strconv" "strings" + ct "github.com/launchdarkly/go-configtypes" "gopkg.in/launchdarkly/go-sdk-common.v2/ldlog" ) @@ -16,42 +15,21 @@ import ( // // The Config parameter should be initialized with default values first. func LoadConfigFromEnvironment(c *Config, loggers ldlog.Loggers) error { - errs := make([]error, 0, 20) - - maybeSetFromEnvInt(&c.Main.Port, "PORT", &errs) - maybeSetFromEnvAny(&c.Main.BaseURI, "BASE_URI", &errs) - maybeSetFromEnvAny(&c.Main.StreamURI, "STREAM_URI", &errs) - maybeSetFromEnvBool(&c.Main.ExitOnError, "EXIT_ON_ERROR") - maybeSetFromEnvBool(&c.Main.ExitAlways, "EXIT_ALWAYS") - maybeSetFromEnvBool(&c.Main.IgnoreConnectionErrors, "IGNORE_CONNECTION_ERRORS") - maybeSetFromEnvAny(&c.Main.HeartbeatInterval, "HEARTBEAT_INTERVAL", &errs) - maybeSetFromEnvAny(&c.Main.MaxClientConnectionTime, "MAX_CLIENT_CONNECTION_TIME", &errs) - maybeSetFromEnvBool(&c.Main.TLSEnabled, "TLS_ENABLED") - maybeSetFromEnv(&c.Main.TLSCert, "TLS_CERT") - maybeSetFromEnv(&c.Main.TLSKey, "TLS_KEY") - maybeSetFromEnvAny(&c.Main.LogLevel, "LOG_LEVEL", &errs) - - maybeSetFromEnvBool(&c.Events.SendEvents, "USE_EVENTS") - maybeSetFromEnvAny(&c.Events.EventsURI, "EVENTS_HOST", &errs) - maybeSetFromEnvAny(&c.Events.FlushInterval, "EVENTS_FLUSH_INTERVAL", &errs) - maybeSetFromEnvInt32(&c.Events.SamplingInterval, "EVENTS_SAMPLING_INTERVAL", &errs) - maybeSetFromEnvInt(&c.Events.Capacity, "EVENTS_CAPACITY", &errs) - maybeSetFromEnvBool(&c.Events.InlineUsers, "EVENTS_INLINE_USERS") - - envNames, envKeys := getEnvVarsWithPrefix("LD_ENV_") - for _, envName := range envNames { - ec := EnvConfig{SDKKey: SDKKey(envKeys[envName])} + reader := ct.NewVarReaderFromEnvironment() + + reader.ReadStruct(&c.Main, false) + reader.ReadStruct(&c.Events, false) + + maybeSetFromEnvInt32(&c.Events.SamplingInterval, "EVENTS_SAMPLING_INTERVAL", reader) + + for envName, envKey := range reader.FindPrefixedValues("LD_ENV_") { + ec := EnvConfig{SDKKey: SDKKey(envKey)} + subReader := reader.WithVarNameSuffix(envName) + subReader.ReadStruct(&ec, false) + ec.MobileKey = MobileKey(maybeEnvStr("LD_MOBILE_KEY_"+envName, string(ec.MobileKey))) ec.EnvID = EnvironmentID(maybeEnvStr("LD_CLIENT_SIDE_ID_"+envName, string(ec.EnvID))) - maybeSetFromEnvBool(&ec.SecureMode, "LD_SECURE_MODE_"+envName) - maybeSetFromEnv(&ec.Prefix, "LD_PREFIX_"+envName) - maybeSetFromEnv(&ec.TableName, "LD_TABLE_NAME_"+envName) - maybeSetFromEnvAny(&ec.TTL, "LD_TTL_"+envName, &errs) - rejectObsoleteVariableName("LD_TTL_MINUTES_"+envName, "LD_TTL_"+envName, &errs) - if s := os.Getenv("LD_ALLOWED_ORIGIN_" + envName); s != "" { - ec.AllowedOrigin = strings.Split(s, ",") - } - maybeSetFromEnvAny(&ec.LogLevel, "LD_LOG_LEVEL_"+envName, &errs) + rejectObsoleteVariableName("LD_TTL_MINUTES_"+envName, "LD_TTL_"+envName, reader) // Not supported: EnvConfig.InsecureSkipVerify (that flag should only be used for testing, never in production) if c.Environment == nil { c.Environment = make(map[string]*EnvConfig) @@ -60,15 +38,14 @@ func LoadConfigFromEnvironment(c *Config, loggers ldlog.Loggers) error { } useRedis := false - maybeSetFromEnvBool(&useRedis, "USE_REDIS") + reader.Read("USE_REDIS", &useRedis) if useRedis || c.Redis.Host != "" || c.Redis.URL.IsDefined() { portStr := "" if c.Redis.Port > 0 { portStr = fmt.Sprintf("%d", c.Redis.Port) } - maybeSetFromEnvAny(&c.Redis.URL, "REDIS_URL", &errs) - maybeSetFromEnv(&c.Redis.Host, "REDIS_HOST") - maybeSetFromEnv(&portStr, "REDIS_PORT") + reader.ReadStruct(&c.Redis, false) + reader.Read("REDIS_PORT", &portStr) // handled separately because it could be a string or a number if portStr != "" { if strings.HasPrefix(portStr, "tcp://") { @@ -77,88 +54,71 @@ func LoadConfigFromEnvironment(c *Config, loggers ldlog.Loggers) error { fields := strings.Split(hostAndPort, ":") c.Redis.Host = fields[0] if len(fields) > 0 { - setInt(&c.Redis.Port, "REDIS_PORT", fields[1], &errs) + setInt(&c.Redis.Port, "REDIS_PORT", fields[1], reader) } } else { if c.Redis.Host == "" { c.Redis.Host = defaultRedisHost } - setInt(&c.Redis.Port, "REDIS_PORT", portStr, &errs) + setInt(&c.Redis.Port, "REDIS_PORT", portStr, reader) } } if !c.Redis.URL.IsDefined() && c.Redis.Host == "" && c.Redis.Port == 0 { // all they specified was USE_REDIS c.Redis.URL = defaultRedisURL } - maybeSetFromEnvBool(&c.Redis.TLS, "REDIS_TLS") - maybeSetFromEnv(&c.Redis.Password, "REDIS_PASSWORD") - maybeSetFromEnvAny(&c.Redis.LocalTTL, "CACHE_TTL", &errs) - rejectObsoleteVariableName("REDIS_TTL", "CACHE_TTL", &errs) + rejectObsoleteVariableName("REDIS_TTL", "CACHE_TTL", reader) } useConsul := false - maybeSetFromEnvBool(&useConsul, "USE_CONSUL") + reader.Read("USE_CONSUL", &useConsul) if useConsul { c.Consul.Host = defaultConsulHost - maybeSetFromEnv(&c.Consul.Host, "CONSUL_HOST") - maybeSetFromEnvAny(&c.Consul.LocalTTL, "CACHE_TTL", &errs) + reader.ReadStruct(&c.Consul, false) } - maybeSetFromEnvBool(&c.DynamoDB.Enabled, "USE_DYNAMODB") + reader.Read("USE_DYNAMODB", &c.DynamoDB.Enabled) if c.DynamoDB.Enabled { - maybeSetFromEnv(&c.DynamoDB.TableName, "DYNAMODB_TABLE") - maybeSetFromEnvAny(&c.DynamoDB.URL, "DYNAMODB_URL", &errs) - maybeSetFromEnvAny(&c.DynamoDB.LocalTTL, "CACHE_TTL", &errs) + reader.ReadStruct(&c.DynamoDB, false) } - maybeSetFromEnvBool(&c.MetricsConfig.Datadog.Enabled, "USE_DATADOG") + reader.Read("USE_DATADOG", &c.MetricsConfig.Datadog.Enabled) if c.MetricsConfig.Datadog.Enabled { - maybeSetFromEnv(&c.MetricsConfig.Datadog.Prefix, "DATADOG_PREFIX") - maybeSetFromEnv(&c.MetricsConfig.Datadog.TraceAddr, "DATADOG_TRACE_ADDR") - maybeSetFromEnv(&c.MetricsConfig.Datadog.StatsAddr, "DATADOG_STATS_ADDR") - tagNames, tagVals := getEnvVarsWithPrefix("DATADOG_TAG_") - for _, tagName := range tagNames { - c.MetricsConfig.Datadog.Tag = append(c.MetricsConfig.Datadog.Tag, tagName+":"+tagVals[tagName]) + reader.Read("DATADOG_PREFIX", &c.MetricsConfig.Datadog.Prefix) + reader.ReadStruct(&c.MetricsConfig.Datadog, false) + for tagName, tagVal := range reader.FindPrefixedValues("DATADOG_TAG_") { + c.MetricsConfig.Datadog.Tag = append(c.MetricsConfig.Datadog.Tag, tagName+":"+tagVal) } } - maybeSetFromEnvBool(&c.MetricsConfig.Stackdriver.Enabled, "USE_STACKDRIVER") + reader.Read("USE_STACKDRIVER", &c.MetricsConfig.Stackdriver.Enabled) if c.MetricsConfig.Stackdriver.Enabled { - maybeSetFromEnv(&c.MetricsConfig.Stackdriver.Prefix, "STACKDRIVER_PREFIX") - maybeSetFromEnv(&c.MetricsConfig.Stackdriver.ProjectID, "STACKDRIVER_PROJECT_ID") + reader.ReadStruct(&c.MetricsConfig.Stackdriver, false) + reader.Read("STACKDRIVER_PREFIX", &c.MetricsConfig.Stackdriver.Prefix) } - maybeSetFromEnvBool(&c.MetricsConfig.Prometheus.Enabled, "USE_PROMETHEUS") + reader.Read("USE_PROMETHEUS", &c.MetricsConfig.Prometheus.Enabled) if c.MetricsConfig.Prometheus.Enabled { - maybeSetFromEnv(&c.MetricsConfig.Prometheus.Prefix, "PROMETHEUS_PREFIX") - maybeSetFromEnvInt(&c.MetricsConfig.Prometheus.Port, "PROMETHEUS_PORT", &errs) + reader.ReadStruct(&c.MetricsConfig.Prometheus, false) + reader.Read("PROMETHEUS_PREFIX", &c.MetricsConfig.Prometheus.Prefix) } - maybeSetFromEnvAny(&c.Proxy.URL, "PROXY_URL", &errs) - maybeSetFromEnv(&c.Proxy.User, "PROXY_AUTH_USER") - maybeSetFromEnv(&c.Proxy.Password, "PROXY_AUTH_PASSWORD") - maybeSetFromEnv(&c.Proxy.Domain, "PROXY_AUTH_DOMAIN") - maybeSetFromEnvBool(&c.Proxy.NTLMAuth, "PROXY_AUTH_NTLM") - maybeSetFromEnv(&c.Proxy.CACertFiles, "PROXY_CA_CERTS") - - if len(errs) > 0 { - ss := make([]string, 0, len(errs)) - for _, e := range errs { - ss = append(ss, e.Error()) - } - return errors.New(strings.Join(ss, ", ")) + reader.ReadStruct(&c.Proxy, false) + + if !reader.Result().OK() { + return reader.Result().GetError() } return ValidateConfig(c, loggers) } -func rejectObsoleteVariableName(oldName, preferredName string, errs *[]error) { +func rejectObsoleteVariableName(oldName, preferredName string, reader *ct.VarReader) { // Unrecognized environment variables are normally ignored, but if someone has set a variable that // used to be used in configuration and is no longer used, we want to raise an error rather than just // silently omitting part of the configuration that they thought they had set. if os.Getenv(oldName) != "" { - *errs = append(*errs, fmt.Errorf("environment variable %s is no longer supported; use %s", - oldName, preferredName)) + reader.AddError(ct.ValidationPath{oldName}, + fmt.Errorf("this variable is no longer supported; use %s", preferredName)) } } @@ -169,81 +129,17 @@ func maybeEnvStr(name string, defaultVal string) string { return defaultVal } -func maybeSetFromEnv(prop *string, name string) bool { - if s := os.Getenv(name); s != "" { - *prop = s - return true - } - return false -} - -func maybeSetFromEnvAny(prop encoding.TextUnmarshaler, name string, errs *[]error) bool { - if s := os.Getenv(name); s != "" { - err := prop.UnmarshalText([]byte(s)) - if err != nil { - *errs = append(*errs, fmt.Errorf("%s: %s", name, err.Error())) - } - return true - } - return false -} - -func maybeSetFromEnvInt(prop *int, name string, errs *[]error) bool { - if s := os.Getenv(name); s != "" { - setInt(prop, name, s, errs) - return true - } - return false -} - -func setInt(prop *int, name string, value string, errs *[]error) { +func setInt(prop *int, name string, value string, reader *ct.VarReader) { if n, err := strconv.Atoi(value); err != nil { - *errs = append(*errs, fmt.Errorf("%s: must be an integer", name)) + reader.AddError(ct.ValidationPath{name}, errors.New("not a valid integer")) } else { *prop = n } } -func maybeSetFromEnvInt32(prop *int32, name string, errs *[]error) bool { - if s := os.Getenv(name); s != "" { - var n int64 - var err error - if n, err = strconv.ParseInt(s, 10, 32); err != nil { - *errs = append(*errs, fmt.Errorf("%s: must be an integer", name)) - } else { - *prop = int32(n) - } - return true - } - return false -} - -func maybeSetFromEnvBool(prop *bool, name string) bool { - if s, found := os.LookupEnv(name); found { - if s == "1" || s == "true" { - *prop = true - } else { - *prop = false - } - return true - } - return false -} - -func getEnvVarsWithPrefix(prefix string) ([]string, map[string]string) { - names := []string{} - values := make(map[string]string) - allVars := os.Environ() - sort.Strings(allVars) - for _, e := range allVars { - if strings.HasPrefix(e, prefix) { - fields := strings.Split(e, "=") - if len(fields) == 2 { - strippedName := strings.TrimPrefix(fields[0], prefix) - names = append(names, strippedName) - values[strippedName] = fields[1] - } - } +func maybeSetFromEnvInt32(prop *int32, name string, reader *ct.VarReader) { + var n int + if reader.Read(name, &n) { + *prop = int32(n) } - return names, values } diff --git a/config/config_from_env_test.go b/config/config_from_env_test.go index c9aa343d..aa2db74e 100644 --- a/config/config_from_env_test.go +++ b/config/config_from_env_test.go @@ -36,7 +36,7 @@ func TestConfigFromEnvironmentDisallowsObsoleteVariables(t *testing.T) { "USE_REDIS": "1", "REDIS_TTL": "500", }, - "environment variable REDIS_TTL is no longer supported; use CACHE_TTL", + "REDIS_TTL: this variable is no longer supported; use CACHE_TTL", ) }) @@ -46,7 +46,7 @@ func TestConfigFromEnvironmentDisallowsObsoleteVariables(t *testing.T) { "LD_ENV_envname": "key", "LD_TTL_MINUTES_envname": "3", }, - "environment variable LD_TTL_MINUTES_envname is no longer supported; use LD_TTL_envname", + "LD_TTL_MINUTES_envname: this variable is no longer supported; use LD_TTL_envname", ) }) } @@ -71,11 +71,10 @@ func TestConfigFromEnvironmentFieldValidation(t *testing.T) { ) }) - t.Run("treats unrecognized boolean values as false", func(t *testing.T) { - // TODO: not sure this is desirable - testValidConfigVars(t, - func(c *Config) { c.Main.ExitOnError = false }, + t.Run("rejects invalid boolean", func(t *testing.T) { + testInvalidConfigVars(t, map[string]string{"EXIT_ON_ERROR": "not really"}, + "EXIT_ON_ERROR: not a valid boolean", ) }) @@ -89,7 +88,7 @@ func TestConfigFromEnvironmentFieldValidation(t *testing.T) { t.Run("rejects invalid int", func(t *testing.T) { testInvalidConfigVars(t, map[string]string{"PORT": "not-numeric"}, - "PORT: must be an integer", + "PORT: not a valid integer", ) }) diff --git a/config/test_data_test.go b/config/test_data_test.go index 241202e1..b7e7f717 100644 --- a/config/test_data_test.go +++ b/config/test_data_test.go @@ -96,7 +96,7 @@ func makeValidConfigAllBaseProperties() testDataValidConfig { SecureMode: true, Prefix: "krypton-", TableName: "krypton-table", - AllowedOrigin: []string{"https://oa", "https://rann"}, + AllowedOrigin: ct.NewOptStringList([]string{"https://oa", "https://rann"}), TTL: ct.NewOptDuration(5 * time.Minute), }, } @@ -495,7 +495,7 @@ func makeValidConfigProxy() testDataValidConfig { Password: "pass", Domain: "domain", NTLMAuth: true, - CACertFiles: "cert", + CACertFiles: ct.NewOptStringList([]string{"cert"}), } } c.envVars = map[string]string{ diff --git a/internal/httpconfig/httpconfig.go b/internal/httpconfig/httpconfig.go index d23172c6..91017dea 100644 --- a/internal/httpconfig/httpconfig.go +++ b/internal/httpconfig/httpconfig.go @@ -3,7 +3,6 @@ package httpconfig import ( "errors" "net/http" - "strings" "github.com/launchdarkly/ld-relay/v6/config" "github.com/launchdarkly/ld-relay/v6/internal/version" @@ -35,7 +34,7 @@ func NewHTTPConfig(proxyConfig config.ProxyConfig, sdkKey config.SDKKey, loggers loggers.Infof("Using proxy server at %s", proxyConfig.URL) } - caCertFiles := strings.Split(strings.TrimSpace(proxyConfig.CACertFiles), ",") + caCertFiles := proxyConfig.CACertFiles.Values() if proxyConfig.NTLMAuth { if proxyConfig.User == "" || proxyConfig.Password == "" { diff --git a/relay.go b/relay.go index e9821b1c..4479e348 100644 --- a/relay.go +++ b/relay.go @@ -142,10 +142,7 @@ func NewRelay(c config.Config, loggers ldlog.Loggers, clientFactory sdkconfig.Cl } if envConfig.EnvID != "" { - var allowedOrigins []string - if len(envConfig.AllowedOrigin) != 0 { - allowedOrigins = envConfig.AllowedOrigin - } + allowedOrigins := envConfig.AllowedOrigin.Values() cachingTransport := httpcache.NewMemoryCacheTransport() if envConfig.InsecureSkipVerify { transport := &(*http.DefaultTransport.(*http.Transport)) From 033a8c402cd688ad591942013cebc323f4d731a0 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Jul 2020 13:48:09 -0700 Subject: [PATCH 03/15] simplify validation --- config/config.go | 13 ++++++------- config/config_field_types.go | 18 ++++++++++++++++++ config/config_from_env.go | 14 +++----------- config/config_from_env_test.go | 8 +++++--- config/config_from_file_test.go | 29 ----------------------------- config/config_validation.go | 24 +++++++++--------------- config/test_data_test.go | 25 +++++++++++++++++++++++++ 7 files changed, 66 insertions(+), 65 deletions(-) diff --git a/config/config.go b/config/config.go index 80798469..9562482c 100644 --- a/config/config.go +++ b/config/config.go @@ -129,17 +129,16 @@ type DynamoDBConfig struct { // This corresponds to one of the [environment "env-name"] sections in the configuration file. In the // Config.Environment map, each key is an environment name and each value is an EnvConfig. type EnvConfig struct { - SDKKey SDKKey - APIKey string // deprecated, equivalent to SdkKey - MobileKey MobileKey - EnvID EnvironmentID + SDKKey SDKKey // set from env var LD_ENV_envname + MobileKey MobileKey `conf:"LD_MOBILE_KEY_"` + EnvID EnvironmentID `conf:"LD_CLIENT_SIDE_ID_"` Prefix string `conf:"LD_PREFIX_"` // used only if Redis, Consul, or DynamoDB is enabled TableName string `conf:"LD_TABLE_NAME_"` // used only if DynamoDB is enabled AllowedOrigin ct.OptStringList `conf:"LD_ALLOWED_ORIGIN_"` SecureMode bool `conf:"LD_SECURE_MODE_"` - InsecureSkipVerify bool - LogLevel OptLogLevel `conf:"LD_LOG_LEVEL_"` - TTL ct.OptDuration `conf:"LD_TTL_"` + InsecureSkipVerify bool // deliberately not settable by env vars + LogLevel OptLogLevel `conf:"LD_LOG_LEVEL_"` + TTL ct.OptDuration `conf:"LD_TTL_"` } // ProxyConfig represents all the supported proxy options. diff --git a/config/config_field_types.go b/config/config_field_types.go index cb7ac162..3b1f017f 100644 --- a/config/config_field_types.go +++ b/config/config_field_types.go @@ -45,6 +45,24 @@ func (k EnvironmentID) GetAuthorizationHeaderValue() string { return "" } +// UnmarshalText allows the SDKKey type to be set from environment variables. +func (k *SDKKey) UnmarshalText(data []byte) error { + *k = SDKKey(string(data)) + return nil +} + +// UnmarshalText allows the MobileKey type to be set from environment variables. +func (k *MobileKey) UnmarshalText(data []byte) error { + *k = MobileKey(string(data)) + return nil +} + +// UnmarshalText allows the EnvironmentID type to be set from environment variables. +func (k *EnvironmentID) UnmarshalText(data []byte) error { + *k = EnvironmentID(string(data)) + return nil +} + func newOptURLAbsoluteMustBeValid(urlString string) ct.OptURLAbsolute { o, err := ct.NewOptURLAbsoluteFromString(urlString) if err != nil { diff --git a/config/config_from_env.go b/config/config_from_env.go index 80bfce77..208fddc0 100644 --- a/config/config_from_env.go +++ b/config/config_from_env.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "sort" "strconv" "strings" @@ -26,9 +27,6 @@ func LoadConfigFromEnvironment(c *Config, loggers ldlog.Loggers) error { ec := EnvConfig{SDKKey: SDKKey(envKey)} subReader := reader.WithVarNameSuffix(envName) subReader.ReadStruct(&ec, false) - - ec.MobileKey = MobileKey(maybeEnvStr("LD_MOBILE_KEY_"+envName, string(ec.MobileKey))) - ec.EnvID = EnvironmentID(maybeEnvStr("LD_CLIENT_SIDE_ID_"+envName, string(ec.EnvID))) rejectObsoleteVariableName("LD_TTL_MINUTES_"+envName, "LD_TTL_"+envName, reader) // Not supported: EnvConfig.InsecureSkipVerify (that flag should only be used for testing, never in production) if c.Environment == nil { @@ -60,7 +58,7 @@ func LoadConfigFromEnvironment(c *Config, loggers ldlog.Loggers) error { if c.Redis.Host == "" { c.Redis.Host = defaultRedisHost } - setInt(&c.Redis.Port, "REDIS_PORT", portStr, reader) + reader.Read("REDIS_PORT", &c.Redis.Port) } } if !c.Redis.URL.IsDefined() && c.Redis.Host == "" && c.Redis.Port == 0 { @@ -89,6 +87,7 @@ func LoadConfigFromEnvironment(c *Config, loggers ldlog.Loggers) error { for tagName, tagVal := range reader.FindPrefixedValues("DATADOG_TAG_") { c.MetricsConfig.Datadog.Tag = append(c.MetricsConfig.Datadog.Tag, tagName+":"+tagVal) } + sort.Strings(c.MetricsConfig.Datadog.Tag) // for test determinacy } reader.Read("USE_STACKDRIVER", &c.MetricsConfig.Stackdriver.Enabled) @@ -122,13 +121,6 @@ func rejectObsoleteVariableName(oldName, preferredName string, reader *ct.VarRea } } -func maybeEnvStr(name string, defaultVal string) string { - if s := os.Getenv(name); s != "" { - return s - } - return defaultVal -} - func setInt(prop *int, name string, value string, reader *ct.VarReader) { if n, err := strconv.Atoi(value); err != nil { reader.AddError(ct.ValidationPath{name}, errors.New("not a valid integer")) diff --git a/config/config_from_env_test.go b/config/config_from_env_test.go index aa2db74e..8586b454 100644 --- a/config/config_from_env_test.go +++ b/config/config_from_env_test.go @@ -23,9 +23,11 @@ func TestConfigFromEnvironmentWithValidProperties(t *testing.T) { func TestConfigFromEnvironmentWithInvalidProperties(t *testing.T) { for _, tdc := range makeInvalidConfigs() { - t.Run(tdc.name, func(t *testing.T) { - testInvalidConfigVars(t, tdc.envVars, tdc.envVarsError) - }) + if len(tdc.envVars) != 0 { + t.Run(tdc.name, func(t *testing.T) { + testInvalidConfigVars(t, tdc.envVars, tdc.envVarsError) + }) + } } } diff --git a/config/config_from_file_test.go b/config/config_from_file_test.go index 47639160..6965badb 100644 --- a/config/config_from_file_test.go +++ b/config/config_from_file_test.go @@ -42,35 +42,6 @@ func TestConfigFromFileWithInvalidProperties(t *testing.T) { } } -func TestConfigFromFileDeprecatedUsage(t *testing.T) { - t.Run("apiKey is allowed instead of sdkKey", func(t *testing.T) { - testFileWithValidConfig(t, - func(c *Config) { - c.Environment = make(map[string]*EnvConfig) - c.Environment["envname"] = &EnvConfig{ - SDKKey: SDKKey("key"), - } - }, - `[Environment "envname"] -apiKey = key`, - ) - }) - - t.Run("if both apiKey and sdkKey are set, sdkKey is used", func(t *testing.T) { - testFileWithValidConfig(t, - func(c *Config) { - c.Environment = make(map[string]*EnvConfig) - c.Environment["envname"] = &EnvConfig{ - SDKKey: SDKKey("key"), - } - }, - `[Environment "envname"] -sdkKey = key -apiKey = wrong`, - ) - }) -} - func TestConfigFromFileBasicValidation(t *testing.T) { t.Run("allows boolean values 0/1 or true/false", func(t *testing.T) { testFileWithValidConfig(t, diff --git a/config/config_validation.go b/config/config_validation.go index b22e858f..25868211 100644 --- a/config/config_validation.go +++ b/config/config_validation.go @@ -21,26 +21,20 @@ import ( // called again by the Relay constructor because it is possible for application code that uses Relay as a // library to construct a Config programmatically. func ValidateConfig(c *Config, loggers ldlog.Loggers) error { + var result ct.ValidationResult if c.Main.TLSEnabled && (c.Main.TLSCert == "" || c.Main.TLSKey == "") { - return errors.New("TLS cert and key are required if TLS is enabled") + result.AddError(nil, errors.New("TLS cert and key are required if TLS is enabled")) } for envName, envConfig := range c.Environment { - if envConfig.APIKey != "" { - if envConfig.SDKKey == "" { - envConfig.SDKKey = SDKKey(envConfig.APIKey) - c.Environment[envName] = envConfig - loggers.Warn(`"apiKey" is deprecated, please use "sdkKey"`) - } else { - loggers.Warn(`"apiKey" and "sdkKey" were both specified; "apiKey" is deprecated, will use "sdkKey" value`) - } - envConfig.APIKey = "" // to avoid confusion if any other code sees this + if envConfig.SDKKey == "" { + result.AddError(nil, fmt.Errorf("SDK key is required for environment %q", envName)) } } if c.Redis.URL.IsDefined() { if c.Redis.Host != "" || c.Redis.Port != 0 { - return errors.New("please specify Redis URL or host/port, but not both") + result.AddError(nil, errors.New("please specify Redis URL or host/port, but not both")) } } else if c.Redis.Host != "" || c.Redis.Port != 0 { host, port := c.Redis.Host, c.Redis.Port @@ -52,7 +46,7 @@ func ValidateConfig(c *Config, loggers ldlog.Loggers) error { } url, err := ct.NewOptURLAbsoluteFromString(fmt.Sprintf("redis://%s:%d", host, port)) if err != nil { - return errors.New("invalid Redis hostname") + result.AddError(nil, errors.New("invalid Redis hostname")) } c.Redis.URL = url c.Redis.Host = "" @@ -70,9 +64,9 @@ func ValidateConfig(c *Config, loggers ldlog.Loggers) error { databases = append(databases, "DynamoDB") } if len(databases) > 1 { - return fmt.Errorf("multiple databases are enabled (%s); only one is allowed", - strings.Join(databases, ", ")) + result.AddError(nil, fmt.Errorf("multiple databases are enabled (%s); only one is allowed", + strings.Join(databases, ", "))) } - return nil + return result.GetError() } diff --git a/config/test_data_test.go b/config/test_data_test.go index b7e7f717..a97866fa 100644 --- a/config/test_data_test.go +++ b/config/test_data_test.go @@ -46,6 +46,7 @@ func makeValidConfigs() []testDataValidConfig { func makeInvalidConfigs() []testDataInvalidConfig { return []testDataInvalidConfig{ + makeInvalidConfigMissingSDKKey(), makeInvalidConfigTLSWithNoCertOrKey(), makeInvalidConfigTLSWithNoCert(), makeInvalidConfigTLSWithNoKey(), @@ -518,10 +519,24 @@ CaCertFiles = "cert" return c } +func makeInvalidConfigMissingSDKKey() testDataInvalidConfig { + c := testDataInvalidConfig{name: "environment without SDK key"} + c.fileContent = ` +[Environment "envname"] +MobileKey = mob-xxx +` + c.fileError = `SDK key is required for environment "envname"` + return c +} + func makeInvalidConfigTLSWithNoCertOrKey() testDataInvalidConfig { c := testDataInvalidConfig{name: "TLS without cert/key"} c.envVarsError = "TLS cert and key are required if TLS is enabled" c.envVars = map[string]string{"TLS_ENABLED": "1"} + c.fileContent = ` +[Main] +TLSEnabled = true +` return c } @@ -529,6 +544,11 @@ func makeInvalidConfigTLSWithNoCert() testDataInvalidConfig { c := testDataInvalidConfig{name: "TLS without cert"} c.envVarsError = "TLS cert and key are required if TLS is enabled" c.envVars = map[string]string{"TLS_ENABLED": "1", "TLS_KEY": "key"} + c.fileContent = ` +[Main] +TLSEnabled = true +TLSKey = keyfile +` return c } @@ -536,6 +556,11 @@ func makeInvalidConfigTLSWithNoKey() testDataInvalidConfig { c := testDataInvalidConfig{name: "TLS without key"} c.envVarsError = "TLS cert and key are required if TLS is enabled" c.envVars = map[string]string{"TLS_ENABLED": "1", "TLS_CERT": "cert"} + c.fileContent = ` +[Main] +TLSEnabled = true +TLSCert = certfile +` return c } From c22a39aeb3257ad709e84eb208ee5f0b3effa442 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Jul 2020 16:27:53 -0700 Subject: [PATCH 04/15] use OptIntGreaterThanZero + more validation fixes --- config/config.go | 139 +++++++++++++-------- config/config_field_types.go | 11 +- config/config_from_env.go | 35 ++---- config/config_from_env_test.go | 79 +++++++++++- config/config_from_file_test.go | 23 +++- config/config_validation.go | 12 +- config/test_data_test.go | 45 +++++-- internal/events/event-relay.go | 6 +- internal/events/event-summarizing-relay.go | 2 +- internal/metrics/prometheus.go | 5 +- internal/metrics/prometheus_test.go | 3 +- relay_endpoints_eval_test.go | 6 +- relay_endpoints_events_test.go | 7 +- relay_endpoints_php_test.go | 2 +- relay_endpoints_status_test.go | 2 +- relay_endpoints_stream_test.go | 6 +- relay_other_test.go | 2 +- 17 files changed, 249 insertions(+), 136 deletions(-) diff --git a/config/config.go b/config/config.go index 9562482c..59dbd866 100644 --- a/config/config.go +++ b/config/config.go @@ -8,6 +8,9 @@ import ( ) const ( + // DefaultPort is the port that Relay runs on if not otherwise specified. + DefaultPort = 8030 + // DefaultBaseURI is the default value for the base URI of LaunchDarkly services (polling endpoints). DefaultBaseURI = "https://app.launchdarkly.com" @@ -17,6 +20,9 @@ const ( // DefaultEventsURI is the default value for the base URI of LaunchDarkly services (event endpoints). DefaultEventsURI = "https://events.launchdarkly.com" + // DefaultEventCapacity is the default value for EventsConfig.Capacity if not specified. + DefaultEventCapacity = 1000 + // DefaultHeartbeatInterval is the default value for MainConfig.HeartBeatInterval if not specified. DefaultHeartbeatInterval = time.Minute * 3 @@ -25,19 +31,19 @@ const ( // DefaultDatabaseCacheTTL is the default value for the LocalTTL parameter for databases if not specified. DefaultDatabaseCacheTTL = time.Second * 30 + + // DefaultPrometheusPort is the default value for PrometheusConfig.Port if not specified. + DefaultPrometheusPort = 8031 ) const ( - defaultPort = 8030 - defaultEventCapacity = 1000 - defaultRedisHost = "localhost" - defaultRedisPort = 6379 - defaultConsulHost = "localhost" - defaultPrometheusPort = 8031 + defaultRedisHost = "localhost" + defaultRedisPort = 6379 + defaultConsulHost = "localhost" ) var ( - defaultRedisURL = newOptURLAbsoluteMustBeValid("redis://localhost:6379") + defaultRedisURL, _ = ct.NewOptURLAbsoluteFromString("redis://localhost:6379") ) // DefaultLoggers is the default logging configuration used by Relay. @@ -47,8 +53,17 @@ var DefaultLoggers = logging.MakeDefaultLoggers() // Config describes the configuration for a relay instance. // -// If you are incorporating Relay into your own code and configuring it programmatically, it is best to -// start by copying relay.DefaultConfig and then changing only the fields you need to change. +// Some fields use special types that enforce validation rules, such as URL fields which must +// be absolute URLs, port numbers which must be greater than zero, or durations. This validation +// is done automatically when reading the configuration from a file or from environment variables. +// +// If you are incorporating Relay into your own code and configuring it programmatically, you +// may need to use functions from go-configtypes such as NewOptDuration to set fields that have +// validation rules. +// +// Since configuration options can be set either programmatically, or from a file, or from environment +// variables, individual fields are not documented here; instead, see the `README.md` section on +// configuration. type Config struct { Main MainConfig Events EventsConfig @@ -57,47 +72,62 @@ type Config struct { DynamoDB DynamoDBConfig Environment map[string]*EnvConfig Proxy ProxyConfig + + // Optional configuration for metrics integrations. Note that unlike the other fields in Config, + // MetricsConfig is not the name of a configuration file section; the actual sections are the + // structs within this struct (Datadog, etc.). MetricsConfig } // MainConfig contains global configuration options for Relay. // // This corresponds to the [Main] section in the configuration file. +// +// Since configuration options can be set either programmatically, or from a file, or from environment +// variables, individual fields are not documented here; instead, see the `README.md` section on +// configuration. type MainConfig struct { - ExitOnError bool `conf:"EXIT_ON_ERROR"` - ExitAlways bool `conf:"EXIT_ALWAYS"` - IgnoreConnectionErrors bool `conf:"IGNORE_CONNECTION_ERRORS"` - StreamURI ct.OptURLAbsolute `conf:"STREAM_URI"` - BaseURI ct.OptURLAbsolute `conf:"BASE_URI"` - Port int `conf:"PORT"` - HeartbeatInterval ct.OptDuration `conf:"HEARTBEAT_INTERVAL"` - MaxClientConnectionTime ct.OptDuration `conf:"MAX_CLIENT_CONNECTION_TIME"` - TLSEnabled bool `conf:"TLS_ENABLED"` - TLSCert string `conf:"TLS_CERT"` - TLSKey string `conf:"TLS_KEY"` - LogLevel OptLogLevel `conf:"LOG_LEVEL"` + ExitOnError bool `conf:"EXIT_ON_ERROR"` + ExitAlways bool `conf:"EXIT_ALWAYS"` + IgnoreConnectionErrors bool `conf:"IGNORE_CONNECTION_ERRORS"` + StreamURI ct.OptURLAbsolute `conf:"STREAM_URI"` + BaseURI ct.OptURLAbsolute `conf:"BASE_URI"` + Port ct.OptIntGreaterThanZero `conf:"PORT"` + HeartbeatInterval ct.OptDuration `conf:"HEARTBEAT_INTERVAL"` + MaxClientConnectionTime ct.OptDuration `conf:"MAX_CLIENT_CONNECTION_TIME"` + TLSEnabled bool `conf:"TLS_ENABLED"` + TLSCert string `conf:"TLS_CERT"` + TLSKey string `conf:"TLS_KEY"` + LogLevel OptLogLevel `conf:"LOG_LEVEL"` } // EventsConfig contains configuration parameters for proxying events. +// +// Since configuration options can be set either programmatically, or from a file, or from environment +// variables, individual fields are not documented here; instead, see the `README.md` section on +// configuration. type EventsConfig struct { - EventsURI ct.OptURLAbsolute `conf:"EVENTS_HOST"` - SendEvents bool `conf:"USE_EVENTS"` - FlushInterval ct.OptDuration `conf:"EVENTS_FLUSH_INTERVAL"` - SamplingInterval int32 - Capacity int `conf:"EVENTS_CAPACITY"` - InlineUsers bool `conf:"EVENTS_INLINE_USERS"` + EventsURI ct.OptURLAbsolute `conf:"EVENTS_HOST"` + SendEvents bool `conf:"USE_EVENTS"` + FlushInterval ct.OptDuration `conf:"EVENTS_FLUSH_INTERVAL"` + Capacity ct.OptIntGreaterThanZero `conf:"EVENTS_CAPACITY"` + InlineUsers bool `conf:"EVENTS_INLINE_USERS"` } // RedisConfig configures the optional Redis integration. // -// Redis is enabled if URL or Host is non-empty or if Port is non-zero. If only Host or Port is set, +// Redis is enabled if URL or Host is non-empty or if Port is set. If only Host or Port is set, // the other value is set to defaultRedisPort or defaultRedisHost. It is an error to set Host or // Port if URL is also set. // // This corresponds to the [Redis] section in the configuration file. +// +// Since configuration options can be set either programmatically, or from a file, or from environment +// variables, individual fields are not documented here; instead, see the `README.md` section on +// configuration. type RedisConfig struct { Host string `conf:"REDIS_HOST"` - Port int + Port ct.OptIntGreaterThanZero URL ct.OptURLAbsolute `conf:"REDIS_URL"` LocalTTL ct.OptDuration `conf:"CACHE_TTL"` TLS bool `conf:"REDIS_TLS"` @@ -109,6 +139,10 @@ type RedisConfig struct { // Consul is enabled if Host is non-empty. // // This corresponds to the [Consul] section in the configuration file. +// +// Since configuration options can be set either programmatically, or from a file, or from environment +// variables, individual fields are not documented here; instead, see the `README.md` section on +// configuration. type ConsulConfig struct { Host string `conf:"CONSUL_HOST"` LocalTTL ct.OptDuration `conf:"CACHE_TTL"` @@ -117,6 +151,10 @@ type ConsulConfig struct { // DynamoDBConfig configures the optional DynamoDB integration, which is used only if Enabled is true. // // This corresponds to the [DynamoDB] section in the configuration file. +// +// Since configuration options can be set either programmatically, or from a file, or from environment +// variables, individual fields are not documented here; instead, see the `README.md` section on +// configuration. type DynamoDBConfig struct { Enabled bool `conf:"USE_DYNAMODB"` TableName string `conf:"DYNAMODB_TABLE"` @@ -128,6 +166,10 @@ type DynamoDBConfig struct { // // This corresponds to one of the [environment "env-name"] sections in the configuration file. In the // Config.Environment map, each key is an environment name and each value is an EnvConfig. +// +// Since configuration options can be set either programmatically, or from a file, or from environment +// variables, individual fields are not documented here; instead, see the `README.md` section on +// configuration. type EnvConfig struct { SDKKey SDKKey // set from env var LD_ENV_envname MobileKey MobileKey `conf:"LD_MOBILE_KEY_"` @@ -142,6 +184,10 @@ type EnvConfig struct { } // ProxyConfig represents all the supported proxy options. +// +// Since configuration options can be set either programmatically, or from a file, or from environment +// variables, individual fields are not documented here; instead, see the `README.md` section on +// configuration. type ProxyConfig struct { URL ct.OptURLAbsolute `conf:"PROXY_URL"` NTLMAuth bool `conf:"PROXY_AUTH_NTLM"` @@ -169,6 +215,10 @@ type CommonMetricsConfig struct { // DatadogConfig configures the optional Datadog integration, which is used only if Enabled is true. // // This corresponds to the [Datadog] section in the configuration file. +// +// Since configuration options can be set either programmatically, or from a file, or from environment +// variables, individual fields are not documented here; instead, see the `README.md` section on +// configuration. type DatadogConfig struct { TraceAddr string `conf:"DATADOG_TRACE_ADDR"` StatsAddr string `conf:"DATADOG_STATS_ADDR"` @@ -179,6 +229,10 @@ type DatadogConfig struct { // StackdriverConfig configures the optional Stackdriver integration, which is used only if Enabled is true. // // This corresponds to the [StackdriverConfig] section in the configuration file. +// +// Since configuration options can be set either programmatically, or from a file, or from environment +// variables, individual fields are not documented here; instead, see the `README.md` section on +// configuration. type StackdriverConfig struct { ProjectID string `conf:"STACKDRIVER_PROJECT_ID"` CommonMetricsConfig @@ -187,28 +241,11 @@ type StackdriverConfig struct { // PrometheusConfig configures the optional Prometheus integration, which is used only if Enabled is true. // // This corresponds to the [PrometheusConfig] section in the configuration file. +// +// Since configuration options can be set either programmatically, or from a file, or from environment +// variables, individual fields are not documented here; instead, see the `README.md` section on +// configuration. type PrometheusConfig struct { - Port int `conf:"PROMETHEUS_PORT"` + Port ct.OptIntGreaterThanZero `conf:"PROMETHEUS_PORT"` CommonMetricsConfig } - -// DefaultConfig contains defaults for all relay configuration sections. -// -// If you are incorporating Relay into your own code and configuring it programmatically, it is best to -// start by copying relay.DefaultConfig and then changing only the fields you need to change. -var DefaultConfig = Config{ - Main: MainConfig{ - BaseURI: newOptURLAbsoluteMustBeValid(DefaultBaseURI), - StreamURI: newOptURLAbsoluteMustBeValid(DefaultStreamURI), - Port: defaultPort, - }, - Events: EventsConfig{ - Capacity: defaultEventCapacity, - EventsURI: newOptURLAbsoluteMustBeValid(DefaultEventsURI), - }, - MetricsConfig: MetricsConfig{ - Prometheus: PrometheusConfig{ - Port: defaultPrometheusPort, - }, - }, -} diff --git a/config/config_field_types.go b/config/config_field_types.go index 3b1f017f..a1840bdf 100644 --- a/config/config_field_types.go +++ b/config/config_field_types.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - ct "github.com/launchdarkly/go-configtypes" "gopkg.in/launchdarkly/go-sdk-common.v2/ldlog" ) @@ -20,7 +19,7 @@ type MobileKey string // LaunchDarkly environment. type EnvironmentID string -// SDKCredential is implemented by types that represent an SDK authorization credential (SDKKey, etc.) +// SDKCredential is implemented by types that represent an SDK authorization credential (SDKKey, etc.). type SDKCredential interface { // GetAuthorizationHeaderValue returns the value that should be passed in an HTTP Authorization header // when using this credential, or "" if the header is not used. @@ -63,14 +62,6 @@ func (k *EnvironmentID) UnmarshalText(data []byte) error { return nil } -func newOptURLAbsoluteMustBeValid(urlString string) ct.OptURLAbsolute { - o, err := ct.NewOptURLAbsoluteFromString(urlString) - if err != nil { - panic(err) - } - return o -} - // OptLogLevel represents an optional log level parameter. It must match one of the level names "debug", // "info", "warn", or "error" (case-insensitive). // diff --git a/config/config_from_env.go b/config/config_from_env.go index 208fddc0..0d82ea31 100644 --- a/config/config_from_env.go +++ b/config/config_from_env.go @@ -1,11 +1,9 @@ package config import ( - "errors" "fmt" "os" "sort" - "strconv" "strings" ct "github.com/launchdarkly/go-configtypes" @@ -21,10 +19,12 @@ func LoadConfigFromEnvironment(c *Config, loggers ldlog.Loggers) error { reader.ReadStruct(&c.Main, false) reader.ReadStruct(&c.Events, false) - maybeSetFromEnvInt32(&c.Events.SamplingInterval, "EVENTS_SAMPLING_INTERVAL", reader) - for envName, envKey := range reader.FindPrefixedValues("LD_ENV_") { - ec := EnvConfig{SDKKey: SDKKey(envKey)} + var ec EnvConfig + if c.Environment[envName] != nil { + ec = *c.Environment[envName] + } + ec.SDKKey = SDKKey(envKey) subReader := reader.WithVarNameSuffix(envName) subReader.ReadStruct(&ec, false) rejectObsoleteVariableName("LD_TTL_MINUTES_"+envName, "LD_TTL_"+envName, reader) @@ -39,8 +39,8 @@ func LoadConfigFromEnvironment(c *Config, loggers ldlog.Loggers) error { reader.Read("USE_REDIS", &useRedis) if useRedis || c.Redis.Host != "" || c.Redis.URL.IsDefined() { portStr := "" - if c.Redis.Port > 0 { - portStr = fmt.Sprintf("%d", c.Redis.Port) + if c.Redis.Port.IsDefined() { + portStr = fmt.Sprintf("%d", c.Redis.Port.GetOrElse(0)) } reader.ReadStruct(&c.Redis, false) reader.Read("REDIS_PORT", &portStr) // handled separately because it could be a string or a number @@ -52,7 +52,9 @@ func LoadConfigFromEnvironment(c *Config, loggers ldlog.Loggers) error { fields := strings.Split(hostAndPort, ":") c.Redis.Host = fields[0] if len(fields) > 0 { - setInt(&c.Redis.Port, "REDIS_PORT", fields[1], reader) + if err := c.Redis.Port.UnmarshalText([]byte(fields[1])); err != nil { + reader.AddError(ct.ValidationPath{"REDIS_PORT"}, err) + } } } else { if c.Redis.Host == "" { @@ -61,7 +63,7 @@ func LoadConfigFromEnvironment(c *Config, loggers ldlog.Loggers) error { reader.Read("REDIS_PORT", &c.Redis.Port) } } - if !c.Redis.URL.IsDefined() && c.Redis.Host == "" && c.Redis.Port == 0 { + if !c.Redis.URL.IsDefined() && c.Redis.Host == "" && !c.Redis.Port.IsDefined() { // all they specified was USE_REDIS c.Redis.URL = defaultRedisURL } @@ -120,18 +122,3 @@ func rejectObsoleteVariableName(oldName, preferredName string, reader *ct.VarRea fmt.Errorf("this variable is no longer supported; use %s", preferredName)) } } - -func setInt(prop *int, name string, value string, reader *ct.VarReader) { - if n, err := strconv.Atoi(value); err != nil { - reader.AddError(ct.ValidationPath{name}, errors.New("not a valid integer")) - } else { - *prop = n - } -} - -func maybeSetFromEnvInt32(prop *int32, name string, reader *ct.VarReader) { - var n int - if reader.Read(name, &n) { - *prop = int32(n) - } -} diff --git a/config/config_from_env_test.go b/config/config_from_env_test.go index 8586b454..7d4c7f96 100644 --- a/config/config_from_env_test.go +++ b/config/config_from_env_test.go @@ -31,6 +31,66 @@ func TestConfigFromEnvironmentWithInvalidProperties(t *testing.T) { } } +func TestConfigFromEnvironmentOverridesExistingSettings(t *testing.T) { + t.Run("can add SDK key to existing environment", func(t *testing.T) { + var startingConfig, expectedConfig Config + startingConfig.Environment = map[string]*EnvConfig{ + "envname": &EnvConfig{ + Prefix: "p", + }, + } + vars := map[string]string{ + "LD_ENV_envname": "my-key", + } + expectedConfig.Environment = map[string]*EnvConfig{ + "envname": &EnvConfig{ + SDKKey: SDKKey("my-key"), + Prefix: "p", + }, + } + withEnvironment(vars, func() { + c := startingConfig + err := LoadConfigFromEnvironment(&c, ldlog.NewDisabledLoggers()) + require.NoError(t, err) + + assert.Equal(t, expectedConfig, c) + }) + }) + + t.Run("can change REDIS_PORT when REDIS_HOST was set", func(t *testing.T) { + var startingConfig, expectedConfig Config + startingConfig.Redis.Host = "redishost" + vars := map[string]string{ + "REDIS_PORT": "2222", + } + expectedConfig.Redis.URL = newOptURLAbsoluteMustBeValid("redis://redishost:2222") + withEnvironment(vars, func() { + c := startingConfig + err := LoadConfigFromEnvironment(&c, ldlog.NewDisabledLoggers()) + require.NoError(t, err) + + assert.Equal(t, expectedConfig, c) + }) + }) + + t.Run("can change REDIS_HOST when REDIS_PORT was set", func(t *testing.T) { + var startingConfig, expectedConfig Config + startingConfig.Redis.Port = mustOptIntGreaterThanZero(2222) + vars := map[string]string{ + "USE_REDIS": "1", + "REDIS_HOST": "redishost", + } + expectedConfig.Redis.URL = newOptURLAbsoluteMustBeValid("redis://redishost:2222") + withEnvironment(vars, func() { + c := startingConfig + err := LoadConfigFromEnvironment(&c, ldlog.NewDisabledLoggers()) + require.NoError(t, err) + + assert.Equal(t, expectedConfig, c) + }) + }) +} + func TestConfigFromEnvironmentDisallowsObsoleteVariables(t *testing.T) { t.Run("REDIS_TTL", func(t *testing.T) { testInvalidConfigVars(t, @@ -82,7 +142,7 @@ func TestConfigFromEnvironmentFieldValidation(t *testing.T) { t.Run("parses valid int", func(t *testing.T) { testValidConfigVars(t, - func(c *Config) { c.Main.Port = 222 }, + func(c *Config) { c.Main.Port = mustOptIntGreaterThanZero(222) }, map[string]string{"PORT": "222"}, ) }) @@ -94,6 +154,17 @@ func TestConfigFromEnvironmentFieldValidation(t *testing.T) { ) }) + t.Run("rejects <=0 value for int that must be >0", func(t *testing.T) { + testInvalidConfigVars(t, + map[string]string{"PORT": "0"}, + "PORT: value must be greater than zero", + ) + testInvalidConfigVars(t, + map[string]string{"PORT": "-1"}, + "PORT: value must be greater than zero", + ) + }) + t.Run("parses valid URI", func(t *testing.T) { testValidConfigVars(t, func(c *Config) { c.Main.BaseURI = newOptURLAbsoluteMustBeValid("http://some/uri") }, @@ -147,10 +218,10 @@ func TestConfigFromEnvironmentFieldValidation(t *testing.T) { func testValidConfigVars(t *testing.T, buildConfig func(c *Config), vars map[string]string) { withEnvironment(vars, func() { - expectedConfig := DefaultConfig + var expectedConfig Config buildConfig(&expectedConfig) - c := DefaultConfig + var c Config err := LoadConfigFromEnvironment(&c, ldlog.NewDisabledLoggers()) require.NoError(t, err) @@ -160,7 +231,7 @@ func testValidConfigVars(t *testing.T, buildConfig func(c *Config), vars map[str func testInvalidConfigVars(t *testing.T, vars map[string]string, errMessage string) { withEnvironment(vars, func() { - c := DefaultConfig + var c Config err := LoadConfigFromEnvironment(&c, ldlog.NewDisabledLoggers()) require.Error(t, err) assert.Contains(t, err.Error(), errMessage) diff --git a/config/config_from_file_test.go b/config/config_from_file_test.go index 6965badb..5f036628 100644 --- a/config/config_from_file_test.go +++ b/config/config_from_file_test.go @@ -76,7 +76,7 @@ ExitOnError = "x"`, t.Run("parses valid int", func(t *testing.T) { testFileWithValidConfig(t, - func(c *Config) { c.Main.Port = 222 }, + func(c *Config) { c.Main.Port = mustOptIntGreaterThanZero(222) }, `[Main] Port = 222`, ) @@ -86,7 +86,20 @@ Port = 222`, testFileWithInvalidConfig(t, `[Main] Port = "x"`, - "failed to parse \"x\" as int: expected integer", + "not a valid integer", + ) + }) + + t.Run("rejects <=0 value for int that must be >0", func(t *testing.T) { + testFileWithInvalidConfig(t, + `[Main] +Port = "0"`, + "value must be greater than zero", + ) + testFileWithInvalidConfig(t, + `[Main] +Port = "-1"`, + "value must be greater than zero", ) }) @@ -150,13 +163,13 @@ LogLevel = "wrong"`, } func testFileWithValidConfig(t *testing.T, buildConfig func(c *Config), fileContent string) { - expectedConfig := DefaultConfig + var expectedConfig Config buildConfig(&expectedConfig) helpers.WithTempFile(func(filename string) { require.NoError(t, ioutil.WriteFile(filename, []byte(fileContent), 0)) - c := DefaultConfig + var c Config err := LoadConfigFile(&c, filename, ldlog.NewDisabledLoggers()) require.NoError(t, err) assert.Equal(t, expectedConfig, c) @@ -167,7 +180,7 @@ func testFileWithInvalidConfig(t *testing.T, fileContent string, errMessage stri helpers.WithTempFile(func(filename string) { require.NoError(t, ioutil.WriteFile(filename, []byte(fileContent), 0)) - c := DefaultConfig + var c Config err := LoadConfigFile(&c, filename, ldlog.NewDisabledLoggers()) require.Error(t, err) assert.Contains(t, err.Error(), errMessage) diff --git a/config/config_validation.go b/config/config_validation.go index 25868211..8c5436e3 100644 --- a/config/config_validation.go +++ b/config/config_validation.go @@ -33,24 +33,22 @@ func ValidateConfig(c *Config, loggers ldlog.Loggers) error { } if c.Redis.URL.IsDefined() { - if c.Redis.Host != "" || c.Redis.Port != 0 { + if c.Redis.Host != "" || c.Redis.Port.IsDefined() { result.AddError(nil, errors.New("please specify Redis URL or host/port, but not both")) } - } else if c.Redis.Host != "" || c.Redis.Port != 0 { - host, port := c.Redis.Host, c.Redis.Port + } else if c.Redis.Host != "" || c.Redis.Port.IsDefined() { + host := c.Redis.Host if host == "" { host = defaultRedisHost } - if port <= 0 { - port = defaultRedisPort - } + port := c.Redis.Port.GetOrElse(defaultRedisPort) url, err := ct.NewOptURLAbsoluteFromString(fmt.Sprintf("redis://%s:%d", host, port)) if err != nil { result.AddError(nil, errors.New("invalid Redis hostname")) } c.Redis.URL = url c.Redis.Host = "" - c.Redis.Port = 0 + c.Redis.Port = ct.OptIntGreaterThanZero{} } databases := []string{} diff --git a/config/test_data_test.go b/config/test_data_test.go index a97866fa..af614516 100644 --- a/config/test_data_test.go +++ b/config/test_data_test.go @@ -22,6 +22,22 @@ type testDataInvalidConfig struct { fileContent string } +func mustOptIntGreaterThanZero(n int) ct.OptIntGreaterThanZero { + o, err := ct.NewOptIntGreaterThanZero(n) + if err != nil { + panic(err) + } + return o +} + +func newOptURLAbsoluteMustBeValid(urlString string) ct.OptURLAbsolute { + o, err := ct.NewOptURLAbsoluteFromString(urlString) + if err != nil { + panic(err) + } + return o +} + func makeValidConfigs() []testDataValidConfig { return []testDataValidConfig{ makeValidConfigAllBaseProperties(), @@ -51,6 +67,7 @@ func makeInvalidConfigs() []testDataInvalidConfig { makeInvalidConfigTLSWithNoCert(), makeInvalidConfigTLSWithNoKey(), makeInvalidConfigRedisInvalidHostname(), + makeInvalidConfigRedisInvalidDockerPort(), makeInvalidConfigRedisConflictingParams(), makeInvalidConfigMultipleDatabases(), } @@ -60,7 +77,7 @@ func makeValidConfigAllBaseProperties() testDataValidConfig { c := testDataValidConfig{name: "all base properties"} c.makeConfig = func(c *Config) { c.Main = MainConfig{ - Port: 8333, + Port: mustOptIntGreaterThanZero(8333), BaseURI: newOptURLAbsoluteMustBeValid("http://base"), StreamURI: newOptURLAbsoluteMustBeValid("http://stream"), ExitOnError: true, @@ -74,12 +91,11 @@ func makeValidConfigAllBaseProperties() testDataValidConfig { LogLevel: NewOptLogLevel(ldlog.Warn), } c.Events = EventsConfig{ - SendEvents: true, - EventsURI: newOptURLAbsoluteMustBeValid("http://events"), - FlushInterval: ct.NewOptDuration(120 * time.Second), - SamplingInterval: 3, - Capacity: 500, - InlineUsers: true, + SendEvents: true, + EventsURI: newOptURLAbsoluteMustBeValid("http://events"), + FlushInterval: ct.NewOptDuration(120 * time.Second), + Capacity: mustOptIntGreaterThanZero(500), + InlineUsers: true, } c.Environment = map[string]*EnvConfig{ "earth": &EnvConfig{ @@ -118,7 +134,6 @@ func makeValidConfigAllBaseProperties() testDataValidConfig { "USE_EVENTS": "1", "EVENTS_HOST": "http://events", "EVENTS_FLUSH_INTERVAL": "120s", - "EVENTS_SAMPLING_INTERVAL": "3", "EVENTS_CAPACITY": "500", "EVENTS_INLINE_USERS": "1", "LD_ENV_earth": "earth-sdk", @@ -155,7 +170,6 @@ LogLevel = "warn" SendEvents = 1 EventsUri = "http://events" FlushInterval = 120s -SamplingInterval = 3 Capacity = 500 InlineUsers = 1 @@ -452,7 +466,6 @@ func makeValidConfigPrometheusMinimal() testDataValidConfig { c.makeConfig = func(c *Config) { c.Prometheus = PrometheusConfig{ CommonMetricsConfig: CommonMetricsConfig{Enabled: true, Prefix: ""}, - Port: 8031, } } c.envVars = map[string]string{ @@ -470,7 +483,7 @@ func makeValidConfigPrometheusAll() testDataValidConfig { c.makeConfig = func(c *Config) { c.Prometheus = PrometheusConfig{ CommonMetricsConfig: CommonMetricsConfig{Enabled: true, Prefix: "pre-"}, - Port: 8333, + Port: mustOptIntGreaterThanZero(8333), } } c.envVars = map[string]string{ @@ -578,6 +591,16 @@ Host = "\\" return c } +func makeInvalidConfigRedisInvalidDockerPort() testDataInvalidConfig { + c := testDataInvalidConfig{name: "Redis - Docker port syntax with invalid port"} + c.envVarsError = "REDIS_PORT: not a valid integer" + c.envVars = map[string]string{ + "USE_REDIS": "1", + "REDIS_PORT": "tcp://redishost:xxx", + } + return c +} + func makeInvalidConfigRedisConflictingParams() testDataInvalidConfig { c := testDataInvalidConfig{name: "Redis - conflicting parameters"} c.envVarsError = "please specify Redis URL or host/port, but not both" diff --git a/internal/events/event-relay.go b/internal/events/event-relay.go index 2f02b969..f34f6346 100644 --- a/internal/events/event-relay.go +++ b/internal/events/event-relay.go @@ -300,7 +300,7 @@ func newEventVerbatimRelay(authKey c.SDKCredential, config c.EventsConfig, httpC eventsURI = c.DefaultEventsURI } opts := []OptionType{ - OptionCapacity(config.Capacity), + OptionCapacity(config.Capacity.GetOrElse(c.DefaultEventCapacity)), OptionEndpointURI(strings.TrimRight(eventsURI, "/") + remotePath), OptionClient{Client: httpClient}, } @@ -322,9 +322,5 @@ func (er *eventVerbatimRelay) enqueue(evts []json.RawMessage) { return } - if er.config.SamplingInterval > 0 && rGen.Int31n(er.config.SamplingInterval) != 0 { - return - } - er.publisher.PublishRaw(evts...) } diff --git a/internal/events/event-summarizing-relay.go b/internal/events/event-summarizing-relay.go index 81eb6bac..b0ccf417 100644 --- a/internal/events/event-summarizing-relay.go +++ b/internal/events/event-summarizing-relay.go @@ -73,7 +73,7 @@ func newEventSummarizingRelay(authKey c.SDKCredential, config c.EventsConfig, ht eventSender := ldevents.NewDefaultEventSender(httpClient, eventsURI, "", headers, loggers) eventsConfig := ldevents.EventsConfiguration{ - Capacity: config.Capacity, + Capacity: config.Capacity.GetOrElse(c.DefaultEventCapacity), InlineUsersInEvents: config.InlineUsers, EventSender: eventSender, FlushInterval: config.FlushInterval.GetOrElse(c.DefaultEventsFlushInterval), diff --git a/internal/metrics/prometheus.go b/internal/metrics/prometheus.go index 3c6d1d2a..f935890d 100644 --- a/internal/metrics/prometheus.go +++ b/internal/metrics/prometheus.go @@ -36,10 +36,7 @@ func (p prometheusExporterTypeImpl) createExporterIfEnabled( return nil, nil } - port := defaultPrometheusPort - if mc.Prometheus.Port != 0 { - port = mc.Prometheus.Port - } + port := mc.Prometheus.Port.GetOrElse(config.DefaultPrometheusPort) logPrometheusError := func(e error) { loggers.Errorf("Prometheus exporter error: %s", e) diff --git a/internal/metrics/prometheus_test.go b/internal/metrics/prometheus_test.go index db7de4a5..a9bcc4e1 100644 --- a/internal/metrics/prometheus_test.go +++ b/internal/metrics/prometheus_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + ct "github.com/launchdarkly/go-configtypes" "github.com/launchdarkly/ld-relay/v6/config" "gopkg.in/launchdarkly/go-sdk-common.v2/ldlog" ) @@ -101,7 +102,7 @@ func TestPrometheusExporterType(t *testing.T) { var mc config.MetricsConfig mc.Prometheus.Enabled = true - mc.Prometheus.Port = availablePort + mc.Prometheus.Port, _ = ct.NewOptIntGreaterThanZero(availablePort) e, err := exporterType.createExporterIfEnabled(mc, ldlog.NewDisabledLoggers()) require.NoError(t, err) require.NotNil(t, e) diff --git a/relay_endpoints_eval_test.go b/relay_endpoints_eval_test.go index a523782a..1d9b1df6 100644 --- a/relay_endpoints_eval_test.go +++ b/relay_endpoints_eval_test.go @@ -28,7 +28,7 @@ func TestRelayServerSideEvalRoutes(t *testing.T) { {"server-side report evalx with reasons", "REPORT", "/sdk/evalx/user?withReasons=true", userJSON, sdkKey, http.StatusOK, expectedMobileEvalxBodyWithReasons}, } - config := c.DefaultConfig + var config c.Config config.Environment = makeEnvConfigs(env) relayTest(config, func(p relayTestParams) { @@ -86,7 +86,7 @@ func TestRelayMobileEvalRoutes(t *testing.T) { {"mobile get evalx", "GET", "/msdk/evalx/users/$USER", userJSON, mobileKey, http.StatusOK, nil}, } - config := c.DefaultConfig + var config c.Config config.Environment = makeEnvConfigs(env) relayTest(config, func(p relayTestParams) { @@ -145,7 +145,7 @@ func TestRelayJSClientEvalRoutes(t *testing.T) { http.StatusOK, expectedJSEvalxBodyWithReasons}, } - config := c.DefaultConfig + var config c.Config config.Environment = makeEnvConfigs(testEnvClientSide, testEnvClientSideSecureMode) relayTest(config, func(p relayTestParams) { diff --git a/relay_endpoints_events_test.go b/relay_endpoints_events_test.go index bd7a28d7..d59cac1d 100644 --- a/relay_endpoints_events_test.go +++ b/relay_endpoints_events_test.go @@ -57,7 +57,6 @@ func relayEventsTest(config config.Config, action func(relayEventsTestParams)) { config.Events.SendEvents = true config.Events.EventsURI, _ = ct.NewOptURLAbsoluteFromString(eventsServer.URL) config.Events.FlushInterval = ct.NewOptDuration(time.Second) - config.Events.Capacity = c.DefaultConfig.Events.Capacity relayTest(config, func(pBase relayTestParams) { p := relayEventsTestParams{ @@ -96,7 +95,7 @@ func makeTestFeatureEventPayload(userKey string) []byte { func TestRelayServerSideEventProxy(t *testing.T) { env := testEnvMain sdkKey := env.config.SDKKey - config := c.DefaultConfig + var config c.Config config.Environment = makeEnvConfigs(env) body := makeTestFeatureEventPayload("me") @@ -145,7 +144,7 @@ func TestRelayServerSideEventProxy(t *testing.T) { func TestRelayMobileEventProxy(t *testing.T) { env := testEnvMobile mobileKey := env.config.MobileKey - config := c.DefaultConfig + var config c.Config config.Environment = makeEnvConfigs(env) relayEventsTest(config, func(p relayEventsTestParams) { @@ -206,7 +205,7 @@ func TestRelayJSClientEventProxy(t *testing.T) { expectBody(string(transparent1PixelImg))}, } - config := c.DefaultConfig + var config c.Config config.Environment = makeEnvConfigs(env) relayEventsTest(config, func(p relayEventsTestParams) { diff --git a/relay_endpoints_php_test.go b/relay_endpoints_php_test.go index 5bfd6284..fb70dfc7 100644 --- a/relay_endpoints_php_test.go +++ b/relay_endpoints_php_test.go @@ -27,7 +27,7 @@ func TestRelayPHPPollingEndpoints(t *testing.T) { http.StatusNotFound, nil}, } - config := c.DefaultConfig + var config c.Config config.Environment = makeEnvConfigs(testEnvMain, testEnvWithTTL) relayTest(config, func(p relayTestParams) { diff --git a/relay_endpoints_status_test.go b/relay_endpoints_status_test.go index 3c719bcb..6c28cb56 100644 --- a/relay_endpoints_status_test.go +++ b/relay_endpoints_status_test.go @@ -14,7 +14,7 @@ import ( ) func TestRelayStatusEndpoint(t *testing.T) { - config := c.DefaultConfig + var config c.Config config.Environment = makeEnvConfigs(testEnvMain, testEnvClientSide, testEnvMobile) relayTest(config, func(p relayTestParams) { diff --git a/relay_endpoints_stream_test.go b/relay_endpoints_stream_test.go index 955cc87d..d4bab700 100644 --- a/relay_endpoints_stream_test.go +++ b/relay_endpoints_stream_test.go @@ -141,7 +141,7 @@ func TestRelayServerSideStreams(t *testing.T) { {endpointTestParams{"all stream", "GET", "/all", nil, sdkKey, 200, nil}, "put", expectedAllData}, } - config := c.DefaultConfig + var config c.Config config.Environment = makeEnvConfigs(env) for _, s := range specs { @@ -162,7 +162,7 @@ func TestRelayMobileStreams(t *testing.T) { "ping", nil}, } - config := c.DefaultConfig + var config c.Config config.Environment = makeEnvConfigs(env) for _, s := range specs { @@ -185,7 +185,7 @@ func TestRelayJSClientStreams(t *testing.T) { "ping", nil}, } - config := c.DefaultConfig + var config c.Config config.Environment = makeEnvConfigs(testEnvClientSide, testEnvClientSideSecureMode) for _, s := range specs { diff --git a/relay_other_test.go b/relay_other_test.go index 84c07dd1..eeff684c 100644 --- a/relay_other_test.go +++ b/relay_other_test.go @@ -75,7 +75,7 @@ func TestRelayJSClientGoalsRoute(t *testing.T) { fakeServerWithGoalsEndpoint := httptest.NewServer(fakeGoalsEndpoint) defer fakeServerWithGoalsEndpoint.Close() - config := c.DefaultConfig + var config c.Config config.Main.BaseURI, _ = ct.NewOptURLAbsoluteFromString(fakeServerWithGoalsEndpoint.URL) config.Environment = makeEnvConfigs(env) From 4368b957207890ab212ae48eff43a2012c15715b Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Jul 2020 16:50:10 -0700 Subject: [PATCH 05/15] fix command-line config logic --- cmd/ld-relay/ld-relay.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/cmd/ld-relay/ld-relay.go b/cmd/ld-relay/ld-relay.go index 7555cd19..68315dbf 100644 --- a/cmd/ld-relay/ld-relay.go +++ b/cmd/ld-relay/ld-relay.go @@ -36,7 +36,7 @@ func main() { flag.BoolVar(&useEnvironment, "from-env", false, "read configuration from environment variables") flag.Parse() - c := config.DefaultConfig + var c config.Config loggers := logging.MakeDefaultLoggers() if configFile == "" && !useEnvironment { @@ -87,23 +87,25 @@ func main() { errs := make(chan error) defer close(errs) - startHTTPServer(&c, r, loggers, errs) + port := c.Main.Port.GetOrElse(config.DefaultPort) + + startHTTPServer(&c, port, r, loggers, errs) for err := range errs { - loggers.Errorf("Error starting http listener on port: %d %s", c.Main.Port, err) + loggers.Errorf("Error starting http listener on port: %d %s", port, err) os.Exit(1) } } -func startHTTPServer(c *config.Config, r *relay.Relay, loggers ldlog.Loggers, errs chan<- error) { +func startHTTPServer(c *config.Config, port int, r *relay.Relay, loggers ldlog.Loggers, errs chan<- error) { srv := &http.Server{ - Addr: fmt.Sprintf(":%d", c.Main.Port), + Addr: fmt.Sprintf(":%d", port), Handler: r, } go func() { var err error - loggers.Infof("Starting server listening on port %d\n", c.Main.Port) + loggers.Infof("Starting server listening on port %d\n", port) if c.Main.TLSEnabled { loggers.Infof("TLS Enabled for server") err = srv.ListenAndServeTLS(c.Main.TLSCert, c.Main.TLSKey) From 9fe03c2ed6a2e74bc1a11349d53bf221d8ba19ce Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Jul 2020 16:54:22 -0700 Subject: [PATCH 06/15] rm unused --- internal/events/event-relay.go | 8 -------- internal/metrics/prometheus.go | 2 -- 2 files changed, 10 deletions(-) diff --git a/internal/events/event-relay.go b/internal/events/event-relay.go index f34f6346..6699d75c 100644 --- a/internal/events/event-relay.go +++ b/internal/events/event-relay.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "io/ioutil" - "math/rand" "net/http" "strconv" "strings" @@ -48,13 +47,6 @@ type eventVerbatimRelay struct { publisher EventPublisher } -var rGen *rand.Rand - -func init() { - s1 := rand.NewSource(time.Now().UnixNano()) - rGen = rand.New(s1) -} - const ( // SummaryEventsSchemaVersion is the minimum event schema that supports summary events SummaryEventsSchemaVersion = 3 diff --git a/internal/metrics/prometheus.go b/internal/metrics/prometheus.go index f935890d..181dc549 100644 --- a/internal/metrics/prometheus.go +++ b/internal/metrics/prometheus.go @@ -13,8 +13,6 @@ import ( "github.com/launchdarkly/ld-relay/v6/internal/logging" ) -const defaultPrometheusPort = 8031 - var prometheusExporterType exporterType = prometheusExporterTypeImpl{} type prometheusExporterTypeImpl struct{} From 9595f78a81b7868dc38f08982d66ac89be86d83c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Jul 2020 17:14:11 -0700 Subject: [PATCH 07/15] rm prematurely added file --- config/extended_config.go | 46 --------------------------------------- 1 file changed, 46 deletions(-) delete mode 100644 config/extended_config.go diff --git a/config/extended_config.go b/config/extended_config.go deleted file mode 100644 index 9e35fdb7..00000000 --- a/config/extended_config.go +++ /dev/null @@ -1,46 +0,0 @@ -package config - -import ct "github.com/launchdarkly/go-configtypes" - -// This file contains extended configuration for Relay Proxy Enterprise. It will be moved to a -// separate project later in development. - -const ( - // This string can appear within MainExtendedConfig.EnvDataStorePrefix or - // MainExtendedConfig.EnvDataStoreTableName to indicate that the environment ID should be - // substituted at that point. - // - // For instance, if EnvDataStorePrefix is "LD-$CID", the value of that setting for an environment - // whose ID is "12345" would be "LD-12345". - AutoConfigEnvironmentIDPlaceholder = "$CID" -) - -type ExtendedConfig struct { - Main MainExtendedConfig - Events EventsConfig - Redis RedisConfig - Consul ConsulConfig - DynamoDB DynamoDBConfig - Environment map[string]*EnvConfig - Proxy ProxyConfig - - // Optional configuration for metrics integrations. Note that unlike the other fields in Config, - // MetricsConfig is not the name of a configuration file section; the actual sections are the - // structs within this struct (Datadog, etc.). - MetricsConfig -} - -// MainConfig contains global configuration options for Relay Proxy Enterprise. -// -// This corresponds to the [Main] section in the configuration file. -// -// Since configuration options can be set either programmatically, or from a file, or from environment -// variables, individual fields are not documented here; instead, see the `README.md` section on -// configuration. -type MainExtendedConfig struct { - MainConfig - AutoConfigKey string `conf:"AUTO_CONFIG_KEY"` - EnvDatastorePrefix string `conf:"ENV_DATASTORE_PREFIX"` - EnvDatastoreTableName string `conf:"ENV_DATASTORE_TABLE_NAME"` - EnvAllowedOrigin ct.OptStringList `conf:"ENV_ALLOWED_ORIGIN"` -} From 60b4376d207b6c4b9bfac60ca2a3207b33c777f2 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Jul 2020 17:14:18 -0700 Subject: [PATCH 08/15] fix test --- internal/metrics/prometheus_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/metrics/prometheus_test.go b/internal/metrics/prometheus_test.go index a9bcc4e1..bf85e629 100644 --- a/internal/metrics/prometheus_test.go +++ b/internal/metrics/prometheus_test.go @@ -86,7 +86,7 @@ func TestPrometheusExporterType(t *testing.T) { defer e.close() require.NoError(t, e.register()) - verifyPrometheusEndpointIsReachable(t, defaultPrometheusPort, time.Second) + verifyPrometheusEndpointIsReachable(t, config.DefaultPrometheusPort, time.Second) }) t.Run("listens on custom port", func(t *testing.T) { From be33dfa22c389fdfedcacbd47f2ed1ff96ff15b1 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Jul 2020 17:17:20 -0700 Subject: [PATCH 09/15] rm prematurely added files --- internal/autoconf/make_config.go | 29 ----------------------------- internal/autoconf/reps.go | 13 ------------- 2 files changed, 42 deletions(-) delete mode 100644 internal/autoconf/make_config.go delete mode 100644 internal/autoconf/reps.go diff --git a/internal/autoconf/make_config.go b/internal/autoconf/make_config.go deleted file mode 100644 index e06e8a83..00000000 --- a/internal/autoconf/make_config.go +++ /dev/null @@ -1,29 +0,0 @@ -package autoconf - -import ( - "strings" - "time" - - ct "github.com/launchdarkly/go-configtypes" - "github.com/launchdarkly/ld-relay/v6/config" -) - -func CreateEnvironmentConfig(rep AutoConfigEnvironmentRep, mainConfig config.MainExtendedConfig) config.EnvConfig { - ret := config.EnvConfig{ - SDKKey: config.SDKKey(rep.SDKKey), - MobileKey: config.MobileKey(rep.MobKey), - EnvID: config.EnvironmentID(rep.EnvID), - Prefix: maybeSubstituteEnvironmentID(mainConfig.EnvDatastorePrefix, rep.EnvID), - TableName: maybeSubstituteEnvironmentID(mainConfig.EnvDatastoreTableName, rep.EnvID), - AllowedOrigin: mainConfig.EnvAllowedOrigin, - SecureMode: rep.SecureMode, - } - if rep.DefaultTTL != 0 { - ret.TTL = ct.NewOptDuration(time.Duration(rep.DefaultTTL) * time.Minute) - } - return ret -} - -func maybeSubstituteEnvironmentID(s, envID string) string { - return strings.ReplaceAll(s, config.AutoConfigEnvironmentIDPlaceholder, envID) -} diff --git a/internal/autoconf/reps.go b/internal/autoconf/reps.go deleted file mode 100644 index 87f7452c..00000000 --- a/internal/autoconf/reps.go +++ /dev/null @@ -1,13 +0,0 @@ -package autoconf - -type AutoConfigEnvironmentRep struct { - EnvID string `json:"envID"` - EnvKey string `json:"envKey"` - EnvName string `json:"envName"` - MobKey string `json:"mobKey"` - ProjKey string `json:"projKey"` - ProjName string `json:"projName"` - SDKKey string `json:"sdkKey"` - DefaultTTL int `json:"defaultTtl"` - SecureMode bool `json:"secureMode"` -} From 3061ef49a48adb370a309f76650fa3296e160ec8 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Jul 2020 17:32:00 -0700 Subject: [PATCH 10/15] simplify config logic even more --- config/config.go | 23 ++++++++++------------- config/config_from_env.go | 17 +++-------------- config/test_data_test.go | 25 ++++++++++++++----------- internal/metrics/datadog.go | 4 ++-- internal/metrics/exporters.go | 6 +++--- internal/metrics/exporters_test.go | 4 ++-- internal/metrics/prometheus.go | 2 +- internal/metrics/stackdriver.go | 2 +- 8 files changed, 36 insertions(+), 47 deletions(-) diff --git a/config/config.go b/config/config.go index 59dbd866..6497f0bc 100644 --- a/config/config.go +++ b/config/config.go @@ -206,12 +206,6 @@ type MetricsConfig struct { Prometheus PrometheusConfig } -// CommonMetricsConfig contains fields that are common to DatadogCOnfig, StackdriverConfig, and PrometheusConfig. -type CommonMetricsConfig struct { - Enabled bool - Prefix string -} - // DatadogConfig configures the optional Datadog integration, which is used only if Enabled is true. // // This corresponds to the [Datadog] section in the configuration file. @@ -220,10 +214,11 @@ type CommonMetricsConfig struct { // variables, individual fields are not documented here; instead, see the `README.md` section on // configuration. type DatadogConfig struct { - TraceAddr string `conf:"DATADOG_TRACE_ADDR"` - StatsAddr string `conf:"DATADOG_STATS_ADDR"` - Tag []string - CommonMetricsConfig + Enabled bool `conf:"USE_DATADOG"` + Prefix string `conf:"DATADOG_PREFIX"` + TraceAddr string `conf:"DATADOG_TRACE_ADDR"` + StatsAddr string `conf:"DATADOG_STATS_ADDR"` + Tag []string // special handling in LoadConfigFromEnvironment } // StackdriverConfig configures the optional Stackdriver integration, which is used only if Enabled is true. @@ -234,8 +229,9 @@ type DatadogConfig struct { // variables, individual fields are not documented here; instead, see the `README.md` section on // configuration. type StackdriverConfig struct { + Enabled bool `conf:"USE_STACKDRIVER"` + Prefix string `conf:"STACKDRIVER_PREFIX"` ProjectID string `conf:"STACKDRIVER_PROJECT_ID"` - CommonMetricsConfig } // PrometheusConfig configures the optional Prometheus integration, which is used only if Enabled is true. @@ -246,6 +242,7 @@ type StackdriverConfig struct { // variables, individual fields are not documented here; instead, see the `README.md` section on // configuration. type PrometheusConfig struct { - Port ct.OptIntGreaterThanZero `conf:"PROMETHEUS_PORT"` - CommonMetricsConfig + Enabled bool `conf:"USE_PROMETHEUS"` + Prefix string `conf:"PROMETHEUS_PREFIX"` + Port ct.OptIntGreaterThanZero `conf:"PROMETHEUS_PORT"` } diff --git a/config/config_from_env.go b/config/config_from_env.go index b4052978..0f1f11f8 100644 --- a/config/config_from_env.go +++ b/config/config_from_env.go @@ -85,27 +85,16 @@ func LoadConfigFromEnvironment(c *Config, loggers ldlog.Loggers) error { reader.ReadStruct(&c.DynamoDB, false) } - reader.Read("USE_DATADOG", &c.MetricsConfig.Datadog.Enabled) + reader.ReadStruct(&c.MetricsConfig.Datadog, false) if c.MetricsConfig.Datadog.Enabled { - reader.Read("DATADOG_PREFIX", &c.MetricsConfig.Datadog.Prefix) - reader.ReadStruct(&c.MetricsConfig.Datadog, false) for tagName, tagVal := range reader.FindPrefixedValues("DATADOG_TAG_") { c.MetricsConfig.Datadog.Tag = append(c.MetricsConfig.Datadog.Tag, tagName+":"+tagVal) } sort.Strings(c.MetricsConfig.Datadog.Tag) // for test determinacy } - reader.Read("USE_STACKDRIVER", &c.MetricsConfig.Stackdriver.Enabled) - if c.MetricsConfig.Stackdriver.Enabled { - reader.ReadStruct(&c.MetricsConfig.Stackdriver, false) - reader.Read("STACKDRIVER_PREFIX", &c.MetricsConfig.Stackdriver.Prefix) - } - - reader.Read("USE_PROMETHEUS", &c.MetricsConfig.Prometheus.Enabled) - if c.MetricsConfig.Prometheus.Enabled { - reader.ReadStruct(&c.MetricsConfig.Prometheus, false) - reader.Read("PROMETHEUS_PREFIX", &c.MetricsConfig.Prometheus.Prefix) - } + reader.ReadStruct(&c.MetricsConfig.Stackdriver, false) + reader.ReadStruct(&c.MetricsConfig.Prometheus, false) reader.ReadStruct(&c.Proxy, false) diff --git a/config/test_data_test.go b/config/test_data_test.go index af614516..7572b90f 100644 --- a/config/test_data_test.go +++ b/config/test_data_test.go @@ -379,7 +379,7 @@ func makeValidConfigDatadogMinimal() testDataValidConfig { c := testDataValidConfig{name: "Datadog - minimal parameters"} c.makeConfig = func(c *Config) { c.Datadog = DatadogConfig{ - CommonMetricsConfig: CommonMetricsConfig{Enabled: true, Prefix: ""}, + Enabled: true, } } c.envVars = map[string]string{ @@ -396,10 +396,11 @@ func makeValidConfigDatadogAll() testDataValidConfig { c := testDataValidConfig{name: "Datadog - all parameters"} c.makeConfig = func(c *Config) { c.Datadog = DatadogConfig{ - CommonMetricsConfig: CommonMetricsConfig{Enabled: true, Prefix: "pre-"}, - TraceAddr: "trace", - StatsAddr: "stats", - Tag: []string{"tag1:value1", "tag2:value2"}, + Enabled: true, + Prefix: "pre-", + TraceAddr: "trace", + StatsAddr: "stats", + Tag: []string{"tag1:value1", "tag2:value2"}, } } c.envVars = map[string]string{ @@ -426,7 +427,7 @@ func makeValidConfigStackdriverMinimal() testDataValidConfig { c := testDataValidConfig{name: "Stackdriver - minimal parameters"} c.makeConfig = func(c *Config) { c.Stackdriver = StackdriverConfig{ - CommonMetricsConfig: CommonMetricsConfig{Enabled: true, Prefix: ""}, + Enabled: true, } } c.envVars = map[string]string{ @@ -443,8 +444,9 @@ func makeValidConfigStackdriverAll() testDataValidConfig { c := testDataValidConfig{name: "Stackdriver - all parameters"} c.makeConfig = func(c *Config) { c.Stackdriver = StackdriverConfig{ - CommonMetricsConfig: CommonMetricsConfig{Enabled: true, Prefix: "pre-"}, - ProjectID: "proj", + Enabled: true, + Prefix: "pre-", + ProjectID: "proj", } } c.envVars = map[string]string{ @@ -465,7 +467,7 @@ func makeValidConfigPrometheusMinimal() testDataValidConfig { c := testDataValidConfig{name: "Prometheus - minimal parameters"} c.makeConfig = func(c *Config) { c.Prometheus = PrometheusConfig{ - CommonMetricsConfig: CommonMetricsConfig{Enabled: true, Prefix: ""}, + Enabled: true, } } c.envVars = map[string]string{ @@ -482,8 +484,9 @@ func makeValidConfigPrometheusAll() testDataValidConfig { c := testDataValidConfig{name: "Prometheus - all parameters"} c.makeConfig = func(c *Config) { c.Prometheus = PrometheusConfig{ - CommonMetricsConfig: CommonMetricsConfig{Enabled: true, Prefix: "pre-"}, - Port: mustOptIntGreaterThanZero(8333), + Enabled: true, + Prefix: "pre-", + Port: mustOptIntGreaterThanZero(8333), } } c.envVars = map[string]string{ diff --git a/internal/metrics/datadog.go b/internal/metrics/datadog.go index 95bd6a7f..dcc1feba 100644 --- a/internal/metrics/datadog.go +++ b/internal/metrics/datadog.go @@ -30,8 +30,8 @@ func (d datadogExporterTypeImpl) createExporterIfEnabled( } options := datadog.Options{ - Namespace: getPrefix(mc.Datadog.CommonMetricsConfig), - Service: getPrefix(mc.Datadog.CommonMetricsConfig), + Namespace: getPrefix(mc.Datadog.Prefix), + Service: getPrefix(mc.Datadog.Prefix), TraceAddr: mc.Datadog.TraceAddr, StatsAddr: mc.Datadog.StatsAddr, Tags: mc.Datadog.Tag, diff --git a/internal/metrics/exporters.go b/internal/metrics/exporters.go index 67344770..e1c09353 100644 --- a/internal/metrics/exporters.go +++ b/internal/metrics/exporters.go @@ -69,9 +69,9 @@ func closeExporters(exporters exportersSet, loggers ldlog.Loggers) { } } -func getPrefix(c config.CommonMetricsConfig) string { - if c.Prefix != "" { - return c.Prefix +func getPrefix(configuredPrefix string) string { + if configuredPrefix != "" { + return configuredPrefix } return defaultMetricsPrefix } diff --git a/internal/metrics/exporters_test.go b/internal/metrics/exporters_test.go index 6667c667..16e9f355 100644 --- a/internal/metrics/exporters_test.go +++ b/internal/metrics/exporters_test.go @@ -134,6 +134,6 @@ func TestCloseExporters(t *testing.T) { } func TestGetPrefix(t *testing.T) { - assert.Equal(t, "x", getPrefix(config.CommonMetricsConfig{Prefix: "x"})) - assert.Equal(t, defaultMetricsPrefix, getPrefix(config.CommonMetricsConfig{})) + assert.Equal(t, "x", getPrefix("x")) + assert.Equal(t, defaultMetricsPrefix, getPrefix("")) } diff --git a/internal/metrics/prometheus.go b/internal/metrics/prometheus.go index 181dc549..9b093cfe 100644 --- a/internal/metrics/prometheus.go +++ b/internal/metrics/prometheus.go @@ -41,7 +41,7 @@ func (p prometheusExporterTypeImpl) createExporterIfEnabled( } options := prometheus.Options{ - Namespace: getPrefix(mc.Prometheus.CommonMetricsConfig), + Namespace: getPrefix(mc.Prometheus.Prefix), OnError: logPrometheusError, } exporter, err := prometheus.NewExporter(options) diff --git a/internal/metrics/stackdriver.go b/internal/metrics/stackdriver.go index f7f92b04..6a30fb09 100644 --- a/internal/metrics/stackdriver.go +++ b/internal/metrics/stackdriver.go @@ -31,7 +31,7 @@ func (s stackdriverExporterTypeImpl) createExporterIfEnabled( } options := stackdriver.Options{ - MetricPrefix: getPrefix(mc.Stackdriver.CommonMetricsConfig), + MetricPrefix: getPrefix(mc.Stackdriver.Prefix), ProjectID: mc.Stackdriver.ProjectID, } exporter, err := stackdriver.NewExporter(options) From 9349e8a82c1653c7c925140bd7506ca394904836 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Jul 2020 17:41:27 -0700 Subject: [PATCH 11/15] debugging --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 6eb4464d..25cfd16d 100644 --- a/Makefile +++ b/Makefile @@ -44,6 +44,7 @@ DOCKER_COMPOSE_TEST=docker-compose -f docker-compose.test.yml test-centos test-debian test-docker test-docker-standalone: release $(DOCKER_COMPOSE_TEST) up --force-recreate -d $(subst test,relay,$@) + docker-compose logs -f -t trap "$(DOCKER_COMPOSE_TEST) logs && $(DOCKER_COMPOSE_TEST) rm -f" EXIT; $(DOCKER_COMPOSE_TEST) run --rm $@ integration-test: test-centos test-debian test-docker test-docker-standalone From b7606e2ddfdf687671d7a0046a4d17357d9fca11 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Jul 2020 18:11:02 -0700 Subject: [PATCH 12/15] debuggin --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 25cfd16d..07afd52c 100644 --- a/Makefile +++ b/Makefile @@ -44,7 +44,7 @@ DOCKER_COMPOSE_TEST=docker-compose -f docker-compose.test.yml test-centos test-debian test-docker test-docker-standalone: release $(DOCKER_COMPOSE_TEST) up --force-recreate -d $(subst test,relay,$@) - docker-compose logs -f -t + $(DOCKER_COMPOSE_TEST) logs -f -t trap "$(DOCKER_COMPOSE_TEST) logs && $(DOCKER_COMPOSE_TEST) rm -f" EXIT; $(DOCKER_COMPOSE_TEST) run --rm $@ integration-test: test-centos test-debian test-docker test-docker-standalone From e623b36238f5a5d2a1d0887d77e9bc5dde103868 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Jul 2020 18:16:46 -0700 Subject: [PATCH 13/15] debugging --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 07afd52c..f057b8cc 100644 --- a/Makefile +++ b/Makefile @@ -44,7 +44,7 @@ DOCKER_COMPOSE_TEST=docker-compose -f docker-compose.test.yml test-centos test-debian test-docker test-docker-standalone: release $(DOCKER_COMPOSE_TEST) up --force-recreate -d $(subst test,relay,$@) - $(DOCKER_COMPOSE_TEST) logs -f -t + $(DOCKER_COMPOSE_TEST) logs -f -t & trap "$(DOCKER_COMPOSE_TEST) logs && $(DOCKER_COMPOSE_TEST) rm -f" EXIT; $(DOCKER_COMPOSE_TEST) run --rm $@ integration-test: test-centos test-debian test-docker test-docker-standalone From cb9ada23452348343a977b7691e5a95c0cc0d3b3 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 21 Jul 2020 10:17:05 -0700 Subject: [PATCH 14/15] fix obsolete conf properties that caused errors in Linux Docker run --- linux/etc/ld-relay.conf | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/linux/etc/ld-relay.conf b/linux/etc/ld-relay.conf index de2d2c08..36a41cba 100644 --- a/linux/etc/ld-relay.conf +++ b/linux/etc/ld-relay.conf @@ -3,28 +3,27 @@ streamUri = "https://stream.launchdarkly.com" baseUri = "https://app.launchdarkly.com" exitOnError = false port = 8030 -; You can tell the relay to send heartbeats every few seconds -; which can be useful if you have an intermediate proxy (e.g. a load balancer) that -; has timeouts -heartbeatIntervalSecs = 15 +; You can tell the Relay Proxy to send heartbeats every few seconds, which +; can be useful if you have an intermediate proxy (e.g. a load balancer) that +; has timeouts. +heartbeatInterval = 15s -; You can pass in an optional Redis configuration, -; where ld-relay can persist its flag data. This provides extra -; durability for ld-relay. +; You can pass in an optional Redis configuration, where the Relay Proxy +; can persist its flag data. This provides extra durability. ; [redis] ; host = "localhost" ; port = 6379 -; localTtl = 30000 +; localTtl = 30s -; You can configure LDR to synchronize as many environments as you want, -; even across different projects. If using Redis to persist flag data, -; you must specify a distinct Redis key prefix for each environment. All -; keys for this environment in Redis will start with this prefix. +; You can configure the Relay Proxy to synchronize as many environments as +; you want, even across different projects. If using Redis to persist flag +; data, you must specify a distinct Redis key prefix for each environment. +; All keys for this environment in Redis will start with this prefix. [environment "Default Project Production"] -apiKey = "YOUR_PRODUCTION_API_KEY" +sdkKey = "YOUR_PRODUCTION_API_KEY" ; prefix = "ld:default:prod" [environment "Default Project Test"] -apiKey = "YOUR_TEST_API_KEY" +sdkKey = "YOUR_TEST_API_KEY" From 75af80d424af2ef3440c3e5cafd91165f5498cfd Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 21 Jul 2020 10:35:31 -0700 Subject: [PATCH 15/15] rm redundant debugging --- Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Makefile b/Makefile index f057b8cc..6eb4464d 100644 --- a/Makefile +++ b/Makefile @@ -44,7 +44,6 @@ DOCKER_COMPOSE_TEST=docker-compose -f docker-compose.test.yml test-centos test-debian test-docker test-docker-standalone: release $(DOCKER_COMPOSE_TEST) up --force-recreate -d $(subst test,relay,$@) - $(DOCKER_COMPOSE_TEST) logs -f -t & trap "$(DOCKER_COMPOSE_TEST) logs && $(DOCKER_COMPOSE_TEST) rm -f" EXIT; $(DOCKER_COMPOSE_TEST) run --rm $@ integration-test: test-centos test-debian test-docker test-docker-standalone