fix: only care about ecosystem suffix if present in both ecosystems when determining equality#1007
Conversation
|
|
||
| --- | ||
|
|
||
| [TestRun/PURL_SBOM_case_sensitivity_(api) - 1] |
There was a problem hiding this comment.
the fact that the offline output has vulnerabilities whereas the online one doesn't indicates there's a potential bug in the api's Alpine comparator 🤔
There was a problem hiding this comment.
Yes there is a bug in alpine version enumeration, just fixed this week in: google/osv.dev#2241 but not in production yet.
There was a problem hiding this comment.
Cool - I'd seen a few of those go around this/last week, so figured it might have been known 😅
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1007 +/- ##
=======================================
Coverage ? 65.30%
=======================================
Files ? 150
Lines ? 12535
Branches ? 0
=======================================
Hits ? 8186
Misses ? 3884
Partials ? 465 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
another-rex
left a comment
There was a problem hiding this comment.
LGTM, thanks!. Going to wait till the next OSV.dev release (with the alpine enumeration fix) before merging this, so we'll have the correct snapshots.
|
Turns out we're not accurately sorting the table output at least for local - I'll tackle that somewhere... |
1efc103 to
c2baa5d
Compare
|
I cannot for the life of me replicate that sorting difference locally 😕 |
I tried running |
|
Yeah, it seems that every couple of hours the order changes - I'm not sure if its coinciding with something on the osv.dev side like work being landed or if it's just some clock-shift based randomness, but it seems to be "hours" based rather than "seconds" based... |
Yeah I see what you mean. I just tested again and the result shows different. |
|
I had a look with @michaelkedar on this issue. We noticed that the result changes after the |
|
oohh good find - fwiw locally I've been playing around with sorting the table rows within packages by their url though that got blocked by it being an |
|
Hmm... is this behaviour new? All.zip is built concurrently, but that has always been the case if I understand correctly. |
I don't think this is a new behaviour. Even though all.zip is built concurrently, the files are usually modified in alphabetical order, just a few are out of place. That's probably why most of our tests are passing. But this PR has a bunch of vulnerabilities with adjacency IDs (like 9840, 9841, 9842, and 9843), which is probably why it's failing. But I am not 100% sure if this is the issue as the modified order doesn't really align with the scanner output order. |
|
I think this just needs a flag change in the test to match the new download-offline-db flag and should be good to merge. |
This changes how we compare ecosystems so that the suffix is only considered when it is present for both ecosystems being compared, since we can't reliably extract that.
Resolves #769