Skip to content

Fix wall clock reset issue - #131

Merged
bob2681312 merged 5 commits into
aws:masterfrom
bob2681312:master
Aug 4, 2026
Merged

Fix wall clock reset issue#131
bob2681312 merged 5 commits into
aws:masterfrom
bob2681312:master

Conversation

@bob2681312

@bob2681312 bob2681312 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Description

Why is this change being made?

  1. To make the library robust to wall clock resets

What is changing?

  1. Instead of storing the time as an int64, it is instead stored as a Time object.
  2. Used Built in functions from the Time object for comparison to allow for comparing with the monotonic time instead of wall-clock time.
  • Monotonic time is similar to a local counter and usually implement based on the number of CPU cycles since wake up. Therefore, it is immune to wall clock resets.
  • Note: An issue with Monotonic time is that in certain OS like Linux, it will stop when the computer goes to sleep, leading to secrets existing for longer than expected.
  1. For checking if the current time has gone past the nextRefreshTime, we do a check against both the monotonic and wall-clock time. If either time shows that the current time is past the nextRefreshTime, a refresh will occur.
  • Since both monotonic and wall-clock time have there issues and there doesn't seem to be any better way of doing this, using them together will decrease the chance of a secret not being refreshed in time. It, however, could lead to a higher chance of secrets being refreshed early, but this is not a big deal as it is just a few extra API calls and the retrieval time being slightly longer on certain tries.
  1. Updated cacheVersion error backoff to start from 1ms from 1 nanosecond.

Testing

How was this tested?

  1. go test
  2. Added and passed additional Unit Tests simulating a wall clock reset in different scenarios

When testing locally, provide testing artifact(s):

N/A


Reviewee Checklist

Update the checklist after submitting the PR

  • I have reviewed, tested and understand all changes
    If not, why:
  • I have filled out the Description and Testing sections above
    If not, why:
  • Build and Unit tests are passing
    If not, why:
  • Unit test coverage check is passing
    If not, why:
  • Integration tests pass locally
    If not, why:
  • I have updated integration tests (if needed)
    If not, why:
  • I have ensured no sensitive information is leaking (i.e., no logging of sensitive fields, or otherwise)
    If not, why:
  • I have added explanatory comments for complex logic, new classes/methods and new tests
    If not, why:
  • I have updated README/documentation (if needed)
    If not, why:
  • I have clearly called out breaking changes (if any)
    If not, why:

Reviewer Checklist

All reviewers please ensure the following are true before reviewing:

  • Reviewee checklist has been accurately filled out
  • Code changes align with stated purpose in description
  • Test coverage adequately validates the changes

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov

This comment was marked as outdated.

@bob2681312
bob2681312 marked this pull request as ready for review July 27, 2026 22:17
@bob2681312
bob2681312 requested a review from a team as a code owner July 27, 2026 22:17
Comment thread secretcache/cacheObjects_test.go Outdated
Comment thread secretcache/cacheObjects_test.go Outdated

@i-am-SR i-am-SR left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: No test validates that a wall-clock jump backward is caught by the monotonic side. This is the primary benefit of the monotonic check and would strengthen the test suite.

@bob2681312

bob2681312 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

nit: No test validates that a wall-clock jump backward is caught by the monotonic side. This is the primary benefit of the monotonic check and would strengthen the test suite.

Yeah, what you said is true, but this is indirectly tested in the tests from line 129-247 when only the monotonic time was moved forward and the wall-clock was frozen in time. Sorry if it was confusing at all, but I wrote those original methods without the AdvanceWall method and I forgot to go back and change them so that it made more sense.

@bob2681312
bob2681312 merged commit 0f06393 into aws:master Aug 4, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants