Skip to content

ROX-33977: risk component mapper#19841

Open
dashrews78 wants to merge 4 commits intomasterfrom
dashrews/risk-component-mapper-33977
Open

ROX-33977: risk component mapper#19841
dashrews78 wants to merge 4 commits intomasterfrom
dashrews/risk-component-mapper-33977

Conversation

@dashrews78
Copy link
Copy Markdown
Contributor

Description

fix: restore component risk score population in ranker

PR #17422 removed UpsertRisk calls from reprocessImageComponentRisk and
reprocessNodeComponentRisk because the risk table rows were never read.
However, UpsertRisk had a side effect: it called ranker.Add(), which
populated the in-memory component ranker. Downstream, updateComponentRisk
in the image/node datastores reads from this ranker before every DB write,
overwriting component.RiskScore with the ranker's value (0 for missing
entries). This caused image_component_v2.riskscore to always be 0.

Fix: call ranker.Add() directly after calculating the risk score.
Tests: add regression tests verifying the ranker is updated.

Partially generated by AI.

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

Added a unit test to catch this regression in the future. Also tested manually.

dashrews78 and others added 2 commits April 6, 2026 12:10
PR #17422 removed UpsertRisk calls from reprocessImageComponentRisk and
reprocessNodeComponentRisk because the risk table rows were never read.
However, UpsertRisk had a side effect: it called ranker.Add(), which
populated the in-memory component ranker. Downstream, updateComponentRisk
in the image/node datastores reads from this ranker before every DB write,
overwriting component.RiskScore with the ranker value (0 for missing
entries). This caused image_component_v2.riskscore to always be 0.

Fix: call ranker.Add() directly after calculating the risk score.
Tests: add regression tests verifying the ranker is updated.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 6, 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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

🚀 Build Images Ready

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

export MAIN_IMAGE_TAG=4.11.x-571-g2fcf3441a8

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro

Run ID: 1f9c5843-6839-49de-b158-1f861c40bfc5

📥 Commits

Reviewing files that changed from the base of the PR and between a4c1d5f and 2fcf344.

📒 Files selected for processing (3)
  • central/image/datastore/datastore_impl_flat_postgres_test.go
  • central/imagev2/datastoretest/datastore_impl_test.go
  • central/risk/manager/manager_test.go
✅ Files skipped from review due to trivial changes (1)
  • central/image/datastore/datastore_impl_flat_postgres_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • central/imagev2/datastoretest/datastore_impl_test.go
  • central/risk/manager/manager_test.go

📝 Walkthrough

Summary by CodeRabbit

  • Tests

    • Added integration tests to verify component risk scores are persisted and retrieved correctly.
    • Added unit tests ensuring risk-score recalculation updates in-memory tracking during processing.
  • Bug Fixes

    • Improved risk-score tracking so image and node component scores are consistently updated in-memory as well as persisted.

Walkthrough

Adds tests verifying component risk scores persist through image upserts and changes the risk manager to immediately record computed component risk scores in in-memory rankers during reprocessing.

Changes

Cohort / File(s) Summary
Image datastore tests
central/image/datastore/datastore_impl_flat_postgres_test.go, central/imagev2/datastoretest/datastore_impl_test.go
Added TestComponentRiskScorePersistedToDB tests that pre-populate component rankers with deterministic scores, call UpsertImage for images with three components, then GetImage and assert each component's persisted risk equals the expected score (within tolerance).
Risk manager implementation
central/risk/manager/manager.go
Updated reprocessImageComponentRisk and reprocessNodeComponentRisk to write computed risk scores into the in-memory component rankers (scancomponent.ComponentIDV2(...) and scancomponent.ComponentID(...)) immediately after computing RiskScore.
Risk manager unit tests
central/risk/manager/manager_test.go
Added test-only stub scorers and unit tests (TestReprocessImageComponentRiskUpdatesRanker, TestReprocessNodeComponentRiskUpdatesRanker) to assert reprocessing sets component RiskScore and updates the corresponding ranker entries.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Manager
participant Ranker
participant Datastore
participant Database

Manager->>Manager: compute component risk
Manager->>Ranker: update ranker with componentID and score
Manager->>Datastore: UpsertImage(image with components)
Datastore->>Database: persist image and component entries (including risk_score)
Datastore->>Database: commit
Datastore->>Datastore: read back persisted image
Datastore->>Manager: return image/components (with persisted risk_score)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'ROX-33977: risk component mapper' directly references the issue and describes the main change: fixing the component risk score mapping/ranker population issue.
Description check ✅ Passed The description provides a clear explanation of the bug fix, root cause analysis, solution approach, and testing strategy. It addresses all key sections of the template with substantive content.

✏️ 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 dashrews/risk-component-mapper-33977

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

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.61%. Comparing base (065e233) to head (2fcf344).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #19841   +/-   ##
=======================================
  Coverage   49.60%   49.61%           
=======================================
  Files        2763     2763           
  Lines      208339   208351   +12     
=======================================
+ Hits       103342   103364   +22     
+ Misses      97331    97319   -12     
- Partials     7666     7668    +2     
Flag Coverage Δ
go-unit-tests 49.61% <100.00%> (+<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.

dashrews78 and others added 2 commits April 6, 2026 13:10
Add TestComponentRiskScorePersistedToDB to both v1 and v2 image datastore
test suites. These tests pre-populate the component ranker with known
scores, upsert an image, then read back from DB to verify the scores
survive through updateComponentRisk and into the database.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@dashrews78 dashrews78 force-pushed the dashrews/risk-component-mapper-33977 branch from a4c1d5f to 2fcf344 Compare April 6, 2026 17:11
@dashrews78 dashrews78 marked this pull request as ready for review April 6, 2026 17:44
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.

1 participant