APIGOV-31191 validate cache

[alrosca]

Cache validation on start up and reconnect

[jcollins-axway]

Looks good from a high level, needs pounded testing wise

[jcollins-axway]

copilots review

Code Review: APIGOV-31191 vs main

Summary

• Scope: reviewed 8 files (314 insertions, 4 deletions), focused on new cache validation flow, reconnect hooks, and cache manager interfaces.
• Verdict: request changes
• Severity: medium (2), low (0), high (0), blocker (0)

Findings

• Medium — Cache validation uses non-unique key (name) and can collide across scoped resources

  • Where: pkg/agent/cache/cachevalidation.go, pkg/agent/cache/cachevalidation.go, pkg/agent/cachevalidationjob.go, pkg/agent/cachevalidationjob.go
  • Description: both server and cache summaries are keyed only by resource.Name. If multiple resources share the same name under different scopes, entries overwrite each other, making the comparison unreliable.
  • Possible impacts to the codebase: false negatives (stale cache undetected) or false positives (unnecessary full cache rebuilds), especially in multi-scope environments.

• Medium — Cache validator hard-codes API version to v1alpha1, which can skip/incorrectly query some filters

  • Where: pkg/agent/cachevalidationjob.go, pkg/agent/cachevalidationjob.go
  • Description: validation URL is built from a synthetic ResourceInstance with fixed APIVersion: "v1alpha1". For filters whose resource version differs, GetKindLink() may produce the wrong endpoint or empty link, and validation is skipped (return true).
  • Possible impacts to the codebase: out-of-sync persisted cache may pass validation and remain in use after startup/reconnect for affected resource kinds.

Positives

• Cache validation logic is cleanly encapsulated (cacheValidator) and integrated into both startup and reconnect paths.
• Reconnect hooks were added consistently to poll and stream clients with minimal API surface change (WithOnReconnect options).
• Logging around validation failures includes useful operational context (resource counts, resource names, timestamps).

Recommendations

• Use a stable unique key for comparisons (for example selfLink or a composite of group/kind/scope/name) instead of name alone.
• Derive API version from filter/resource metadata or from server-discovered GVK mappings rather than hard-coding "v1alpha1".
• Add targeted unit tests for: duplicate names across scopes, non-v1alpha1 resources, and reconnect-triggered validation behavior.

[alrosca]

Not a bad review at all

[jcollins-axway]

as we spoke of we prefer test cases over single tests

[jcollins-axway]

caching changes require an extra amount of testing as its usually to blame for our duplicate service issues