diff --git a/doc/koji.rst b/doc/koji.rst index 7403e551..411da9fc 100644 --- a/doc/koji.rst +++ b/doc/koji.rst @@ -96,8 +96,10 @@ If the first compose already managed to hardlink the file before it gets replaced, there will be two copies of the file present locally. -Caveats -------- +Integrity checking +------------------ -There is no integrity checking. Ideally Koji should provide checksums for the -RPMs that would be verified after downloading. This is not yet available. +There is minimal integrity checking. RPM packages belonging to real builds will +be check to match the checksum provided by Koji hub. + +There is no checking for scratch builds or any images. diff --git a/pungi/phases/pkgset/pkgsets.py b/pungi/phases/pkgset/pkgsets.py index e8e84bb0..b3f4f6e1 100644 --- a/pungi/phases/pkgset/pkgsets.py +++ b/pungi/phases/pkgset/pkgsets.py @@ -26,10 +26,12 @@ import time import pgpy import rpm from six.moves import cPickle as pickle +from functools import partial import kobo.log import kobo.pkgset import kobo.rpmlib +from kobo.shortcuts import compute_file_checksums from kobo.threads import WorkerThread, ThreadPool @@ -545,6 +547,23 @@ class KojiPackageSet(PackageSetBase): pathinfo = self.koji_wrapper.koji_module.pathinfo paths = [] + if "getRPMChecksums" in self.koji_proxy.system.listMethods(): + + def checksum_validator(keyname, pkg_path): + checksums = self.koji_proxy.getRPMChecksums( + rpm_info["id"], checksum_types=("sha256",) + ) + if "sha256" in checksums.get(keyname, {}): + computed = compute_file_checksums(pkg_path, ("sha256",)) + if computed["sha256"] != checksums[keyname]["sha256"]: + raise RuntimeError("Checksum mismatch for %s" % pkg_path) + + else: + + def checksum_validator(keyname, pkg_path): + # Koji doesn't support checksums yet + pass + attempts_left = self.signed_packages_retries + 1 while attempts_left > 0: for sigkey in self.sigkey_ordering: @@ -557,7 +576,9 @@ class KojiPackageSet(PackageSetBase): ) if rpm_path not in paths: paths.append(rpm_path) - path = self.downloader.get_file(rpm_path) + path = self.downloader.get_file( + rpm_path, partial(checksum_validator, sigkey) + ) if path: return path @@ -572,7 +593,7 @@ class KojiPackageSet(PackageSetBase): # use an unsigned copy (if allowed) rpm_path = os.path.join(pathinfo.build(build_info), pathinfo.rpm(rpm_info)) paths.append(rpm_path) - path = self.downloader.get_file(rpm_path) + path = self.downloader.get_file(rpm_path, partial(checksum_validator, "")) if path: return path diff --git a/pungi/wrappers/kojiwrapper.py b/pungi/wrappers/kojiwrapper.py index 6358f882..889b80dc 100644 --- a/pungi/wrappers/kojiwrapper.py +++ b/pungi/wrappers/kojiwrapper.py @@ -1001,7 +1001,14 @@ class KojiDownloadProxy: shutil.copyfileobj(r.raw, f) return dest - def _atomic_download(self, url, dest): + def _delete(self, path): + """Try to delete file at given path and ignore errors.""" + try: + os.remove(path) + except Exception: + self.logger.warning("Failed to delete %s", path) + + def _atomic_download(self, url, dest, validator): """Atomically download a file :param str url: URL of the file to download @@ -1019,18 +1026,25 @@ class KojiDownloadProxy: except Exception: # Download failed, let's make sure to clean up potentially partial # temporary file. - try: - os.remove(temp_file) - except Exception: - self.logger.warning("Failed to delete %s", temp_file) - pass + self._delete(temp_file) + raise + + # Check if the temporary file is correct (assuming we were provided a + # validator function). + try: + if validator: + validator(temp_file) + except Exception: + # Validation failed. Let's delete the problematic file and re-raise + # the exception. + self._delete(temp_file) raise # Atomically move the temporary file into final location os.rename(temp_file, dest) return dest - def _download_file(self, path): + def _download_file(self, path, validator): """Ensure file on Koji volume in ``path`` is present in the local cache. @@ -1065,12 +1079,17 @@ class KojiDownloadProxy: os.utime(destination_file) return destination_file - return self._atomic_download(url, destination_file) + return self._atomic_download(url, destination_file, validator) - def get_file(self, path): + def get_file(self, path, validator=None): """ If path refers to an existing file in Koji, return a valid local path to it. If no such file exists, return None. + + :param validator: A callable that will be called with the path to the + downloaded file if and only if the file was actually downloaded. + Any exception raised from there will be abort the download and be + propagated. """ if self.has_local_access: # We have koji volume mounted locally. No transformation needed for @@ -1080,4 +1099,4 @@ class KojiDownloadProxy: return None else: # We need to download the file. - return self._download_file(path) + return self._download_file(path, validator)