fix: use cached client for handler to reduce audit log volume - #325
fix: use cached client for handler to reduce audit log volume#325kseiter-rh wants to merge 2 commits into
Conversation
The handler previously used an uncached client.New() for all Kubernetes API calls, causing every reconciliation cycle (~10+ GET calls every 5 minutes per cluster) to hit the API server directly. These direct API calls generate audit log entries that flow to Splunk, contributing to the ingestion spike tracked in HCMSEC-3645. Switch the handler to use mgr.GetClient() (the manager's cached client) and expand the cache configuration to cover the cross-namespace resources the handler needs: - Secrets in openshift-config (for pull-secret reads) - ConfigMaps in openshift-monitoring (for CAMO ConfigMap) - Cluster-scoped resources (Proxy, ClusterVersion) are already cacheable since DefaultNamespaces only restricts namespaced resources With this change, steady-state GET operations are served from the informer cache without hitting the API server, eliminating the corresponding audit log entries. Write operations (Create/Update/Delete) still go through the API server as expected. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Skipping CI for Draft Pull Request. |
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe manager now caches selected Secrets and ConfigMaps. ChangesOCM agent manager and reconciliation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kseiter-rh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Narrow the ByObject cache entries to watch only the exact resources needed (pull-secret in openshift-config, ocm-agent in openshift-monitoring) rather than all Secrets/ConfigMaps in those namespaces. This avoids caching sensitive secrets the operator doesn't need access to. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
client.New()to the manager's cachedmgr.GetClient(), eliminating ~10+ direct API server GET calls per reconciliation cycle (every 5 minutes per cluster)ByObjectentries to cover cross-namespace resources the handler needs: Secrets inopenshift-config(pull-secret) and ConfigMaps inopenshift-monitoring(CAMO ConfigMap)DefaultNamespacesonly restricts namespaced resourcesContext
Investigation of HCMSEC-3645 (Splunk ingestion spike in
osdsecuritylogs) identifiedopenshift-ocm-agent-operatoras a contributor. The handler was created with a separate uncached client because the manager's cache was namespace-scoped to the operator namespace and couldn't serve cross-namespace reads. Every reconciliation cycle made direct API calls that generated audit log entries flowing to Splunk. Across thousands of clusters, this adds up significantly.Controller-runtime v0.23.3 supports
ByObjectcache configuration with per-object namespace overrides, which lets us expand the cache to cover the specific cross-namespace resources without watching entire namespaces.What changes
client.New()(uncached)mgr.GetClient()(cached)openshift-configand ConfigMaps inopenshift-monitoringWrite operations (Create/Update/Delete) still hit the API server as expected — only read operations benefit from caching.
Test plan
go build ./...passesgo test ./...— all unit tests pass (no handler test changes needed since tests mock theclient.Clientinterface, which is the same regardless of cached/uncached backing)openshift-configare still detected (cache watch delivers update events)🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Performance & Reliability