Fix wall clock reset issue

secretcache/cacheItem.go

@@ -29,7 +29,8 @@ type secretCacheItem struct {
 
 	// The next scheduled refresh time for this item.  Once the item is accessed
 	// after this time, the item will be synchronously refreshed.
-	nextRefreshTime int64
+	nextRefreshTime time.Time
+
 	*cacheObject
 }
 
@@ -38,17 +39,22 @@ func newSecretCacheItem(config CacheConfig, client SecretsManagerAPIClient, secr
 	return secretCacheItem{
 		versions:        newLRUCache(10),
 		cacheObject:     &cacheObject{config: config, client: client, secretId: secretId, refreshNeeded: true},
-		nextRefreshTime: time.Now().UnixNano(),
+		nextRefreshTime: time.Now(),
 	}
 }
 
 // isRefreshNeeded determines if the cached item should be refreshed.
+// Dual-check: monotonic clock (immune to wall clock jumps) OR wall clock
+// (advances during macOS sleep when monotonic freezes). Extra API calls
+// from a forward wall clock jump could occur, but is acceptable
+// to avoid serving stale secrets.
 func (ci *secretCacheItem) isRefreshNeeded() bool {
 	if ci.cacheObject.isRefreshNeeded() {
 		return true
 	}
 
-	return ci.nextRefreshTime <= time.Now().UnixNano()
+	// Check both monotonic and wall clock to determine if refresh is needed
+	return ci.nextRefreshTime.Compare(ci.timeNow()) <= 0 || ci.nextRefreshTime.Round(0).Compare(ci.timeNowWall()) <= 0
 }
 
 // getVersionId gets the version id for the given version stage.
@@ -103,7 +109,7 @@ func (ci *secretCacheItem) executeRefresh(ctx context.Context) (*secretsmanager.
 		ttl = rand.Int63n(maxTTL/2) + maxTTL/2
 	}
 
-	ci.nextRefreshTime = time.Now().Add(time.Nanosecond * time.Duration(ttl)).UnixNano()
+	ci.nextRefreshTime = ci.timeNow().Add(time.Nanosecond * time.Duration(ttl))
 	return result, err
 }
 
@@ -127,20 +133,19 @@ func (ci *secretCacheItem) getVersion(versionStage string) (*cacheVersion, bool)
 	return secretCacheVersion, true
 }
 
-// refresh the cached object on demand
+// refreshNow forces a refresh with a jittered sleep to avoid retry storms.
 func (ci *secretCacheItem) refreshNow(ctx context.Context) {
 	ci.refreshNeeded = true
-	// Generate a random number to have a sleep jitter to not get stuck in a retry loop
-	sleep := rand.Int63n((forceRefreshJitterSleep+1)-(forceRefreshJitterSleep/2)+1) + (forceRefreshJitterSleep / 2)
+	sleep := (rand.Int63n((forceRefreshJitterSleep+1)-(forceRefreshJitterSleep/2)+1) + (forceRefreshJitterSleep / 2)) * int64(time.Millisecond)
 
 	if ci.err != nil {
-		exceptionSleep := ci.nextRefreshTime - time.Now().UnixNano()
+		exceptionSleep := int64(ci.nextRefreshTime.Sub(ci.timeNow()))
 		if exceptionSleep > sleep {
 			sleep = exceptionSleep
 		}
 	}
 
-	time.Sleep(time.Millisecond * time.Duration(sleep))
+	time.Sleep(time.Duration(sleep))
 	ci.refresh(ctx)
 }
 
@@ -160,7 +165,7 @@ func (ci *secretCacheItem) refresh(ctx context.Context) {
 		delay := exceptionRetryDelayBase * math.Pow(exceptionRetryGrowthFactor, float64(ci.errorCount))
 		delay = math.Min(delay, exceptionRetryDelayMax)
 		delayDuration := time.Millisecond * time.Duration(delay)
-		ci.nextRetryTime = time.Now().Add(delayDuration).UnixNano()
+		ci.nextRetryTime = ci.timeNow().Add(delayDuration)
 		return
 	}
 

secretcache/cacheObject.go

@@ -36,8 +36,33 @@ type cacheObject struct {
 	refreshNeeded bool
 
 	// The time to wait before retrying a failed AWS Secrets Manager request.
-	nextRetryTime int64
+	nextRetryTime time.Time
 	data          interface{}
+
+	// now overrides time.Now in tests. nil in production.
+	now func() time.Time
+
+	// nowWall overrides the wall clock reading in tests. nil in production.
+	nowWall func() time.Time
+}
+
+// Function used for overriding the time.Now in tests. In production, it will
+// just return the result of the normal time.Now function
+func (o *cacheObject) timeNow() time.Time {
+	if o.now != nil {
+		return o.now()
+	}
+	return time.Now()
+}
+
+// timeNowWall returns the current time with the monotonic reading stripped, so
+// comparisons against it use the wall clock. Utilized in tests to set a wall clock
+// time
+func (o *cacheObject) timeNowWall() time.Time {
+	if o.nowWall != nil {
+		return o.nowWall().Round(0)
+	}
+	return o.timeNow().Round(0)
 }
 
 // isRefreshNeeded determines if the cached object should be refreshed.
@@ -50,9 +75,11 @@ func (o *cacheObject) isRefreshNeeded() bool {
 		return false
 	}
 
-	if o.nextRetryTime == 0 {
+	if o.nextRetryTime.IsZero() {
 		return true
 	}
 
-	return o.nextRetryTime <= time.Now().UnixNano()
+	// Compare both the monotonic and wall clock time to reduce possibility of secrets living longer than they should be
+	// Note: During normal comparison, the monotonic clock is used. Round(0) will force the wall clock reading to be used.
+	return o.nextRetryTime.Compare(o.timeNow()) <= 0 || o.nextRetryTime.Round(0).Compare(o.timeNowWall()) <= 0
 }