Support checksum algorithm mismatch (+ other fixes)

CHANGELOG.md

@@ -3,6 +3,13 @@
 ### Added
 ### Fixed
 
+## 0.4.4 - 2025-07-01
+
+### Fixed
+- Make GPG config be consistently ordered in lockfile.
+- Remove debug print statement.
+- Handle case where RPMs have a different checksum algorithm than expected when they're being downloaded, while still verifying the checksum.
+
 ## 0.4.3 - 2025-05-28
 
 ### Added
@@ -32,7 +39,7 @@
 
 ### Fixed
 - Set size in hardlink headers correctly.
-  - Fixes integrity failures during this `docker push` 
+  - Fixes integrity failures during this `docker push`
 
 
 ## 0.3.1 - 2024-07-24

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rpmoci"
-version = "0.4.3"
+version = "0.4.4"
 edition = "2024"
 description = "Build container images from RPMs"
 # rpmoci uses DNF (via pyo3) which is GPLV2+ licensed,

src/cli.rs

@@ -38,7 +38,7 @@ pub struct Cli {
 fn label_parser(s: &str) -> Result<(String, String), String> {
     match s.split_once('=') {
         Some((key, value)) => Ok((key.to_string(), value.to_string())),
-        None => Err(format!("`{}` should be of the form KEY=VALUE.", s)),
+        None => Err(format!("`{s}` should be of the form KEY=VALUE.")),
     }
 }
 

src/config.rs

@@ -176,7 +176,7 @@ impl ImageConfig {
             .entrypoint(entrypoint.clone())
             .env(
                 envs.iter()
-                    .map(|(k, v)| format!("{}={}", k, v))
+                    .map(|(k, v)| format!("{k}={v}"))
                     .collect::<Vec<_>>(),
             )
             .exposed_ports(exposed_ports.clone())

src/lib.rs

@@ -130,13 +130,12 @@ pub fn main(command: Command) -> anyhow::Result<()> {
                     write::error(
                         "Warning",
                         format!(
-                            "failed to parse existing lock file. Generating a new one. Error: {}",
-                            err
+                            "failed to parse existing lock file. Generating a new one. Error: {err}"
                         ),
                     )?;
                     err.chain()
                         .skip(1)
-                        .for_each(|cause| eprintln!("caused by: {}", cause));
+                        .for_each(|cause| eprintln!("caused by: {cause}"));
                     changed = true;
                     Lockfile::resolve_from_config(&cfg)?
                 }

src/lockfile/build.rs

@@ -142,7 +142,7 @@ impl Lockfile {
                             .context(format!("Failed to read file: {}", path.display()))?;
                         let checksum = format!("{:x}", hasher.finalize());
 
-                        eprintln!("Checksum: {}", checksum);
+                        eprintln!("Checksum: {checksum}");
 
                         if checksum != p.checksum.checksum {
                             bail!(
@@ -217,7 +217,7 @@ impl Lockfile {
             }
         }
         write::ok("Installing", "packages")?;
-        log::debug!("Running `{:?}`", dnf_install);
+        log::debug!("Running `{dnf_install:?}`");
         let status = dnf_install.status().context("Failed to run dnf")?;
         if !status.success() {
             bail!("failed to dnf install");
@@ -239,7 +239,7 @@ impl Lockfile {
                 rpm_erase.arg(pkg);
             }
             write::ok("Removing", "excluded packages")?;
-            log::debug!("Running `{:?}`", rpm_erase);
+            log::debug!("Running `{rpm_erase:?}`");
             let status = rpm_erase.status().context("Failed to run rpm")?;
             if !status.success() {
                 bail!("failed to rpm erase excluded packages");
@@ -272,7 +272,7 @@ fn creation_time() -> Result<DateTime<chrono::Utc>, anyhow::Error> {
     let creation_time = if let Ok(sde) = std::env::var("SOURCE_DATE_EPOCH") {
         let timestamp = sde
             .parse::<i64>()
-            .with_context(|| format!("Failed to parse SOURCE_DATE_EPOCH `{}`", sde))?;
+            .with_context(|| format!("Failed to parse SOURCE_DATE_EPOCH `{sde}`"))?;
         DateTime::from_timestamp(timestamp, 0)
             .ok_or_else(|| anyhow::anyhow!("SOURCE_DATE_EPOCH out of range: `{}`", sde))?
     } else {

src/lockfile/download.py

@@ -19,32 +19,114 @@
 from dnf.i18n import _
 import dnf
 from dnf.cli.progress import MultiFileProgressMeter
+import hashlib
+import hawkey
+
+
+class Package:
+    """Store information about RPM packages."""
+
+    def __init__(self, name, evr, algo, checksum, arch) -> None:
+        self.name = name
+        self.evr = evr
+        self.algo = algo
+        self.checksum = checksum
+        self.arch = arch
+        self.package = None  # dnf.package.Package
+        self.checked = False
 
 
 def download(base, packages, directory):
     """Downloads packages.
     Parameters:
     - base needs to be a dnf.Base() object that has had repos configured and fill_sack called.
-    packages is an array of requested package specifications
-    - packages is a list of [name, evr, checksum, arch] lists.
-    - directory, where to copy the RPMs to
+    - packages is a list of [name, evr, checksum algorithm, checksum, arch] lists, of requested package specifications.
+    - directory, where to copy the RPMs to.
     """
-    pkgs = [get_package(base, p[0], p[1], p[2], p[3]) for p in packages]
-    base.download_packages(pkgs, MultiFileProgressMeter(fo=sys.stdout))
-    for pkg in pkgs:
-        shutil.copy(pkg.localPkg(), directory)
+    # Convert input to our own Package type for convenience.
+    pkgs = [Package(p[0], p[1], p[2], p[3], p[4]) for p in packages]
+
+    # Fill in DNF package info, and take a note of whichever checksums already match.
+    for p in pkgs:
+        get_package(base, p)
+
+    # Download the RPMs.
+    base.download_packages(
+        (p.package for p in pkgs), MultiFileProgressMeter(fo=sys.stdout)
+    )
+
+    # Download each RPM. If we haven't been able to verify the checksum yet because we got a different checksum
+    # algorithm than we had when we originally resolved the RPM, then verify it now by hashing the file.
+    for p in pkgs:
+        if not p.checked:
+            # Checksum types must not have matched, hash the RPM now.
+            hasher = hashlib.new(p.algo)
+            with open(p.package.localPkg(), "rb") as f:
+                while True:
+                    data = f.read(65536)
+                    if not data:
+                        break
+                    hasher.update(data)
+
+            checksum = hasher.hexdigest()
+            if p.checksum != checksum:
+                msg = (
+                    "Package checksum didn't match: "
+                    f"Name: '{p.name}', evr: '{p.evr}', algo: '{p.algo}', expected: '{p.checksum}', found: '{checksum}'"
+                )
+                raise dnf.exceptions.DepsolveError(msg)
+
+        shutil.copy(p.package.localPkg(), directory)
 
 
-def get_package(base, name, evr, checksum, arch):
+def hawkey_chksum_to_name(id):
+    if id == hawkey.CHKSUM_MD5:  # Devskim: ignore DS126858
+        return "md5"  # Devskim: ignore DS126858
+    elif id == hawkey.CHKSUM_SHA1:  # Devskim: ignore DS126858
+        return "sha1"  # Devskim: ignore DS126858
+    elif id == hawkey.CHKSUM_SHA256:
+        return "sha256"
+    elif id == hawkey.CHKSUM_SHA384:
+        return "sha384"
+    elif id == hawkey.CHKSUM_SHA512:
+        return "sha512"
+    raise dnf.exceptions.Error("Unknown checksum value %d" % id)
+
+
+def raise_no_package_error(pkg):
+    msg = f"Package could no longer be found in repositories. Name: '{pkg.name}', evr: '{pkg.evr}'"
+    raise dnf.exceptions.DepsolveError(msg)
+
+
+def get_package(base, pkg):
     """Find packages matching given spec."""
-    if arch:
-        pkgs = base.sack.query().filter(name=name, evr=evr, arch=arch).run()
+    if pkg.arch:
+        pkgs = base.sack.query().filter(name=pkg.name, evr=pkg.evr, arch=pkg.arch).run()
     else:
-        pkgs = base.sack.query().filter(name=name, evr=evr).run()
-    # Filter by checksum manually as hawkey does not support it
-    pkgs = [p for p in pkgs if p.chksum and p.chksum[1].hex() == checksum]
+        pkgs = base.sack.query().filter(name=pkg.name, evr=pkg.evr).run()
 
     if not pkgs:
-        msg = f"Package could no longer be found in repositories. Name: '{name}', evr: '{evr}'"
-        raise dnf.exceptions.DepsolveError(msg)
-    return pkgs[0]
+        raise_no_package_error(pkg)
+
+    # Filter by checksum manually as hawkey does not support it.
+    # A package may be presented with a different checksum algorithm here than when it was originally resolved.
+    # Therefore, only bail out here if we find the right type of checksum but no matches, otherwise we'll verify the
+    # checksum after downloading the package.
+    found_correct_checksum_type = False
+    for p in pkgs:
+        if p.chksum:
+            if p.chksum[1].hex() == pkg.checksum:
+                # Found a match, indicate that the checksum is correct and return it.
+                pkg.package = p
+                pkg.checked = True
+                return
+
+            if hawkey_chksum_to_name(p.chksum[0]) == pkg.algo:
+                found_correct_checksum_type = True
+
+    if found_correct_checksum_type:
+        # Should have found a matching checksum because we found at least one of the same type, but none did.
+        raise_no_package_error(pkg)
+
+    # Fallback, just pick the first package and verify the checksum after downloading it.
+    pkg.package = pkgs[0]

src/lockfile/download.rs

@@ -50,6 +50,7 @@ impl Lockfile {
                     (
                         p.name.clone(),
                         p.evr.clone(),
+                        p.checksum.algorithm.to_string(),
                         p.checksum.checksum.clone(),
                         p.arch.clone().unwrap_or_default(),
                     )
@@ -84,7 +85,7 @@ impl Lockfile {
         for (repoid, repo_key_info) in &self.repo_gpg_config {
             if repo_key_info.gpgcheck {
                 for (i, key) in repo_key_info.keys.iter().enumerate() {
-                    load_key(&tmp_dir, &format!("{}-{}", repoid, i), key)?;
+                    load_key(&tmp_dir, &format!("{repoid}-{i}"), key)?;
                 }
             }
         }

src/lockfile/mod.rs

@@ -14,7 +14,7 @@
 //!
 //! You should have received a copy of the GNU General Public License
 //! along with this program.  If not, see <https://www.gnu.org/licenses/>.
-use std::collections::{BTreeMap, BTreeSet, HashMap};
+use std::collections::{BTreeMap, BTreeSet};
 use std::fmt;
 use std::io::Write;
 use std::path::Path;
@@ -36,8 +36,8 @@ pub struct Lockfile {
     packages: BTreeSet<Package>,
     #[serde(default, skip_serializing_if = "BTreeSet::is_empty")]
     local_packages: BTreeSet<LocalPackage>,
-    #[serde(default, skip_serializing_if = "HashMap::is_empty")]
-    repo_gpg_config: HashMap<String, RepoKeyInfo>,
+    #[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
+    repo_gpg_config: BTreeMap<String, RepoKeyInfo>,
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     global_key_specs: Vec<url::Url>,
 }
@@ -65,7 +65,7 @@ struct DnfOutput {
     /// Local packages
     local_packages: Vec<LocalPackage>,
     /// Repository GPG configuration
-    repo_gpg_config: HashMap<String, RepoKeyInfo>,
+    repo_gpg_config: BTreeMap<String, RepoKeyInfo>,
 }
 
 /// GPG key configuration for a specified repository
@@ -194,14 +194,14 @@ impl Lockfile {
         for (name, evr) in old {
             if let Some(new_evr) = new.remove(name) {
                 if new_evr != evr {
-                    write::ok("Updating", format!("{} {} -> {}", name, evr, new_evr))?;
+                    write::ok("Updating", format!("{name} {evr} -> {new_evr}"))?;
                 }
             } else {
-                write::ok("Removing", format!("{} {}", name, evr))?;
+                write::ok("Removing", format!("{name} {evr}"))?;
             }
         }
         for (name, evr) in new {
-            write::ok("Adding", format!("{} {}", name, evr))?;
+            write::ok("Adding", format!("{name} {evr}"))?;
         }
 
         Ok(())

src/lockfile/resolve.rs

@@ -56,7 +56,6 @@ impl Lockfile {
             } else {
                 pkg_specs.clone()
             };
-            eprintln!("3");
 
             let args = PyTuple::new(py, [base.as_any(), &specs.into_pyobject(py)?])?;
             // Run the resolve function, returning a json string, which we shall deserialize.

src/main.rs

@@ -22,7 +22,7 @@ fn main() {
         write::error("Error", err.to_string()).unwrap();
         err.chain()
             .skip(1)
-            .for_each(|cause| eprintln!("caused by: {}", cause));
+            .for_each(|cause| eprintln!("caused by: {cause}"));
         std::process::exit(1);
     }
 }

src/write.rs

@@ -21,9 +21,9 @@ use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};
 fn msg(label: &str, message: impl Display, color: &ColorSpec) -> io::Result<()> {
     let mut stderr = StandardStream::stderr(ColorChoice::Auto);
     stderr.set_color(color)?;
-    write!(&mut stderr, "{:>20} ", label)?;
+    write!(&mut stderr, "{label:>20} ")?;
     stderr.set_color(ColorSpec::new().set_fg(None))?;
-    writeln!(&mut stderr, "{}", message)?;
+    writeln!(&mut stderr, "{message}")?;
     Ok(())
 }