Skip to content

ROX-33333: Sensor targeted cache invalidation changes#19708

Merged
clickboo merged 2 commits intomasterfrom
boo-sensor-send-invalidate-cache
Apr 3, 2026
Merged

ROX-33333: Sensor targeted cache invalidation changes#19708
clickboo merged 2 commits intomasterfrom
boo-sensor-send-invalidate-cache

Conversation

@clickboo
Copy link
Copy Markdown
Contributor

@clickboo clickboo commented Mar 31, 2026

Description

This PR introduces:

  1. Sensor forwarding targeted cache invalidation messages received from central to admission controller instead of invoking a FlushCache on admission controller.
  2. Admission controller side processing of those messages.

Please see merged foundational proto PR here.

Note:
Central already sends targeted cache invalidation messages to sensor in two cases:

  1. When an image is deleted, and
  2. When a vuln deferral or mark as false positive is approved

We are now forwarding those to admission controller instead of aggressively flushing admission controller cache in these cases.

These are secured cluster side changes only, and hence no version compatibility concerns.

Follow on Central PR to send a targeted cache invalidation message when ScanImage is complete is here

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

Manual testing on an openshift infra cluster using delete image workflow, for images deployed after the deployments pass the admission review requests.

admission-control/manager: 2026/04/02 07:49:47.562386 manager_impl.go:522: Info: Updated cluster labels: map[]
admission-control/manager: 2026/04/02 07:49:47.562450 manager_impl.go:512: Info: Initial resource sync with Sensor complete
2026/04/02 07:50:10 http: TLS handshake error from 10.131.0.17:36630: client sent an HTTP request to an HTTPS server
2026/04/02 07:50:12 http: TLS handshake error from 10.131.0.17:57824: client sent an HTTP request to an HTTPS server
admission-control/settingswatch: 2026/04/02 07:50:18.192295 sensor_push.go:100: Info: Received and propagated updated admission controller settings via sensor push, timestamp: seconds:1775116218 nanos:191075690
admission-control/manager: 2026/04/02 07:50:18.192933 manager_impl.go:425: Info: Applied new admission control settings (enforcing on 0 deploy-time policies: 0 deployment metadata only, 0 image enrichment data required; detecting on 3 run-time policies; enforcing on 0 run-time policies).
admission-control/settingswatch: 2026/04/02 07:50:18.214284 k8s.go:143: Info: Detected and propagated updated admission controller settings via Kubernetes config map watch, timestamp: 2026-04-02T07:50:18.19107569Z
admission-control/settingswatch: 2026/04/02 07:51:17.597528 mount.go:145: Info: Detected and propagated updated admission controller settings via config map mount, timestamp: seconds:1775116218 nanos:191075690
admission-control/manager: 2026/04/02 07:52:22.067124 manager_impl.go:555: Info: Targeted image cache invalidation: invalidated 1 entries
admission-control/manager: 2026/04/02 07:54:12.633256 manager_impl.go:555: Info: Targeted image cache invalidation: invalidated 1 entries

@clickboo
Copy link
Copy Markdown
Contributor Author

clickboo commented Mar 31, 2026

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Mar 31, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@rhacs-bot
Copy link
Copy Markdown
Contributor

rhacs-bot commented Mar 31, 2026

Images are ready for the commit at d82be84.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-529-gd82be84a20.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 31, 2026

Codecov Report

❌ Patch coverage is 31.03448% with 60 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.60%. Comparing base (02759e7) to head (6fed273).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
...r/common/admissioncontroller/management_service.go 0.00% 26 Missing ⚠️
sensor/admission-control/manager/manager_impl.go 62.85% 13 Missing ⚠️
...sor/admission-control/settingswatch/sensor_push.go 0.00% 12 Missing ⚠️
...ommon/admissioncontroller/settings_manager_impl.go 40.00% 6 Missing ⚠️
sensor/common/reprocessor/handler.go 25.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19708      +/-   ##
==========================================
- Coverage   49.60%   49.60%   -0.01%     
==========================================
  Files        2763     2763              
  Lines      208271   208339      +68     
