feat: add in-memory LRU+TTL cache for validation results#1933
Open
nissessenap wants to merge 5 commits intosigstore:mainfrom
Open
feat: add in-memory LRU+TTL cache for validation results#1933nissessenap wants to merge 5 commits intosigstore:mainfrom
nissessenap wants to merge 5 commits intosigstore:mainfrom
Conversation
Author
|
I need to perform some validation, of this before changing over from draft. I will also create a PR that builds on this which implements some basic metrics make the cach visaiable, but that is a sperate PR since it's the first time we use custom metrics in this repo, but I wanted to showcase that I was working on the issues with this PR at least. |
nissessenap
force-pushed
the
cache_impl
branch
from
3038a32 to
8013c95
Compare
Add tests for the upcoming LRU+TTL cache implementation (sigstore#647). Unit tests cover set/get, TTL expiry, eviction, key isolation, error skipping, and resource version invalidation. Integration tests verify ValidatePolicy cache hit/miss behavior. All tests currently fail to compile (NewLRUCache undefined), confirming the TDD Red phase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Edvin Norling <edvin.norling@kognic.com>
Add LRUCache implementing ResultCache using hashicorp/golang-lru/v2 expirable. Only successful validations (PolicyResult non-nil) are cached; failed validations are skipped to allow immediate retries. Fix cache key mismatch bug: ref.Name() in Set vs ref.String() in Get caused cache to never hit. Both now use ref.String(). Move cache Set into ValidatePolicy so caching is self-contained regardless of call path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Edvin Norling <edvin.norling@kognic.com>
Wire the LRU+TTL cache into the validating webhook via --enable-cache, --cache-size, and --cache-ttl flags. Cache is off by default and only injected into the validating admission controller context. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Edvin Norling <edvin.norling@kognic.com>
- Copy CacheResult.Errors slice in LRUCache.Set to prevent callers from mutating cached entries through the shared backing array - Update copyright year to 2026 on new files (lrucache.go, lrucache_test.go) - Extract cacheTestFixtures helper to reduce boilerplate in cache integration tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Edvin Norling <edvin.norling@kognic.com>
Move cache observability into LRUCache.Get() so the implementation
owns its own logging
Signed-off-by: Edvin Norling <edvin.norling@kognic.com>
nissessenap
force-pushed
the
cache_impl
branch
from
70790b3 to
e870c6c
Compare
nissessenap
force-pushed
the
cache_impl
branch
from
e870c6c to
85c8ec6
Compare
Author
|
I have ran this locally and it seams to works as intended. I removed third_party/VENDOR-LICENSE/github.com/hashicorp/golang-lru in one commit, but saw on my local PR that it made a CI fail. I hope you think this is a sufisant start to get this PR merged and we can build uppon it to create two seperate PR
|
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ResultCacheinterface with an LRU+TTL cache usinghashicorp/golang-lru/v2/expirable, opt-in via--enable-cacheCLI flagref.Name()inSetvsref.String()inGetcaused the cache to never hit — both now useref.String()cache.Setcall fromvalidatePoliciesintoValidatePolicyso caching is self-contained regardless of call path (previously it was in the goroutine caller, usingref.Name()which strips the digest)Closes #647, closes #1887
Details
New files:
pkg/webhook/lrucache.go—LRUCachestruct wrappingexpirable.LRU[string, *CacheResult]pkg/webhook/lrucache_test.go— 8 unit tests (set/get, miss, skip errors, partial success, TTL expiry, eviction, key isolation, resourceVersion invalidation)Modified files:
pkg/webhook/validator.go— Fixref.Name()→ref.String()bug, movecache.SetintoValidatePolicy(was invalidatePoliciesgoroutine)pkg/webhook/validator_test.go— 4 integration tests (cache hit, skip errors, partial success, no-cache default)cmd/webhook/main.go— Add--enable-cache,--cache-size,--cache-ttlflags; inject cache into validating webhook contextgo.mod— Promotehashicorp/golang-lru/v2from indirect to direct dependencyCLI flags (all opt-in, cache disabled by default):
--enable-cachefalse--cache-size1024--cache-ttl1hTest plan
go test ./pkg/webhook/... -run TestLRUCachego test ./pkg/webhook/... -run TestValidatePolicyCachego test $(go list ./... | grep -v third_party/)go build ./cmd/webhook/...--enable-cache,--cache-size,--cache-ttl--enable-cacheand verify repeated admissions use cache--enable-cachebehavior is identical to before🤖 Generated with Claude Code