From ce2c222dc28e99c22573b008794e99dcfdd06507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Thu, 2 May 2024 15:09:43 +0200 Subject: [PATCH] createiso: Block reuse if unsigned packages are allowed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can have a compose with unsigned packages. By the time the next compose is generated, the packages could have been signed. However, the new compose would still reuse the ISO with unsigned copies. Signed-off-by: Lubomír Sedlář (cherry picked from commit d546a49299f756d7b7deb33cc744e8b0e33e572e) --- pungi/phases/createiso.py | 8 +++++ pungi/phases/extra_isos.py | 8 +++++ tests/test_createiso_phase.py | 36 ++++++++++++++++++--- tests/test_extra_isos_phase.py | 58 ++++++++++++++++++++++++++++++---- 4 files changed, 98 insertions(+), 12 deletions(-) diff --git a/pungi/phases/createiso.py b/pungi/phases/createiso.py index ec653ad6..57cde7d9 100644 --- a/pungi/phases/createiso.py +++ b/pungi/phases/createiso.py @@ -189,6 +189,14 @@ class CreateisoPhase(PhaseLoggerMixin, PhaseBase): if not old_config: self.logger.info("%s - no config for old compose", log_msg) return False + + # Disable reuse if unsigned packages are allowed. The older compose + # could have unsigned packages, and those may have been signed since + # then. We want to regenerate the ISO to have signatures. + if None in self.compose.conf["sigkeys"]: + self.logger.info("%s - unsigned packages are allowed", log_msg) + return False + # Convert current configuration to JSON and back to encode it similarly # to the old one config = json.loads(json.dumps(self.compose.conf)) diff --git a/pungi/phases/extra_isos.py b/pungi/phases/extra_isos.py index 30dc20fc..ca161b48 100644 --- a/pungi/phases/extra_isos.py +++ b/pungi/phases/extra_isos.py @@ -205,6 +205,14 @@ class ExtraIsosThread(WorkerThread): if not old_config: self.pool.log_info("%s - no config for old compose", log_msg) return False + + # Disable reuse if unsigned packages are allowed. The older compose + # could have unsigned packages, and those may have been signed since + # then. We want to regenerate the ISO to have signatures. + if None in compose.conf["sigkeys"]: + self.pool.log_info("%s - unsigned packages are allowed", log_msg) + return False + # Convert current configuration to JSON and back to encode it similarly # to the old one config = json.loads(json.dumps(compose.conf)) diff --git a/tests/test_createiso_phase.py b/tests/test_createiso_phase.py index 1c57b430..03f141ed 100644 --- a/tests/test_createiso_phase.py +++ b/tests/test_createiso_phase.py @@ -1386,7 +1386,9 @@ class CreateisoTryReusePhaseTest(helpers.PungiTestCase): ) def test_old_config_changed(self): - compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) + compose = helpers.DummyCompose( + self.topdir, {"createiso_allow_reuse": True, "sigkeys": ["abcdef"]} + ) old_config = compose.conf.copy() old_config["release_version"] = "2" compose.load_old_compose_config.return_value = old_config @@ -1399,12 +1401,30 @@ class CreateisoTryReusePhaseTest(helpers.PungiTestCase): phase.try_reuse(cmd, compose.variants["Server"], "x86_64", opts) ) - def test_no_old_metadata(self): + @mock.patch("pungi.phases.createiso.read_json_file") + def test_unsigned_packages_allowed(self, read_json_file): compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) compose.load_old_compose_config.return_value = compose.conf.copy() phase = createiso.CreateisoPhase(compose, mock.Mock()) phase.logger = self.logger cmd = {"disc_num": 1, "disc_count": 1} + + opts = CreateIsoOpts(volid="new-volid") + + read_json_file.return_value = {"opts": {"volid": "old-volid"}} + + self.assertFalse( + phase.try_reuse(cmd, compose.variants["Server"], "x86_64", opts) + ) + + def test_no_old_metadata(self): + compose = helpers.DummyCompose( + self.topdir, {"createiso_allow_reuse": True, "sigkeys": ["abcdef"]} + ) + compose.load_old_compose_config.return_value = compose.conf.copy() + phase = createiso.CreateisoPhase(compose, mock.Mock()) + phase.logger = self.logger + cmd = {"disc_num": 1, "disc_count": 1} opts = CreateIsoOpts() self.assertFalse( @@ -1413,7 +1433,9 @@ class CreateisoTryReusePhaseTest(helpers.PungiTestCase): @mock.patch("pungi.phases.createiso.read_json_file") def test_volume_id_differs(self, read_json_file): - compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) + compose = helpers.DummyCompose( + self.topdir, {"createiso_allow_reuse": True, "sigkeys": ["abcdef"]} + ) compose.load_old_compose_config.return_value = compose.conf.copy() phase = createiso.CreateisoPhase(compose, mock.Mock()) phase.logger = self.logger @@ -1429,7 +1451,9 @@ class CreateisoTryReusePhaseTest(helpers.PungiTestCase): @mock.patch("pungi.phases.createiso.read_json_file") def test_packages_differ(self, read_json_file): - compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) + compose = helpers.DummyCompose( + self.topdir, {"createiso_allow_reuse": True, "sigkeys": ["abcdef"]} + ) compose.load_old_compose_config.return_value = compose.conf.copy() phase = createiso.CreateisoPhase(compose, mock.Mock()) phase.logger = self.logger @@ -1451,7 +1475,9 @@ class CreateisoTryReusePhaseTest(helpers.PungiTestCase): @mock.patch("pungi.phases.createiso.read_json_file") def test_runs_perform_reuse(self, read_json_file): - compose = helpers.DummyCompose(self.topdir, {"createiso_allow_reuse": True}) + compose = helpers.DummyCompose( + self.topdir, {"createiso_allow_reuse": True, "sigkeys": ["abcdef"]} + ) compose.load_old_compose_config.return_value = compose.conf.copy() phase = createiso.CreateisoPhase(compose, mock.Mock()) phase.logger = self.logger diff --git a/tests/test_extra_isos_phase.py b/tests/test_extra_isos_phase.py index 1cd30cb8..6826a9ab 100644 --- a/tests/test_extra_isos_phase.py +++ b/tests/test_extra_isos_phase.py @@ -1156,7 +1156,9 @@ class ExtraisoTryReusePhaseTest(helpers.PungiTestCase): ) def test_buildinstall_changed(self): - compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True}) + compose = helpers.DummyCompose( + self.topdir, {"extraiso_allow_reuse": True, "sigkeys": ["abcdef"]} + ) thread = extra_isos.ExtraIsosThread(compose, mock.Mock()) thread.logger = self.logger thread.bi = mock.Mock() @@ -1170,7 +1172,9 @@ class ExtraisoTryReusePhaseTest(helpers.PungiTestCase): ) def test_no_old_config(self): - compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True}) + compose = helpers.DummyCompose( + self.topdir, {"extraiso_allow_reuse": True, "sigkeys": ["abcdef"]} + ) thread = extra_isos.ExtraIsosThread(compose, mock.Mock()) thread.logger = self.logger opts = CreateIsoOpts() @@ -1182,7 +1186,9 @@ class ExtraisoTryReusePhaseTest(helpers.PungiTestCase): ) def test_old_config_changed(self): - compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True}) + compose = helpers.DummyCompose( + self.topdir, {"extraiso_allow_reuse": True, "sigkeys": ["abcdef"]} + ) old_config = compose.conf.copy() old_config["release_version"] = "2" compose.load_old_compose_config.return_value = old_config @@ -1197,7 +1203,9 @@ class ExtraisoTryReusePhaseTest(helpers.PungiTestCase): ) def test_no_old_metadata(self): - compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True}) + compose = helpers.DummyCompose( + self.topdir, {"extraiso_allow_reuse": True, "sigkeys": ["abcdef"]} + ) compose.load_old_compose_config.return_value = compose.conf.copy() thread = extra_isos.ExtraIsosThread(compose, mock.Mock()) thread.logger = self.logger @@ -1211,7 +1219,9 @@ class ExtraisoTryReusePhaseTest(helpers.PungiTestCase): @mock.patch("pungi.phases.extra_isos.read_json_file") def test_volume_id_differs(self, read_json_file): - compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True}) + compose = helpers.DummyCompose( + self.topdir, {"extraiso_allow_reuse": True, "sigkeys": ["abcdef"]} + ) compose.load_old_compose_config.return_value = compose.conf.copy() thread = extra_isos.ExtraIsosThread(compose, mock.Mock()) thread.logger = self.logger @@ -1228,7 +1238,9 @@ class ExtraisoTryReusePhaseTest(helpers.PungiTestCase): @mock.patch("pungi.phases.extra_isos.read_json_file") def test_packages_differ(self, read_json_file): - compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True}) + compose = helpers.DummyCompose( + self.topdir, {"extraiso_allow_reuse": True, "sigkeys": ["abcdef"]} + ) compose.load_old_compose_config.return_value = compose.conf.copy() thread = extra_isos.ExtraIsosThread(compose, mock.Mock()) thread.logger = self.logger @@ -1250,7 +1262,7 @@ class ExtraisoTryReusePhaseTest(helpers.PungiTestCase): ) @mock.patch("pungi.phases.extra_isos.read_json_file") - def test_runs_perform_reuse(self, read_json_file): + def test_unsigned_packages(self, read_json_file): compose = helpers.DummyCompose(self.topdir, {"extraiso_allow_reuse": True}) compose.load_old_compose_config.return_value = compose.conf.copy() thread = extra_isos.ExtraIsosThread(compose, mock.Mock()) @@ -1273,6 +1285,38 @@ class ExtraisoTryReusePhaseTest(helpers.PungiTestCase): }, } + self.assertFalse( + thread.try_reuse( + compose, compose.variants["Server"], "x86_64", "abcdef", opts + ) + ) + + @mock.patch("pungi.phases.extra_isos.read_json_file") + def test_runs_perform_reuse(self, read_json_file): + compose = helpers.DummyCompose( + self.topdir, {"extraiso_allow_reuse": True, "sigkeys": ["abcdef"]} + ) + compose.load_old_compose_config.return_value = compose.conf.copy() + thread = extra_isos.ExtraIsosThread(compose, mock.Mock()) + thread.logger = self.logger + thread.perform_reuse = mock.Mock() + + new_graft_points = os.path.join(self.topdir, "new_graft_points") + helpers.touch(new_graft_points) + opts = CreateIsoOpts(graft_points=new_graft_points, volid="volid") + + old_graft_points = os.path.join(self.topdir, "old_graft_points") + helpers.touch(old_graft_points) + dummy_iso_path = "dummy-iso-path/dummy.iso" + read_json_file.return_value = { + "opts": { + "graft_points": old_graft_points, + "volid": "volid", + "output_dir": os.path.dirname(dummy_iso_path), + "iso_name": os.path.basename(dummy_iso_path), + }, + } + self.assertTrue( thread.try_reuse( compose, compose.variants["Server"], "x86_64", "abcdef", opts