diff --git a/pungi/createiso.py b/pungi/createiso.py index 26f7a643..e68932eb 100644 --- a/pungi/createiso.py +++ b/pungi/createiso.py @@ -159,15 +159,11 @@ def write_xorriso_commands(opts): script = os.path.join(opts.script_dir, "xorriso-%s.txt" % id(opts)) with open(script, "w") as f: - emit(f, "-indev %s" % opts.boot_iso) - emit(f, "-outdev %s" % os.path.join(opts.output_dir, opts.iso_name)) - emit(f, "-boot_image any replay") + for cmd in iso.xorriso_commands( + opts.arch, opts.boot_iso, os.path.join(opts.output_dir, opts.iso_name) + ): + emit(f, " ".join(cmd)) emit(f, "-volid %s" % opts.volid) - # isoinfo -J uses the Joliet tree, and it's used by virt-install - emit(f, "-joliet on") - # Support long filenames in the Joliet trees. Repodata is particularly - # likely to run into this limit. - emit(f, "-compliance joliet_long_names") with open(opts.graft_points) as gp: for line in gp: @@ -178,10 +174,6 @@ def write_xorriso_commands(opts): emit(f, "%s %s %s" % (cmd, fs_path, iso_path)) emit(f, "-chmod 0%o %s" % (_get_perms(fs_path), iso_path)) - if opts.arch == "ppc64le": - # This is needed for the image to be bootable. - emit(f, "-as mkisofs -U --") - emit(f, "-chown_r 0 /") emit(f, "-chgrp_r 0 /") emit(f, "-end") diff --git a/pungi/phases/createiso.py b/pungi/phases/createiso.py index dd197899..ec653ad6 100644 --- a/pungi/phases/createiso.py +++ b/pungi/phases/createiso.py @@ -14,6 +14,7 @@ # along with this program; if not, see . +import itertools import os import random import shutil @@ -23,7 +24,7 @@ import json import productmd.treeinfo from productmd.images import Image from kobo.threads import ThreadPool, WorkerThread -from kobo.shortcuts import run, relative_path +from kobo.shortcuts import run, relative_path, compute_file_checksums from six.moves import shlex_quote from pungi.wrappers import iso @@ -457,7 +458,14 @@ class CreateIsoThread(WorkerThread): try: run_createiso_command( - num, compose, bootable, arch, cmd["cmd"], mounts, log_file + num, + compose, + bootable, + arch, + cmd["cmd"], + mounts, + log_file, + cmd["iso_path"], ) except Exception: self.fail(compose, cmd, variant, arch) @@ -538,7 +546,9 @@ def add_iso_to_metadata( return img -def run_createiso_command(num, compose, bootable, arch, cmd, mounts, log_file): +def run_createiso_command( + num, compose, bootable, arch, cmd, mounts, log_file, iso_path +): packages = [ "coreutils", "xorriso" if compose.conf.get("createiso_use_xorrisofs") else "genisoimage", @@ -580,6 +590,76 @@ def run_createiso_command(num, compose, bootable, arch, cmd, mounts, log_file): weight=compose.conf["runroot_weights"].get("createiso"), ) + if bootable and compose.conf.get("createiso_use_xorrisofs"): + fix_treeinfo_checksums(compose, iso_path, arch) + + +def fix_treeinfo_checksums(compose, iso_path, arch): + """It is possible for the ISO to contain a .treefile with incorrect + checksums. By modifying the ISO (adding files) some of the images may + change. + + This function fixes that after the fact by looking for incorrect checksums, + recalculating them and updating the .treeinfo file. Since the size of the + file doesn't change, this seems to not change any images. + """ + modified = False + with iso.mount(iso_path, compose._logger) as mountpoint: + ti = productmd.TreeInfo() + ti.load(os.path.join(mountpoint, ".treeinfo")) + for image, (type_, expected) in ti.checksums.checksums.items(): + checksums = compute_file_checksums(os.path.join(mountpoint, image), [type_]) + actual = checksums[type_] + if actual == expected: + # Everything fine here, skip to next image. + continue + + compose.log_debug("%s: %s: checksum mismatch", iso_path, image) + # Update treeinfo with correct checksum + ti.checksums.checksums[image] = (type_, actual) + modified = True + + if not modified: + compose.log_debug("%s: All checksums match, nothing to do.", iso_path) + return + + try: + tmpdir = compose.mkdtemp(arch, prefix="fix-checksum-") + # Write modified .treeinfo + ti_path = os.path.join(tmpdir, ".treeinfo") + compose.log_debug("Storing modified .treeinfo in %s", ti_path) + ti.dump(ti_path) + # Write a modified DVD into a temporary path, that is atomically moved + # over the original file. + fixed_path = os.path.join(tmpdir, "fixed-checksum-dvd.iso") + cmd = ["xorriso"] + cmd.extend( + itertools.chain.from_iterable( + iso.xorriso_commands(arch, iso_path, fixed_path) + ) + ) + cmd.extend(["-map", ti_path, ".treeinfo"]) + run( + cmd, + logfile=compose.paths.log.log_file( + arch, "checksum-fix_generate_%s" % os.path.basename(iso_path) + ), + ) + # The modified ISO no longer has implanted MD5, so that needs to be + # fixed again. + compose.log_debug("Implanting new MD5 to %s fixed_path") + run( + iso.get_implantisomd5_cmd(fixed_path, compose.supported), + logfile=compose.paths.log.log_file( + arch, "checksum-fix_implantisomd5_%s" % os.path.basename(iso_path) + ), + ) + # All done, move the updated image to the final location. + compose.log_debug("Updating %s", iso_path) + os.rename(fixed_path, iso_path) + finally: + shutil.rmtree(tmpdir) + def split_iso(compose, arch, variant, no_split=False, logger=None): """ diff --git a/pungi/phases/extra_isos.py b/pungi/phases/extra_isos.py index 3b0e25ec..da396d2b 100644 --- a/pungi/phases/extra_isos.py +++ b/pungi/phases/extra_isos.py @@ -166,6 +166,7 @@ class ExtraIsosThread(WorkerThread): log_file=compose.paths.log.log_file( arch, "extraiso-%s" % os.path.basename(iso_path) ), + iso_path=iso_path, ) img = add_iso_to_metadata( diff --git a/pungi/wrappers/iso.py b/pungi/wrappers/iso.py index ab273027..1f017b7c 100644 --- a/pungi/wrappers/iso.py +++ b/pungi/wrappers/iso.py @@ -516,3 +516,21 @@ def mount(image, logger=None, use_guestmount=True): util.run_unmount_cmd(["fusermount", "-u", mount_dir], path=mount_dir) else: util.run_unmount_cmd(["umount", mount_dir], path=mount_dir) + + +def xorriso_commands(arch, input, output): + """List of xorriso commands to modify a bootable image.""" + commands = [ + ("-indev", input), + ("-outdev", output), + # isoinfo -J uses the Joliet tree, and it's used by virt-install + ("-joliet", "on"), + # Support long filenames in the Joliet trees. Repodata is particularly + # likely to run into this limit. + ("-compliance", "joliet_long_names"), + ("-boot_image", "any", "replay"), + ] + if arch == "ppc64le": + # This is needed for the image to be bootable. + commands.append(("-as", "mkisofs", "-U", "--")) + return commands diff --git a/tests/helpers.py b/tests/helpers.py index 41df06e8..5fc788bd 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -2,6 +2,7 @@ import difflib import errno +import hashlib import os import shutil import tempfile @@ -364,3 +365,9 @@ def fake_run_in_threads(func, params, threads=None): """Like run_in_threads from Kobo, but actually runs tasks serially.""" for num, param in enumerate(params): func(None, param, num) + + +def hash_string(alg, s): + m = hashlib.new(alg) + m.update(s.encode("utf-8")) + return m.hexdigest() diff --git a/tests/test_createiso_phase.py b/tests/test_createiso_phase.py index d135f811..bb9b6892 100644 --- a/tests/test_createiso_phase.py +++ b/tests/test_createiso_phase.py @@ -2,6 +2,7 @@ import logging +import contextlib try: from unittest import mock @@ -9,6 +10,7 @@ except ImportError: import mock import six +import productmd import os from tests import helpers @@ -1607,3 +1609,103 @@ class ComposeConfGetIsoLevelTest(helpers.PungiTestCase): compose, compose.variants["Client"], "x86_64" ), ) + + +def mk_mount(topdir, images): + @contextlib.contextmanager + def dummy_mount(path, logger): + treeinfo = [ + "[general]", + "family = Test", + "version = 1.0", + "arch = x86_64", + "variant = Server", + "[checksums]", + ] + for image in images: + helpers.touch(os.path.join(topdir, image.path), image.content) + treeinfo.append("%s = sha256:%s" % (image.path, image.checksum)) + helpers.touch(os.path.join(topdir, ".treeinfo"), "\n".join(treeinfo)) + yield topdir + + return dummy_mount + + +class _MockRun: + """This class replaces kobo.shortcuts.run and validates that the correct + two commands are called. The assertions can not be done after the tested + function finishes because it will clean up the .treeinfo file that needs to + be checked. + """ + + def __init__(self): + self.num_calls = 0 + self.asserts = [self._assert_xorriso, self._assert_implantisomd5] + + def __call__(self, cmd, logfile): + self.num_calls += 1 + self.asserts.pop(0)(cmd) + + def _assert_xorriso(self, cmd): + assert cmd[0] == "xorriso" + ti = productmd.TreeInfo() + input_iso = None + for i, arg in enumerate(cmd): + if arg == "-map": + ti.load(cmd[i + 1]) + if arg == "-outdev": + self.temp_iso = cmd[i + 1] + if arg == "-indev": + input_iso = cmd[i + 1] + assert self.input_iso == input_iso + assert ti.checksums.checksums[self.image_relative_path] == self.image_checksum + + def _assert_implantisomd5(self, cmd): + assert cmd[0] == "/usr/bin/implantisomd5" + assert cmd[-1] == self.temp_iso + + +class DummyImage: + def __init__(self, path, content, checksum=None): + self.path = path + self.content = content + self.checksum = checksum or helpers.hash_string("sha256", content) + + +@mock.patch("os.rename") +@mock.patch("pungi.phases.createiso.run", new_callable=_MockRun) +class FixChecksumsTest(helpers.PungiTestCase): + def test_checksum_matches(self, mock_run, mock_rename): + compose = helpers.DummyCompose(self.topdir, {}) + arch = "x86_64" + iso_path = "DUMMY_ISO" + + with mock.patch( + "pungi.wrappers.iso.mount", + new=mk_mount(self.topdir, [DummyImage("images/eltorito.img", "eltorito")]), + ): + createiso.fix_treeinfo_checksums(compose, iso_path, arch) + + self.assertEqual(mock_run.num_calls, 0) + self.assertEqual(mock_rename.call_args_list, []) + + def test_checksum_fix(self, mock_run, mock_rename): + compose = helpers.DummyCompose(self.topdir, {}) + arch = "x86_64" + img = "images/eltorito.img" + content = "eltorito" + iso_path = "DUMMY_ISO" + mock_run.input_iso = iso_path + mock_run.image_relative_path = "images/eltorito.img" + mock_run.image_checksum = ("sha256", helpers.hash_string("sha256", content)) + + with mock.patch( + "pungi.wrappers.iso.mount", + new=mk_mount(self.topdir, [DummyImage(img, content, "abc")]), + ): + createiso.fix_treeinfo_checksums(compose, iso_path, arch) + + # The new image was copied over the old one + self.assertEqual( + mock_rename.call_args_list, [mock.call(mock_run.temp_iso, iso_path)] + ) diff --git a/tests/test_extra_isos_phase.py b/tests/test_extra_isos_phase.py index 9d60eee3..b90deeb0 100644 --- a/tests/test_extra_isos_phase.py +++ b/tests/test_extra_isos_phase.py @@ -134,6 +134,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): ), ], ) + iso_path = os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso") self.assertEqual( rcc.call_args_list, [ @@ -152,6 +153,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): log_file=os.path.join( self.topdir, "logs/x86_64/extraiso-my.iso.x86_64.log" ), + iso_path=iso_path, ) ], ) @@ -162,7 +164,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): compose, server, "x86_64", - os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso"), + iso_path, True, additional_variants=["Client"], ) @@ -209,6 +211,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): ), ], ) + iso_path = os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso") self.assertEqual( rcc.call_args_list, [ @@ -227,6 +230,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): log_file=os.path.join( self.topdir, "logs/x86_64/extraiso-my.iso.x86_64.log" ), + iso_path=iso_path, ) ], ) @@ -237,7 +241,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): compose, server, "x86_64", - os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso"), + iso_path, True, additional_variants=["Client"], ) @@ -282,6 +286,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): ), ], ) + iso_path = os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso") self.assertEqual( rcc.call_args_list, [ @@ -300,6 +305,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): log_file=os.path.join( self.topdir, "logs/x86_64/extraiso-my.iso.x86_64.log" ), + iso_path=iso_path, ) ], ) @@ -310,7 +316,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): compose, server, "x86_64", - os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso"), + iso_path, True, additional_variants=["Client"], ) @@ -357,6 +363,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): ), ], ) + iso_path = os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso") self.assertEqual( rcc.call_args_list, [ @@ -375,6 +382,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): log_file=os.path.join( self.topdir, "logs/x86_64/extraiso-my.iso.x86_64.log" ), + iso_path=iso_path, ) ], ) @@ -385,7 +393,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): compose, server, "x86_64", - os.path.join(self.topdir, "compose/Server/x86_64/iso/my.iso"), + iso_path, False, additional_variants=["Client"], ) @@ -427,6 +435,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): ), ], ) + iso_path = os.path.join(self.topdir, "compose/Server/source/iso/my.iso") self.assertEqual( rcc.call_args_list, [ @@ -445,6 +454,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): log_file=os.path.join( self.topdir, "logs/src/extraiso-my.iso.src.log" ), + iso_path=iso_path, ) ], ) @@ -455,7 +465,7 @@ class ExtraIsosThreadTest(helpers.PungiTestCase): compose, server, "src", - os.path.join(self.topdir, "compose/Server/source/iso/my.iso"), + iso_path, False, additional_variants=["Client"], )