From 7e1685c0d131e8c451051f5ac592e72ef1380fb0 Mon Sep 17 00:00:00 2001
From: Tom Fay <tomfay@microsoft.com>
Date: Thu, 21 Nov 2024 09:30:52 +0000
Subject: [PATCH] download correct arch packages (#173)

for repos that use $basearch, we may download the package of the wrong architecture.
fix by selecting the package with correct checksum, rather than the first with the correct name/evr
---
 Cargo.lock                          |  5 +--
 Cargo.toml                          |  1 +
 src/lockfile/download.py            |  5 +++
 tests/fixtures/basearch/rpmoci.toml |  7 ++++
 tests/it.rs                         | 50 +++++++++++++++++++++++------
 5 files changed, 57 insertions(+), 11 deletions(-)
 create mode 100644 tests/fixtures/basearch/rpmoci.toml

diff --git a/Cargo.lock b/Cargo.lock
index 842f934..c1bb1b8 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1236,9 +1236,9 @@ dependencies = [
 
 [[package]]
 name = "libc"
-version = "0.2.159"
+version = "0.2.164"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "561d97a539a36e26a9a5fad1ea11a3039a67714694aaa379433e580854bc3dc5"
+checksum = "433bfe06b8c75da9b2e3fbea6e5329ff87748f0b144ef75306e674c3f6f7c13f"
 
 [[package]]
 name = "libredox"
@@ -1873,6 +1873,7 @@ dependencies = [
  "filetime",
  "flate2",
  "glob",
+ "libc",
  "log",
  "ocidir",
  "pathdiff",
diff --git a/Cargo.toml b/Cargo.toml
index f0117dc..c2626ef 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -36,6 +36,7 @@ xattr = "1.0.1"
 ocidir = "0.3.1"
 
 [dev-dependencies]
+libc = "0.2.164"
 test-temp-dir = "0.3.0"
 testcontainers = { version = "0.23.1", features = ["blocking"] }
 testcontainers-modules = { version = "0.11.2", features = [
diff --git a/src/lockfile/download.py b/src/lockfile/download.py
index 6f03f82..297c7f7 100644
--- a/src/lockfile/download.py
+++ b/src/lockfile/download.py
@@ -1,4 +1,5 @@
 """RPM downloader module."""
+
 # Copyright (C) Microsoft Corporation.
 #
 # This program is free software: you can redistribute it and/or modify
@@ -37,6 +38,10 @@ def download(base, packages, directory):
 def get_package(base, name, evr, checksum):
     """Find packages matching given spec."""
     pkgs = base.sack.query().filter(name=name, evr=evr).run()
+    # Filter by checksum manually as hawkey does not support it
+    # This also ensures that we get the package for the correct arch
+    # in case $basearch is used in the URL.
+    pkgs = [p for p in pkgs if p.chksum and p.chksum[1].hex() == checksum]
 
     if not pkgs:
         msg = (
diff --git a/tests/fixtures/basearch/rpmoci.toml b/tests/fixtures/basearch/rpmoci.toml
new file mode 100644
index 0000000..094da32
--- /dev/null
+++ b/tests/fixtures/basearch/rpmoci.toml
@@ -0,0 +1,7 @@
+[contents]
+packages = ["basesystem"]
+
+[[contents.repositories]]
+url = "https://cdn-ubi.redhat.com/content/public/ubi/dist/ubi9/9/$basearch/baseos/os"
+id = "ubi"
+options = { gpgcheck = "false" }
diff --git a/tests/it.rs b/tests/it.rs
index 7e7630c..2311fe8 100644
--- a/tests/it.rs
+++ b/tests/it.rs
@@ -16,12 +16,18 @@ use testcontainers_modules::cncf_distribution::CncfDistribution;
 const EXE: &str = env!("CARGO_BIN_EXE_rpmoci");
 
 fn rpmoci() -> Command {
-    // if running as root
-    let mut cmd = Command::new("unshare");
-    // Don't use --map-auto here as that doesn't work on Azure Linux 2.0's unshare
-    // This will cause failures if tests install RPMs which create users
-    cmd.arg("--map-root-user").arg("--user").arg(EXE);
-    cmd
+    // if running as root, don't unshare
+    let is_root = unsafe { libc::geteuid() == 0 };
+    if is_root {
+        Command::new(EXE)
+    } else {
+        // Run in user namespace
+        let mut cmd = Command::new("unshare");
+        // Don't use --map-auto here as that doesn't work on Azure Linux 2.0's unshare
+        // This will cause failures if tests install RPMs which create users
+        cmd.arg("--map-root-user").arg("--user").arg(EXE);
+        cmd
+    }
 }
 
 fn setup_test(fixture: &str) -> (TestTempDir, PathBuf) {
@@ -250,20 +256,46 @@ fn test_explicit_etc_os_release() {
     let output = rpmoci().arg("update").current_dir(&root).output().unwrap();
     let stderr = std::str::from_utf8(&output.stderr).unwrap();
     eprintln!("stderr: {}. {}. {}", stderr, root.display(), EXE);
+    assert!(output.status.success());
+    // Open the lockfile and verify /etc/os-release was added as a dependency
+    let lockfile_path = root.join("rpmoci.lock");
+    eprintln!("lockfile_path: {}", lockfile_path.display());
+    let lockfile: Lockfile = toml::from_str(&fs::read_to_string(lockfile_path).unwrap()).unwrap();
+    assert_eq!(
+        lockfile
+            .iter_packages()
+            .filter(|p| p.name == "mariner-release")
+            .count(),
+        1
+    );
 }
 
 #[test]
 fn test_weak_deps() {
     // Verify a build without weak dependencies succeeds
     let (_tmp_dir, root) = setup_test("weakdeps");
-    let output = rpmoci()
+    let status = rpmoci()
         .arg("build")
         .arg("--image=weak")
         .arg("--tag=deps")
         .current_dir(&root)
-        .output()
+        .status()
         .unwrap();
-    assert!(output.status.success());
+    assert!(status.success());
+}
+
+#[test]
+fn test_base_arch() {
+    // Verify a build using a repo with a $basearch variable in the URL succeeds
+    let (_tmp_dir, root) = setup_test("basearch");
+    let status = rpmoci()
+        .arg("build")
+        .arg("--image=base")
+        .arg("--tag=arch")
+        .current_dir(&root)
+        .status()
+        .unwrap();
+    assert!(status.success());
 }
 
 #[cfg(feature = "test-docker")]