Open joshuai96 opened 3 months ago
It was intentional to only validate the DB on download and not on start (for performance reasons) since validating every time could lead to startup performance issues. That being said, I can also see the point where getting grype results with a known bad database is also not ideal, since we can no longer trust those results.
I think ultimately it's the right call to turn on validation by default, however, I feel that it should coincide with attempting to improve performance. Right now we're working on grype DB v6, which is very different than DB v1-5. Part of this work may also switch the distributing sha256 digests to xxh64, which is much faster.
One question I have: I have an anecdote from the past that startup was taking an extra second due to hashing the DB -- what extra time at startup are you seeing with this change @joshuai96 ?
In the setup I described in https://github.com/anchore/grype/issues/1975#issuecomment-2208251742 I've seen grype db update
stating Vulnerability database updated to latest version!
and then having some files missing for example metadata.json
, what leads to failing scan runs. So I think validation on download seems a little bit buggy, too. I am positive this is some kind of freak accident based on network hiccups, as our GitLab CI runners are known to have some networking issue.
You mean the startup before a scan runs, @wagoodman ? I didn't notice any inconvenient increase in time, but I can run some tests, to have some numbers backing that up. IMHO a second or two, has no impact on a scan that usually takes a few seconds anyway, but I'm not as sensitive to time, as this scans run over night in a CI/CD setup.
Hello @wagoodman, sorry for the delay.
I've run some simple timing tests for both versions.
Setup:
time
s real
output in combination with a small image like apline (using alpine:3.20.2
with hash 0a4eaa0eecf5f8c050e5bba433f58c052be7587ee8af3e8b3910ef9ab5fbe9f5
) to reduce scan time and see db hash time.go build -ldflags "-s -w" -o bin/grype cmd/grype/main.go
Test:
Test No. | Unmodified | Modified |
---|---|---|
1 | 3,632 | 3,628 |
2 | 3,730 | 3,591 |
3 | 3,653 | 3,640 |
4 | 3,749 | 3,661 |
5 | 3,526 | 3,621 |
6 | 3,596 | 3,577 |
7 | 3,707 | 3,740 |
8 | 3,568 | 3,613 |
9 | 3,675 | 3,662 |
10 | 3,585 | 3,600 |
11 | 3,724 | 3,653 |
12 | 3,630 | 3,588 |
13 | 3,582 | 3,680 |
14 | 3,647 | 3,626 |
15 | 3,649 | 3,673 |
16 | 3,657 | 3,607 |
17 | 3,695 | 3,567 |
18 | 3,640 | 3,608 |
19 | 3,902 | 3,616 |
20 | 3,686 | 3,700 |
Average | 3,662 | 3,633 |
Standard deviation | 0,079 | 0,043 |
Result:
Looking at these numbers it may indicate that checking the database on startup decreased scanning time and made it more stable. But the sample size and incrrease are so small, that this can not be stated with confidence.
FWIW, enabling this option in the latest grype seems to increase the scan time of alpine:latest
by ~7 seconds on my x86 mac.
@wagoodman any idea, why I don't see a slowdown of scans?
I think we'll include this with the grype DB v6 work (as mentioned above) -- so we won't make this change in v5. Keep this PR open and we can rebase on that work and pull it in 👍
Fixes: #1975 Fixes: #1648
Setting
ValidateByHashOnStart
totrue
by default, enables the database curator to executeValidateByHash
invalidateIntegrity
and report an invalid database.Scans with
grype
now give a better error too: