PR#406: Rebuild only affected pkgs by the CVE - freshmaker

freshmaker

#406 Rebuild only affected pkgs by the CVE

Merged 4 years ago by jkaluza. Opened 4 years ago by gnaponie.

gnaponie/freshmaker get-only-affected-pkgs into master

Rebuild only affected pkgs by the CVE

gnaponie • 4 years ago

93a4f8a

freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py

file modified

+13

		`@@ -24,6 +24,7 @@`

		`import json`
		`import koji`
		`+ import kobo`

		`from freshmaker import conf, db`
		`from freshmaker.events import ErrataAdvisoryRPMsSignedEvent`
		`@@ -411,6 +412,18 @@`
		`# For each SRPM NVR, find out all the containers which include`
		`# this SRPM NVR.`
		`srpm_nvrs = set(errata.get_builds(errata_id))`
		`+ # affected_pkgs contains the list of actually affected pkgs from the CVE.`
		`+ # We don't need to build images that really don't affect the CVE, even if they are`
		`+ # listed in the RHSA. So let's just remove the ones that are not listed in here.`
		`+ # In case this is empty we are just going to use the "old" behavior before this`
		`+ # change was made, and rebuild everything.`
		`+ affected_pkgs = set([pkg['pkg_name'] for pkg in self.event.advisory.affected_pkgs])`
		`+ if affected_pkgs:`
		`+ tmp_srpm_nvrs = srpm_nvrs`
		`+ srpm_nvrs = set([srpm_nvr for srpm_nvr in srpm_nvrs if kobo.rpmlib.parse_nvr(srpm_nvr)["name"] in affected_pkgs])`
		`+ self.log_info(("Not going to rebuild these container images,"`
		`+ "because they're not affected: %r"), tmp_srpm_nvrs.difference(srpm_nvrs))`
		`+`
		`self.log_info(`
		`"Going to find all the container images to rebuild as "`
		`"result of %r update.", srpm_nvrs)`

tests/handlers/koji/test_rebuild_images_on_rpm_advisory_change.py

file modified

+23

		`@@ -516,6 +516,29 @@`
		`published=True, release_categories=('Generally Available', 'Tech Preview', 'Beta'),`
		`leaf_container_images=["foo", "bar"])`

		`+ @patch.object(freshmaker.conf, 'handler_build_whitelist', new={`
		`+ 'RebuildImagesOnRPMAdvisoryChange': {`
		`+ 'image': {'advisory_name': 'RHBA-*'}`
		`+ }`
		`+ })`
		`+ @patch('os.path.exists', return_value=True)`
		`+ def test_whitelist_affected_packages(self, exists):`
		`+ """`
		`+ In case there are more pkgs in a RHSA, but not all of them are actually affected from the CVE,`
		`+ the images that will have to be rebuild will be only the ones affected.`
		`+ This test is checking this process.`
		`+ """`
		`+ self.event.advisory.affected_pkgs = [{'product': 'whatever', 'pkg_name': 'httpd'}]`
		`+ self.get_builds.return_value = ["httpd-2.4-11.el7", "foo-1-1"]`
		`+ for x in self.handler._find_images_to_rebuild(123456):`
		`+ pass`
		`+`
		`+ self.find_images_to_rebuild.assert_called_once_with(`
		`+ set(['httpd-2.4-11.el7']), ['content-set-1'],`
		`+ filter_fnc=self.handler._filter_out_not_allowed_builds,`
		`+ published=True, release_categories=('Generally Available', 'Tech Preview', 'Beta'),`
		`+ leaf_container_images=None)`
		`+`

		`class TestAllowBuild(helpers.ModelsTestCase):`
		`"""Test RebuildImagesOnRPMAdvisoryChange.allow_build"""`

gnaponie commented 4 years ago

An RHSA that addresses a specific advisory will contain RPM build(s)
for addressing a specific CVE. Freshmaker uses these RPM build(s) to
find out which images should be rebuilt. In some cases, additional
RPM build(s) are attached to RHSA. These are unrelated to the CVE.
With this change Freshmaker is going to rebuild only the affected
pkgs by the CVE, and not all the contained RPMs in the RHSA.

Signed-off-by: gnaponie gnaponie@redhat.com

jkaluza commented on line 7 of freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py 4 years ago

ones

jkaluza commented on line 10 of freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py 4 years ago

I think the affected_pkgs here is the list of package names, while the srpm_nvrs is list of NVRs. For example, the values will be like this:

affected_pkgs = set(["httpd"])
srpm_nvrs = set(["httpd-2.4.6-11", "foo-1-1"])

See my comment below for more info.

Edited 4 years ago by jkaluza

jkaluza commented on line 12 of freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py 4 years ago

The goal of this line should be to keep only "affected" NVRs in the srpm_nvrs - it means you should keep the srpm_nvr which has the same name as the one in affected_pkgs.

Maybe something like this:

if affected_pkgs:
    set([srpm_nvr for srpm_nvr in srpm_nvrs if kobo.rpmlib.parse_nvr(srpm_nvr)["name"] in affected_pkgs])

jkaluza commented on line 12 of freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py 4 years ago

Can you also call self.log_info() with some message showing the list of removed SRPM NVRs, so we know that some NVRs were removed?

jkaluza commented on line 17 of tests/handlers/koji/test_rebuild_images_on_rpm_advisory_change.py 4 years ago

The pkg_name returned by Bugzilla is just httpd.

jkaluza commented on line 23 of tests/handlers/koji/test_rebuild_images_on_rpm_advisory_change.py 4 years ago

You will need to change the test to include more packages, not just httpd. But even in the current test, this should be httpd-2.4-11.el7, because that's the NVR affected by CVE.

gnaponie commented 4 years ago

You will need to change the test to include more packages, not just httpd. But even in the current test, this should be httpd-2.4-11.el7, because that's the NVR affected by CVE.

Yeah... actually I made the opposite. I shouldn't remove the affected pkgs, I should use them as a whitelist.

rebased onto 93a4f8a

4 years ago

gnaponie commented 4 years ago

@jkaluza this is ready for another review.

jkaluza commented 4 years ago