==========================================
+ Hits       103317   103346      +29     
- Misses      97288    97329      +41     
+ Partials     7666     7664       -2     
Flag Coverage Δ
go-unit-tests 49.60% <31.03%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@stackrox stackrox deleted a comment from openshift-ci bot Mar 31, 2026
@clickboo clickboo force-pushed the boo-sensor-send-invalidate-cache branch from 9a957fc to eb11813 Compare March 31, 2026 19:25
@clickboo clickboo force-pushed the boo-adm-cntrl-targeted-invalidation-foundation branch from 1ad1e5f to 782897c Compare April 1, 2026 18:35
@clickboo clickboo force-pushed the boo-sensor-send-invalidate-cache branch from eb11813 to cbd7a3e Compare April 1, 2026 18:35
Base automatically changed from boo-adm-cntrl-targeted-invalidation-foundation to master April 1, 2026 20:49
@clickboo clickboo force-pushed the boo-sensor-send-invalidate-cache branch from cbd7a3e to d2e46f3 Compare April 1, 2026 21:02
@clickboo
Copy link
Copy Markdown
Contributor Author

clickboo commented Apr 1, 2026

/test all

@clickboo clickboo force-pushed the boo-sensor-send-invalidate-cache branch 2 times, most recently from 9c3d8c2 to 8625ac2 Compare April 2, 2026 07:02
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 2, 2026

📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • New Features
    • Added support for targeted image cache invalidation, allowing selective removal of specific images from cache instead of flushing the entire cache. This enables more efficient cache management by invalidating only the necessary entries based on provided image keys.

Walkthrough

This pull request introduces targeted image cache invalidation capability to the admission control system. A new sensor capability constant is added, and the settings manager, manager interface, and management service are extended to support cache invalidation via streams. The reprocessor handler is updated to use targeted invalidation instead of flushing the entire cache.

Changes

Cohort / File(s) Summary
Capability Constants
pkg/centralsensor/caps_list.go
Added new exported sensor capability constant TargetedImageCacheInvalidationCap with value "TargetedImageCacheInvalidation".
Manager Interface & Implementation
sensor/admission-control/manager/manager.go, sensor/admission-control/manager/manager_impl.go
Extended Manager interface with ImageCacheInvalidationC() channel method. Implementation adds imageCacheInvalidationC channel, integrates it into the run() select loop, and processes cache invalidation by iterating image keys, computing appropriate cache keys based on flatten image data setting, removing entries from imageCache, and calling imageFetchGroup.Forget.
Settings Manager Interface & Implementation
sensor/common/admissioncontroller/settings_manager.go, sensor/common/admissioncontroller/settings_manager_impl.go
Extended interface with InvalidateImageCache(keys) method and ImageCacheInvalidationStream() accessor. Implementation initializes imageCacheInvalidationStream, pushes AdmCtrlImageCacheInvalidation objects containing image keys to the stream, and exposes the stream as read-only.
Management Service
sensor/common/admissioncontroller/management_service.go, sensor/common/admissioncontroller/management_service_test.go
Extended Communicate to handle image cache invalidation streams. Added imageCacheInvalidationStream field initialization from manager, created iterator over invalidation stream with select case handling, and introduced sendImageCacheInvalidation() helper to dispatch MsgToAdmissionControl_ImageCacheInvalidation messages. Updated test setup to mock new stream method.
Sensor Push Watch
sensor/admission-control/settingswatch/sensor_push.go
Added support for MsgToAdmissionControl_ImageCacheInvalidation message type by introducing imageCacheInvalidationOutC channel, wiring from manager's ImageCacheInvalidationC(), and dispatching invalidation payloads with context cancellation error handling.
Reprocessor Handler
sensor/common/reprocessor/handler.go, sensor/common/reprocessor/handler_test.go
Updated Capabilities() to advertise TargetedImageCacheInvalidationCap. Changed ProcessInvalidateImageCache() to call InvalidateImageCache(req.GetImageKeys()) instead of FlushCache(), shifting from full cache flush to targeted invalidation. Updated test expectations to match new method call.
Mock Settings Manager
sensor/common/admissioncontroller/mocks/settings_manager.go
Added GoMock support for ImageCacheInvalidationStream() and InvalidateImageCache(keys) methods with corresponding recorder methods for test expectations.

Sequence Diagram

sequenceDiagram
    participant Reprocessor as Reprocessor Handler
    participant SettingsMgr as Settings Manager
    participant Stream as Invalidation Stream
    participant MgmtService as Management Service
    participant Manager as Admission Control Manager
    participant Sensor as Sensor/Client

    Reprocessor->>SettingsMgr: InvalidateImageCache(imageKeys)
    SettingsMgr->>Stream: Push AdmCtrlImageCacheInvalidation
    
    par Management Service Processing
        MgmtService->>Stream: Iterator.Next()
        Stream-->>MgmtService: Return invalidation object
        MgmtService->>Sensor: Send MsgToAdmissionControl_ImageCacheInvalidation
    end
    
    par Manager Processing
        Manager->>Manager: ImageCacheInvalidationC receives message
        Manager->>Manager: Iterate image keys
        Manager->>Manager: Compute cache keys (considering flatten setting)
        Manager->>Manager: Remove entries from imageCache
        Manager->>Manager: Clear name-to-cache-key mappings
        Manager->>Manager: Call imageFetchGroup.Forget()
        Manager->>Manager: Log invalidation count
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is entirely missing. The template requires a detailed description of changes, user-facing documentation checklist, testing and quality checklist, and validation details. Add a detailed PR description covering what changes were made, why they were made, testing performed, and complete the required checklists for documentation and quality assurance.
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'ROX-33333: Sensor targeted cache invalidation changes' is directly related to the main changes, which implement targeted image cache invalidation in the sensor admission controller system across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch boo-sensor-send-invalidate-cache

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
sensor/common/reprocessor/handler_test.go (1)

37-37: Assert invalidation payload content, not only invocation.

Using gomock.Any() can hide regressions where wrong or empty key sets are forwarded. Please assert that InvalidateImageCache receives expected keys (IDs/full names) for each test case.

Also applies to: 121-121

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sensor/common/reprocessor/handler_test.go` at line 37, The test currently
uses gomock.Any() for mockAdmCtrlSettingsMgr.EXPECT().InvalidateImageCache which
only asserts invocation; change the expectation to assert the actual payload
content by matching the expected key set for each test case (use
gomock.Eq(expectedKeys) or gomock.Match(func(v interface{}) bool { /* compare
against expectedKeys */ }) as appropriate) and update both occurrences (the call
at mockAdmCtrlSettingsMgr.EXPECT().InvalidateImageCache and the other occurrence
around lines referenced) so the mock validates the exact IDs/full names
forwarded to InvalidateImageCache rather than accepting any value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@sensor/admission-control/manager/manager_impl.go`:
- Around line 533-541: The loop in the invalidation path currently chooses a
single cache key variant based on m.currentState() flatten mode (using cacheKey
= key.GetImageId() or key.GetImageIdV2()), which can leave the other variant
stale; instead, in the loop over inv.GetImageKeys() always attempt eviction for
both non-empty forms: call the evict/remove logic for key.GetImageId() if
non-empty and separately for key.GetImageIdV2() if non-empty (do not gate on
flatten), and increment invalidated appropriately for each successful eviction;
update the code around the cacheKey/flatten logic in the loop that references
m.currentState(), key.GetImageId(), key.GetImageIdV2(), and the eviction call so
both variants are evicted unconditionally when present.

In `@sensor/common/admissioncontroller/management_service.go`:
- Line 114: Create the invalidation iterator before calling sync so it won't
miss events: move the call to s.imageCacheInvalidationStream.Iterator(true) (the
imageCacheInvIt variable) to immediately before the sync(stream) invocation (or
at the start of the method where the stream is set up) so the iterator exists
prior to s.sync(stream) and will observe any invalidation events emitted during
sync.

In `@sensor/common/reprocessor/handler.go`:
- Around line 65-69: The sender (central) must check sensor capabilities before
broadcasting InvalidateImageCache messages: update the broadcast sites in
service_impl.go (the InvalidateImageCache calls around lines 1277 and 1293) to
gate sending with
AllSensorsHaveCapability(centralsensor.TargetedImageCacheInvalidationCap) or
implement a fallback broadcast path for sensors lacking the capability; ensure
the capability symbol centralsensor.TargetedImageCacheInvalidationCap and the
capability-advertising handler (the function returning
[]centralsensor.SensorCapability in sensor/common/reprocessor/handler.go) are
used to decide routing so old sensors don’t receive unsupported targeted
invalidation messages.

---

Nitpick comments:
In `@sensor/common/reprocessor/handler_test.go`:
- Line 37: The test currently uses gomock.Any() for
mockAdmCtrlSettingsMgr.EXPECT().InvalidateImageCache which only asserts
invocation; change the expectation to assert the actual payload content by
matching the expected key set for each test case (use gomock.Eq(expectedKeys) or
gomock.Match(func(v interface{}) bool { /* compare against expectedKeys */ }) as
appropriate) and update both occurrences (the call at
mockAdmCtrlSettingsMgr.EXPECT().InvalidateImageCache and the other occurrence
around lines referenced) so the mock validates the exact IDs/full names
forwarded to InvalidateImageCache rather than accepting any value.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 7c015b2e-67ba-46bf-94aa-5a2301df47a7

📥 Commits

Reviewing files that changed from the base of the PR and between f042134 and 8625ac2.

📒 Files selected for processing (11)
  • pkg/centralsensor/caps_list.go
  • sensor/admission-control/manager/manager.go
  • sensor/admission-control/manager/manager_impl.go
  • sensor/admission-control/settingswatch/sensor_push.go
  • sensor/common/admissioncontroller/management_service.go
  • sensor/common/admissioncontroller/management_service_test.go
  • sensor/common/admissioncontroller/mocks/settings_manager.go
  • sensor/common/admissioncontroller/settings_manager.go
  • sensor/common/admissioncontroller/settings_manager_impl.go
  • sensor/common/reprocessor/handler.go
  • sensor/common/reprocessor/handler_test.go

Comment thread sensor/admission-control/manager/manager_impl.go
Comment thread sensor/common/admissioncontroller/management_service.go Outdated
Comment thread sensor/common/reprocessor/handler.go Outdated
@clickboo clickboo force-pushed the boo-sensor-send-invalidate-cache branch 2 times, most recently from e354972 to d82be84 Compare April 2, 2026 08:41
@clickboo clickboo marked this pull request as ready for review April 2, 2026 09:26
@clickboo clickboo requested review from a team as code owners April 2, 2026 09:26
sourcery-ai[bot]

This comment was marked as low quality.

Comment thread sensor/admission-control/manager/manager_impl.go
@clickboo clickboo force-pushed the boo-sensor-send-invalidate-cache branch from 0d7675a to 6fed273 Compare April 2, 2026 19:49
@clickboo
Copy link
Copy Markdown
Contributor Author

clickboo commented Apr 2, 2026

/test all

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

🚀 Build Images Ready

Images are ready for commit 2e6eff0. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.11.x-559-g2e6eff05a1

Copy link
Copy Markdown
Contributor

@dcaravel dcaravel left a comment

Choose a reason for hiding this comment

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

LGTM

@clickboo clickboo requested a review from AlexVulaj April 3, 2026 17:59
@clickboo clickboo merged commit 2e6eff0 into master Apr 3, 2026
188 of 190 checks passed
@clickboo clickboo deleted the boo-sensor-send-invalidate-cache branch April 3, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants