Skip to content

core: Use source root for repos if set #5284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cgwalters merged 2 commits into from
Feb 10, 2025
Merged

core: Use source root for repos if set #5284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f19575
core: Use source root for repos and dnf vars
cgwalters Feb 7, 2025
1c6fae8
compose-rootfs: Add --source-root-rw
cgwalters Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
164 changes: 160 additions & 4 deletions rust/src/compose.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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";
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,

Expand Down Expand Up @@ -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()?;
Expand Down Expand Up @@ -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(())
}
}
2 changes: 1 addition & 1 deletion rust/src/composepost.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
26 changes: 19 additions & 7 deletions src/app/rpmostree-compose-builtin-tree.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 ())
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions src/libpriv/rpmostree-core-private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ struct _RpmOstreeContext
gboolean enable_rofiles;
OstreeRepoDevInoCache *devino_cache;
gboolean unprivileged;
gboolean repos_dir_configured;
OstreeSePolicy *sepolicy;
char *passwd_dir;

Expand Down
33 changes: 30 additions & 3 deletions src/libpriv/rpmostree-core.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/libpriv/rpmostree-core.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 4 additions & 10 deletions tests/compose-rootfs/Containerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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`
Copy link
Member

@jlebon jlebon Feb 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, how about instead we mount an install tree and then rsync that in the container env? Similar to #5286.

Copy link
Member Author

@cgwalters cgwalters Feb 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the CI sprawl is a huge pain. Lately what I've been thinking a lot about is getting really strict and saying: all build stuff is always in a container.

OK to just leave this bit for now and circle back to the larger CI problem...well, not sure when 😢 ?

FROM quay.io/fedora/fedora:41 as builder
Expand All @@ -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
Expand Down