fix(api): align latest_resources scan selection with completed_at#10802
Merged
Conversation
Contributor
|
✅ All necessary |
Contributor
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
The /finding-groups/latest/<check_id>/resources endpoint picked the latest scan per provider by -inserted_at, while /finding-groups/latest and the daily-summary upsert key by midnight(completed_at). When two scans for the same provider overlap (one started earlier but completed later than a short scan that ran in between), the two endpoints read from different scans and reported diverging delta / new_count values. Reorder by -completed_at, -inserted_at so all paths resolve to the same scan. Adds a regression test covering the overlapping-scan case.
74aeec8 to
9c82f78
Compare
Contributor
🔒 Container Security ScanImage: 📊 Vulnerability Summary
4 package(s) affected
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10802 +/- ##
=======================================
Coverage 93.47% 93.47%
=======================================
Files 228 228
Lines 32027 32055 +28
=======================================
+ Hits 29936 29964 +28
Misses 2091 2091
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Collaborator
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
/finding-groups/latest/<check_id>/resourcesand/finding-groups/latestwere disagreeing about which scan represents the "latest state" for a provider, which showed up as divergingdelta/new_countvalues between the two endpoints on the same finding group.Reproduced against a tenant with two overlapping completed scans for the same AWS provider:
/latest/<check_id>/resourcesreturned 4 resources withdelta=new(findings from the short scan)./latestfor the samecheck_idreturnednew_count=1/new_fail_count=1(summed from the long scan's daily summary).reaggregate_finding_groups_for_providerdid not fix it, because that command also orders by-completed_atand therefore refreshes the long scan's summary every time.Root cause:
latest_resourcespicked the latest completed scan per provider by-inserted_at, while the/latestsummary path reads theFindingGroupDailySummaryrow with the greatestinserted_at(which the upsert keys onmidnight(scan.completed_at)), and the reaggregation management command orders by-completed_at, -inserted_at. Whenever two scans overlap, the two criteria resolve to different scans and the two endpoints report counts derived from different underlying data.Description
Change
FindingGroupViewSet.latest_resources(api/src/backend/api/v1/views.py) to order candidate completed scans by-completed_at, -inserted_atbeforedistinct("provider_id"), matching the criterion already used by the/latestsummary path and byreaggregate_finding_groups_for_provider. After the fix all three paths always converge on the same scan for a given provider.Added a regression test
TestFindingGroupViewSet::test_latest_resources_picks_scan_by_completed_at_when_overlapthat builds the overlapping-scan shape above in the fixture, creates one finding in each scan for the samecheck_id, and asserts the endpoint returns the finding from the scan with the greatestcompleted_at(not the greatestinserted_at).No schema, serializer, or response-shape change. Other endpoints that also pick "latest scan per provider" by
-inserted_at(e.g. resources listings atviews.py:2970,3107,3688,3716,5028,5085,5138) are left untouched in this PR and can be aligned separately.Steps to review
cd api poetry run pytest src/backend/api/tests/test_views.py::TestFindingGroupViewSet::test_latest_resources_picks_scan_by_completed_at_when_overlap -x --tb=shortcheck_id;/latesttotals should match the sum ofdelta=newrows in/latest/<check_id>/resources:views.pyand confirm the ordering is now("provider_id", "-completed_at", "-inserted_at").Checklist
Community Checklist
API (if applicable)
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.