ROX-6988: Fix RHEL updates by deleting old CVEs when RH advisories are updated #456
Conversation
connorgorman
force-pushed
the
cgorman-rhel
branch
from
1551775 to
1ab54c9
Compare
connorgorman
added
the
generate-dumps-on-pr
connorgorman
force-pushed
the
cgorman-rhel
branch
from
7ad1283 to
3d90b7a
Compare
|
@RTann I cleaned this up. There are 900k fewer vulns in the DB on start. Also, modules are correctly scoped. The vuln creation does delete some CVEs and I think those are expected and are oval issues |
|
|
||
| func TestRHELv2VulnerabilityCVERemoval(t *testing.T) { | ||
| datastore, err := openDatabaseForTest("TestRHELv2VulnerabilityCVERemoval", false) | ||
| if err != nil { |
There was a problem hiding this comment.
you fixed a separate one but not this one lol
| CVEs: []string{"CVE-2021-1234", "CVE-2021-1235"}, | ||
| } | ||
|
|
||
| err = datastore.InsertRHELv2Vulnerabilities([]*database.RHELv2Vulnerability{v3}) |
There was a problem hiding this comment.
what happens if i insert all 3 cves in this call?
There was a problem hiding this comment.
If the vulnerability is added after the RHSA then it will be in the DB. Realistically, this shouldn't happen though because the RHSAs and CVEs should not overlap and mostly this is the case as seen through the genesis dump
| } | ||
| } | ||
| for module, packages := range packagesByModule { | ||
| result, err := vulnDeleteStatement.Exec(pq.Array(vuln.CVEs), pq.Array(packages), pq.Array(vuln.CPEs), module) |
There was a problem hiding this comment.
I'm still curious if it's possible we find a value in this cross-product that is in the DB but should not actually be deleted. Is that possible?
There was a problem hiding this comment.
I went through the genesis dump vulns that were printed and they all looked like they should be legitimately removed. My understanding of the code (you would know better) is that we'll be inserting an RHSA with all the packages and cpes so the vulns will match against that. I would not expect both an RHSA and CVE to match on the same package and cpe if the RHSA fixes that CVE
| cris := cris[:0] | ||
| walkCriterion(&def.Criteria, &cris) | ||
| enabledModules := getEnabledModules(cris) |
There was a problem hiding this comment.
so before we'd get all the modules mentions in the CVE and attribute them to each package? Now we are only attributing the module relevant to the package? We should sync with Hank since this is in claircore as well
There was a problem hiding this comment.
Yes, this is not correct because the criterions AND the module and the packages
There was a problem hiding this comment.
Basically, we were deleting too many things prior to this change
There was a problem hiding this comment.
makes me concerned we were inserting entries into the DB that should have never existed. At least, they won't match any vulnerabilities (I don't think), so not the end of the world. But rather, potentially, wasteful, no?
There was a problem hiding this comment.
Yes, we were inserting entries that should never have existed, but honestly not very many
| cris := cris[:0] | ||
| walkCriterion(&def.Criteria, &cris) | ||
| enabledModules := getEnabledModules(cris) |
There was a problem hiding this comment.
makes me concerned we were inserting entries into the DB that should have never existed. At least, they won't match any vulnerabilities (I don't think), so not the end of the world. But rather, potentially, wasteful, no?
|
|
||
| func TestRHELv2VulnerabilityCVERemoval(t *testing.T) { | ||
| datastore, err := openDatabaseForTest("TestRHELv2VulnerabilityCVERemoval", false) | ||
| if err != nil { |
There was a problem hiding this comment.
you fixed a separate one but not this one lol
connorgorman
force-pushed
the
cgorman-rhel
branch
from
ff51949 to
a89f432
Compare
connorgorman
changed the title
No description provided.