core: Use source root for repos if set

rust/src/compose.rs

@@ -4,13 +4,14 @@
 
 use std::ffi::OsStr;
 use std::fs::File;
-use std::io::{BufWriter, Write};
+use std::io::{BufRead, BufReader, BufWriter, Write};
 use std::os::fd::{AsFd, AsRawFd};
 use std::process::Command;
 
 use anyhow::{anyhow, Context, Result};
 use camino::{Utf8Path, Utf8PathBuf};
 use cap_std::fs::{Dir, MetadataExt};
+use cap_std_ext::dirext::CapStdExtDirExt;
 use clap::Parser;
 use fn_error_context::context;
 use oci_spec::image::ImageManifest;
@@ -26,6 +27,7 @@ use crate::cmdutils::CommandRunExt;
 use crate::containers_storage::Mount;
 use crate::cxxrsutil::{CxxResult, FFIGObjectWrapper};
 use crate::isolation::self_command;
+use crate::{RPMOSTREE_RPMDB_LOCATION, RPMOSTREE_SYSIMAGE_RPMDB};
 
 const SYSROOT: &str = "sysroot";
 const USR: &str = "usr";
@@ -94,7 +96,8 @@ struct ComposeImageOpts {
 
     #[clap(long)]
     #[clap(value_parser)]
-    /// Rootfs to use for resolving releasever if unset
+    /// Rootfs to use for resolving package system configuration, such
+    /// as the yum repository configuration, releasever, etc.
     source_root: Option<Utf8PathBuf>,
 
     /// Container authentication file
@@ -202,9 +205,16 @@ pub(crate) struct RootfsOpts {
     ostree_repo: Option<Utf8PathBuf>,
 
     /// Source root for package system configuration.
-    #[clap(long, value_parser)]
+    #[clap(long, value_parser, conflicts_with = "source_root_rw")]
     source_root: Option<Utf8PathBuf>,
 
+    #[clap(long, value_parser, conflicts_with = "source_root")]
+    /// Rootfs to use for resolving package system configuration, such
+    /// as the yum repository configuration, releasever, etc.
+    ///
+    /// The source root may be mutated to work around bugs.
+    source_root_rw: Option<Utf8PathBuf>,
+
     /// Path to the input manifest
     manifest: Utf8PathBuf,
 
@@ -312,14 +322,118 @@ impl BuildChunkedOCIOpts {
     }
 }
 
+/// Given a .repo file, rewrite all references to gpgkey=file:// inside
+/// it to point into the source_root (if the key can be found there).
+/// This is a workaround for https://github.com/coreos/rpm-ostree/issues/5285
+fn mutate_one_dnf_repo(
+    exec_root: &Dir,
+    source_root: &Utf8Path,
+    reposdir: &Dir,
+    name: &Utf8Path,
+) -> Result<()> {
+    let r = reposdir.open(name).map(BufReader::new)?;
+    let mut w = Vec::new();
+    let mut modified = false;
+    for line in r.lines() {
+        let line = line?;
+        // Extract the value of gpgkey=, if it doesn't match then
+        // pass through the line.
+        let Some(value) = line
+            .split_once('=')
+            .filter(|kv| kv.0 == "gpgkey")
+            .map(|kv| kv.1)
+        else {
+            writeln!(w, "{line}")?;
+            continue;
+        };
+        // If the gpg key isn't a local file, pass through the line.
+        let Some(relpath) = value
+            .strip_prefix("file://")
+            .and_then(|path| path.strip_prefix('/'))
+        else {
+            writeln!(w, "{line}")?;
+            continue;
+        };
+        // If it doesn't exist in the source root, then assume the absolute
+        // reference is intentional.
+        let target_repo_file = source_root.join(relpath);
+        if !exec_root.try_exists(&target_repo_file)? {
+            writeln!(w, "{line}")?;
+            continue;
+        }
+        modified = true;
+        writeln!(w, "gpgkey=file:///{target_repo_file}")?;
+    }
+    if modified {
+        reposdir.write(name, w)?;
+    }
+    Ok(())
+}
+
+#[context("Preparing source root")]
+fn mutate_source_root(exec_root: &Dir, source_root: &Utf8Path) -> Result<()> {
+    let source_root_dir = exec_root
+        .open_dir(source_root)
+        .with_context(|| format!("Opening {source_root}"))?;
+    if source_root_dir
+        .symlink_metadata_optional(RPMOSTREE_RPMDB_LOCATION)?
+        .is_none()
+        && source_root_dir
+            .symlink_metadata_optional(RPMOSTREE_SYSIMAGE_RPMDB)?
+            .is_some()
+    {
+        source_root_dir
+            .symlink_contents("../lib/sysimage/rpm", RPMOSTREE_RPMDB_LOCATION)
+            .context("Symlinking rpmdb")?;
+        println!("Symlinked {RPMOSTREE_RPMDB_LOCATION} in source root");
+    }
+
+    if !source_root_dir.try_exists("etc")? {
+        return Ok(());
+    }
+    if let Some(repos) = source_root_dir
+        .open_dir_optional("etc/yum.repos.d")
+        .context("Opening yum.repos.d")?
+    {
+        for ent in repos.entries_utf8()? {
+            let ent = ent?;
+            if !ent.file_type()?.is_file() {
+                continue;
+            }
+            let name: Utf8PathBuf = ent.file_name()?.into();
+            let Some("repo") = name.extension() else {
+                continue;
+            };
+            mutate_one_dnf_repo(exec_root, source_root, &repos, &name)?;
+        }
+    }
+
+    Ok(())
+}
+
 impl RootfsOpts {
-    pub(crate) fn run(self) -> Result<()> {
+    pub(crate) fn run(mut self) -> Result<()> {
         let manifest = self.manifest.as_path();
 
         if self.dest.try_exists()? {
             anyhow::bail!("Refusing to operate on extant target {}", self.dest);
         }
 
+        // If we were passed a mutable source root, then let's work around some bugs
+        if let Some(rw) = self.source_root_rw.take() {
+            // Clap ensures this
+            assert!(self.source_root.is_none());
+            let exec_root = Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
+            // We could handle relative paths, but it's easier to require absolute.
+            // The mutation work below all happens via cap-std because it's way easier
+            // to unit test with that.
+            let Some(rw_relpath) = rw.strip_prefix("/").ok() else {
+                anyhow::bail!("Expected absolute path for source-root: {rw}");
+            };
+            mutate_source_root(&exec_root, rw_relpath)?;
+            self.source_root = Some(rw);
+        }
+
         // Create a temporary directory for things
         let td = tempfile::tempdir_in("/var/tmp")?;
         let td_path: Utf8PathBuf = td.path().to_owned().try_into()?;
@@ -1029,4 +1143,46 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn test_mutate_source_root() -> Result<()> {
+        let rootfs = &cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
+        // Should be a no-op in an empty root
+        rootfs.create_dir("repos")?;
+        mutate_source_root(rootfs, "repos".into()).unwrap();
+        rootfs.create_dir_all("repos/usr/share")?;
+        rootfs.create_dir_all("repos/usr/lib/sysimage/rpm")?;
+        mutate_source_root(rootfs, "repos".into()).unwrap();
+        assert!(rootfs
+            .symlink_metadata("repos/usr/share/rpm")
+            .unwrap()
+            .is_symlink());
+
+        rootfs.create_dir_all("repos/etc/yum.repos.d")?;
+        rootfs.create_dir_all("repos/etc/pki/rpm-gpg")?;
+        rootfs.write("repos/etc/pki/rpm-gpg/repo2.key", "repo2 gpg key")?;
+        let orig_repo_content = indoc::indoc! { r#"
+        [repo]
+        baseurl=blah
+        gpgkey=https://example.com
+
+        [repo2]
+        baseurl=other
+        gpgkey=file:///etc/pki/rpm-gpg/repo2.key
+
+        [repo3]
+        baseurl=blah
+        gpgkey=file:///absolute/path/not-in-source-root
+    "#};
+        rootfs.write("repos/etc/yum.repos.d/test.repo", orig_repo_content)?;
+        mutate_source_root(rootfs, "repos".into()).unwrap();
+        let found_repos = rootfs.read_to_string("repos/etc/yum.repos.d/test.repo")?;
+        let expected = orig_repo_content.replace(
+            "gpgkey=file:///etc/pki/rpm-gpg/repo2.key",
+            "gpgkey=file:///repos/etc/pki/rpm-gpg/repo2.key",
+        );
+        similar_asserts::assert_eq!(expected, found_repos);
+
+        Ok(())
+    }
 }

rust/src/composepost.rs

@@ -47,7 +47,7 @@ const OSTREE_HOME_SYMLINKS: &[(&str, &str)] = &[("var/roothome", "root"), ("var/
 /* See rpmostree-core.h */
 const RPMOSTREE_BASE_RPMDB: &str = "usr/lib/sysimage/rpm-ostree-base-db";
 pub(crate) const RPMOSTREE_RPMDB_LOCATION: &str = "usr/share/rpm";
-const RPMOSTREE_SYSIMAGE_RPMDB: &str = "usr/lib/sysimage/rpm";
+pub(crate) const RPMOSTREE_SYSIMAGE_RPMDB: &str = "usr/lib/sysimage/rpm";
 pub(crate) const TRADITIONAL_RPMDB_LOCATION: &str = "var/lib/rpm";
 
 const SD_LOCAL_FS_TARGET_REQUIRES: &str = "usr/lib/systemd/system/local-fs.target.requires";

src/app/rpmostree-compose-builtin-tree.cxx

@@ -106,7 +106,9 @@ static GOptionEntry install_option_entries[]
           "Assume cache is present, do not attempt to update it", NULL },
         { "cachedir", 0, 0, G_OPTION_ARG_STRING, &opt_cachedir, "Cached state", "CACHEDIR" },
         { "source-root", 0, 0, G_OPTION_ARG_STRING, &opt_source_root,
-          "Rootfs to use for resolving releasever if unset", "PATH" },
+          "Rootfs to use for configuring libdnf, such as releasever, dnf variables, and input "
+          "rpm-md repositories.",
+          "PATH" },
         { "download-only", 0, 0, G_OPTION_ARG_NONE, &opt_download_only,
           "Like --dry-run, but download and import RPMs as well; requires --cachedir", NULL },
         { "download-only-rpms", 0, 0, G_OPTION_ARG_NONE, &opt_download_only_rpms,
@@ -285,13 +287,24 @@ try_load_previous_sepolicy (RpmOstreeTreeComposeContext *self, GCancellable *can
 }
 
 static gboolean
-set_repos_dir (DnfContext *dnfctx, rpmostreecxx::Treefile &treefile, int workdir_dfd,
+set_repos_dir (RpmOstreeContext *ctx, rpmostreecxx::Treefile &treefile, int workdir_dfd,
                GCancellable *cancellable, GError **error)
 {
-  auto treefile_dir = std::string (treefile.get_workdir ());
-  dnf_context_set_repo_dir (dnfctx, treefile_dir.c_str ());
+  DnfContext *dnfctx = rpmostree_context_get_dnf (ctx);
+  if (!opt_source_root)
+    {
+      auto treefile_dir = std::string (treefile.get_workdir ());
+      rpmostree_context_set_repos_dir (ctx, treefile_dir.c_str ());
+    }
   g_autoptr (GPtrArray) var_dirs = g_ptr_array_new ();
 
+  g_autofree char *source_vars = NULL;
+  if (opt_source_root)
+    {
+      source_vars = g_build_filename (opt_source_root, "etc/dnf/vars", NULL);
+      g_ptr_array_add (var_dirs, (char *)source_vars);
+    }
+
   /* And add our own vars if defined. */
   CXX_TRY_VAR (repovars, treefile.write_repovars (workdir_dfd), error);
   if (!repovars.empty ())
@@ -324,7 +337,7 @@ install_packages (RpmOstreeTreeComposeContext *self, gboolean *out_unmodified,
     rpmlogSetFile (NULL);
   }
 
-  if (!set_repos_dir (dnfctx, **self->treefile_rs, self->workdir_dfd, cancellable, error))
+  if (!set_repos_dir (self->corectx, **self->treefile_rs, self->workdir_dfd, cancellable, error))
     return FALSE;
 
   /* By default, retain packages in addition to metadata with --cachedir, unless
@@ -1651,8 +1664,7 @@ rpmostree_compose_builtin_extensions (int argc, char **argv, RpmOstreeCommandInv
       = rpmostree_context_new_compose (cachedir_dfd, repo, *extension_tf);
 
   // we're abusing the cachedir as a workdir here, like we do below too
-  if (!set_repos_dir (rpmostree_context_get_dnf (ctx), *extension_tf, cachedir_dfd, cancellable,
-                      error))
+  if (!set_repos_dir (ctx, *extension_tf, cachedir_dfd, cancellable, error))
     return FALSE;
 
 #define TMP_EXTENSIONS_ROOTFS "rpmostree-extensions.tmp"

src/libpriv/rpmostree-core-private.h

@@ -51,6 +51,7 @@ struct _RpmOstreeContext
   gboolean enable_rofiles;
   OstreeRepoDevInoCache *devino_cache;
   gboolean unprivileged;
+  gboolean repos_dir_configured;
   OstreeSePolicy *sepolicy;
   char *passwd_dir;
 

src/libpriv/rpmostree-core.cxx

@@ -346,6 +346,16 @@ rpmostree_context_set_dnf_caching (RpmOstreeContext *self, RpmOstreeContextDnfCa
   self->dnf_cache_policy = policy;
 }
 
+// Override the location of the yum.repos.d, and also record that we did
+// an override so we know not to auto-set it later.
+void
+rpmostree_context_set_repos_dir (RpmOstreeContext *self, const char *reposdir)
+{
+  dnf_context_set_repo_dir (self->dnfctx, reposdir);
+  self->repos_dir_configured = TRUE;
+  g_debug ("set override for repos dir");
+}
+
 /* Pick up repos dir and passwd from @cfg_deployment. */
 void
 rpmostree_context_configure_from_deployment (RpmOstreeContext *self, OstreeSysroot *sysroot,
@@ -355,7 +365,7 @@ rpmostree_context_configure_from_deployment (RpmOstreeContext *self, OstreeSysro
   g_autofree char *reposdir = g_build_filename (cfg_deployment_root, "etc/yum.repos.d", NULL);
 
   /* point libhif to the yum.repos.d and os-release of the merge deployment */
-  dnf_context_set_repo_dir (self->dnfctx, reposdir);
+  rpmostree_context_set_repos_dir (self, reposdir);
 
   /* point the core to the passwd & group of the merge deployment */
   g_assert (!self->passwd_dir);
@@ -663,8 +673,25 @@ rpmostree_context_setup (RpmOstreeContext *self, const char *install_root, const
         releasever = "rpmostree-unset-releasever";
       dnf_context_set_release_ver (self->dnfctx, releasever.c_str ());
     }
-  else if (releasever.length () > 0)
-    dnf_context_set_release_ver (self->dnfctx, releasever.c_str ());
+  else
+    {
+      // Source root is set. But check if we have a releasever from the treefile,
+      // if so that overrides the one from the source root.
+      if (releasever.length () > 0)
+        dnf_context_set_release_ver (self->dnfctx, releasever.c_str ());
+
+      // Override what rpmostree_context_set_cache_root() did.
+      // TODO disentangle these things so that we only call set_repo_dir if
+      // we didn't have a source root.
+      // Note we also need to skip this in cases where the caller explicitly
+      // overrode the repos dir.
+      if (!self->repos_dir_configured)
+        {
+          g_autofree char *source_repos = g_build_filename (source_root, "etc/yum.repos.d", NULL);
+          dnf_context_set_repo_dir (self->dnfctx, source_repos);
+          g_debug ("Set repos dir from source root");
+        }
+    }
 
   dnf_context_set_install_root (self->dnfctx, install_root);
   dnf_context_set_source_root (self->dnfctx, source_root);

src/libpriv/rpmostree-core.h

@@ -112,6 +112,8 @@ void rpmostree_context_set_is_empty (RpmOstreeContext *self);
 void rpmostree_context_disable_selinux (RpmOstreeContext *self);
 const char *rpmostree_context_get_ref (RpmOstreeContext *self);
 
+void rpmostree_context_set_repos_dir (RpmOstreeContext *self, const char *reposdir);
+
 void rpmostree_context_set_repos (RpmOstreeContext *self, OstreeRepo *base_repo,
                                   OstreeRepo *pkgcache_repo);
 void rpmostree_context_set_devino_cache (RpmOstreeContext *self,

tests/compose-rootfs/Containerfile

@@ -1,5 +1,5 @@
-# In theory this can skew from the builder
-FROM quay.io/fedora/fedora:41 as repos
+# Demonstrate skew from the builder
+FROM quay.io/centos/centos:stream10 as repos
 
 # You must run this build with `-v /path/to/rpm-ostree:/run/build/rpm-ostree:ro`
 FROM quay.io/fedora/fedora:41 as builder
@@ -15,15 +15,9 @@ EORUN
 # Copy in our source code.
 COPY . /src
 WORKDIR /src
-RUN --mount=type=bind,from=repos,src=/,dst=/repos <<EORUN
+RUN --mount=type=bind,from=repos,src=/,dst=/repos,rw <<EORUN
 set -xeuo pipefail
-# Synchronize the dnf/rpm configs from the repos container.
-for x in etc/dnf etc/yum.repos.d etc/pki/rpm-gpg; do
-  rm -rf /"$x" && cp -a /repos/${x} /$x
-done
-# And copy to the workdir; TODO fix this in rpm-ostree
-cp /etc/yum.repos.d/*.repo .
-exec rpm-ostree experimental compose rootfs --source-root=/repos manifest.yaml /target-rootfs
+exec rpm-ostree experimental compose rootfs --source-root-rw=/repos manifest.yaml /target-rootfs
 EORUN
 
 # This pulls in the rootfs generated in the previous step