refactor the new offline bool out of EnvParams

internal/credential/rotator.go

@@ -218,23 +218,36 @@ func (r *Rotator) swapPrimaryKey(newKey config.SDKKey) config.SDKKey {
 
 	return previous
 }
+func (r *Rotator) immediatelyRevoke(key config.SDKKey) {
+	if key.Defined() {
+		r.expirations = append(r.expirations, key)
+		r.loggers.Infof("SDK key %s has been immediately revoked", key.Masked())
+	}
+	return
+}
+
 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())
+	// If there's no deprecation notice, then the previous key (if any) needs to be immediately revoked so it doesn't
+	// hang around forever valid.
+	if grace == nil {
+		r.immediatelyRevoke(previous)
 		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)
 			}
+			// When a key is deprecated by LD, it will stick around in the deprecated field of the message until something
+			// else is deprecated. This means that if a key is rotated *without* a deprecation period set for the previous key,
+			// then we'll receive that new primary key but the deprecation message will be stale - it'll be referring to some
+			// even older key. We detect this case here (since we already saw the deprecation message in our map) and
+			// ensure the previous key is revoked.
+			r.immediatelyRevoke(previous)
 			return
 		}
 

internal/credential/rotator_test.go

@@ -197,3 +197,29 @@ func TestSDKKeyExpiredInThePastIsNotAdded(t *testing.T) {
 	assert.ElementsMatch(t, []SDKCredential{primaryKey}, additions)
 	assert.Empty(t, expirations)
 }
+
+func TestSDKKeyIsImmediatelyRotatedIfPreviousDeprecationAlreadyTookPlace(t *testing.T) {
+	mockLog := ldlogtest.NewMockLog()
+	rotator := NewRotator(mockLog.Loggers)
+
+	rotator.Initialize([]SDKCredential{config.SDKKey("key0")})
+
+	now := time.Unix(1000, 0)
+	expiry := now.Add(1 * time.Hour)
+	rotator.RotateWithGrace(config.SDKKey("key1"), NewGracePeriod("key0", expiry, now))
+
+	additions, expirations := rotator.StepTime(now.Add(30 * time.Minute))
+	assert.ElementsMatch(t, []SDKCredential{config.SDKKey("key1")}, additions)
+	assert.Empty(t, expirations)
+
+	rotator.RotateWithGrace(config.SDKKey("key2"), NewGracePeriod("key0", expiry, now.Add(31*time.Minute)))
+
+	additions, expirations = rotator.StepTime(now.Add(31 * time.Minute))
+	assert.ElementsMatch(t, []SDKCredential{config.SDKKey("key2")}, additions)
+	assert.ElementsMatch(t, []SDKCredential{config.SDKKey("key1")}, expirations)
+
+	additions, expirations = rotator.StepTime(expiry.Add(1 * time.Millisecond))
+	assert.Empty(t, additions)
+	assert.ElementsMatch(t, []SDKCredential{config.SDKKey("key0")}, expirations)
+
+}

internal/envfactory/env_config_factory.go

@@ -15,10 +15,11 @@ type EnvConfigFactory struct {
 	// DataStorePrefix is the configured data store prefix, which may contain a per-environment placeholder.
 	DataStorePrefix string
 	// DataStorePrefix is the configured data store table name, which may contain a per-environment placeholder.
-	TableName string
-	//
+	TableName     string
 	AllowedOrigin ct.OptStringList
 	AllowedHeader ct.OptStringList
+	// Whether this factory is used for offline mode or not.
+	Offline bool
 }
 
 // NewEnvConfigFactoryForAutoConfig creates an EnvConfigFactory based on the auto-configuration mode settings.
@@ -28,6 +29,7 @@ func NewEnvConfigFactoryForAutoConfig(c config.AutoConfigConfig) EnvConfigFactor
 		TableName:       c.EnvDatastoreTableName,
 		AllowedOrigin:   c.EnvAllowedOrigin,
 		AllowedHeader:   c.EnvAllowedHeader,
+		Offline:         false,
 	}
 }
 
@@ -38,6 +40,7 @@ func NewEnvConfigFactoryForOfflineMode(c config.OfflineModeConfig) EnvConfigFact
 		TableName:       c.EnvDatastoreTableName,
 		AllowedOrigin:   c.EnvAllowedOrigin,
 		AllowedHeader:   c.EnvAllowedHeader,
+		Offline:         true,
 	}
 }
 
@@ -54,7 +57,7 @@ func (f EnvConfigFactory) MakeEnvironmentConfig(params EnvironmentParams) config
 		AllowedHeader: f.AllowedHeader,
 		SecureMode:    params.SecureMode,
 		FilterKey:     params.Identifiers.FilterKey,
-		Offline:       params.Offline,
+		Offline:       f.Offline,
 	}
 	if params.TTL != 0 {
 		ret.TTL = ct.NewOptDuration(params.TTL)

internal/envfactory/env_params.go

@@ -35,9 +35,6 @@ type EnvironmentParams struct {
 
 	// SecureMode is true if secure mode is required for this environment.
 	SecureMode bool
-
-	// True if the environment was created from an offline file data source.
-	Offline bool
 }
 
 type ExpiringSDKKey struct {

internal/filedata/archive_reader.go

@@ -121,12 +121,8 @@ func (ar *archiveReader) GetEnvironmentMetadata(envID config.EnvironmentID) (env
 	if err := json.Unmarshal(data, &rep); err != nil {
 		return environmentMetadata{}, err
 	}
-
-	params := rep.Env.ToParams()
-	params.Offline = true // Signify that this environment is from an offline mode source
-
 	return environmentMetadata{
-		params:  params,
+		params:  rep.Env.ToParams(),
 		version: rep.Env.Version,
 		dataID:  rep.DataID,
 	}, nil