perf: cache webhook secret resolution with a TTL cache

[dejanzele]

How

Adds a caching decorator in pkg/secret and wraps the one client the webhook loader builds. Webhook parameters are resolved on every notification, so an event burst previously turned into one uncached secret GET per event per parameter.

Only Get and GetObject are cached. Every mutating method passes through to the inner client and invalidates the affected key. Entries are keyed by name plus the namespace the call resolves to, so a read with an explicit namespace and a read that falls back to the default share one entry. Returned maps and objects are copies, so a caller cannot mutate what the cache holds.

TESTKUBE_SECRET_CACHE_TTL sets the lifetime and defaults to 30s. A zero value returns the inner client unchanged, which disables caching completely.

Missing secrets are cached too, for a third of the TTL, so a webhook pointing at a secret that does not exist does not hammer the API server either. Every other error is treated as possibly transient and is never cached.

A read that started before an invalidation does not repopulate the cache afterwards, so a mutation cannot be overwritten by an older in flight result.

The saving is larger than one read per event. processTemplate rebuilds the resolved config map for every templated field, so an uncached notification reads the same secret once for the uri, once for the payload and once more for every header. A webhook with a templated uri and a payload resolves it twice per event before this change and once per TTL window after it.

Storage is the shared pkg/cache, rather than another map with an expiry sweep next to it. That package needed two things it did not have: Delete, without which a mutation cannot invalidate what it replaced, and a way to substitute the time source, without which TTL behavior can only be tested by waiting. Both are added here, and Clear comes with Delete for DeleteAll, which changes secrets it cannot map back to individual keys.

What stays in pkg/secret is the part that is specific to reading secrets: a shorter lifetime for not found results, coalescing of concurrent misses, and the revision that lets a mutation discard reads it superseded.

Notes for review

This trades immediacy for request volume, and the staleness bound is worth an explicit decision: rotating a secret takes up to the TTL to reach webhook parameters, and creating a secret that was just looked up and missing takes up to a third of the TTL to become visible. Setting the TTL to zero restores the current behavior for anyone who needs rotation to apply immediately.

This branch includes the Get clock fix from #8053, since injecting a time source is pointless while Get reads the wall clock. Merge that first and this diff shrinks accordingly.

Widening cache.Cache touches every implementer. In this repository that is InMemoryCache and one test fake in pkg/imageinspector, both updated here.

The decorator is applied at exactly one construction site. Other pkg/secret callers are unchanged, and adopting it elsewhere is now a one line change per site.

Concurrent misses for the same key are coalesced with singleflight, so a burst arriving on a cold key produces one read rather than one per listener. Without that the cache only helps after the first read completes, which is not the case the change exists for.

[greptile-apps]

Greptile Summary

Adds TTL-based caching for webhook secret resolution while preserving mutation consistency.

• Coalesces concurrent cache misses with singleflight, addressing the previously reported burst of duplicate Kubernetes API reads.
• Keys flights by cache revision so reads beginning after invalidation cannot join an older in-flight request, addressing the previously reported stale-read race.
• Adds configurable positive and negative TTLs, defensive copies, mutation invalidation, and injectable cache timing for deterministic tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; same-revision misses are coalesced, while revision changes isolate post-mutation reads from older in-flight results.

Important Files Changed

Filename	Overview
pkg/secret/cached_client.go	Implements copied TTL-cached secret reads, negative caching, concurrent miss coalescing, and revision-aware invalidation; both previously reported concurrency defects are addressed.
pkg/cache/inmem.go	Adds deterministic time-source injection and thread-safe delete and clear operations required by secret-cache invalidation.
cmd/api-server/main.go	Wraps the webhook loader's Kubernetes secret client with the configured cache.
internal/config/config.go	Adds the TESTKUBE_SECRET_CACHE_TTL duration setting with a 30-second default and zero-value opt-out.
pkg/secret/cached_client_test.go	Covers TTL behavior, copies, invalidation, negative caching, concurrent miss coalescing, and post-invalidation flight isolation.

Sequence Diagram

sequenceDiagram
    participant W as Webhook listeners
    participant C as Cached secret client
    participant F as singleflight
    participant K as Kubernetes API
    W->>C: Concurrent Get(namespace/name)
    C->>C: Check TTL cache and capture revision
    C->>F: Do(revision/key)
    F->>K: One secret read
    K-->>F: Secret or not-found
    F-->>C: Shared result
    C->>C: Cache defensive copy
    C-->>W: Independent copies
    Note over C: Mutation increments revision and invalidates entries
    W->>C: Get after mutation
    C->>F: Do(newRevision/key)
    F->>K: Fresh secret read

Loading

_{Reviews (4): Last reviewed commit: "refactor(secret): build the secret read ..." | Re-trigger Greptile}

[dejanzele]

@greptileai

[dejanzele]

@greptileai

[dejanzele]

@greptileai