diff --git a/config/config.go b/config/config.go index bc02d915..9fa82a86 100644 --- a/config/config.go +++ b/config/config.go @@ -88,6 +88,10 @@ const ( // the CPU time it takes to read the archive over and over again. It is somewhat arbitrary. // It likely doesn't make sense to use an interval this frequent in production use-cases. minimumFileDataSourceMonitoringInterval = 100 * time.Millisecond + // This minimum was chosen to protect the host system from unnecessary work, while also allowing expired + // credentials to be revoked nearly instantaneously. It is not necessarily a recommendation. + // It likely doesn't make sense to use an interval this frequent in production use-cases. + minimumCredentialCleanupInterval = 100 * time.Millisecond ) // DefaultLoggers is the default logging configuration used by Relay. @@ -134,24 +138,25 @@ type Config struct { // 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"` - ClientSideBaseURI ct.OptURLAbsolute `conf:"CLIENT_SIDE_BASE_URI"` - Port ct.OptIntGreaterThanZero `conf:"PORT"` - InitTimeout ct.OptDuration `conf:"INIT_TIMEOUT"` - HeartbeatInterval ct.OptDuration `conf:"HEARTBEAT_INTERVAL"` - MaxClientConnectionTime ct.OptDuration `conf:"MAX_CLIENT_CONNECTION_TIME"` - DisconnectedStatusTime ct.OptDuration `conf:"DISCONNECTED_STATUS_TIME"` - TLSEnabled bool `conf:"TLS_ENABLED"` - TLSCert string `conf:"TLS_CERT"` - TLSKey string `conf:"TLS_KEY"` - TLSMinVersion OptTLSVersion `conf:"TLS_MIN_VERSION"` - LogLevel OptLogLevel `conf:"LOG_LEVEL"` - BigSegmentsStaleAsDegraded bool `conf:"BIG_SEGMENTS_STALE_AS_DEGRADED"` - BigSegmentsStaleThreshold ct.OptDuration `conf:"BIG_SEGMENTS_STALE_THRESHOLD"` + 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"` + ClientSideBaseURI ct.OptURLAbsolute `conf:"CLIENT_SIDE_BASE_URI"` + Port ct.OptIntGreaterThanZero `conf:"PORT"` + InitTimeout ct.OptDuration `conf:"INIT_TIMEOUT"` + HeartbeatInterval ct.OptDuration `conf:"HEARTBEAT_INTERVAL"` + MaxClientConnectionTime ct.OptDuration `conf:"MAX_CLIENT_CONNECTION_TIME"` + DisconnectedStatusTime ct.OptDuration `conf:"DISCONNECTED_STATUS_TIME"` + TLSEnabled bool `conf:"TLS_ENABLED"` + TLSCert string `conf:"TLS_CERT"` + TLSKey string `conf:"TLS_KEY"` + TLSMinVersion OptTLSVersion `conf:"TLS_MIN_VERSION"` + LogLevel OptLogLevel `conf:"LOG_LEVEL"` + BigSegmentsStaleAsDegraded bool `conf:"BIG_SEGMENTS_STALE_AS_DEGRADED"` + BigSegmentsStaleThreshold ct.OptDuration `conf:"BIG_SEGMENTS_STALE_THRESHOLD"` + ExpiredCredentialCleanupInterval ct.OptDuration `conf:"EXPIRED_CREDENTIAL_CLEANUP_INTERVAL"` } // AutoConfigConfig contains configuration parameters for the auto-configuration feature. diff --git a/config/config_field_types.go b/config/config_field_types.go index c7ecc563..de676b42 100644 --- a/config/config_field_types.go +++ b/config/config_field_types.go @@ -5,8 +5,6 @@ import ( "fmt" "strings" - "github.com/launchdarkly/ld-relay/v8/internal/credential" - "github.com/launchdarkly/go-sdk-common/v3/ldlog" ) @@ -44,6 +42,13 @@ type FilterKey string // DefaultFilter represents the lack of a filter, meaning a full LaunchDarkly environment. const DefaultFilter = FilterKey("") +func last4Chars(s string) string { + if len(s) < 4 { // COVERAGE: doesn't happen in unit tests, also can't happen with real environments + return s + } + return s[len(s)-4:] +} + // GetAuthorizationHeaderValue for SDKKey returns the same string, since SDK keys are passed in // the Authorization header. func (k SDKKey) GetAuthorizationHeaderValue() string { @@ -57,21 +62,7 @@ func (k SDKKey) Defined() bool { func (k SDKKey) String() string { return string(k) } - -func (k SDKKey) Compare(cr credential.AutoConfig) (credential.SDKCredential, credential.Status) { - if cr.SDKKey == k { - return nil, credential.Unchanged - } - if cr.ExpiringSDKKey == k { - // If the AutoConfig update contains an ExpiringSDKKey that is equal to *this* key, then it means - // this key is now considered deprecated. - return cr.SDKKey, credential.Deprecated - } else { - // Otherwise if the AutoConfig update contains *some other* key, then it means this one must be considered - // expired. - return cr.SDKKey, credential.Expired - } -} +func (k SDKKey) Masked() string { return "..." + last4Chars(k.String()) } // GetAuthorizationHeaderValue for MobileKey returns the same string, since mobile keys are passed in the // Authorization header. @@ -87,12 +78,7 @@ func (k MobileKey) String() string { return string(k) } -func (k MobileKey) Compare(cr credential.AutoConfig) (credential.SDKCredential, credential.Status) { - if cr.MobileKey == k { - return nil, credential.Unchanged - } - return cr.MobileKey, credential.Expired -} +func (k MobileKey) Masked() string { return "..." + last4Chars(k.String()) } // GetAuthorizationHeaderValue for EnvironmentID returns an empty string, since environment IDs are not // passed in a header but as part of the request URL. @@ -108,10 +94,8 @@ func (k EnvironmentID) String() string { return string(k) } -func (k EnvironmentID) Compare(_ credential.AutoConfig) (credential.SDKCredential, credential.Status) { - // EnvironmentIDs should not change. - return nil, credential.Unchanged -} +// Masked is an alias for String(), because EnvironmentIDs are considered non-sensitive public information. +func (k EnvironmentID) Masked() string { return k.String() } // GetAuthorizationHeaderValue for AutoConfigKey returns the same string, since these keys are passed in // the Authorization header. Note that unlike the other kinds of authorization keys, this one is never @@ -120,15 +104,12 @@ func (k AutoConfigKey) GetAuthorizationHeaderValue() string { return string(k) } -func (k AutoConfigKey) Compare(_ credential.AutoConfig) (credential.SDKCredential, credential.Status) { - // AutoConfigKeys should not change. - return nil, credential.Unchanged -} - func (k AutoConfigKey) String() string { return string(k) } +func (k AutoConfigKey) Masked() string { return last4Chars(string(k)) } + // UnmarshalText allows the SDKKey type to be set from environment variables. func (k *SDKKey) UnmarshalText(data []byte) error { *k = SDKKey(string(data)) diff --git a/config/config_validation.go b/config/config_validation.go index b126062b..9708c076 100644 --- a/config/config_validation.go +++ b/config/config_validation.go @@ -24,6 +24,7 @@ var ( errAutoConfWithFilters = errors.New("cannot configure filters if auto-configuration is enabled") errMissingProjKey = errors.New("when filters are configured, all environments must specify a 'projKey'") errInvalidFileDataSourceMonitoringInterval = fmt.Errorf("file data source monitoring interval must be >= %s", minimumFileDataSourceMonitoringInterval) + errInvalidCredentialCleanupInterval = fmt.Errorf("expired credential cleanup interval must be >= %s", minimumCredentialCleanupInterval) ) func errEnvironmentWithNoSDKKey(envName string) error { @@ -78,6 +79,7 @@ func ValidateConfig(c *Config, loggers ldlog.Loggers) error { validateConfigDatabases(&result, c, loggers) validateConfigFilters(&result, c) validateOfflineMode(&result, c) + validateCredentialCleanupInterval(&result, c) return result.GetError() } @@ -196,6 +198,14 @@ func validateOfflineMode(result *ct.ValidationResult, c *Config) { } } +func validateCredentialCleanupInterval(result *ct.ValidationResult, c *Config) { + if c.Main.ExpiredCredentialCleanupInterval.IsDefined() { + interval := c.Main.ExpiredCredentialCleanupInterval.GetOrElse(0) + if interval < minimumCredentialCleanupInterval { + result.AddError(nil, errInvalidCredentialCleanupInterval) + } + } +} func validateConfigDatabases(result *ct.ValidationResult, c *Config, loggers ldlog.Loggers) { normalizeRedisConfig(result, c) diff --git a/config/test_data_configs_invalid_test.go b/config/test_data_configs_invalid_test.go index 41cd917b..9d583bbc 100644 --- a/config/test_data_configs_invalid_test.go +++ b/config/test_data_configs_invalid_test.go @@ -11,6 +11,9 @@ type testDataInvalidConfig struct { func makeInvalidConfigs() []testDataInvalidConfig { return []testDataInvalidConfig{ makeInvalidConfigMissingSDKKey(), + makeInvalidConfigCredentialCleanupInterval("0s"), + makeInvalidConfigCredentialCleanupInterval("-1s"), + makeInvalidConfigCredentialCleanupInterval("99ms"), makeInvalidConfigTLSWithNoCertOrKey(), makeInvalidConfigTLSWithNoCert(), makeInvalidConfigTLSWithNoKey(), @@ -255,6 +258,16 @@ fileDataSourceMonitoringInterval = ` + interval + ` return c } +func makeInvalidConfigCredentialCleanupInterval(interval string) testDataInvalidConfig { + c := testDataInvalidConfig{name: "credential cleanup interval with invalid value"} + c.fileError = errInvalidCredentialCleanupInterval.Error() + c.fileContent = ` +[Main] +expiredCredentialCleanupInterval = ` + interval + ` +` + return c +} + func makeInvalidConfigRedisInvalidHostname() testDataInvalidConfig { c := testDataInvalidConfig{name: "Redis - invalid hostname"} c.envVarsError = "invalid Redis hostname" diff --git a/config/test_data_configs_valid_test.go b/config/test_data_configs_valid_test.go index a06bdcd0..b5d875a0 100644 --- a/config/test_data_configs_valid_test.go +++ b/config/test_data_configs_valid_test.go @@ -98,23 +98,24 @@ func makeValidConfigAllBaseProperties() testDataValidConfig { c := testDataValidConfig{name: "all base properties"} c.makeConfig = func(c *Config) { c.Main = MainConfig{ - Port: mustOptIntGreaterThanZero(8333), - BaseURI: newOptURLAbsoluteMustBeValid("http://base"), - ClientSideBaseURI: newOptURLAbsoluteMustBeValid("http://clientbase"), - StreamURI: newOptURLAbsoluteMustBeValid("http://stream"), - ExitOnError: true, - ExitAlways: true, - IgnoreConnectionErrors: true, - HeartbeatInterval: ct.NewOptDuration(90 * time.Second), - MaxClientConnectionTime: ct.NewOptDuration(30 * time.Minute), - DisconnectedStatusTime: ct.NewOptDuration(3 * time.Minute), - TLSEnabled: true, - TLSCert: "cert", - TLSKey: "key", - TLSMinVersion: NewOptTLSVersion(tls.VersionTLS12), - LogLevel: NewOptLogLevel(ldlog.Warn), - BigSegmentsStaleAsDegraded: true, - BigSegmentsStaleThreshold: ct.NewOptDuration(10 * time.Minute), + Port: mustOptIntGreaterThanZero(8333), + BaseURI: newOptURLAbsoluteMustBeValid("http://base"), + ClientSideBaseURI: newOptURLAbsoluteMustBeValid("http://clientbase"), + StreamURI: newOptURLAbsoluteMustBeValid("http://stream"), + ExitOnError: true, + ExitAlways: true, + IgnoreConnectionErrors: true, + HeartbeatInterval: ct.NewOptDuration(90 * time.Second), + MaxClientConnectionTime: ct.NewOptDuration(30 * time.Minute), + DisconnectedStatusTime: ct.NewOptDuration(3 * time.Minute), + TLSEnabled: true, + TLSCert: "cert", + TLSKey: "key", + TLSMinVersion: NewOptTLSVersion(tls.VersionTLS12), + LogLevel: NewOptLogLevel(ldlog.Warn), + BigSegmentsStaleAsDegraded: true, + BigSegmentsStaleThreshold: ct.NewOptDuration(10 * time.Minute), + ExpiredCredentialCleanupInterval: ct.NewOptDuration(1 * time.Minute), } c.Events = EventsConfig{ SendEvents: true, @@ -146,43 +147,44 @@ func makeValidConfigAllBaseProperties() testDataValidConfig { } } c.envVars = map[string]string{ - "PORT": "8333", - "BASE_URI": "http://base", - "CLIENT_SIDE_BASE_URI": "http://clientbase", - "STREAM_URI": "http://stream", - "EXIT_ON_ERROR": "1", - "EXIT_ALWAYS": "1", - "IGNORE_CONNECTION_ERRORS": "1", - "HEARTBEAT_INTERVAL": "90s", - "MAX_CLIENT_CONNECTION_TIME": "30m", - "DISCONNECTED_STATUS_TIME": "3m", - "TLS_ENABLED": "1", - "TLS_CERT": "cert", - "TLS_KEY": "key", - "TLS_MIN_VERSION": "1.2", - "LOG_LEVEL": "warn", - "BIG_SEGMENTS_STALE_AS_DEGRADED": "true", - "BIG_SEGMENTS_STALE_THRESHOLD": "10m", - "USE_EVENTS": "1", - "EVENTS_HOST": "http://events", - "EVENTS_FLUSH_INTERVAL": "120s", - "EVENTS_CAPACITY": "500", - "EVENTS_INLINE_USERS": "1", - "LD_ENV_earth": "earth-sdk", - "LD_MOBILE_KEY_earth": "earth-mob", - "LD_CLIENT_SIDE_ID_earth": "earth-env", - "LD_PREFIX_earth": "earth-", - "LD_TABLE_NAME_earth": "earth-table", - "LD_LOG_LEVEL_earth": "debug", - "LD_ENV_krypton": "krypton-sdk", - "LD_MOBILE_KEY_krypton": "krypton-mob", - "LD_CLIENT_SIDE_ID_krypton": "krypton-env", - "LD_SECURE_MODE_krypton": "1", - "LD_PREFIX_krypton": "krypton-", - "LD_TABLE_NAME_krypton": "krypton-table", - "LD_ALLOWED_ORIGIN_krypton": "https://oa,https://rann", - "LD_ALLOWED_HEADER_krypton": "Timestamp-Valid,Random-Id-Valid", - "LD_TTL_krypton": "5m", + "PORT": "8333", + "BASE_URI": "http://base", + "CLIENT_SIDE_BASE_URI": "http://clientbase", + "STREAM_URI": "http://stream", + "EXIT_ON_ERROR": "1", + "EXIT_ALWAYS": "1", + "IGNORE_CONNECTION_ERRORS": "1", + "HEARTBEAT_INTERVAL": "90s", + "MAX_CLIENT_CONNECTION_TIME": "30m", + "DISCONNECTED_STATUS_TIME": "3m", + "TLS_ENABLED": "1", + "TLS_CERT": "cert", + "TLS_KEY": "key", + "TLS_MIN_VERSION": "1.2", + "LOG_LEVEL": "warn", + "BIG_SEGMENTS_STALE_AS_DEGRADED": "true", + "BIG_SEGMENTS_STALE_THRESHOLD": "10m", + "USE_EVENTS": "1", + "EVENTS_HOST": "http://events", + "EVENTS_FLUSH_INTERVAL": "120s", + "EVENTS_CAPACITY": "500", + "EVENTS_INLINE_USERS": "1", + "LD_ENV_earth": "earth-sdk", + "LD_MOBILE_KEY_earth": "earth-mob", + "LD_CLIENT_SIDE_ID_earth": "earth-env", + "LD_PREFIX_earth": "earth-", + "LD_TABLE_NAME_earth": "earth-table", + "LD_LOG_LEVEL_earth": "debug", + "LD_ENV_krypton": "krypton-sdk", + "LD_MOBILE_KEY_krypton": "krypton-mob", + "LD_CLIENT_SIDE_ID_krypton": "krypton-env", + "LD_SECURE_MODE_krypton": "1", + "LD_PREFIX_krypton": "krypton-", + "LD_TABLE_NAME_krypton": "krypton-table", + "LD_ALLOWED_ORIGIN_krypton": "https://oa,https://rann", + "LD_ALLOWED_HEADER_krypton": "Timestamp-Valid,Random-Id-Valid", + "LD_TTL_krypton": "5m", + "EXPIRED_CREDENTIAL_CLEANUP_INTERVAL": "1m", } c.fileContent = ` [Main] @@ -203,6 +205,7 @@ TLSMinVersion = "1.2" LogLevel = "warn" BigSegmentsStaleAsDegraded = 1 BigSegmentsStaleThreshold = 10m +ExpiredCredentialCleanupInterval = 1m [Events] SendEvents = 1 diff --git a/docs/configuration.md b/docs/configuration.md index f937df4b..f7e4a436 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -45,26 +45,27 @@ For **Duration** settings, the value should be be an integer followed by `ms`, ` ### File section: `[Main]` -| Property in file | Environment var | Type | Default | Description | -|-------------------------------|----------------------------------|:--------:|:--------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `streamUri` | `STREAM_URI` | URI | _(1)_ | URI for the LaunchDarkly streaming service. | -| `baseUri` | `BASE_URI` | URI | _(1)_ | URI for the LaunchDarkly polling service for server-side SDKs. | -| `clientSideBaseUri` | `CLIENT_SIDE_BASE_URI` | URI | _(1)_ | URI for the LaunchDarkly polling service for client-side SDKs. | -| `exitOnError` | `EXIT_ON_ERROR` | Boolean | `false` | Close the Relay Proxy if it encounters any error during initialization. The default behavior is that it will terminate with a non-zero exit code if the configuration options are completely invalid, or if there is an incorrect `AutoConfig` key, but will remain running if there is an error specific to one environment, such as an invalid SDK key. Setting this option to `true` makes it terminate in both cases. | -| `exitAlways` | `EXIT_ALWAYS` | Boolean | `false` | Close the Relay Proxy immediately after initializing all environments. Do not start an HTTP server. _(2)_ | -| `ignoreConnectionErrors` | `IGNORE_CONNECTION_ERRORS` | Boolean | `false` | Ignore any initial connectivity issues with LaunchDarkly. Best used when network connectivity is not reliable. | -| `port` | `PORT` | Number | `8030` | Port the Relay Proxy should listen on. | -| `initTimeout` | `INIT_TIMEOUT` | Duration | `10s` | How long the Relay Proxy should wait for an initial connection to LaunchDarkly. If this timeout elapses, the behavior depends on `ignoreConnectionErrors`: by default, it will quit, but if `ignoreConnectionErrors` is true it will go on trying to connect in the background while still allowing clients to connect to the Relay Proxy. To learn more, read [How connections are handled in error conditions](./proxy-mode.md#how-connections-are-handled-in-error-conditions). | -| `heartbeatInterval` | `HEARTBEAT_INTERVAL` | Number | `3m` | Interval for heartbeat messages to prevent read timeouts on streaming connections. Assumed to be in seconds if no unit is specified. | -| `maxClientConnectionTime` | `MAX_CLIENT_CONNECTION_TIME` | Duration | none | Maximum amount of time that Relay will allow a streaming connection from an SDK client to remain open. _(3)_ | -| `disconnectedStatusTime` | `DISCONNECTED_STATUS_TIME` | Duration | `1m` | How long a stream connection can be interrupted before Relay reports the status as "disconnected." _(4)_ | -| `tlsEnabled` | `TLS_ENABLED` | Boolean | `false` | Enable TLS on the Relay Proxy. Read: [Using TLS](./tls.md). | -| `tlsCert` | `TLS_CERT` | String | | Required if `tlsEnabled` is true. Path to TLS certificate file. | -| `tlsKey` | `TLS_KEY` | String | | Required if `tlsEnabled` is true. Path to TLS private key file. | -| `tlsMinVersion` | `TLS_MIN_VERSION` | String | | Set to "1.2", etc., to enforce a minimum TLS version for secure requests. | -| `logLevel` | `LOG_LEVEL` | String | `info` | Should be `debug`, `info`, `warn`, `error`, or `none`. To learn more, read [Logging](./logging.md). | -| `bigSegmentsStaleAsDegraded` | `BIG_SEGMENTS_STALE_AS_DEGRADED` | Boolean | `false` | Indicates if environments should be considered degraded if Big Segments are not fully synchronized. | -| `bigSegmentsStaleThreshold` | `BIG_SEGMENTS_STALE_THRESHOLD` | Duration | `5m` | Indicates how long until Big Segments should be considered stale. | +| Property in file | Environment var | Type | Default | Description | +|------------------------------------|---------------------------------------|:--------:|:--------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `streamUri` | `STREAM_URI` | URI | _(1)_ | URI for the LaunchDarkly streaming service. | +| `baseUri` | `BASE_URI` | URI | _(1)_ | URI for the LaunchDarkly polling service for server-side SDKs. | +| `clientSideBaseUri` | `CLIENT_SIDE_BASE_URI` | URI | _(1)_ | URI for the LaunchDarkly polling service for client-side SDKs. | +| `exitOnError` | `EXIT_ON_ERROR` | Boolean | `false` | Close the Relay Proxy if it encounters any error during initialization. The default behavior is that it will terminate with a non-zero exit code if the configuration options are completely invalid, or if there is an incorrect `AutoConfig` key, but will remain running if there is an error specific to one environment, such as an invalid SDK key. Setting this option to `true` makes it terminate in both cases. | +| `exitAlways` | `EXIT_ALWAYS` | Boolean | `false` | Close the Relay Proxy immediately after initializing all environments. Do not start an HTTP server. _(2)_ | +| `ignoreConnectionErrors` | `IGNORE_CONNECTION_ERRORS` | Boolean | `false` | Ignore any initial connectivity issues with LaunchDarkly. Best used when network connectivity is not reliable. | +| `port` | `PORT` | Number | `8030` | Port the Relay Proxy should listen on. | +| `initTimeout` | `INIT_TIMEOUT` | Duration | `10s` | How long the Relay Proxy should wait for an initial connection to LaunchDarkly. If this timeout elapses, the behavior depends on `ignoreConnectionErrors`: by default, it will quit, but if `ignoreConnectionErrors` is true it will go on trying to connect in the background while still allowing clients to connect to the Relay Proxy. To learn more, read [How connections are handled in error conditions](./proxy-mode.md#how-connections-are-handled-in-error-conditions). | +| `heartbeatInterval` | `HEARTBEAT_INTERVAL` | Number | `3m` | Interval for heartbeat messages to prevent read timeouts on streaming connections. Assumed to be in seconds if no unit is specified. | +| `maxClientConnectionTime` | `MAX_CLIENT_CONNECTION_TIME` | Duration | none | Maximum amount of time that Relay will allow a streaming connection from an SDK client to remain open. _(3)_ | +| `disconnectedStatusTime` | `DISCONNECTED_STATUS_TIME` | Duration | `1m` | How long a stream connection can be interrupted before Relay reports the status as "disconnected." _(4)_ | +| `tlsEnabled` | `TLS_ENABLED` | Boolean | `false` | Enable TLS on the Relay Proxy. Read: [Using TLS](./tls.md). | +| `tlsCert` | `TLS_CERT` | String | | Required if `tlsEnabled` is true. Path to TLS certificate file. | +| `tlsKey` | `TLS_KEY` | String | | Required if `tlsEnabled` is true. Path to TLS private key file. | +| `tlsMinVersion` | `TLS_MIN_VERSION` | String | | Set to "1.2", etc., to enforce a minimum TLS version for secure requests. | +| `logLevel` | `LOG_LEVEL` | String | `info` | Should be `debug`, `info`, `warn`, `error`, or `none`. To learn more, read [Logging](./logging.md). | +| `bigSegmentsStaleAsDegraded` | `BIG_SEGMENTS_STALE_AS_DEGRADED` | Boolean | `false` | Indicates if environments should be considered degraded if Big Segments are not fully synchronized. | +| `bigSegmentsStaleThreshold` | `BIG_SEGMENTS_STALE_THRESHOLD` | Duration | `5m` | Indicates how long until Big Segments should be considered stale. | +| `expiredCredentialCleanupInterval` | `EXPIRED_CREDENTIAL_CLEANUP_INTERVAL` | Duration | `1m` | Specifies how often expired credentials for environments are cleaned up. _(5)_ | _(1)_ The default values for `streamUri`, `baseUri`, and `clientSideBaseUri` are `https://stream.launchdarkly.com`, `https://sdk.launchdarkly.com`, and `https://clientsdk.launchdarkly.com`, respectively. You should never need to change these URIs unless you are either using a special instance of the LaunchDarkly service, in which case Support will tell you how to set them, or you are accessing LaunchDarkly using a reverse proxy or some other mechanism that rewrites URLs. @@ -74,6 +75,10 @@ _(3)_ The optional `maxClientConnectionTime` setting may be useful in load-balan _(4)_ For details about `disconnectedStatusTime`, read [Service endpoints - Status (health check)](./endpoints.md#status-health-check). +_(5)_ Relevant only when using AutoConfig or Offline Mode. In these modes, when an environment's SDK key is rotated in +LaunchDarkly, it's possible to specify a deprecation/grace period for the previous key where existing SDKs are still able +to authorize using that credential. Relay will periodically check for expired credentials and remove them on this interval. + ### File section: `[AutoConfig]` This section is only applicable if [automatic configuration](https://docs.launchdarkly.com/home/advanced/relay-proxy-enterprise/automatic-configuration) is enabled for your account. diff --git a/internal/autoconfig/message_handler.go b/internal/autoconfig/message_handler.go index 864c1a03..15b5a007 100644 --- a/internal/autoconfig/message_handler.go +++ b/internal/autoconfig/message_handler.go @@ -26,11 +26,6 @@ type MessageHandler interface { // message, or a "put" that no longer contains that environment. DeleteEnvironment(id config.EnvironmentID) - // KeyExpired is called when a key that was previously provided in EnvironmentParams.ExpiringSDKKey - // has now expired. Relay should disconnect any clients currently using that key and reject any - // future requests that use it. - KeyExpired(id config.EnvironmentID, projKey string, oldKey config.SDKKey) - // AddFilter is called whenever a new filter should be added, either in a "put" or "patch" message. AddFilter(params envfactory.FilterParams) diff --git a/internal/autoconfig/stream_manager.go b/internal/autoconfig/stream_manager.go index 079030a4..ed6d1c10 100644 --- a/internal/autoconfig/stream_manager.go +++ b/internal/autoconfig/stream_manager.go @@ -3,7 +3,6 @@ package autoconfig import ( "encoding/json" "errors" - "fmt" "net/http" "net/url" "path" @@ -14,7 +13,6 @@ import ( es "github.com/launchdarkly/eventsource" "github.com/launchdarkly/go-sdk-common/v3/ldlog" - "github.com/launchdarkly/go-sdk-common/v3/ldtime" "github.com/launchdarkly/ld-relay/v8/config" "github.com/launchdarkly/ld-relay/v8/internal/envfactory" "github.com/launchdarkly/ld-relay/v8/internal/httpconfig" @@ -50,8 +48,6 @@ type StreamManager struct { uri *url.URL handler MessageHandler lastKnownEnvs map[config.EnvironmentID]envfactory.EnvironmentRep - expiredKeys chan expiredKey - expiryTimers map[config.SDKKey]*time.Timer httpConfig httpconfig.HTTPConfig initialRetryDelay time.Duration loggers ldlog.Loggers @@ -62,12 +58,6 @@ type StreamManager struct { filterReceiver *MessageReceiver[envfactory.FilterRep] } -type expiredKey struct { - envID config.EnvironmentID - projKey string - key config.SDKKey -} - // NewStreamManager creates a StreamManager, but does not start the connection. func NewStreamManager( key config.AutoConfigKey, @@ -89,8 +79,6 @@ func NewStreamManager( uri: streamURI, handler: handler, lastKnownEnvs: make(map[config.EnvironmentID]envfactory.EnvironmentRep), - expiredKeys: make(chan expiredKey), - expiryTimers: make(map[config.SDKKey]*time.Timer), httpConfig: httpConfig, initialRetryDelay: initialRetryDelay, loggers: loggers, @@ -307,17 +295,8 @@ func (s *StreamManager) consumeStream(stream *es.Stream) { if shouldRestart { stream.Restart() } - - case expiredKey := <-s.expiredKeys: - s.loggers.Warnf(logMsgKeyExpired, last4Chars(string(expiredKey.key)), expiredKey.envID, - makeEnvName(s.lastKnownEnvs[expiredKey.envID])) - s.handler.KeyExpired(expiredKey.envID, expiredKey.projKey, expiredKey.key) - case <-s.halt: stream.Close() - for _, t := range s.expiryTimers { - t.Stop() - } return } } @@ -329,17 +308,11 @@ func (s *StreamManager) dispatchEnvAction(id config.EnvironmentID, rep envfactor return case ActionInsert: params := rep.ToParams() - if s.IgnoreExpiringSDKKey(rep) { - params.ExpiringSDKKey = "" - } s.handler.AddEnvironment(params) case ActionDelete: s.handler.DeleteEnvironment(id) case ActionUpdate: params := rep.ToParams() - if s.IgnoreExpiringSDKKey(rep) { - params.ExpiringSDKKey = "" - } s.handler.UpdateEnvironment(params) } } @@ -394,54 +367,6 @@ func (s *StreamManager) handlePut(content PutContent) { s.handler.ReceivedAllEnvironments() } -// IgnoreExpiringSDKKey implements the EnvironmentMsgAdapter's KeyChecker interface. Its main purpose is to -// create a goroutine that triggers SDK key expiration, if the EnvironmentRep specifies that. Additionally, it returns -// true if an ExpiringSDKKey should be ignored (since the expiry is stale). -func (s *StreamManager) IgnoreExpiringSDKKey(env envfactory.EnvironmentRep) bool { - expiringKey := env.SDKKey.Expiring.Value - expiryTime := env.SDKKey.Expiring.Timestamp - - if expiringKey == "" || expiryTime == 0 { - return false - } - - if _, alreadyHaveTimer := s.expiryTimers[expiringKey]; alreadyHaveTimer { - return false - } - - timeFromNow := time.Duration(expiryTime-ldtime.UnixMillisNow()) * time.Millisecond - if timeFromNow <= 0 { - // LD might sometimes tell us about an "expiring" key that has really already expired. If so, - // just ignore it. - return true - } - - dateTime := time.Unix(int64(expiryTime)/1000, 0) - s.loggers.Warnf(logMsgKeyWillExpire, last4Chars(string(expiringKey)), env.Describe(), dateTime) - - timer := time.NewTimer(timeFromNow) - s.expiryTimers[expiringKey] = timer - - go func() { - if _, ok := <-timer.C; ok { - s.expiredKeys <- expiredKey{env.EnvID, env.ProjKey, expiringKey} - } - }() - - return false -} - -func makeEnvName(rep envfactory.EnvironmentRep) string { - return fmt.Sprintf("%s %s", rep.ProjName, rep.EnvName) -} - -func last4Chars(s string) string { - if len(s) < 4 { // COVERAGE: doesn't happen in unit tests, also can't happen with real environments - return s - } - return s[len(s)-4:] -} - func obfuscateEventData(data string) string { // Used for debug logging to obscure the SDK keys and mobile keys in the JSON data data = sdkKeyJSONRegex.ReplaceAllString(data, `"value":"...$1"`) diff --git a/internal/autoconfig/stream_manager_expiring_key_test.go b/internal/autoconfig/stream_manager_expiring_key_test.go deleted file mode 100644 index 67077fef..00000000 --- a/internal/autoconfig/stream_manager_expiring_key_test.go +++ /dev/null @@ -1,162 +0,0 @@ -package autoconfig - -import ( - "testing" - - "github.com/launchdarkly/ld-relay/v8/config" - "github.com/launchdarkly/ld-relay/v8/internal/envfactory" - - "github.com/launchdarkly/go-sdk-common/v3/ldlog" - "github.com/launchdarkly/go-sdk-common/v3/ldtime" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -const ( - oldKey = config.SDKKey("oldkey") - briefExpiryMillis = 300 -) - -func makeEnvWithExpiringKey(fromEnv envfactory.EnvironmentRep, oldKey config.SDKKey) envfactory.EnvironmentRep { - ret := fromEnv - ret.SDKKey.Expiring = envfactory.ExpiringKeyRep{ - Value: oldKey, - Timestamp: ldtime.UnixMillisNow() + briefExpiryMillis, - } - return ret -} - -func makeEnvWithAlreadyExpiredKey(fromEnv envfactory.EnvironmentRep, oldKey config.SDKKey) envfactory.EnvironmentRep { - ret := fromEnv - ret.SDKKey.Expiring = envfactory.ExpiringKeyRep{ - Value: oldKey, - Timestamp: ldtime.UnixMillisNow() - 1, - } - return ret -} - -func expectOldKeyWillExpire(p streamManagerTestParams, envID config.EnvironmentID) { - p.mockLog.AssertMessageMatch(p.t, true, ldlog.Warn, "Old SDK key ending in dkey .* will expire") - assert.Len(p.t, p.mockLog.GetOutput(ldlog.Error), 0) - - msg := p.requireMessage() - require.NotNil(p.t, msg.expired) - assert.Equal(p.t, envID, msg.expired.envID) - assert.Equal(p.t, oldKey, msg.expired.key) - - p.mockLog.AssertMessageMatch(p.t, true, ldlog.Warn, "Old SDK key ending in dkey .* has expired") -} - -func expectNoKeyExpiryMessage(p streamManagerTestParams) { - p.mockLog.AssertMessageMatch(p.t, false, ldlog.Warn, "Old SDK key .* will expire") -} - -func TestExpiringKeyInPutMessage(t *testing.T) { - envWithExpiringKey := makeEnvWithExpiringKey(testEnv1, oldKey) - event := makeEnvPutEvent(envWithExpiringKey) - streamManagerTest(t, &event, func(p streamManagerTestParams) { - p.startStream() - - msg := p.requireMessage() - require.NotNil(t, msg.add) - p.requireReceivedAllMessage() - - assert.Equal(t, envWithExpiringKey.ToParams(), *msg.add) - assert.Equal(t, oldKey, msg.add.ExpiringSDKKey) - - expectOldKeyWillExpire(p, envWithExpiringKey.EnvID) - }) -} - -func TestExpiringKeyInPatchAdd(t *testing.T) { - envWithExpiringKey := makeEnvWithExpiringKey(testEnv1, oldKey) - event := makePatchEnvEvent(envWithExpiringKey) - streamManagerTest(t, nil, func(p streamManagerTestParams) { - p.startStream() - p.stream.Enqueue(event) - - msg := p.requireMessage() - require.NotNil(t, msg.add) - - assert.Equal(t, envWithExpiringKey.ToParams(), *msg.add) - assert.Equal(t, oldKey, msg.add.ExpiringSDKKey) - - expectOldKeyWillExpire(p, envWithExpiringKey.EnvID) - }) -} - -func TestExpiringKeyInPatchUpdate(t *testing.T) { - streamManagerTest(t, nil, func(p streamManagerTestParams) { - p.startStream() - p.stream.Enqueue(makePatchEnvEvent(testEnv1)) - - _ = p.requireMessage() - - envWithExpiringKey := makeEnvWithExpiringKey(testEnv1, oldKey) - envWithExpiringKey.Version++ - - p.stream.Enqueue(makePatchEnvEvent(envWithExpiringKey)) - - msg := p.requireMessage() - require.NotNil(t, msg.update) - assert.Equal(t, envWithExpiringKey.ToParams(), *msg.update) - assert.Equal(t, oldKey, msg.update.ExpiringSDKKey) - - expectOldKeyWillExpire(p, envWithExpiringKey.EnvID) - }) -} - -func TestExpiringKeyHasAlreadyExpiredInPutMessage(t *testing.T) { - envWithExpiringKey := makeEnvWithAlreadyExpiredKey(testEnv1, oldKey) - event := makeEnvPutEvent(envWithExpiringKey) - streamManagerTest(t, &event, func(p streamManagerTestParams) { - p.startStream() - - msg := p.requireMessage() - require.NotNil(t, msg.add) - p.requireReceivedAllMessage() - - assert.Equal(t, testEnv1.ToParams(), *msg.add) - assert.Equal(t, config.SDKKey(""), msg.add.ExpiringSDKKey) - - expectNoKeyExpiryMessage(p) - }) -} - -func TestExpiringKeyHasAlreadyExpiredInPatchAdd(t *testing.T) { - envWithExpiringKey := makeEnvWithAlreadyExpiredKey(testEnv1, oldKey) - event := makePatchEnvEvent(envWithExpiringKey) - streamManagerTest(t, nil, func(p streamManagerTestParams) { - p.startStream() - p.stream.Enqueue(event) - - msg := p.requireMessage() - require.NotNil(t, msg.add) - assert.Equal(t, testEnv1.ToParams(), *msg.add) - assert.Equal(t, config.SDKKey(""), msg.add.ExpiringSDKKey) - - expectNoKeyExpiryMessage(p) - }) -} - -func TestExpiringKeyHasAlreadyExpiredInPatchUpdate(t *testing.T) { - streamManagerTest(t, nil, func(p streamManagerTestParams) { - p.startStream() - p.stream.Enqueue(makePatchEnvEvent(testEnv1)) - - _ = p.requireMessage() - - envWithExpiringKey := makeEnvWithAlreadyExpiredKey(testEnv1, oldKey) - envWithExpiringKey.Version++ - - p.stream.Enqueue(makePatchEnvEvent(envWithExpiringKey)) - - msg := p.requireMessage() - require.NotNil(t, msg.update) - assert.Equal(t, testEnv1.ToParams(), *msg.update) - assert.Equal(t, config.SDKKey(""), msg.update.ExpiringSDKKey) - - expectNoKeyExpiryMessage(p) - }) -} diff --git a/internal/autoconfig/stream_manager_test_base_test.go b/internal/autoconfig/stream_manager_test_base_test.go index e22fe992..6f7390ed 100644 --- a/internal/autoconfig/stream_manager_test_base_test.go +++ b/internal/autoconfig/stream_manager_test_base_test.go @@ -176,7 +176,6 @@ type testMessage struct { delete *config.EnvironmentID deleteFilter *config.FilterID receivedAll bool - expired *expiredKey } func (m testMessage) String() string { @@ -192,9 +191,6 @@ func (m testMessage) String() string { if m.receivedAll { return "receivedAllEnvironments" } - if m.expired != nil { - return fmt.Sprintf("expired(%+v)", *m.expired) - } return "???" } @@ -302,10 +298,6 @@ func (h *testMessageHandler) ReceivedAllEnvironments() { h.received <- testMessage{receivedAll: true} } -func (h *testMessageHandler) KeyExpired(envID config.EnvironmentID, projKey string, key config.SDKKey) { - h.received <- testMessage{expired: &expiredKey{envID, projKey, key}} -} - func (h *testMessageHandler) AddFilter(params envfactory.FilterParams) { h.received <- testMessage{addFilter: ¶ms} } diff --git a/internal/credential/credential.go b/internal/credential/credential.go index 14dd270c..09354588 100644 --- a/internal/credential/credential.go +++ b/internal/credential/credential.go @@ -11,24 +11,10 @@ type SDKCredential interface { Defined() bool // String returns the string form of the credential. String() string - // Compare accepts a collection of AutoConfig credentials and inspects it, determining if this credential has - // changed in any way. If so, it should return the new credential and a status. - Compare(creds AutoConfig) (SDKCredential, Status) + // Masked returns a masked form of the credential suitable for log messages. + Masked() string } -// Status represents that difference between an existing credential and one found in a new AutoConfig configuration -// struct. -type Status string - -const ( - // Unchanged means the credential has not changed. - Unchanged = Status("unchanged") - // Deprecated means the existing credential has been deprecated in favor of a new one. - Deprecated = Status("deprecated") - // Expired means the existing credential should be removed in favor of a new one. - Expired = Status("expired") -) - // AutoConfig represents credentials that are updated via AutoConfig protocol. type AutoConfig struct { // SDKKey is the environment's SDK key; if there is more than one active key, it is the latest. diff --git a/internal/credential/rotator.go b/internal/credential/rotator.go new file mode 100644 index 00000000..3ece1494 --- /dev/null +++ b/internal/credential/rotator.go @@ -0,0 +1,278 @@ +package credential + +import ( + "slices" + "sync" + "time" + + "github.com/launchdarkly/go-sdk-common/v3/ldlog" + "github.com/launchdarkly/ld-relay/v8/config" +) + +type Rotator struct { + loggers ldlog.Loggers + + // There is only one mobile key active at a given time; it does not support a deprecation period. + primaryMobileKey config.MobileKey + + // There is only one environment ID active at a given time, and it won't actually be rotated. The mechanism is + // here to allow setting it in a deferred manner. + primaryEnvironmentID config.EnvironmentID + + // There can be multiple SDK keys active at a given time, but only one is primary. + primarySdkKey config.SDKKey + + // Deprecated keys are stored in a map with a started timer for each key representing the deprecation period. + // Upon expiration, they are removed. + deprecatedSdkKeys map[config.SDKKey]time.Time + + expirations []SDKCredential + additions []SDKCredential + + mu sync.RWMutex +} + +type InitialCredentials struct { + SDKKey config.SDKKey + MobileKey config.MobileKey + EnvironmentID config.EnvironmentID +} + +// NewRotator constructs a rotator with the provided loggers. A new rotator +// contains no credentials and can optionally be initialized via Initialize. +func NewRotator(loggers ldlog.Loggers) *Rotator { + r := &Rotator{ + loggers: loggers, + deprecatedSdkKeys: make(map[config.SDKKey]time.Time), + } + return r +} + +// Initialize sets the initial credentials. Only credentials that are defined +// will be stored. +func (r *Rotator) Initialize(credentials []SDKCredential) { + r.mu.Lock() + defer r.mu.Unlock() + + for _, cred := range credentials { + if !cred.Defined() { + continue + } + switch cred := cred.(type) { + case config.SDKKey: + r.primarySdkKey = cred + case config.MobileKey: + r.primaryMobileKey = cred + case config.EnvironmentID: + r.primaryEnvironmentID = cred + } + } +} + +// MobileKey returns the primary mobile key. +func (r *Rotator) MobileKey() config.MobileKey { + r.mu.RLock() + defer r.mu.RUnlock() + return r.primaryMobileKey +} + +// SDKKey returns the primary SDK key. +func (r *Rotator) SDKKey() config.SDKKey { + r.mu.RLock() + defer r.mu.RUnlock() + return r.primarySdkKey +} + +// EnvironmentID returns the environment ID. +func (r *Rotator) EnvironmentID() config.EnvironmentID { + r.mu.RLock() + defer r.mu.RUnlock() + return r.primaryEnvironmentID +} + +// PrimaryCredentials returns the primary (non-deprecated) credentials. +func (r *Rotator) PrimaryCredentials() []SDKCredential { + r.mu.RLock() + defer r.mu.RUnlock() + return r.primaryCredentials() +} + +func (r *Rotator) primaryCredentials() []SDKCredential { + return slices.DeleteFunc([]SDKCredential{ + r.primarySdkKey, + r.primaryMobileKey, + r.primaryEnvironmentID, + }, func(cred SDKCredential) bool { + return !cred.Defined() + }) +} + +func (r *Rotator) deprecatedCredentials() []SDKCredential { + deprecated := make([]SDKCredential, 0, len(r.deprecatedSdkKeys)) + for key := range r.deprecatedSdkKeys { + deprecated = append(deprecated, key) + } + return deprecated +} + +// DeprecatedCredentials returns deprecated credentials (not expired or primary.) +func (r *Rotator) DeprecatedCredentials() []SDKCredential { + r.mu.RLock() + defer r.mu.RUnlock() + return r.deprecatedCredentials() +} + +// AllCredentials returns the primary and deprecated credentials as one list. +func (r *Rotator) AllCredentials() []SDKCredential { + r.mu.RLock() + defer r.mu.RUnlock() + return append(r.primaryCredentials(), r.deprecatedCredentials()...) +} + +// Rotate sets a new primary credential while revoking the previous. +func (r *Rotator) Rotate(cred SDKCredential) { + r.RotateWithGrace(cred, nil) +} + +// GracePeriod represents a grace period (or deprecation period) within which +// a particular SDK key is still valid, pending revocation. +type GracePeriod struct { + // The SDK key that is being deprecated. + key config.SDKKey + // When the key will expire. + expiry time.Time + // The current timestamp. + now time.Time +} + +// NewGracePeriod constructs a new grace period. The current time must be provided in order to +// determine if the credential is already expired. +func NewGracePeriod(key config.SDKKey, expiry time.Time, now time.Time) *GracePeriod { + return &GracePeriod{key, expiry, now} +} + +// RotateWithGrace sets a new primary credential while deprecating a previous credential. The grace +// parameter may be nil to immediately revoke the previous credential. +// It is invalid to specify a grace period when the credential being rotate is a mobile key or +// environment ID. +func (r *Rotator) RotateWithGrace(primary SDKCredential, grace *GracePeriod) { + switch primary := primary.(type) { + case config.SDKKey: + r.updateSDKKey(primary, grace) + case config.MobileKey: + if grace != nil { + panic("programmer error: mobile keys do not support deprecation") + } + r.updateMobileKey(primary) + case config.EnvironmentID: + if grace != nil { + panic("programmer error: environment IDs do not support deprecation") + } + r.updateEnvironmentID(primary) + } +} + +func (r *Rotator) updateEnvironmentID(envID config.EnvironmentID) { + if envID == r.EnvironmentID() { + return + } + r.mu.Lock() + defer r.mu.Unlock() + previous := r.primaryEnvironmentID + r.primaryEnvironmentID = envID + r.additions = append(r.additions, envID) + if previous.Defined() { + r.loggers.Infof("Environment ID %s was rotated, new environment ID is %s", r.primaryEnvironmentID, envID) + r.expirations = append(r.expirations, previous) + } else { + r.loggers.Infof("New environment ID is %s", envID) + } +} + +func (r *Rotator) updateMobileKey(mobileKey config.MobileKey) { + if mobileKey == r.MobileKey() { + return + } + r.mu.Lock() + defer r.mu.Unlock() + previous := r.primaryMobileKey + r.primaryMobileKey = mobileKey + r.additions = append(r.additions, mobileKey) + if previous.Defined() { + r.expirations = append(r.expirations, previous) + r.loggers.Infof("Mobile key %s was rotated, new primary mobile key is %s", previous.Masked(), mobileKey.Masked()) + } else { + r.loggers.Infof("New primary mobile key is %s", mobileKey.Masked()) + } +} + +func (r *Rotator) swapPrimaryKey(newKey config.SDKKey) config.SDKKey { + if newKey == r.primarySdkKey { + // There's no swap to be done, we already are using this as primary. + return "" + } + previous := r.primarySdkKey + r.primarySdkKey = newKey + r.additions = append(r.additions, newKey) + r.loggers.Infof("New primary SDK key is %s", newKey.Masked()) + + return previous +} +func (r *Rotator) updateSDKKey(sdkKey config.SDKKey, grace *GracePeriod) { + r.mu.Lock() + defer r.mu.Unlock() + + previous := r.swapPrimaryKey(sdkKey) + // Immediately revoke the previous SDK key if there's no explicit deprecation notice, otherwise it would + // hang around forever. + if previous.Defined() && grace == nil { + r.expirations = append(r.expirations, previous) + r.loggers.Infof("SDK key %s has been immediately revoked", previous.Masked()) + return + } + if grace != nil { + if previousExpiry, ok := r.deprecatedSdkKeys[grace.key]; ok { + if previousExpiry != grace.expiry { + r.loggers.Warnf("SDK key %s was marked for deprecation with an expiry at %v, but it was previously deprecated with an expiry at %v. The previous expiry will be used. ", grace.key.Masked(), grace.expiry, previousExpiry) + } + return + } + + if grace.now.After(grace.expiry) { + r.loggers.Infof("Deprecated SDK key %s already expired; ignoring", grace.key.Masked()) + return + } + + r.loggers.Infof("SDK key %s was marked for deprecation with an expiry at %v", grace.key.Masked(), grace.expiry) + r.deprecatedSdkKeys[grace.key] = grace.expiry + + if grace.key != previous { + r.loggers.Infof("Deprecated SDK key %s was not previously managed by Relay", grace.key.Masked()) + r.additions = append(r.additions, grace.key) + } + } +} + +func (r *Rotator) expireSDKKey(sdkKey config.SDKKey) { + r.loggers.Infof("Deprecated SDK key %s has expired and is no longer valid for authentication", sdkKey.Masked()) + delete(r.deprecatedSdkKeys, sdkKey) + r.expirations = append(r.expirations, sdkKey) +} + +// StepTime provides the current time to the Rotator, allowing it to compute the set of additions and expirations +// for the tracked credentials since the last time this method was called. +func (r *Rotator) StepTime(now time.Time) (additions []SDKCredential, expirations []SDKCredential) { + r.mu.Lock() + defer r.mu.Unlock() + + for key, expiry := range r.deprecatedSdkKeys { + if now.After(expiry) { + r.expireSDKKey(key) + } + } + + additions, expirations = r.additions, r.expirations + r.additions = nil + r.expirations = nil + return +} diff --git a/internal/credential/rotator_test.go b/internal/credential/rotator_test.go new file mode 100644 index 00000000..9acbc57f --- /dev/null +++ b/internal/credential/rotator_test.go @@ -0,0 +1,199 @@ +package credential + +import ( + "fmt" + "testing" + "time" + + "github.com/launchdarkly/go-sdk-common/v3/ldlogtest" + "github.com/launchdarkly/ld-relay/v8/config" + "github.com/stretchr/testify/assert" +) + +func TestNewRotator(t *testing.T) { + mockLog := ldlogtest.NewMockLog() + rotator := NewRotator(mockLog.Loggers) + assert.NotNil(t, rotator) +} + +func TestImmediateKeyExpiration(t *testing.T) { + kinds := []struct { + name string + keys []SDKCredential + getKey func(*Rotator) SDKCredential + }{ + { + name: "sdk keys", + keys: []SDKCredential{config.SDKKey("key1"), config.SDKKey("key2"), config.SDKKey("key3")}, + getKey: func(r *Rotator) SDKCredential { return r.SDKKey() }, + }, + { + name: "mobile keys", + keys: []SDKCredential{config.MobileKey("key1"), config.MobileKey("key2"), config.MobileKey("key3")}, + getKey: func(r *Rotator) SDKCredential { return r.MobileKey() }, + }, + { + name: "environment IDs", + keys: []SDKCredential{config.EnvironmentID("id1"), config.EnvironmentID("id2"), config.EnvironmentID("id3")}, + getKey: func(r *Rotator) SDKCredential { return r.EnvironmentID() }, + }, + } + + for _, c := range kinds { + t.Run(c.name, func(t *testing.T) { + mockLog := ldlogtest.NewMockLog() + rotator := NewRotator(mockLog.Loggers) + + // The first rotation shouldn't trigger any expirations because there was no previous key. + rotator.Rotate(c.keys[0]) + additions, _ := rotator.StepTime(time.Now()) + assert.ElementsMatch(t, c.keys[0:1], additions) + assert.Equal(t, c.keys[0], c.getKey(rotator)) + + // The second rotation should trigger a deprecation of key1. + rotator.Rotate(c.keys[1]) + additions, expirations := rotator.StepTime(time.Now()) + assert.ElementsMatch(t, c.keys[1:2], additions) + assert.ElementsMatch(t, c.keys[0:1], expirations) + assert.Equal(t, c.keys[1], c.getKey(rotator)) + + // The third rotation should trigger a deprecation of key2. + rotator.Rotate(c.keys[2]) + additions, expirations = rotator.StepTime(time.Now()) + assert.ElementsMatch(t, c.keys[2:3], additions) + assert.ElementsMatch(t, c.keys[1:2], expirations) + assert.Equal(t, c.keys[2], c.getKey(rotator)) + }) + } +} + +func TestManyImmediateKeyExpirations(t *testing.T) { + + kinds := []struct { + name string + makeKey func(string) SDKCredential + getKey func(*Rotator) SDKCredential + }{ + { + name: "sdk keys", + makeKey: func(s string) SDKCredential { return config.SDKKey(s) }, + getKey: func(r *Rotator) SDKCredential { return r.SDKKey() }, + }, + { + name: "mobile keys", + makeKey: func(s string) SDKCredential { return config.MobileKey(s) }, + getKey: func(r *Rotator) SDKCredential { return r.MobileKey() }, + }, + { + name: "environment IDs", + makeKey: func(s string) SDKCredential { return config.EnvironmentID(s) }, + getKey: func(r *Rotator) SDKCredential { return r.EnvironmentID() }, + }, + } + + for _, c := range kinds { + t.Run(c.name, func(t *testing.T) { + mockLog := ldlogtest.NewMockLog() + rotator := NewRotator(mockLog.Loggers) + + const numKeys = 100 + for i := 0; i < numKeys; i++ { + key := c.makeKey(fmt.Sprintf("key%v", i)) + rotator.Rotate(key) + } + + assert.Equal(t, c.makeKey(fmt.Sprintf("key%v", numKeys-1)), c.getKey(rotator)) + + additions, expirations := rotator.StepTime(time.Now()) + assert.Len(t, additions, numKeys) + assert.Len(t, expirations, numKeys-1) // because the last key is still active + }) + } +} + +func TestSDKKeyDeprecation(t *testing.T) { + mockLog := ldlogtest.NewMockLog() + rotator := NewRotator(mockLog.Loggers) + + const ( + key1 = config.SDKKey("key1") + key2 = config.SDKKey("key2") + ) + + start := time.Unix(10000, 0) + + halfTime := start.Add(30 * time.Second) + deprecationTime := start.Add(1 * time.Minute) + + rotator.Initialize([]SDKCredential{key1}) + + rotator.RotateWithGrace(key2, NewGracePeriod(key1, deprecationTime, halfTime)) + additions, expirations := rotator.StepTime(halfTime) + assert.ElementsMatch(t, []SDKCredential{key2}, additions) + assert.Empty(t, expirations) + + additions, expirations = rotator.StepTime(deprecationTime) + assert.Empty(t, additions) + assert.Empty(t, expirations) + + additions, expirations = rotator.StepTime(deprecationTime.Add(1 * time.Millisecond)) + assert.Empty(t, additions) + assert.ElementsMatch(t, []SDKCredential{key1}, expirations) +} + +func TestManyConcurrentSDKKeyDeprecation(t *testing.T) { + mockLog := ldlogtest.NewMockLog() + rotator := NewRotator(mockLog.Loggers) + + makeKey := func(i int) config.SDKKey { + return config.SDKKey(fmt.Sprintf("key%v", i)) + } + + rotator.Initialize([]SDKCredential{config.SDKKey("key0")}) + + const numKeys = 250 + now := time.Unix(10000, 0) + expiryTime := now.Add(1 * time.Hour) + + var keysDeprecated []SDKCredential + var keysAdded []SDKCredential + + for i := 0; i < numKeys; i++ { + previousKey := makeKey(i) + nextKey := makeKey(i + 1) + + keysDeprecated = append(keysDeprecated, previousKey) + keysAdded = append(keysAdded, nextKey) + + rotator.RotateWithGrace(nextKey, NewGracePeriod(previousKey, expiryTime, now)) + } + + // The last key added should be the current primary key. + assert.Equal(t, keysAdded[len(keysAdded)-1], rotator.SDKKey()) + + // Until and including the exact expiry timestamp, there should be no expirations. + additions, expirations := rotator.StepTime(expiryTime) + assert.ElementsMatch(t, keysAdded, additions) + assert.Empty(t, expirations) + + // One moment after the expiry time, we should now have a batch of expirations. + additions, expirations = rotator.StepTime(expiryTime.Add(1 * time.Millisecond)) + assert.Empty(t, additions) + assert.ElementsMatch(t, keysDeprecated, expirations) +} + +func TestSDKKeyExpiredInThePastIsNotAdded(t *testing.T) { + mockLog := ldlogtest.NewMockLog() + rotator := NewRotator(mockLog.Loggers) + + primaryKey := config.SDKKey("primary") + obsoleteKey := config.SDKKey("obsolete") + obsoleteExpiry := time.Unix(1000000, 0) + now := obsoleteExpiry.Add(1 * time.Hour) + + rotator.RotateWithGrace(primaryKey, NewGracePeriod(obsoleteKey, obsoleteExpiry, now)) + + additions, expirations := rotator.StepTime(now) + assert.ElementsMatch(t, []SDKCredential{primaryKey}, additions) + assert.Empty(t, expirations) +} diff --git a/internal/envfactory/env_params.go b/internal/envfactory/env_params.go index 318e62fa..d0230c3f 100644 --- a/internal/envfactory/env_params.go +++ b/internal/envfactory/env_params.go @@ -3,8 +3,6 @@ package envfactory import ( "time" - "github.com/launchdarkly/ld-relay/v8/internal/credential" - "github.com/launchdarkly/ld-relay/v8/config" "github.com/launchdarkly/ld-relay/v8/internal/relayenv" ) @@ -29,9 +27,8 @@ type EnvironmentParams struct { MobileKey config.MobileKey // ExpiringSDKKey is an additional SDK key that should also be allowed (but not surfaced as - // the canonical one), or "" if none. The expiry time is not represented here; it is managed - // by lower-level components. - ExpiringSDKKey config.SDKKey + // the canonical one). + ExpiringSDKKey ExpiringSDKKey // TTL is the cache TTL for PHP clients. TTL time.Duration @@ -40,12 +37,13 @@ type EnvironmentParams struct { SecureMode bool } -func (e EnvironmentParams) Credentials() credential.AutoConfig { - return credential.AutoConfig{ - SDKKey: e.SDKKey, - ExpiringSDKKey: e.ExpiringSDKKey, - MobileKey: e.MobileKey, - } +type ExpiringSDKKey struct { + Key config.SDKKey + Expiration time.Time +} + +func (e ExpiringSDKKey) Defined() bool { + return e.Key.Defined() } func (e EnvironmentParams) WithFilter(key config.FilterKey) EnvironmentParams { diff --git a/internal/envfactory/env_rep.go b/internal/envfactory/env_rep.go index a0d39aab..2c080c79 100644 --- a/internal/envfactory/env_rep.go +++ b/internal/envfactory/env_rep.go @@ -52,8 +52,8 @@ func (f FilterRep) ToTestParams() FilterParams { // SDKKeyRep describes an SDK key optionally accompanied by an old expiring key. type SDKKeyRep struct { - Value config.SDKKey `json:"value"` - Expiring ExpiringKeyRep + Value config.SDKKey `json:"value"` + Expiring ExpiringKeyRep `json:"expiring"` } // ExpiringKeyRep describes an old key that will expire at the specified date/time. @@ -62,9 +62,24 @@ type ExpiringKeyRep struct { Timestamp ldtime.UnixMillisecondTime `json:"timestamp"` } +func (e ExpiringKeyRep) ToParams() ExpiringSDKKey { + if e.Value.Defined() { + return ExpiringSDKKey{ + Key: e.Value, + Expiration: ToTime(e.Timestamp), + } + } else { + return ExpiringSDKKey{} + } +} + +func ToTime(millisecondTime ldtime.UnixMillisecondTime) time.Time { + return time.UnixMilli(int64(millisecondTime)) +} + // ToParams converts the JSON properties for an environment into our internal parameter type. func (r EnvironmentRep) ToParams() EnvironmentParams { - return EnvironmentParams{ + params := EnvironmentParams{ EnvID: r.EnvID, Identifiers: relayenv.EnvIdentifiers{ EnvKey: r.EnvKey, @@ -73,11 +88,13 @@ func (r EnvironmentRep) ToParams() EnvironmentParams { ProjName: r.ProjName, }, SDKKey: r.SDKKey.Value, + ExpiringSDKKey: r.SDKKey.Expiring.ToParams(), MobileKey: r.MobKey, - ExpiringSDKKey: r.SDKKey.Expiring.Value, TTL: time.Duration(r.DefaultTTL) * time.Minute, SecureMode: r.SecureMode, } + + return params } func (r EnvironmentRep) Describe() string { diff --git a/internal/envfactory/env_rep_test.go b/internal/envfactory/env_rep_test.go index 5df5687e..b46e7566 100644 --- a/internal/envfactory/env_rep_test.go +++ b/internal/envfactory/env_rep_test.go @@ -64,9 +64,12 @@ func TestEnvironmentRepToParams(t *testing.T) { ProjKey: "projkey2", ProjName: "projname2", }, - SDKKey: env2.SDKKey.Value, - ExpiringSDKKey: env2.SDKKey.Expiring.Value, - MobileKey: env2.MobKey, + SDKKey: env2.SDKKey.Value, + ExpiringSDKKey: ExpiringSDKKey{ + Key: env2.SDKKey.Expiring.Value, + Expiration: time.UnixMilli(int64(env2.SDKKey.Expiring.Timestamp)), + }, + MobileKey: env2.MobKey, }, params2) } diff --git a/internal/projmanager/environment_manager.go b/internal/projmanager/environment_manager.go index b02fcc4c..4529d592 100644 --- a/internal/projmanager/environment_manager.go +++ b/internal/projmanager/environment_manager.go @@ -12,7 +12,6 @@ type EnvironmentActions interface { AddEnvironment(params envfactory.EnvironmentParams) UpdateEnvironment(params envfactory.EnvironmentParams) DeleteEnvironment(id config.EnvironmentID, filter config.FilterKey) - KeyExpired(id config.EnvironmentID, filter config.FilterKey, oldKey config.SDKKey) } type filterMapping struct { @@ -132,13 +131,6 @@ func (e *EnvironmentManager) DeleteFilter(filter config.FilterID) bool { return true } -func (e *EnvironmentManager) KeyExpired(id config.EnvironmentID, oldKey config.SDKKey) { - e.handler.KeyExpired(id, config.DefaultFilter, oldKey) - for _, filter := range e.filtered { - e.handler.KeyExpired(id, filter.key, oldKey) - } -} - func (e *EnvironmentManager) Filters() []config.FilterKey { filters := make([]config.FilterKey, 0, len(e.filtered)) for _, filter := range e.filtered { diff --git a/internal/projmanager/environment_manager_test.go b/internal/projmanager/environment_manager_test.go index 12f28780..08253f86 100644 --- a/internal/projmanager/environment_manager_test.go +++ b/internal/projmanager/environment_manager_test.go @@ -426,34 +426,6 @@ func TestEnvironmentManager_SimpleFilterCombination(t *testing.T) { require.ElementsMatchf(t, out, []config.EnvironmentID{"a", "b", "a/foo", "a/bar", "b/foo", "b/bar"}, "default and filtered environments should be created") } -func TestEnvironmentManager_KeyExpired(t *testing.T) { - t.Run("key expiry is broadcast n times", func(t *testing.T) { - mockLog := ldlogtest.NewMockLog() - defer mockLog.DumpIfTestFailed(t) - mockLog.Loggers.SetMinLevel(ldlog.Debug) - - for i := 0; i < 10; i++ { - spy := newHandlerSpy() - m := NewEnvironmentManager("foo", spy, mockLog.Loggers) - - filters := makeFilters(i, []string{"foo"}) - expected := []expiredParams{{id: "foo", filter: config.DefaultFilter, key: "sdk-123"}} - for _, f := range filters { - expected = append(expected, expiredParams{ - id: "foo", - filter: f.Key, - key: "sdk-123", - }) - m.AddFilter(f) - } - - m.KeyExpired("foo", "sdk-123") - require.ElementsMatch(t, spy.expired, expected) - } - }) - -} - type command struct { op commandType value string diff --git a/internal/projmanager/project_router.go b/internal/projmanager/project_router.go index 75046deb..276cb278 100644 --- a/internal/projmanager/project_router.go +++ b/internal/projmanager/project_router.go @@ -41,15 +41,6 @@ func NewProjectRouter(handler AutoConfigActions, loggers ldlog.Loggers) *Project return &ProjectRouter{managers: make(map[string]*EnvironmentManager), actions: handler, loggers: loggers} } -// KeyExpired indicates that an SDK key, scoped to a particular project, has expired. The command -// is forwarded on to the manager, if any, for the given projKey. -func (e *ProjectRouter) KeyExpired(id config.EnvironmentID, projKey string, oldKey config.SDKKey) { - manager, ok := e.managers[projKey] - if ok { - manager.KeyExpired(id, oldKey) - } -} - // AddEnvironment routes the given EnvironmentParams to the relevant ProjectManager based on its project key, or instantiates // a new ProjectManager if one doesn't already exist. func (e *ProjectRouter) AddEnvironment(params envfactory.EnvironmentParams) { diff --git a/internal/relayenv/env_context.go b/internal/relayenv/env_context.go index d89b4e2b..beded3e2 100644 --- a/internal/relayenv/env_context.go +++ b/internal/relayenv/env_context.go @@ -20,6 +20,43 @@ import ( ldeval "github.com/launchdarkly/go-server-sdk-evaluation/v3" ) +// CredentialUpdate specifies the primary credential of a given credential kind for an environment. +// For example, an environment may have a primary SDK key and a primary mobile key at the same time; each would +// be specified in individual CredentialUpdate objects. +type CredentialUpdate struct { + // The new primary credential + primary credential.SDKCredential + // An optional deprecated credential (only SDK keys are supported currently) + deprecated config.SDKKey + // When the deprecated credential expires + expiry time.Time + // The current time + now time.Time +} + +// NewCredentialUpdate creates a CredentialUpdate from a given primary credential. +// The default behavior of the environment is to immediately revoke the previous credential of this kind. +func NewCredentialUpdate(primary credential.SDKCredential) *CredentialUpdate { + return &CredentialUpdate{primary: primary, now: time.Now()} +} + +// WithGracePeriod modifies the default behavior from immediate revocation to a delayed revocation of the previous +// credential. During the grace period, the previous credential continues to function. +func (c *CredentialUpdate) WithGracePeriod(deprecated config.SDKKey, expiry time.Time) *CredentialUpdate { + c.deprecated = deprecated + c.expiry = expiry + return c +} + +// WithTime overrides the update's current time for testing purposes. +// Because the environment's credential rotation algorithm compares the current time to the specific expiry of +// each credential, this can be used to trigger behavior in a more predictable way than relying on the actual time +// in the test. +func (c *CredentialUpdate) WithTime(t time.Time) *CredentialUpdate { + c.now = t + return c +} + // EnvContext is the interface for all Relay operations that are specific to one configured LD environment. // // The EnvContext is normally associated with an LDClient instance from the Go SDK, and allows direct access @@ -39,29 +76,16 @@ type EnvContext interface { // SetIdentifiers updates the environment and project names and keys. SetIdentifiers(EnvIdentifiers) + // UpdateCredential updates the environment with a new credential, optionally deprecating a previous one + // with a grace period. + UpdateCredential(update *CredentialUpdate) + // GetCredentials returns all currently enabled and non-deprecated credentials for the environment. GetCredentials() []credential.SDKCredential // GetDeprecatedCredentials returns all deprecated and not-yet-removed credentials for the environment. GetDeprecatedCredentials() []credential.SDKCredential - // AddCredential adds a new credential for the environment. - // - // If the credential is an SDK key, then a new SDK client is started with that SDK key, and event forwarding - // to server-side endpoints is switched to use the new key. - AddCredential(credential.SDKCredential) - - // RemoveCredential removes a credential from the environment. Any active stream connections using that - // credential are immediately dropped. - // - // If the credential is an SDK key, then the SDK client that we started with that SDK key is disposed of. - RemoveCredential(credential.SDKCredential) - - // DeprecateCredential marks an existing credential as not being a preferred one, without removing it or - // dropping any connections. It will no longer be included in the return value of GetCredentials(). This is - // used in Relay Proxy Enterprise when an SDK key is being changed but the old key has not expired yet. - DeprecateCredential(credential.SDKCredential) - // GetClient returns the SDK client instance for this environment. This is nil if initialization is not yet // complete. Rather than providing the full client object, we use the simpler sdks.LDClientContext which // includes only the operations Relay needs to do. @@ -83,7 +107,7 @@ type EnvContext interface { // have its own prefix string and, optionally, its own log level. GetLoggers() ldlog.Loggers - // GetHandler returns the HTTP handler for the specified kind of stream requests and credential for this + // GetStreamHandler returns the HTTP handler for the specified kind of stream requests and credential for this // environment. If there is none, it returns a handler for a 404 status (not nil). GetStreamHandler(streams.StreamProvider, credential.SDKCredential) http.Handler diff --git a/internal/relayenv/env_context_impl.go b/internal/relayenv/env_context_impl.go index b9debe3c..a5b271f6 100644 --- a/internal/relayenv/env_context_impl.go +++ b/internal/relayenv/env_context_impl.go @@ -45,6 +45,11 @@ const ( // ID. This is the default behavior for Relay Proxy Enterprise when running in auto-configuration mode, // where we always know the environment ID but the SDK key is subject to change. LogNameIsEnvID LogNameMode = true + + // By default, credentials that have an expiry date in the future (compared to when the message containing the + // expiry was received) will be cleaned up on an interval with this granularity. This means the environment won't accept + // connections for this credential, and it will shut down the SDK client associated with that credential. + defaultCredentialCleanupInterval = 1 * time.Minute ) func errInitPublisher(err error) error { @@ -55,56 +60,66 @@ func errInitMetrics(err error) error { return fmt.Errorf("failed to initialize metrics for environment: %w", err) } +type ConnectionMapper interface { + AddConnectionMapping(scopedCredential sdkauth.ScopedCredential, envContext EnvContext) + RemoveConnectionMapping(scopedCredential sdkauth.ScopedCredential) +} + // EnvContextImplParams contains the constructor parameters for NewEnvContextImpl. These have their // own type because there are a lot of them, and many are irrelevant in tests. type EnvContextImplParams struct { - Identifiers EnvIdentifiers - EnvConfig config.EnvConfig - AllConfig config.Config - ClientFactory sdks.ClientFactoryFunc - DataStoreFactory subsystems.ComponentConfigurer[subsystems.DataStore] - DataStoreInfo sdks.DataStoreEnvironmentInfo - StreamProviders []streams.StreamProvider - JSClientContext JSClientContext - MetricsManager *metrics.Manager - BigSegmentStoreFactory bigsegments.BigSegmentStoreFactory - BigSegmentSynchronizerFactory bigsegments.BigSegmentSynchronizerFactory - SDKBigSegmentsConfigFactory subsystems.ComponentConfigurer[subsystems.BigSegmentsConfiguration] // set only in tests - UserAgent string - LogNameMode LogNameMode - Loggers ldlog.Loggers + Identifiers EnvIdentifiers + EnvConfig config.EnvConfig + AllConfig config.Config + ClientFactory sdks.ClientFactoryFunc + DataStoreFactory subsystems.ComponentConfigurer[subsystems.DataStore] + DataStoreInfo sdks.DataStoreEnvironmentInfo + StreamProviders []streams.StreamProvider + JSClientContext JSClientContext + MetricsManager *metrics.Manager + BigSegmentStoreFactory bigsegments.BigSegmentStoreFactory + BigSegmentSynchronizerFactory bigsegments.BigSegmentSynchronizerFactory + SDKBigSegmentsConfigFactory subsystems.ComponentConfigurer[subsystems.BigSegmentsConfiguration] // set only in tests + UserAgent string + LogNameMode LogNameMode + Loggers ldlog.Loggers + ConnectionMapper ConnectionMapper + ExpiredCredentialCleanupInterval time.Duration } type envContextImpl struct { - mu sync.RWMutex - clients map[config.SDKKey]sdks.LDClientContext - storeAdapter *store.SSERelayDataStoreAdapter - loggers ldlog.Loggers - credentials map[credential.SDKCredential]bool // true if not deprecated - identifiers EnvIdentifiers - secureMode bool - envStreams *streams.EnvStreams - streamProviders []streams.StreamProvider - handlers map[streams.StreamProvider]map[credential.SDKCredential]http.Handler - jsContext JSClientContext - evaluator ldeval.Evaluator - eventDispatcher *events.EventDispatcher - bigSegmentSync bigsegments.BigSegmentSynchronizer - bigSegmentStore bigsegments.BigSegmentStore - bigSegmentsExist bool - sdkBigSegments *ldstoreimpl.BigSegmentStoreWrapper - sdkConfig ld.Config - sdkClientFactory sdks.ClientFactoryFunc - sdkInitTimeout time.Duration - metricsManager *metrics.Manager - metricsEnv *metrics.EnvironmentManager - metricsEventPub events.EventPublisher - dataStoreInfo sdks.DataStoreEnvironmentInfo - globalLoggers ldlog.Loggers - ttl time.Duration - initErr error - creationTime time.Time - filterKey config.FilterKey + mu sync.RWMutex + clients map[config.SDKKey]sdks.LDClientContext + storeAdapter *store.SSERelayDataStoreAdapter + loggers ldlog.Loggers + identifiers EnvIdentifiers + secureMode bool + envStreams *streams.EnvStreams + streamProviders []streams.StreamProvider + handlers map[streams.StreamProvider]map[credential.SDKCredential]http.Handler + jsContext JSClientContext + evaluator ldeval.Evaluator + eventDispatcher *events.EventDispatcher + bigSegmentSync bigsegments.BigSegmentSynchronizer + bigSegmentStore bigsegments.BigSegmentStore + bigSegmentsExist bool + sdkBigSegments *ldstoreimpl.BigSegmentStoreWrapper + sdkConfig ld.Config + sdkClientFactory sdks.ClientFactoryFunc + sdkInitTimeout time.Duration + metricsManager *metrics.Manager + metricsEnv *metrics.EnvironmentManager + metricsEventPub events.EventPublisher + dataStoreInfo sdks.DataStoreEnvironmentInfo + globalLoggers ldlog.Loggers + ttl time.Duration + initErr error + creationTime time.Time + filterKey config.FilterKey + keyRotator *credential.Rotator + stopMonitoringCredentials chan struct{} + doneMonitoringCredentials chan struct{} + connectionMapper ConnectionMapper } // Implementation of the DataStoreQueries interface that the streams package uses as an abstraction of @@ -155,33 +170,33 @@ func NewEnvContext( return nil, err } - credentials := make(map[credential.SDKCredential]bool, 3) - credentials[envConfig.SDKKey] = true - if envConfig.MobileKey.Defined() { - credentials[envConfig.MobileKey] = true - } - if envConfig.EnvID.Defined() { - credentials[envConfig.EnvID] = true - } - envContext := &envContextImpl{ - identifiers: params.Identifiers, - clients: make(map[config.SDKKey]sdks.LDClientContext), - credentials: credentials, - loggers: envLoggers, - secureMode: envConfig.SecureMode, - streamProviders: params.StreamProviders, - handlers: make(map[streams.StreamProvider]map[credential.SDKCredential]http.Handler), - jsContext: params.JSClientContext, - sdkClientFactory: params.ClientFactory, - sdkInitTimeout: allConfig.Main.InitTimeout.GetOrElse(config.DefaultInitTimeout), - metricsManager: params.MetricsManager, - globalLoggers: params.Loggers, - ttl: envConfig.TTL.GetOrElse(0), - dataStoreInfo: params.DataStoreInfo, - creationTime: time.Now(), - filterKey: params.EnvConfig.FilterKey, - } + identifiers: params.Identifiers, + clients: make(map[config.SDKKey]sdks.LDClientContext), + loggers: envLoggers, + secureMode: envConfig.SecureMode, + streamProviders: params.StreamProviders, + handlers: make(map[streams.StreamProvider]map[credential.SDKCredential]http.Handler), + jsContext: params.JSClientContext, + sdkClientFactory: params.ClientFactory, + sdkInitTimeout: allConfig.Main.InitTimeout.GetOrElse(config.DefaultInitTimeout), + metricsManager: params.MetricsManager, + globalLoggers: params.Loggers, + ttl: envConfig.TTL.GetOrElse(0), + dataStoreInfo: params.DataStoreInfo, + creationTime: time.Now(), + filterKey: params.EnvConfig.FilterKey, + keyRotator: credential.NewRotator(params.Loggers), + stopMonitoringCredentials: make(chan struct{}), + doneMonitoringCredentials: make(chan struct{}), + connectionMapper: params.ConnectionMapper, + } + + envContext.keyRotator.Initialize([]credential.SDKCredential{ + envConfig.SDKKey, + envConfig.MobileKey, + envConfig.EnvID, + }) bigSegmentStoreFactory := params.BigSegmentStoreFactory if bigSegmentStoreFactory == nil { @@ -243,12 +258,13 @@ func NewEnvContext( context: envContext, } - for c := range credentials { + allCreds := envContext.keyRotator.AllCredentials() + for _, c := range allCreds { envStreams.AddCredential(c) } for _, sp := range params.StreamProviders { handlers := make(map[credential.SDKCredential]http.Handler) - for c := range credentials { + for _, c := range allCreds { h := sp.Handler(sdkauth.NewScoped(envContext.filterKey, c)) if h != nil { handlers[c] = h @@ -377,11 +393,79 @@ func NewEnvContext( // Connecting may take time, so do this in parallel go envContext.startSDKClient(envConfig.SDKKey, readyCh, allConfig.Main.IgnoreConnectionErrors) + cleanupInterval := params.ExpiredCredentialCleanupInterval + if cleanupInterval == 0 { // 0 means it wasn't specified; the config system disallows 0 as a valid value. + cleanupInterval = defaultCredentialCleanupInterval + } + go envContext.cleanupExpiredCredentials(cleanupInterval) + thingsToCleanUp.Clear() // we've succeeded so we do not want to throw away these things return envContext, nil } +func (c *envContextImpl) cleanupExpiredCredentials(interval time.Duration) { + ticker := time.NewTicker(interval) + defer ticker.Stop() + for { + select { + case <-ticker.C: + c.triggerCredentialChanges(time.Now()) + case <-c.stopMonitoringCredentials: + close(c.doneMonitoringCredentials) + return + } + } +} + +func (c *envContextImpl) addCredential(newCredential credential.SDKCredential) { + c.mu.Lock() + defer c.mu.Unlock() + c.envStreams.AddCredential(newCredential) + for streamProvider, handlers := range c.handlers { + if h := streamProvider.Handler(sdkauth.NewScoped(c.filterKey, newCredential)); h != nil { + handlers[newCredential] = h + } + } + + // A new SDK key means 1. we should start a new SDK client, 2. we should tell all event forwarding + // components that use an SDK key to use the new one. A new mobile key does not require starting a + // new SDK client, but does requiring updating any event forwarding components that use a mobile key. + switch key := newCredential.(type) { + case config.SDKKey: + go c.startSDKClient(key, nil, false) + if c.metricsEventPub != nil { // metrics event publisher always uses SDK key + c.metricsEventPub.ReplaceCredential(key) + } + if c.eventDispatcher != nil { + c.eventDispatcher.ReplaceCredential(key) + } + case config.MobileKey: + if c.eventDispatcher != nil { + c.eventDispatcher.ReplaceCredential(key) + } + } + + c.connectionMapper.AddConnectionMapping(sdkauth.NewScoped(c.filterKey, newCredential), c) +} + +func (c *envContextImpl) removeCredential(oldCredential credential.SDKCredential) { + c.mu.Lock() + defer c.mu.Unlock() + c.connectionMapper.RemoveConnectionMapping(sdkauth.NewScoped(c.filterKey, oldCredential)) + c.envStreams.RemoveCredential(oldCredential) + for _, handlers := range c.handlers { + delete(handlers, oldCredential) + } + if sdkKey, ok := oldCredential.(config.SDKKey); ok { + // The SDK client instance is tied to the SDK key, so get rid of it + if client := c.clients[sdkKey]; client != nil { + delete(c.clients, sdkKey) + _ = client.Close() + } + } +} + func (c *envContextImpl) startSDKClient(sdkKey config.SDKKey, readyCh chan<- EnvContext, suppressErrors bool) { client, err := c.sdkClientFactory(sdkKey, c.sdkConfig, c.sdkInitTimeout) c.mu.Lock() @@ -421,7 +505,7 @@ func (c *envContextImpl) startSDKClient(sdkKey config.SDKKey, readyCh chan<- Env return } } else { - c.globalLoggers.Infof("Initialized LaunchDarkly client for %q", name) + c.globalLoggers.Infof("Initialized LaunchDarkly client for %q (SDK key %s)", name, sdkKey.Masked()) } if readyCh != nil { readyCh <- c @@ -446,98 +530,37 @@ func (c *envContextImpl) SetIdentifiers(ei EnvIdentifiers) { c.identifiers = ei } -func (c *envContextImpl) GetCredentials() []credential.SDKCredential { - return c.getCredentialsInternal(true) -} - -func (c *envContextImpl) GetDeprecatedCredentials() []credential.SDKCredential { - return c.getCredentialsInternal(false) -} - -func (c *envContextImpl) getCredentialsInternal(preferred bool) []credential.SDKCredential { - c.mu.RLock() - defer c.mu.RUnlock() - - ret := make([]credential.SDKCredential, 0, len(c.credentials)) - for c, nonDeprecated := range c.credentials { - if nonDeprecated == preferred { - ret = append(ret, c) - } +func (c *envContextImpl) UpdateCredential(update *CredentialUpdate) { + if !update.deprecated.Defined() { + c.keyRotator.Rotate(update.primary) + } else { + c.keyRotator.RotateWithGrace(update.primary, credential.NewGracePeriod(update.deprecated, update.expiry, update.now)) } - return ret + c.triggerCredentialChanges(update.now) } -func (c *envContextImpl) AddCredential(newCredential credential.SDKCredential) { - c.mu.Lock() - defer c.mu.Unlock() - if _, found := c.credentials[newCredential]; found { - return - } - c.credentials[newCredential] = true - c.envStreams.AddCredential(newCredential) - for streamProvider, handlers := range c.handlers { - if h := streamProvider.Handler(sdkauth.NewScoped(c.filterKey, newCredential)); h != nil { - handlers[newCredential] = h - } +func (c *envContextImpl) triggerCredentialChanges(now time.Time) { + additions, expirations := c.keyRotator.StepTime(now) + for _, cred := range additions { + c.addCredential(cred) } - - // A new SDK key means 1. we should start a new SDK client, 2. we should tell all event forwarding - // components that use an SDK key to use the new one. A new mobile key does not require starting a - // new SDK client, but does requiring updating any event forwarding components that use a mobile key. - switch key := newCredential.(type) { - case config.SDKKey: - go c.startSDKClient(key, nil, false) - if c.metricsEventPub != nil { // metrics event publisher always uses SDK key - c.metricsEventPub.ReplaceCredential(key) - } - if c.eventDispatcher != nil { - c.eventDispatcher.ReplaceCredential(key) - } - case config.MobileKey: - if c.eventDispatcher != nil { - c.eventDispatcher.ReplaceCredential(key) - } + for _, cred := range expirations { + c.removeCredential(cred) } } -func (c *envContextImpl) RemoveCredential(oldCredential credential.SDKCredential) { - c.mu.Lock() - defer c.mu.Unlock() - if _, found := c.credentials[oldCredential]; found { - delete(c.credentials, oldCredential) - c.envStreams.RemoveCredential(oldCredential) - for _, handlers := range c.handlers { - delete(handlers, oldCredential) - } - if sdkKey, ok := oldCredential.(config.SDKKey); ok { - // The SDK client instance is tied to the SDK key, so get rid of it - if client := c.clients[sdkKey]; client != nil { - delete(c.clients, sdkKey) - _ = client.Close() - } - } - } +func (c *envContextImpl) GetCredentials() []credential.SDKCredential { + return c.keyRotator.PrimaryCredentials() } -func (c *envContextImpl) DeprecateCredential(credential credential.SDKCredential) { - c.mu.Lock() - defer c.mu.Unlock() - if _, found := c.credentials[credential]; found { - c.credentials[credential] = false - } +func (c *envContextImpl) GetDeprecatedCredentials() []credential.SDKCredential { + return c.keyRotator.DeprecatedCredentials() } func (c *envContextImpl) GetClient() sdks.LDClientContext { c.mu.RLock() defer c.mu.RUnlock() - // There might be multiple clients if there's an expiring SDK key. Find the SDK key that has a true - // value in our map (meaning it's not deprecated) and return that client. - for cred, valid := range c.credentials { - if sdkKey, ok := cred.(config.SDKKey); ok && valid { - return c.clients[sdkKey] - } - } - return nil + return c.clients[c.keyRotator.SDKKey()] } func (c *envContextImpl) GetStore() subsystems.DataStore { @@ -659,7 +682,12 @@ func (c *envContextImpl) Close() error { } c.clients = make(map[config.SDKKey]sdks.LDClientContext) c.mu.Unlock() + + close(c.stopMonitoringCredentials) + <-c.doneMonitoringCredentials + _ = c.envStreams.Close() + if c.metricsManager != nil && c.metricsEnv != nil { c.metricsManager.RemoveEnvironment(c.metricsEnv) } diff --git a/internal/relayenv/env_context_impl_test.go b/internal/relayenv/env_context_impl_test.go index e285eb00..ed142779 100644 --- a/internal/relayenv/env_context_impl_test.go +++ b/internal/relayenv/env_context_impl_test.go @@ -11,6 +11,8 @@ import ( "testing" "time" + "github.com/launchdarkly/ld-relay/v8/internal/sdkauth" + "github.com/launchdarkly/ld-relay/v8/internal/credential" "github.com/launchdarkly/eventsource" @@ -57,15 +59,26 @@ func requireClientReady(t *testing.T, clientCh chan *testclient.FakeLDClient) *t func makeBasicEnv(t *testing.T, envConfig config.EnvConfig, clientFactory sdks.ClientFactoryFunc, loggers ldlog.Loggers, readyCh chan EnvContext) EnvContext { env, err := NewEnvContext(EnvContextImplParams{ - Identifiers: EnvIdentifiers{ConfiguredName: envName}, - EnvConfig: envConfig, - ClientFactory: clientFactory, - Loggers: loggers, + Identifiers: EnvIdentifiers{ConfiguredName: envName}, + EnvConfig: envConfig, + ClientFactory: clientFactory, + Loggers: loggers, + ConnectionMapper: mockConnectionMapper{}, }, readyCh) require.NoError(t, err) return env } +type mockConnectionMapper struct { +} + +func (m mockConnectionMapper) AddConnectionMapping(scopedCredential sdkauth.ScopedCredential, envContext EnvContext) { + +} +func (m mockConnectionMapper) RemoveConnectionMapping(scopedCredential sdkauth.ScopedCredential) { + +} + func TestConstructorBasicProperties(t *testing.T) { envConfig := st.EnvWithAllCredentials.Config envConfig.TTL = configtypes.NewOptDuration(time.Hour) @@ -175,8 +188,8 @@ func TestAddRemoveCredential(t *testing.T) { assert.Equal(t, []credential.SDKCredential{envConfig.SDKKey}, env.GetCredentials()) - env.AddCredential(st.EnvWithAllCredentials.Config.MobileKey) - env.AddCredential(st.EnvWithAllCredentials.Config.EnvID) + env.UpdateCredential(NewCredentialUpdate(st.EnvWithAllCredentials.Config.MobileKey)) + env.UpdateCredential(NewCredentialUpdate(st.EnvWithAllCredentials.Config.EnvID)) creds := env.GetCredentials() assert.Len(t, creds, 3) @@ -184,11 +197,12 @@ func TestAddRemoveCredential(t *testing.T) { assert.Contains(t, creds, st.EnvWithAllCredentials.Config.MobileKey) assert.Contains(t, creds, st.EnvWithAllCredentials.Config.EnvID) - env.RemoveCredential(st.EnvWithAllCredentials.Config.MobileKey) + env.UpdateCredential(NewCredentialUpdate(config.MobileKey("evict-the-previous-key"))) creds = env.GetCredentials() - assert.Len(t, creds, 2) + assert.Len(t, creds, 3) assert.Contains(t, creds, envConfig.SDKKey) + assert.NotContains(t, creds, st.EnvWithAllCredentials.Config.MobileKey) assert.Contains(t, creds, st.EnvWithAllCredentials.Config.EnvID) } @@ -203,14 +217,14 @@ func TestAddExistingCredentialDoesNothing(t *testing.T) { assert.Equal(t, []credential.SDKCredential{envConfig.SDKKey}, env.GetCredentials()) - env.AddCredential(st.EnvWithAllCredentials.Config.MobileKey) + env.UpdateCredential(NewCredentialUpdate(st.EnvWithAllCredentials.Config.MobileKey)) creds := env.GetCredentials() assert.Len(t, creds, 2) assert.Contains(t, creds, envConfig.SDKKey) assert.Contains(t, creds, st.EnvWithAllCredentials.Config.MobileKey) - env.AddCredential(st.EnvWithAllCredentials.Config.MobileKey) + env.UpdateCredential(NewCredentialUpdate(st.EnvWithAllCredentials.Config.MobileKey)) creds = env.GetCredentials() assert.Len(t, creds, 2) @@ -221,7 +235,7 @@ func TestAddExistingCredentialDoesNothing(t *testing.T) { func TestChangeSDKKey(t *testing.T) { envConfig := st.EnvMain.Config readyCh := make(chan EnvContext, 1) - newKey := config.SDKKey("new-key") + key2 := config.SDKKey("key2") clientCh := make(chan *testclient.FakeLDClient, 1) clientFactory := testclient.FakeLDClientFactoryWithChannel(true, clientCh) @@ -237,24 +251,48 @@ func TestChangeSDKKey(t *testing.T) { assert.Equal(t, env.GetClient(), client1) assert.Nil(t, env.GetInitError()) - env.AddCredential(newKey) - env.DeprecateCredential(envConfig.SDKKey) + // The environment should have been initialized with a single SDK key (found in the envConfig.) + // At this point, there's no deprecated credentials. + assert.Equal(t, []credential.SDKCredential{envConfig.SDKKey}, env.GetCredentials()) + assert.Empty(t, env.GetDeprecatedCredentials()) + + // For the purposes of key rotation, we'll make time deterministic. + start := time.Unix(1000, 0) - assert.Equal(t, []credential.SDKCredential{newKey}, env.GetCredentials()) + // Upon rotating to key2, the original key should still be valid for a hour. + env.UpdateCredential( + NewCredentialUpdate(key2). + WithTime(start). + WithGracePeriod(envConfig.SDKKey, start.Add(1*time.Hour))) + + assert.Equal(t, []credential.SDKCredential{key2}, env.GetCredentials()) + assert.Equal(t, []credential.SDKCredential{envConfig.SDKKey}, env.GetDeprecatedCredentials()) client2 := requireClientReady(t, clientCh) assert.NotEqual(t, client1, client2) assert.Equal(t, env.GetClient(), client2) - if !helpers.AssertNoMoreValues(t, client1.CloseCh, time.Millisecond*20, "client for deprecated key should not have been closed") { + // The client for the original SDK key should not have been closed, since it's valid for an hour. + if !helpers.AssertChannelNotClosed(t, client1.CloseCh, 1*time.Second, "client for envConfig.SDKKey should not have been closed yet") { t.FailNow() } - env.RemoveCredential(envConfig.SDKKey) + // Simulate an amount of time passing that is less than the deprecation period. The original key should still be valid. + env.UpdateCredential(NewCredentialUpdate(key2).WithTime(start.Add(45 * time.Minute))) + if !helpers.AssertChannelNotClosed(t, client1.CloseCh, 1*time.Second, "client for envConfig.SDKKey should not have been closed yet") { + t.FailNow() + } + + // We are now an instant after the deprecation period. This should cause the original key to become expired + // and trigger the client to close. + env.UpdateCredential(NewCredentialUpdate(key2).WithTime(start.Add(1*time.Hour + 1*time.Millisecond))) + assert.Equal(t, []credential.SDKCredential{key2}, env.GetCredentials()) + assert.Empty(t, env.GetDeprecatedCredentials()) - assert.Equal(t, []credential.SDKCredential{newKey}, env.GetCredentials()) + if !helpers.AssertChannelClosed(t, client1.CloseCh, 1*time.Second, "client for envConfig.SDKKey should have been closed") { + t.FailNow() + } - client1.AwaitClose(t, time.Millisecond*20) } func TestSDKClientCreationFails(t *testing.T) { diff --git a/internal/sharedtest/testdata_envs.go b/internal/sharedtest/testdata_envs.go index fee23a66..adc81600 100644 --- a/internal/sharedtest/testdata_envs.go +++ b/internal/sharedtest/testdata_envs.go @@ -3,8 +3,6 @@ package sharedtest import ( "time" - "github.com/launchdarkly/ld-relay/v8/internal/credential" - "github.com/launchdarkly/ld-relay/v8/config" ct "github.com/launchdarkly/go-configtypes" @@ -24,10 +22,6 @@ type TestEnv struct { type UnsupportedSDKCredential struct{} // implements credential.SDKCredential -func (k UnsupportedSDKCredential) Compare(_ credential.AutoConfig) (credential.SDKCredential, credential.Status) { - return nil, credential.Unchanged -} - func (k UnsupportedSDKCredential) GetAuthorizationHeaderValue() string { return "" } func (k UnsupportedSDKCredential) Defined() bool { @@ -38,6 +32,8 @@ func (k UnsupportedSDKCredential) String() string { return "unsupported" } +func (k UnsupportedSDKCredential) Masked() string { return "unsupported" } + const ( // The "undefined" values are well-formed, but do not match any environment in our test data. UndefinedSDKKey = config.SDKKey("sdk-99999999-9999-4999-8999-999999999999") diff --git a/relay/autoconfig_actions.go b/relay/autoconfig_actions.go index 97b8d1d8..9588e8cf 100644 --- a/relay/autoconfig_actions.go +++ b/relay/autoconfig_actions.go @@ -2,8 +2,8 @@ package relay import ( "github.com/launchdarkly/ld-relay/v8/config" - "github.com/launchdarkly/ld-relay/v8/internal/credential" "github.com/launchdarkly/ld-relay/v8/internal/envfactory" + "github.com/launchdarkly/ld-relay/v8/internal/relayenv" "github.com/launchdarkly/ld-relay/v8/internal/sdkauth" ) @@ -34,11 +34,8 @@ func (a *relayAutoConfigActions) AddEnvironment(params envfactory.EnvironmentPar } if params.ExpiringSDKKey.Defined() { - if _, err := a.r.getEnvironment(sdkauth.NewScoped(params.Identifiers.FilterKey, params.ExpiringSDKKey)); err != nil { - env.AddCredential(params.ExpiringSDKKey) - env.DeprecateCredential(params.ExpiringSDKKey) - a.r.addConnectionMapping(sdkauth.NewScoped(params.Identifiers.FilterKey, params.ExpiringSDKKey), env) - } + update := relayenv.NewCredentialUpdate(params.SDKKey) + env.UpdateCredential(update.WithGracePeriod(params.ExpiringSDKKey.Key, params.ExpiringSDKKey.Expiration)) } } @@ -53,24 +50,15 @@ func (a *relayAutoConfigActions) UpdateEnvironment(params envfactory.Environment env.SetTTL(params.TTL) env.SetSecureMode(params.SecureMode) - newCredentials := params.Credentials() - - for _, prevCred := range env.GetCredentials() { - newCred, status := prevCred.Compare(newCredentials) - if status == credential.Unchanged { - continue - } - - env.AddCredential(newCred) - a.r.addConnectionMapping(sdkauth.NewScoped(params.Identifiers.FilterKey, newCred), env) - - switch status { - case credential.Deprecated: - env.DeprecateCredential(prevCred) - case credential.Expired: - a.r.removeConnectionMapping(sdkauth.NewScoped(params.Identifiers.FilterKey, prevCred)) - env.RemoveCredential(prevCred) + if params.MobileKey.Defined() { + env.UpdateCredential(relayenv.NewCredentialUpdate(params.MobileKey)) + } + if params.SDKKey.Defined() { + update := relayenv.NewCredentialUpdate(params.SDKKey) + if params.ExpiringSDKKey.Defined() { + update = update.WithGracePeriod(params.ExpiringSDKKey.Key, params.ExpiringSDKKey.Expiration) } + env.UpdateCredential(update) } } @@ -85,13 +73,3 @@ func (a *relayAutoConfigActions) ReceivedAllEnvironments() { a.r.loggers.Info(logMsgAutoConfReceivedAllEnvironments) a.r.setFullyConfigured(true) } - -func (a *relayAutoConfigActions) KeyExpired(id config.EnvironmentID, filter config.FilterKey, oldKey config.SDKKey) { - env, err := a.r.getEnvironment(sdkauth.NewScoped(filter, id)) - if err != nil { - a.r.loggers.Warnf(logMsgKeyExpiryUnknownEnv, id) - return - } - a.r.removeConnectionMapping(sdkauth.NewScoped(filter, oldKey)) - env.RemoveCredential(oldKey) -} diff --git a/relay/autoconfig_actions_test.go b/relay/autoconfig_actions_test.go index 87a8c137..c957cf1a 100644 --- a/relay/autoconfig_actions_test.go +++ b/relay/autoconfig_actions_test.go @@ -5,6 +5,8 @@ import ( "testing" "time" + "github.com/launchdarkly/ld-relay/v8/internal/envfactory" + c "github.com/launchdarkly/ld-relay/v8/config" "github.com/launchdarkly/ld-relay/v8/internal/sharedtest/testclient" @@ -70,6 +72,10 @@ func autoConfTest( config.Events.EventsURI, _ = configtypes.NewOptURLAbsoluteFromString(eventsServer.URL) config.Events.FlushInterval = configtypes.NewOptDuration(time.Millisecond * 10) + // In tests involving adding/removing credentials, allow Relay to clean up credentials quickly so as not + // to take more time than necessary to verify the test conditions. + config.Main.ExpiredCredentialCleanupInterval = configtypes.NewOptDuration(time.Millisecond * 100) + relay, err := newRelayInternal(config, relayInternalOptions{ loggers: mockLog.Loggers, clientFactory: testclient.FakeLDClientFactoryWithChannel(true, clientsCreatedCh), @@ -87,7 +93,7 @@ func autoConfTest( } func (p autoConfTestParams) awaitClient() *testclient.FakeLDClient { - return helpers.RequireValue(p.t, p.clientsCreatedCh, time.Second, "timed out waiting for client creation") + return helpers.RequireValue(p.t, p.clientsCreatedCh, 1000*time.Second, "timed out waiting for client creation") } func (p autoConfTestParams) shouldNotCreateClient(timeout time.Duration) { @@ -101,11 +107,11 @@ func TestAutoConfigInit(t *testing.T) { autoConfTest(t, testAutoConfDefaultConfig, &initialEvent, func(p autoConfTestParams) { client1 := p.awaitClient() client2 := p.awaitClient() - if client1.Key == testAutoConfEnv2.sdkKey { + if client1.Key == testAutoConfEnv2.SDKKey() { client1, client2 = client2, client1 } - assert.Equal(t, testAutoConfEnv1.sdkKey, client1.Key) - assert.Equal(t, testAutoConfEnv2.sdkKey, client2.Key) + assert.Equal(t, testAutoConfEnv1.SDKKey(), client1.Key) + assert.Equal(t, testAutoConfEnv2.SDKKey(), client2.Key) env1 := p.awaitEnvironment(testAutoConfEnv1.id) assertEnvProps(t, testAutoConfEnv1.params(), env1) @@ -121,9 +127,13 @@ func TestAutoConfigInitWithExpiringSDKKey(t *testing.T) { newKey := c.SDKKey("newsdkkey") oldKey := c.SDKKey("oldsdkkey") envWithKeys := testAutoConfEnv1 - envWithKeys.sdkKey = newKey - envWithKeys.sdkKeyExpiryValue = oldKey - envWithKeys.sdkKeyExpiryTime = ldtime.UnixMillisNow() + 100000 + envWithKeys.sdkKey = envfactory.SDKKeyRep{ + Value: newKey, + Expiring: envfactory.ExpiringKeyRep{ + Value: oldKey, + Timestamp: ldtime.UnixMillisNow() + 100000, + }, + } initialEvent := makeAutoConfPutEvent(envWithKeys) autoConfTest(t, testAutoConfDefaultConfig, &initialEvent, func(p autoConfTestParams) { client1 := p.awaitClient() @@ -148,7 +158,7 @@ func TestAutoConfigInitAfterPreviousInitCanAddAndRemoveEnvs(t *testing.T) { initialEvent := makeAutoConfPutEvent(testAutoConfEnv1) autoConfTest(t, testAutoConfDefaultConfig, &initialEvent, func(p autoConfTestParams) { client1 := p.awaitClient() - assert.Equal(t, testAutoConfEnv1.sdkKey, client1.Key) + assert.Equal(t, testAutoConfEnv1.SDKKey(), client1.Key) env1 := p.awaitEnvironment(testAutoConfEnv1.id) assertEnvProps(t, testAutoConfEnv1.params(), env1) @@ -157,7 +167,7 @@ func TestAutoConfigInitAfterPreviousInitCanAddAndRemoveEnvs(t *testing.T) { p.stream.Enqueue(makeAutoConfPutEvent(testAutoConfEnv2)) client2 := p.awaitClient() - assert.Equal(t, testAutoConfEnv2.sdkKey, client2.Key) + assert.Equal(t, testAutoConfEnv2.SDKKey(), client2.Key) env2 := p.awaitEnvironment(testAutoConfEnv2.id) assertEnvProps(t, testAutoConfEnv2.params(), env2) @@ -168,7 +178,7 @@ func TestAutoConfigInitAfterPreviousInitCanAddAndRemoveEnvs(t *testing.T) { p.shouldNotHaveEnvironment(testAutoConfEnv1.id, time.Millisecond*100) p.assertSDKEndpointsAvailability( false, - testAutoConfEnv1.sdkKey, + testAutoConfEnv1.SDKKey(), testAutoConfEnv1.mobKey, testAutoConfEnv1.id, ) @@ -179,7 +189,7 @@ func TestAutoConfigAddEnvironment(t *testing.T) { initialEvent := makeAutoConfPutEvent(testAutoConfEnv1) autoConfTest(t, testAutoConfDefaultConfig, &initialEvent, func(p autoConfTestParams) { client1 := p.awaitClient() - assert.Equal(t, testAutoConfEnv1.sdkKey, client1.Key) + assert.Equal(t, testAutoConfEnv1.SDKKey(), client1.Key) env1 := p.awaitEnvironment(testAutoConfEnv1.id) assertEnvProps(t, testAutoConfEnv1.params(), env1) @@ -187,7 +197,7 @@ func TestAutoConfigAddEnvironment(t *testing.T) { p.stream.Enqueue(makeAutoConfPatchEvent(testAutoConfEnv2)) client2 := p.awaitClient() - assert.Equal(t, testAutoConfEnv2.sdkKey, client2.Key) + assert.Equal(t, testAutoConfEnv2.SDKKey(), client2.Key) env2 := p.awaitEnvironment(testAutoConfEnv2.id) p.assertEnvLookup(env2, testAutoConfEnv2.params()) @@ -199,10 +209,13 @@ func TestAutoConfigAddEnvironmentWithExpiringSDKKey(t *testing.T) { newKey := c.SDKKey("newsdkkey") oldKey := c.SDKKey("oldsdkkey") envWithKeys := testAutoConfEnv1 - envWithKeys.sdkKey = newKey - envWithKeys.sdkKeyExpiryValue = oldKey - envWithKeys.sdkKeyExpiryTime = ldtime.UnixMillisNow() + 100000 - + envWithKeys.sdkKey = envfactory.SDKKeyRep{ + Value: newKey, + Expiring: envfactory.ExpiringKeyRep{ + Value: oldKey, + Timestamp: ldtime.UnixMillisNow() + 100000, + }, + } initialEvent := makeAutoConfPutEvent() autoConfTest(t, testAutoConfDefaultConfig, &initialEvent, func(p autoConfTestParams) { p.stream.Enqueue(makeAutoConfPatchEvent(envWithKeys)) @@ -218,7 +231,7 @@ func TestAutoConfigAddEnvironmentWithExpiringSDKKey(t *testing.T) { env := p.awaitEnvironment(envWithKeys.id) assertEnvProps(t, envWithKeys.params(), env) - expectedCredentials := credentialsAsSet(envWithKeys.id, envWithKeys.mobKey, envWithKeys.sdkKey) + expectedCredentials := credentialsAsSet(envWithKeys.id, envWithKeys.mobKey, envWithKeys.SDKKey()) assert.Equal(t, expectedCredentials, credentialsAsSet(env.GetCredentials()...)) paramsWithOldKey := envWithKeys.params() @@ -260,7 +273,7 @@ func TestAutoConfigDeleteEnvironment(t *testing.T) { autoConfTest(t, testAutoConfDefaultConfig, &initialEvent, func(p autoConfTestParams) { client1 := p.awaitClient() client2 := p.awaitClient() - if client1.Key == testAutoConfEnv2.sdkKey { + if client1.Key == testAutoConfEnv2.SDKKey() { client1, client2 = client2, client1 } @@ -277,7 +290,7 @@ func TestAutoConfigDeleteEnvironment(t *testing.T) { p.shouldNotHaveEnvironment(testAutoConfEnv1.id, time.Millisecond*100) p.assertSDKEndpointsAvailability( false, - testAutoConfEnv1.sdkKey, + testAutoConfEnv1.SDKKey(), testAutoConfEnv1.mobKey, testAutoConfEnv1.id, ) diff --git a/relay/autoconfig_key_change_test.go b/relay/autoconfig_key_change_test.go index a81409a7..4ca0eb4a 100644 --- a/relay/autoconfig_key_change_test.go +++ b/relay/autoconfig_key_change_test.go @@ -5,6 +5,8 @@ import ( "testing" "time" + "github.com/launchdarkly/ld-relay/v8/internal/envfactory" + "github.com/launchdarkly/ld-relay/v8/internal/sdkauth" "github.com/launchdarkly/ld-relay/v8/internal/credential" @@ -26,7 +28,7 @@ const ( ) func makeEnvWithModifiedSDKKey(e testAutoConfEnv) testAutoConfEnv { - e.sdkKey += "-changed" + e.sdkKey.Value += "-changed" e.version++ return e } @@ -87,12 +89,12 @@ func TestAutoConfigUpdateEnvironmentSDKKeyWithNoExpiry(t *testing.T) { p.stream.Enqueue(makeAutoConfPatchEvent(modified)) client2 := p.awaitClient() - assert.Equal(t, modified.sdkKey, client2.Key) + assert.Equal(t, modified.SDKKey(), client2.Key) - client1.AwaitClose(t, time.Second) + client1.AwaitClose(t, 10000*time.Second) p.awaitCredentialsUpdated(env, modified.params()) - noEnv, _ := p.relay.getEnvironment(sdkauth.New(testAutoConfEnv1.sdkKey)) + noEnv, _ := p.relay.getEnvironment(sdkauth.New(testAutoConfEnv1.SDKKey())) assert.Nil(t, noEnv) }) } @@ -106,16 +108,18 @@ func TestAutoConfigUpdateEnvironmentSDKKeyWithExpiry(t *testing.T) { assertEnvProps(t, testAutoConfEnv1.params(), env) modified := makeEnvWithModifiedSDKKey(testAutoConfEnv1) - modified.sdkKeyExpiryValue = testAutoConfEnv1.sdkKey - modified.sdkKeyExpiryTime = ldtime.UnixMillisNow() + 100000 + modified.sdkKey.Expiring = envfactory.ExpiringKeyRep{ + Value: testAutoConfEnv1.SDKKey(), + Timestamp: ldtime.UnixMillisNow() + 100000, + } p.stream.Enqueue(makeAutoConfPatchEvent(modified)) client2 := p.awaitClient() - assert.Equal(t, modified.sdkKey, client2.Key) + assert.Equal(t, modified.SDKKey(), client2.Key) p.awaitCredentialsUpdated(env, modified.params()) p.assertEnvLookup(env, testAutoConfEnv1.params()) // looking up env by old key still works - assert.Equal(t, []credential.SDKCredential{testAutoConfEnv1.sdkKey}, env.GetDeprecatedCredentials()) + assert.Equal(t, []credential.SDKCredential{testAutoConfEnv1.sdkKey.Value}, env.GetDeprecatedCredentials()) if !helpers.AssertChannelNotClosed(t, client1.CloseCh, time.Millisecond*300, "should not have closed client for deprecated key yet") { t.FailNow() @@ -139,7 +143,7 @@ func TestEventForwardingAfterSDKKeyChange(t *testing.T) { p.awaitCredentialsUpdated(env, modified.params()) - verifyEventProxying(t, p, serverSideEventsURL, modified.sdkKey) + verifyEventProxying(t, p, serverSideEventsURL, modified.SDKKey()) verifyEventProxying(t, p, mobileEventsURL, testAutoConfEnv1.mobKey) verifyEventProxying(t, p, jsEventsURL+string(testAutoConfEnv1.id), testAutoConfEnv1.id) }) @@ -150,7 +154,7 @@ func TestEventForwardingAfterSDKKeyChange(t *testing.T) { env := p.awaitEnvironment(testAutoConfEnv1.id) assertEnvProps(t, testAutoConfEnv1.params(), env) - verifyEventProxying(t, p, serverSideEventsURL, testAutoConfEnv1.sdkKey) + verifyEventProxying(t, p, serverSideEventsURL, testAutoConfEnv1.sdkKey.Value) verifyEventProxying(t, p, mobileEventsURL, testAutoConfEnv1.mobKey) modified := makeEnvWithModifiedSDKKey(testAutoConfEnv1) @@ -158,7 +162,7 @@ func TestEventForwardingAfterSDKKeyChange(t *testing.T) { p.awaitCredentialsUpdated(env, modified.params()) - verifyEventProxying(t, p, serverSideEventsURL, modified.sdkKey) + verifyEventProxying(t, p, serverSideEventsURL, modified.sdkKey.Value) verifyEventProxying(t, p, mobileEventsURL, testAutoConfEnv1.mobKey) verifyEventProxying(t, p, jsEventsURL+string(testAutoConfEnv1.id), testAutoConfEnv1.id) }) @@ -167,7 +171,7 @@ func TestEventForwardingAfterSDKKeyChange(t *testing.T) { func TestAutoConfigRemovesCredentialForExpiredSDKKey(t *testing.T) { briefExpiryMillis := 300 - oldKey := testAutoConfEnv1.sdkKey + oldKey := testAutoConfEnv1.sdkKey.Value initialEvent := makeAutoConfPutEvent(testAutoConfEnv1) @@ -178,21 +182,21 @@ func TestAutoConfigRemovesCredentialForExpiredSDKKey(t *testing.T) { assertEnvProps(t, testAutoConfEnv1.params(), env) modified := makeEnvWithModifiedSDKKey(testAutoConfEnv1) - modified.sdkKeyExpiryValue = oldKey - modified.sdkKeyExpiryTime = ldtime.UnixMillisNow() + ldtime.UnixMillisecondTime(briefExpiryMillis) + modified.sdkKey.Expiring = envfactory.ExpiringKeyRep{ + Value: oldKey, + Timestamp: ldtime.UnixMillisNow() + ldtime.UnixMillisecondTime(briefExpiryMillis), + } p.stream.Enqueue(makeAutoConfPatchEvent(modified)) client2 := p.awaitClient() - assert.Equal(t, modified.sdkKey, client2.Key) + assert.Equal(t, modified.SDKKey(), client2.Key) p.awaitCredentialsUpdated(env, modified.params()) newCredentials := credentialsAsSet(env.GetCredentials()...) foundEnvWithOldKey, _ := p.relay.getEnvironment(sdkauth.New(oldKey)) assert.Equal(t, env, foundEnvWithOldKey) - <-time.After(time.Duration(briefExpiryMillis+100) * time.Millisecond) - - if !helpers.AssertChannelClosed(t, client1.CloseCh, time.Millisecond*300, "timed out waiting for client with old key to close") { + if !helpers.AssertChannelClosed(t, client1.CloseCh, time.Duration(briefExpiryMillis+100)*time.Millisecond, "timed out waiting for client with old key to close") { t.FailNow() } @@ -234,7 +238,7 @@ func TestEventForwardingAfterMobileKeyChange(t *testing.T) { p.awaitCredentialsUpdated(env, modified.params()) - verifyEventProxying(t, p, serverSideEventsURL, testAutoConfEnv1.sdkKey) + verifyEventProxying(t, p, serverSideEventsURL, testAutoConfEnv1.sdkKey.Value) verifyEventProxying(t, p, mobileEventsURL, modified.mobKey) verifyEventProxying(t, p, jsEventsURL+string(testAutoConfEnv1.id), testAutoConfEnv1.id) }) @@ -245,7 +249,7 @@ func TestEventForwardingAfterMobileKeyChange(t *testing.T) { env := p.awaitEnvironment(testAutoConfEnv1.id) assertEnvProps(t, testAutoConfEnv1.params(), env) - verifyEventVerbatimRelay(t, p, serverSideEventsURL, testAutoConfEnv1.sdkKey) + verifyEventVerbatimRelay(t, p, serverSideEventsURL, testAutoConfEnv1.sdkKey.Value) verifyEventVerbatimRelay(t, p, mobileEventsURL, testAutoConfEnv1.mobKey) modified := makeEnvWithModifiedMobileKey(testAutoConfEnv1) @@ -253,7 +257,7 @@ func TestEventForwardingAfterMobileKeyChange(t *testing.T) { p.awaitCredentialsUpdated(env, modified.params()) - verifyEventProxying(t, p, serverSideEventsURL, testAutoConfEnv1.sdkKey) + verifyEventProxying(t, p, serverSideEventsURL, testAutoConfEnv1.sdkKey.Value) verifyEventProxying(t, p, mobileEventsURL, modified.mobKey) verifyEventProxying(t, p, jsEventsURL+string(testAutoConfEnv1.id), testAutoConfEnv1.id) }) diff --git a/relay/autoconfig_testdata_test.go b/relay/autoconfig_testdata_test.go index 17681d50..aa0e3dbc 100644 --- a/relay/autoconfig_testdata_test.go +++ b/relay/autoconfig_testdata_test.go @@ -7,7 +7,6 @@ import ( "github.com/launchdarkly/ld-relay/v8/internal/autoconfig" "github.com/launchdarkly/ld-relay/v8/internal/envfactory" - "github.com/launchdarkly/go-sdk-common/v3/ldtime" "github.com/launchdarkly/go-test-helpers/v3/httphelpers" ) @@ -18,16 +17,18 @@ var testAutoConfDefaultConfig = c.Config{ } type testAutoConfEnv struct { - id c.EnvironmentID - envKey string - envName string - mobKey c.MobileKey - projKey string - projName string - sdkKey c.SDKKey - sdkKeyExpiryValue c.SDKKey - sdkKeyExpiryTime ldtime.UnixMillisecondTime - version int + id c.EnvironmentID + envKey string + envName string + mobKey c.MobileKey + projKey string + projName string + sdkKey envfactory.SDKKeyRep + version int +} + +func (e testAutoConfEnv) SDKKey() c.SDKKey { + return e.sdkKey.Value } var ( @@ -38,7 +39,7 @@ var ( mobKey: c.MobileKey("mobkey1"), projKey: "projkey1", projName: "projname1", - sdkKey: c.SDKKey("sdkkey1"), + sdkKey: envfactory.SDKKeyRep{Value: c.SDKKey("sdkkey1")}, version: 10, } @@ -49,7 +50,7 @@ var ( mobKey: c.MobileKey("mobkey2"), projKey: "projkey2", projName: "projname2", - sdkKey: c.SDKKey("sdkkey2"), + sdkKey: envfactory.SDKKeyRep{Value: c.SDKKey("sdkkey2")}, version: 11, } ) @@ -62,14 +63,8 @@ func (e testAutoConfEnv) toEnvironmentRep() envfactory.EnvironmentRep { MobKey: e.mobKey, ProjKey: e.projKey, ProjName: e.projName, - SDKKey: envfactory.SDKKeyRep{ - Value: e.sdkKey, - }, - Version: e.version, - } - if e.sdkKeyExpiryValue != "" { - rep.SDKKey.Expiring.Value = e.sdkKeyExpiryValue - rep.SDKKey.Expiring.Timestamp = e.sdkKeyExpiryTime + SDKKey: e.sdkKey, + Version: e.version, } return rep } diff --git a/relay/relay.go b/relay/relay.go index 3f47bf21..db617e58 100644 --- a/relay/relay.go +++ b/relay/relay.go @@ -361,8 +361,7 @@ func IsPayloadFilterNotFound(err error) bool { // getEnvironment returns the environment object corresponding to the given credential, or nil // if not found. The credential can be an SDK key, a mobile key, or an environment ID. The second -// return value is normally true, but is false if Relay does not yet have a valid configuration -// (which affects our error handling). +// return value is normally nil, but is present if Relay does not yet have a valid configuration. func (r *Relay) getEnvironment(req sdkauth.ScopedCredential) (relayenv.EnvContext, error) { r.lock.RLock() defer r.lock.RUnlock() @@ -445,18 +444,20 @@ func (r *Relay) addEnvironment( return r.clientFactory(sdkKey, config, timeout) } clientContext, err := relayenv.NewEnvContext(relayenv.EnvContextImplParams{ - Identifiers: identifiers, - EnvConfig: envConfig, - AllConfig: r.config, - ClientFactory: wrappedClientFactory, - DataStoreFactory: dataStoreFactory, - DataStoreInfo: dataStoreInfo, - StreamProviders: r.allStreamProviders(), - JSClientContext: jsClientContext, - MetricsManager: r.metricsManager, - UserAgent: r.userAgent, - LogNameMode: r.envLogNameMode, - Loggers: r.loggers, + Identifiers: identifiers, + EnvConfig: envConfig, + AllConfig: r.config, + ClientFactory: wrappedClientFactory, + DataStoreFactory: dataStoreFactory, + DataStoreInfo: dataStoreInfo, + StreamProviders: r.allStreamProviders(), + JSClientContext: jsClientContext, + MetricsManager: r.metricsManager, + UserAgent: r.userAgent, + LogNameMode: r.envLogNameMode, + Loggers: r.loggers, + ConnectionMapper: r, + ExpiredCredentialCleanupInterval: r.config.Main.ExpiredCredentialCleanupInterval.GetOrElse(0), }, resultCh) if err != nil { return nil, nil, errNewClientContextFailed(identifiers.GetDisplayName(), err) @@ -494,19 +495,19 @@ func (r *Relay) setFullyConfigured(fullyConfigured bool) { r.lock.Unlock() } -// addConnectionMapping updates the RelayCore's environment mapping to reflect that a new +// AddConnectionMapping updates the RelayCore's environment mapping to reflect that a new // credential is now enabled for this EnvContext. This should be done only *after* calling // EnvContext.AddCredential() so that if the RelayCore receives an incoming request with the new // credential immediately after this, it will work. -func (r *Relay) addConnectionMapping(params sdkauth.ScopedCredential, env relayenv.EnvContext) { +func (r *Relay) AddConnectionMapping(params sdkauth.ScopedCredential, env relayenv.EnvContext) { r.envsByCredential.MapRequestParams(params, env) } -// removeConnectionMapping updates the RelayCore's environment mapping to reflect that this +// RemoveConnectionMapping updates the RelayCore's environment mapping to reflect that this // credential is no longer enabled. This should be done *before* calling EnvContext.RemoveCredential() // because RemoveCredential() disconnects all existing streams, and if a client immediately tries to // reconnect using the same credential we want it to be rejected. -func (r *Relay) removeConnectionMapping(params sdkauth.ScopedCredential) { +func (r *Relay) RemoveConnectionMapping(params sdkauth.ScopedCredential) { r.envsByCredential.UnmapRequestParams(params) } diff --git a/relay/relay_environments_test.go b/relay/relay_environments_test.go index c5548cf3..64e93c6e 100644 --- a/relay/relay_environments_test.go +++ b/relay/relay_environments_test.go @@ -186,7 +186,7 @@ func TestRelayAddedEnvironmentCredential(t *testing.T) { noEnv, _ := relay.getEnvironment(sdkauth.New(extraKey)) assert.Nil(t, noEnv) - relay.addConnectionMapping(sdkauth.New(extraKey), env) + relay.AddConnectionMapping(sdkauth.New(extraKey), env) env1, _ := relay.getEnvironment(sdkauth.New(extraKey)) assert.Equal(t, env, env1) @@ -200,7 +200,7 @@ func TestRelayRemovingEnvironmentCredential(t *testing.T) { require.NoError(t, err) defer relay.Close() - relay.removeConnectionMapping(sdkauth.New(st.EnvMain.Config.SDKKey)) + relay.RemoveConnectionMapping(sdkauth.New(st.EnvMain.Config.SDKKey)) _, err = relay.getEnvironment(sdkauth.New(st.EnvMain.Config.SDKKey)) assert.Error(t, err) diff --git a/relay/testutils_test.go b/relay/testutils_test.go index b0120e48..c13225ae 100644 --- a/relay/testutils_test.go +++ b/relay/testutils_test.go @@ -27,6 +27,7 @@ type relayTestHelper struct { } func (h relayTestHelper) awaitEnvironment(envID c.EnvironmentID) relayenv.EnvContext { + h.t.Helper() var e relayenv.EnvContext var err error require.Eventually(h.t, func() bool { @@ -37,6 +38,7 @@ func (h relayTestHelper) awaitEnvironment(envID c.EnvironmentID) relayenv.EnvCon } func (h relayTestHelper) shouldNotHaveEnvironment(envID c.EnvironmentID, timeout time.Duration) { + h.t.Helper() require.Eventually(h.t, func() bool { _, err := h.relay.getEnvironment(sdkauth.New(envID)) return err != nil @@ -44,6 +46,8 @@ func (h relayTestHelper) shouldNotHaveEnvironment(envID c.EnvironmentID, timeout } func (h relayTestHelper) assertEnvLookup(env relayenv.EnvContext, expected envfactory.EnvironmentParams) { + h.t.Helper() + foundEnv, err := h.relay.getEnvironment(sdkauth.New(expected.EnvID)) if assert.NoError(h.t, err) { assert.Equal(h.t, env, foundEnv)