From 5b7c14ad3da97b006c847bfbf4196e8e5f4b204f Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Fri, 28 Nov 2025 18:08:34 +0100 Subject: [PATCH 01/15] install/bootupd: chroot to deployment When `--src-imgref` is passed, the deployed systemd does not match the running environnement. In this case, let's chroot into the deployment before calling bootupd. This makes sure we are using the binaries shipped in the image (and relevant config files such as grub fragements). We could do that in all cases but i kept it behind the `--src-imgref` option since when using the target container as the buildroot it will have no impact, and we expect this scenario to be the most common. In CoreOS we have a specific test that checks if the bootloader was installed with the `grub2-install` of the image. Fixes https://github.com/bootc-dev/bootc/issues/1559 Also see https://github.com/bootc-dev/bootc/issues/1455 Signed-off-by: jbtrystram --- crates/lib/src/bootloader.rs | 76 ++++++++++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 7 deletions(-) diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 7fce2888a..32184d479 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -7,9 +7,12 @@ use camino::Utf8Path; use cap_std_ext::cap_std::fs::Dir; use cap_std_ext::dirext::CapStdExtDirExt; use fn_error_context::context; +use tempfile::TempDir; use bootc_blockdev::{Partition, PartitionTable}; use bootc_mount as mount; +use rustix::mount::UnmountFlags; +use rustix::path::Arg; use crate::bootc_composefs::boot::{SecurebootKeys, get_sysroot_parent_dev, mount_esp}; use crate::{discoverable_partition_specification, utils}; @@ -91,22 +94,81 @@ pub(crate) fn install_via_bootupd( // bootc defaults to only targeting the platform boot method. let bootupd_opts = (!configopts.generic_image).then_some(["--update-firmware", "--auto"]); - let abs_deployment_path = deployment_path.map(|v| rootfs.join(v)); - let src_root_arg = if let Some(p) = abs_deployment_path.as_deref() { - vec!["--src-root", p.as_str()] + let abs_deployment_path = deployment_path.map(|deploy| rootfs.join(deploy)); + // When not running inside the target container (through `--src-imgref`) we chroot + // into the deployment before running bootupd. This makes sure we use binaries + // from the target image rather than the buildroot + // In some cases (e.g. --write-uuid), bootupd needs to find the underlying device + // for /boot. But since we don't control where the destination rootfs is mounted + // let's bind mount it to a temp mountpoint under /run + // so it gets carried over in the chroot. + + let rootfs_mountpoint: TempDir; + let rootfs_mount = if rootfs.starts_with("/run") { + rootfs.as_str() + } else { + rootfs_mountpoint = tempfile::tempdir_in("/run")?; + rustix::mount::mount_bind_recursive( + rootfs.as_std_path(), + &rootfs_mountpoint.path().to_owned(), + )?; + rootfs_mountpoint + .path() + .to_str() + .expect("Invalid tempdir path") + }; + + // We mount the linux API file systems into the target deployment before chrooting + // so bootupd can find the proper backing device. + // xref https://systemd.io/API_FILE_SYSTEMS/ + let bind_mount_dirs = ["/dev", "/run", "/proc", "/sys"]; + let chroot_args = if let Some(target_root) = abs_deployment_path.as_deref() { + tracing::debug!("Setting up bind-mounts before chrooting to the target deployment"); + for src in bind_mount_dirs { + let dest = target_root + // joining an absolute path + // makes it replace self, so we strip the prefix + .join_os(src.strip_prefix("/").unwrap()); + tracing::debug!("bind mounting {}", dest.display()); + rustix::mount::mount_bind_recursive(src, dest)?; + } + // Append the `bootupctl` command, it will be passed as + // an argument to chroot + vec![target_root.as_str(), "bootupctl"] } else { vec![] }; + let devpath = device.path(); println!("Installing bootloader via bootupd"); - Command::new("bootupctl") + let mut bootupctl = if abs_deployment_path.is_some() { + Command::new("chroot") + } else { + Command::new("bootupctl") + }; + let install_result = bootupctl + .args(chroot_args) .args(["backend", "install", "--write-uuid"]) .args(verbose) .args(bootupd_opts.iter().copied().flatten()) - .args(src_root_arg) - .args(["--device", devpath.as_str(), rootfs.as_str()]) + .args(["--device", devpath.as_str(), rootfs_mount]) .log_debug() - .run_inherited_with_cmd_context() + .run_inherited_with_cmd_context(); + + // Clean up the mounts after ourselves + if let Some(target_root) = abs_deployment_path { + for dir in bind_mount_dirs { + let mount = target_root + .join(dir.strip_prefix("/").unwrap()) + .into_std_path_buf(); + if let Err(e) = rustix::mount::unmount(&mount, UnmountFlags::DETACH) { + // let's not propagate the error up because in some cases we can't unmount + // e.g. when running `to-existing-root` + tracing::warn!("Error unmounting {}: {e}", mount.display()); + } + } + } + install_result } #[context("Installing bootloader")] From 65f8859810e9687da92e090c6c24c0494bfc7a0b Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Wed, 7 Jan 2026 12:21:31 +0100 Subject: [PATCH 02/15] DNM: add more debug output to tests Signed-off-by: jbtrystram --- crates/tests-integration/src/install.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/tests-integration/src/install.rs b/crates/tests-integration/src/install.rs index dc9d2afd2..a748763e7 100644 --- a/crates/tests-integration/src/install.rs +++ b/crates/tests-integration/src/install.rs @@ -9,7 +9,17 @@ use fn_error_context::context; use libtest_mimic::Trial; use xshell::{Shell, cmd}; -pub(crate) const BASE_ARGS: &[&str] = &["podman", "run", "--rm", "--privileged", "--pid=host"]; +pub(crate) const BASE_ARGS: &[&str] = &[ + "podman", + "run", + "--rm", + "--privileged", + "--pid=host", + "--env", + "BOOTC_BOOTLOADER_DEBUG=true", + "--env", + "RUST_LOG=debug", +]; // Arbitrary const NON_DEFAULT_STATEROOT: &str = "foo"; From 9f5c3486a709af9198c8fa73a304d2240ab3c1e5 Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Wed, 17 Dec 2025 21:32:21 +0100 Subject: [PATCH 03/15] insert a reasonnable default PATH into the chroot Signed-off-by: jbtrystram --- crates/lib/src/bootloader.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 32184d479..1d2e4cfc6 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -148,6 +148,13 @@ pub(crate) fn install_via_bootupd( }; let install_result = bootupctl .args(chroot_args) + // Inject a reasonnable PATH here so we find the required tools + // when running chrooted in the deployment. Testing show that + // the default $PATH value in the chroot is insufficient. + .env( + "PATH", + "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", + ) .args(["backend", "install", "--write-uuid"]) .args(verbose) .args(bootupd_opts.iter().copied().flatten()) From 827f3c5eceea13ac0a204970037390f4cce43ddb Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Wed, 7 Jan 2026 13:45:08 +0100 Subject: [PATCH 04/15] test: just bind-mount target/boot in deploymnt/boot --- crates/lib/src/bootloader.rs | 41 +++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 1d2e4cfc6..5cc6b5834 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -7,12 +7,10 @@ use camino::Utf8Path; use cap_std_ext::cap_std::fs::Dir; use cap_std_ext::dirext::CapStdExtDirExt; use fn_error_context::context; -use tempfile::TempDir; use bootc_blockdev::{Partition, PartitionTable}; use bootc_mount as mount; use rustix::mount::UnmountFlags; -use rustix::path::Arg; use crate::bootc_composefs::boot::{SecurebootKeys, get_sysroot_parent_dev, mount_esp}; use crate::{discoverable_partition_specification, utils}; @@ -103,21 +101,19 @@ pub(crate) fn install_via_bootupd( // let's bind mount it to a temp mountpoint under /run // so it gets carried over in the chroot. - let rootfs_mountpoint: TempDir; + // let rootfs_mountpoint: TempDir; let rootfs_mount = if rootfs.starts_with("/run") { rootfs.as_str() } else { - rootfs_mountpoint = tempfile::tempdir_in("/run")?; - rustix::mount::mount_bind_recursive( - rootfs.as_std_path(), - &rootfs_mountpoint.path().to_owned(), - )?; - rootfs_mountpoint - .path() - .to_str() - .expect("Invalid tempdir path") + "/" }; + // rootfs_mountpoint + // .path() + // .to_str() + // .expect("Invalid tempdir path") + //}; + // We mount the linux API file systems into the target deployment before chrooting // so bootupd can find the proper backing device. // xref https://systemd.io/API_FILE_SYSTEMS/ @@ -132,6 +128,21 @@ pub(crate) fn install_via_bootupd( tracing::debug!("bind mounting {}", dest.display()); rustix::mount::mount_bind_recursive(src, dest)?; } + // WIP : let's try to bind-mount /target/boot into the deployment as well rather than bind-mounting the whole thing?? + if !rootfs.starts_with("/run") { + tracing::debug!( + "We need to access the target /boot filesystem so let's also bind-mount it" + ); + let trgt_boot = rootfs.as_std_path().join("boot"); + let chrooted_boot = target_root.join_os("boot"); + tracing::debug!( + "bind-mounting {} in {}", + &trgt_boot.display(), + &chrooted_boot.display() + ); + rustix::mount::mount_bind_recursive(trgt_boot, chrooted_boot)?; + } + // Append the `bootupctl` command, it will be passed as // an argument to chroot vec![target_root.as_str(), "bootupctl"] @@ -164,6 +175,12 @@ pub(crate) fn install_via_bootupd( // Clean up the mounts after ourselves if let Some(target_root) = abs_deployment_path { + if let Err(e) = rustix::mount::unmount( + target_root.join("boot").into_std_path_buf(), + UnmountFlags::DETACH, + ) { + tracing::warn!("Error unmounting target/boot: {e}"); + } for dir in bind_mount_dirs { let mount = target_root .join(dir.strip_prefix("/").unwrap()) From 271bf75d631587cf1f82b7abe256e58dc91c2afe Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Fri, 9 Jan 2026 09:44:43 +0100 Subject: [PATCH 05/15] wip --- crates/lib/src/bootloader.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 5cc6b5834..f83688268 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -108,12 +108,6 @@ pub(crate) fn install_via_bootupd( "/" }; - // rootfs_mountpoint - // .path() - // .to_str() - // .expect("Invalid tempdir path") - //}; - // We mount the linux API file systems into the target deployment before chrooting // so bootupd can find the proper backing device. // xref https://systemd.io/API_FILE_SYSTEMS/ From 6f940151f63a0c22f1a5a7ddfd6c1565495318b4 Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Mon, 19 Jan 2026 17:31:41 +0100 Subject: [PATCH 06/15] bootloader: bind-mount target root on itself This make it a mountpoint so the chrooted `bootupd` process will be able to find the underlying filesystem when `/boot` is not on a separate partition --- crates/lib/src/bootloader.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index f83688268..3e293c5ed 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -114,6 +114,12 @@ pub(crate) fn install_via_bootupd( let bind_mount_dirs = ["/dev", "/run", "/proc", "/sys"]; let chroot_args = if let Some(target_root) = abs_deployment_path.as_deref() { tracing::debug!("Setting up bind-mounts before chrooting to the target deployment"); + // First off, we bind-mount target on itself, so it becomes a mount point and the chrooted + // `findmnt` calls are able to resolve the mount in the chroot + // See https://github.com/coreos/bootupd/issues/1051#issuecomment-3768271509 and following comments + tracing::debug!("bind mounting the target deployement on itslelf"); + rustix::mount::mount_bind(target_root.as_std_path(), target_root.as_std_path())?; + for src in bind_mount_dirs { let dest = target_root // joining an absolute path @@ -185,6 +191,11 @@ pub(crate) fn install_via_bootupd( tracing::warn!("Error unmounting {}: {e}", mount.display()); } } + if let Err(e) = + rustix::mount::unmount(&target_root.into_std_path_buf(), UnmountFlags::DETACH) + { + tracing::warn!("Error unmounting target root bind mount: {e}"); + } } install_result } From 9315633a65a8cbb1927e22f3a9ad9a7e07adb7ea Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Fri, 23 Jan 2026 17:06:02 +0100 Subject: [PATCH 07/15] cleanup --- crates/lib/src/bootloader.rs | 52 +++++++++++++++++------------------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 3e293c5ed..89cc72a2c 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -93,55 +93,51 @@ pub(crate) fn install_via_bootupd( let bootupd_opts = (!configopts.generic_image).then_some(["--update-firmware", "--auto"]); let abs_deployment_path = deployment_path.map(|deploy| rootfs.join(deploy)); + // When not running inside the target container (through `--src-imgref`) we chroot // into the deployment before running bootupd. This makes sure we use binaries // from the target image rather than the buildroot - // In some cases (e.g. --write-uuid), bootupd needs to find the underlying device - // for /boot. But since we don't control where the destination rootfs is mounted - // let's bind mount it to a temp mountpoint under /run - // so it gets carried over in the chroot. + // Bootupd needs to find the underlying device for /boot + // (or / if /boot is not on a separate partition) - // let rootfs_mountpoint: TempDir; - let rootfs_mount = if rootfs.starts_with("/run") { + let rootfs_mount = if abs_deployment_path.is_none() { rootfs.as_str() } else { "/" }; - // We mount the linux API file systems into the target deployment before chrooting - // so bootupd can find the proper backing device. - // xref https://systemd.io/API_FILE_SYSTEMS/ - let bind_mount_dirs = ["/dev", "/run", "/proc", "/sys"]; + let bind_mount_dirs = ["/dev", "/proc", "/run", "/sys"]; let chroot_args = if let Some(target_root) = abs_deployment_path.as_deref() { tracing::debug!("Setting up bind-mounts before chrooting to the target deployment"); + // First off, we bind-mount target on itself, so it becomes a mount point and the chrooted // `findmnt` calls are able to resolve the mount in the chroot // See https://github.com/coreos/bootupd/issues/1051#issuecomment-3768271509 and following comments - tracing::debug!("bind mounting the target deployement on itslelf"); rustix::mount::mount_bind(target_root.as_std_path(), target_root.as_std_path())?; + // We mount the linux API file systems into the target deployment before chrooting + // so bootupd can find the proper backing device. + // xref https://systemd.io/API_FILE_SYSTEMS/ for src in bind_mount_dirs { let dest = target_root // joining an absolute path // makes it replace self, so we strip the prefix .join_os(src.strip_prefix("/").unwrap()); - tracing::debug!("bind mounting {}", dest.display()); + tracing::debug!("bind mounting {} in {}", src, dest.display()); + // We mount them recursively so nested API FS get carried over rustix::mount::mount_bind_recursive(src, dest)?; } - // WIP : let's try to bind-mount /target/boot into the deployment as well rather than bind-mounting the whole thing?? - if !rootfs.starts_with("/run") { - tracing::debug!( - "We need to access the target /boot filesystem so let's also bind-mount it" - ); - let trgt_boot = rootfs.as_std_path().join("boot"); - let chrooted_boot = target_root.join_os("boot"); - tracing::debug!( - "bind-mounting {} in {}", - &trgt_boot.display(), - &chrooted_boot.display() - ); - rustix::mount::mount_bind_recursive(trgt_boot, chrooted_boot)?; - } + + // Also mount the target /boot inside the chroot + let trgt_boot = rootfs.as_std_path().join("boot"); + let chrooted_boot = target_root.join_os("boot"); + tracing::debug!( + "bind-mounting {} in {}", + &trgt_boot.display(), + &chrooted_boot.display() + ); + // mounting recursively to get the EFI partition as well + rustix::mount::mount_bind_recursive(trgt_boot, chrooted_boot)?; // Append the `bootupctl` command, it will be passed as // an argument to chroot @@ -177,7 +173,7 @@ pub(crate) fn install_via_bootupd( if let Some(target_root) = abs_deployment_path { if let Err(e) = rustix::mount::unmount( target_root.join("boot").into_std_path_buf(), - UnmountFlags::DETACH, + UnmountFlags::empty(), ) { tracing::warn!("Error unmounting target/boot: {e}"); } @@ -192,7 +188,7 @@ pub(crate) fn install_via_bootupd( } } if let Err(e) = - rustix::mount::unmount(&target_root.into_std_path_buf(), UnmountFlags::DETACH) + rustix::mount::unmount(&target_root.into_std_path_buf(), UnmountFlags::empty()) { tracing::warn!("Error unmounting target root bind mount: {e}"); } From 1a180aa02ef8e763bae42918f4113e2b09d69d47 Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Fri, 23 Jan 2026 17:07:44 +0100 Subject: [PATCH 08/15] debug GH action --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 120c71de0..4bc889da4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,6 +89,10 @@ jobs: # Install tests sudo bootc-integration-tests install-alongside localhost/bootc-install + # inspect system state after the install tests. + sudo lsblk + sudo mount + # system-reinstall-bootc tests cargo build --release -p system-reinstall-bootc From b3cb5d9a1187652c2c40f6dfc6dc731079639487 Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Mon, 26 Jan 2026 17:18:24 +0100 Subject: [PATCH 09/15] shadow mount a /dev/pts inside the chroot and remove cleanups relying on the private mount namespace --- crates/lib/src/bootloader.rs | 54 +++++++++++++++--------------------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 89cc72a2c..d90ad5687 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -1,3 +1,4 @@ +use std::ffi::CString; use std::fs::create_dir_all; use std::process::Command; @@ -7,10 +8,10 @@ use camino::Utf8Path; use cap_std_ext::cap_std::fs::Dir; use cap_std_ext::dirext::CapStdExtDirExt; use fn_error_context::context; +use rustix::mount::MountFlags; use bootc_blockdev::{Partition, PartitionTable}; use bootc_mount as mount; -use rustix::mount::UnmountFlags; use crate::bootc_composefs::boot::{SecurebootKeys, get_sysroot_parent_dev, mount_esp}; use crate::{discoverable_partition_specification, utils}; @@ -106,6 +107,9 @@ pub(crate) fn install_via_bootupd( "/" }; + // API filesystems to bind mount. + // Note that this is safe to use recursive bind mount + // because we're already in a private mount namespace via ensure_self_unshared_mount_namespace()) let bind_mount_dirs = ["/dev", "/proc", "/run", "/sys"]; let chroot_args = if let Some(target_root) = abs_deployment_path.as_deref() { tracing::debug!("Setting up bind-mounts before chrooting to the target deployment"); @@ -115,8 +119,7 @@ pub(crate) fn install_via_bootupd( // See https://github.com/coreos/bootupd/issues/1051#issuecomment-3768271509 and following comments rustix::mount::mount_bind(target_root.as_std_path(), target_root.as_std_path())?; - // We mount the linux API file systems into the target deployment before chrooting - // so bootupd can find the proper backing device. + // Mount API filesystems recursively // xref https://systemd.io/API_FILE_SYSTEMS/ for src in bind_mount_dirs { let dest = target_root @@ -128,6 +131,21 @@ pub(crate) fn install_via_bootupd( rustix::mount::mount_bind_recursive(src, dest)?; } + // Create a fresh devpts mount inside the chroot that will shadow the + // host one to avoid PTY corruption + let target_dev_pts = target_root.join("dev/pts"); + if target_dev_pts.exists() { + tracing::debug!("mounting devpts at {}", target_dev_pts); + let mount_opts = CString::new("newinstance,ptmxmode=0666,mode=620")?; + rustix::mount::mount( + c"devpts", + target_dev_pts.as_std_path(), + c"devpts", + MountFlags::NOSUID | MountFlags::NOEXEC, + Some(mount_opts.as_c_str()), + )?; + } + // Also mount the target /boot inside the chroot let trgt_boot = rootfs.as_std_path().join("boot"); let chrooted_boot = target_root.join_os("boot"); @@ -153,7 +171,7 @@ pub(crate) fn install_via_bootupd( } else { Command::new("bootupctl") }; - let install_result = bootupctl + bootupctl .args(chroot_args) // Inject a reasonnable PATH here so we find the required tools // when running chrooted in the deployment. Testing show that @@ -167,33 +185,7 @@ pub(crate) fn install_via_bootupd( .args(bootupd_opts.iter().copied().flatten()) .args(["--device", devpath.as_str(), rootfs_mount]) .log_debug() - .run_inherited_with_cmd_context(); - - // Clean up the mounts after ourselves - if let Some(target_root) = abs_deployment_path { - if let Err(e) = rustix::mount::unmount( - target_root.join("boot").into_std_path_buf(), - UnmountFlags::empty(), - ) { - tracing::warn!("Error unmounting target/boot: {e}"); - } - for dir in bind_mount_dirs { - let mount = target_root - .join(dir.strip_prefix("/").unwrap()) - .into_std_path_buf(); - if let Err(e) = rustix::mount::unmount(&mount, UnmountFlags::DETACH) { - // let's not propagate the error up because in some cases we can't unmount - // e.g. when running `to-existing-root` - tracing::warn!("Error unmounting {}: {e}", mount.display()); - } - } - if let Err(e) = - rustix::mount::unmount(&target_root.into_std_path_buf(), UnmountFlags::empty()) - { - tracing::warn!("Error unmounting target root bind mount: {e}"); - } - } - install_result + .run_inherited_with_cmd_context() } #[context("Installing bootloader")] From 42921740d5b264059ba935d1d50c564ec6c8aa70 Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Tue, 27 Jan 2026 10:59:14 +0100 Subject: [PATCH 10/15] add a podman rootfs wrapper to handle the chroot --- crates/lib/src/bootloader.rs | 136 ++++++++-------------- crates/utils/Cargo.toml | 1 + crates/utils/src/lib.rs | 16 ++- crates/utils/src/podman.rs | 211 +++++++++++++++++++++++++++++++++++ 4 files changed, 270 insertions(+), 94 deletions(-) create mode 100644 crates/utils/src/podman.rs diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index d90ad5687..ab08ade58 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -1,19 +1,17 @@ -use std::ffi::CString; use std::fs::create_dir_all; use std::process::Command; -use anyhow::{Context, Result, anyhow, bail}; -use bootc_utils::CommandRunExt; +use anyhow::{anyhow, bail, Context, Result}; +use bootc_utils::{CommandRunExt, PodmanCmd}; use camino::Utf8Path; use cap_std_ext::cap_std::fs::Dir; use cap_std_ext::dirext::CapStdExtDirExt; use fn_error_context::context; -use rustix::mount::MountFlags; use bootc_blockdev::{Partition, PartitionTable}; use bootc_mount as mount; -use crate::bootc_composefs::boot::{SecurebootKeys, get_sysroot_parent_dev, mount_esp}; +use crate::bootc_composefs::boot::{get_sysroot_parent_dev, mount_esp, SecurebootKeys}; use crate::{discoverable_partition_specification, utils}; /// The name of the mountpoint for efi (as a subdirectory of /boot, or at the toplevel) @@ -93,99 +91,61 @@ pub(crate) fn install_via_bootupd( // bootc defaults to only targeting the platform boot method. let bootupd_opts = (!configopts.generic_image).then_some(["--update-firmware", "--auto"]); - let abs_deployment_path = deployment_path.map(|deploy| rootfs.join(deploy)); + let devpath = device.path(); - // When not running inside the target container (through `--src-imgref`) we chroot - // into the deployment before running bootupd. This makes sure we use binaries - // from the target image rather than the buildroot - // Bootupd needs to find the underlying device for /boot - // (or / if /boot is not on a separate partition) + // When not running inside the target container (through `--src-imgref`) we use + // systemd-nspawn to run bootupctl in the deployment. + // This makes sure we use binaries from the target image rather than the buildroot. - let rootfs_mount = if abs_deployment_path.is_none() { + let rootfs_mount = if deployment_path.is_none() { rootfs.as_str() } else { "/" }; - // API filesystems to bind mount. - // Note that this is safe to use recursive bind mount - // because we're already in a private mount namespace via ensure_self_unshared_mount_namespace()) - let bind_mount_dirs = ["/dev", "/proc", "/run", "/sys"]; - let chroot_args = if let Some(target_root) = abs_deployment_path.as_deref() { - tracing::debug!("Setting up bind-mounts before chrooting to the target deployment"); - - // First off, we bind-mount target on itself, so it becomes a mount point and the chrooted - // `findmnt` calls are able to resolve the mount in the chroot - // See https://github.com/coreos/bootupd/issues/1051#issuecomment-3768271509 and following comments - rustix::mount::mount_bind(target_root.as_std_path(), target_root.as_std_path())?; - - // Mount API filesystems recursively - // xref https://systemd.io/API_FILE_SYSTEMS/ - for src in bind_mount_dirs { - let dest = target_root - // joining an absolute path - // makes it replace self, so we strip the prefix - .join_os(src.strip_prefix("/").unwrap()); - tracing::debug!("bind mounting {} in {}", src, dest.display()); - // We mount them recursively so nested API FS get carried over - rustix::mount::mount_bind_recursive(src, dest)?; - } - - // Create a fresh devpts mount inside the chroot that will shadow the - // host one to avoid PTY corruption - let target_dev_pts = target_root.join("dev/pts"); - if target_dev_pts.exists() { - tracing::debug!("mounting devpts at {}", target_dev_pts); - let mount_opts = CString::new("newinstance,ptmxmode=0666,mode=620")?; - rustix::mount::mount( - c"devpts", - target_dev_pts.as_std_path(), - c"devpts", - MountFlags::NOSUID | MountFlags::NOEXEC, - Some(mount_opts.as_c_str()), - )?; - } - - // Also mount the target /boot inside the chroot - let trgt_boot = rootfs.as_std_path().join("boot"); - let chrooted_boot = target_root.join_os("boot"); - tracing::debug!( - "bind-mounting {} in {}", - &trgt_boot.display(), - &chrooted_boot.display() - ); - // mounting recursively to get the EFI partition as well - rustix::mount::mount_bind_recursive(trgt_boot, chrooted_boot)?; - - // Append the `bootupctl` command, it will be passed as - // an argument to chroot - vec![target_root.as_str(), "bootupctl"] - } else { - vec![] - }; - - let devpath = device.path(); println!("Installing bootloader via bootupd"); - let mut bootupctl = if abs_deployment_path.is_some() { - Command::new("chroot") + + // Build the bootupctl arguments + let mut bootupd_args: Vec<&str> = vec!["backend", "install", "--write-uuid"]; + if let Some(v) = verbose { + bootupd_args.push(v); + } + if let Some(ref opts) = bootupd_opts { + bootupd_args.extend(opts.iter().copied()); + } + bootupd_args.extend(["--device", devpath.as_str(), rootfs_mount]); + + // Run inside a podman container using --rootfs. Podman takes care of + // mounting and creating the necessary API filesystems in the target deployment. + if let Some(deploy) = deployment_path { + let target_root = rootfs.join(deploy); + let boot_path = rootfs.join("boot"); + + tracing::debug!("Running bootupctl via podman in {}", target_root); + + // Prepend "bootupctl" to the args for podman + let mut podman_args = vec!["bootupctl"]; + podman_args.extend(bootupd_args); + + PodmanCmd::new(target_root.as_str()) + // Bind mount /boot from the physical target root so bootupctl can find + // the boot partition and install the bootloader there + .bind(boot_path.as_str(), "/boot") + // Bind the target block device so bootupctl can access it + .bind_device(devpath.as_str()) + // Set PATH so we find the required tools + .setenv( + "PATH", + "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", + ) + .run(podman_args) } else { + // Running directly without chroot Command::new("bootupctl") - }; - bootupctl - .args(chroot_args) - // Inject a reasonnable PATH here so we find the required tools - // when running chrooted in the deployment. Testing show that - // the default $PATH value in the chroot is insufficient. - .env( - "PATH", - "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", - ) - .args(["backend", "install", "--write-uuid"]) - .args(verbose) - .args(bootupd_opts.iter().copied().flatten()) - .args(["--device", devpath.as_str(), rootfs_mount]) - .log_debug() - .run_inherited_with_cmd_context() + .args(&bootupd_args) + .log_debug() + .run_inherited_with_cmd_context() + } } #[context("Installing bootloader")] diff --git a/crates/utils/Cargo.toml b/crates/utils/Cargo.toml index 4c3f4007c..a20d59b9f 100644 --- a/crates/utils/Cargo.toml +++ b/crates/utils/Cargo.toml @@ -11,6 +11,7 @@ repository = "https://github.com/bootc-dev/bootc" anstream = { workspace = true } anyhow = { workspace = true } chrono = { workspace = true, features = ["std"] } +fn-error-context = { workspace = true } owo-colors = { workspace = true } rustix = { workspace = true } serde = { workspace = true, features = ["derive"] } diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index bd9948daa..6ef185631 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -4,18 +4,22 @@ //! mod command; pub use command::*; -mod path; -pub use path::*; mod iterators; pub use iterators::*; -mod timestamp; -pub use timestamp::*; -mod tracing_util; -pub use tracing_util::*; +mod nspawn; +pub use nspawn::*; +mod path; +pub use path::*; +mod podman; +pub use podman::*; /// Re-execute the current process pub mod reexec; mod result_ext; pub use result_ext::*; +mod timestamp; +pub use timestamp::*; +mod tracing_util; +pub use tracing_util::*; /// The name of our binary pub const NAME: &str = "bootc"; diff --git a/crates/utils/src/podman.rs b/crates/utils/src/podman.rs new file mode 100644 index 000000000..66c9430db --- /dev/null +++ b/crates/utils/src/podman.rs @@ -0,0 +1,211 @@ +//! Builder for running commands inside a container using podman with --rootfs. +//! +//! This provides clean namespace isolation for running commands in a target +//! root filesystem, handling API filesystem setup (/dev, /proc, /sys) and +//! proper cleanup automatically. + +use std::ffi::OsStr; +use std::process::Command; + +use anyhow::Result; +use fn_error_context::context; + +use crate::CommandRunExt; + +/// Builder for running commands inside a container using podman with --rootfs. +/// +/// Helps running commands in a target deployment, +/// handling API filesystem setup (/dev, /proc, /sys) and cleanup automatically. +/// +/// # Example +/// ```ignore +/// use bootc_utils::PodmanCmd; +/// +/// PodmanCmd::new("/path/to/rootfs") +/// .bind("/host/boot", "/boot") +/// .bind_device("/dev/sda") +/// .run(&["bootupctl", "backend", "install", ...])?; +/// ``` +#[derive(Debug)] +pub struct PodmanCmd<'a> { + /// The root directory for the container + rootfs_path: &'a str, + /// Bind mounts in format (source, target) + bind_mounts: Vec<(&'a str, &'a str)>, + /// Read-only bind mounts in format (source, target) + bind_ro_mounts: Vec<(&'a str, &'a str)>, + /// Device nodes to pass into the container + devices: Vec<&'a str>, + /// Environment variables to set + env_vars: Vec<(&'a str, &'a str)>, + /// Whether to run in privileged mode (default: true for device access) + privileged: bool, + /// Whether to remove the container after exit (default: true) + rm: bool, +} + +impl<'a> PodmanCmd<'a> { + /// Create a new PodmanCmd builder with the given root directory. + pub fn new(path: &'a str) -> Self { + Self { + rootfs_path: path, + bind_mounts: Vec::new(), + bind_ro_mounts: Vec::new(), + devices: Vec::new(), + env_vars: Vec::new(), + privileged: true, + rm: true, + } + } + + /// Add a bind mount from source to target inside the container. + /// If target is the same as source, you can pass the same value for both. + pub fn bind(mut self, source: &'a str, target: &'a str) -> Self { + self.bind_mounts.push((source, target)); + self + } + + /// Add a read-only bind mount from source to target inside the container. + #[allow(dead_code)] + pub fn bind_ro(mut self, source: &'a str, target: &'a str) -> Self { + self.bind_ro_mounts.push((source, target)); + self + } + + /// Pass a device node into the container. + pub fn bind_device(mut self, device: &'a str) -> Self { + self.devices.push(device); + self + } + + /// Set an environment variable for the command. + pub fn setenv(mut self, key: &'a str, value: &'a str) -> Self { + self.env_vars.push((key, value)); + self + } + + /// Whether to run in privileged mode (default: true). + #[allow(dead_code)] + pub fn privileged(mut self, privileged: bool) -> Self { + self.privileged = privileged; + self + } + + /// Whether to remove the container after exit (default: true). + #[allow(dead_code)] + pub fn rm(mut self, rm: bool) -> Self { + self.rm = rm; + self + } + + /// Build the podman Command. + pub fn command>(&self, args: impl IntoIterator) -> Command { + let mut cmd = Command::new("podman"); + + cmd.arg("run"); + + if self.rm { + cmd.arg("--rm"); + } + + if self.privileged { + cmd.arg("--privileged"); + } + + // Add bind mounts as volumes + for (source, target) in &self.bind_mounts { + cmd.args(["--volume", &format!("{source}:{target}")]); + } + + // Add read-only bind mounts + for (source, target) in &self.bind_ro_mounts { + cmd.args(["--volume", &format!("{source}:{target}:ro")]); + } + + // Add device nodes + for device in &self.devices { + cmd.args(["--device", device]); + } + + // Add environment variables + for (key, value) in &self.env_vars { + cmd.args(["--env", &format!("{key}={value}")]); + } + + // Use the rootfs as the container filesystem + cmd.args(["--rootfs", self.rootfs_path]); + + // Add the command to run + cmd.args(args); + + cmd + } + + /// Run the specified command inside the container. + #[context("Running command in podman container")] + pub fn run>(self, args: impl IntoIterator) -> Result<()> { + self.command(args) + .log_debug() + .run_inherited_with_cmd_context() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_podman_command_basic() { + let podman = PodmanCmd::new("/some/rootfs"); + let cmd = podman.command(["echo", "hello"]); + + let args: Vec<_> = cmd.get_args().collect(); + assert!(args.contains(&&std::ffi::OsStr::new("run"))); + assert!(args.contains(&&std::ffi::OsStr::new("--rm"))); + assert!(args.contains(&&std::ffi::OsStr::new("--privileged"))); + assert!(args.contains(&&std::ffi::OsStr::new("--rootfs"))); + assert!(args.contains(&&std::ffi::OsStr::new("/some/rootfs"))); + assert!(args.contains(&&std::ffi::OsStr::new("echo"))); + assert!(args.contains(&&std::ffi::OsStr::new("hello"))); + } + + #[test] + fn test_podman_command_with_binds() { + let podman = PodmanCmd::new("/rootfs") + .bind("/host/boot", "/boot") + .bind_ro("/host/etc", "/etc") + .bind_device("/dev/sda"); + + let cmd = podman.command(["ls"]); + let args: Vec<_> = cmd.get_args().collect(); + + // Check volume mount format + assert!(args.contains(&&std::ffi::OsStr::new("--volume"))); + assert!(args.contains(&&std::ffi::OsStr::new("/host/boot:/boot"))); + assert!(args.contains(&&std::ffi::OsStr::new("/host/etc:/etc:ro"))); + assert!(args.contains(&&std::ffi::OsStr::new("--device"))); + assert!(args.contains(&&std::ffi::OsStr::new("/dev/sda"))); + } + + #[test] + fn test_podman_command_with_env() { + let podman = PodmanCmd::new("/rootfs").setenv("PATH", "/usr/bin:/bin"); + + let cmd = podman.command(["ls"]); + let args: Vec<_> = cmd.get_args().collect(); + + assert!(args.contains(&&std::ffi::OsStr::new("--env"))); + assert!(args.contains(&&std::ffi::OsStr::new("PATH=/usr/bin:/bin"))); + } + + #[test] + fn test_podman_command_not_privileged() { + let podman = PodmanCmd::new("/rootfs").privileged(false); + + let cmd = podman.command(["ls"]); + let args: Vec<_> = cmd.get_args().collect(); + + // Should NOT contain --privileged when privileged is false + assert!(!args.contains(&&std::ffi::OsStr::new("--privileged"))); + } +} From f94cf486e28ea7639653073515ddfd70dac88645 Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Tue, 27 Jan 2026 11:00:38 +0100 Subject: [PATCH 11/15] append cargo lock --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index 45399dec0..a55db8a3f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -195,6 +195,7 @@ dependencies = [ "anstream", "anyhow", "chrono", + "fn-error-context", "owo-colors", "rustix", "serde", From 1af726417ebdc791fe67474f9b08c01d2c6f095d Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Tue, 27 Jan 2026 11:07:28 +0100 Subject: [PATCH 12/15] remove nspawn wrapper for now --- crates/utils/src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index 6ef185631..4fcaf8853 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -6,8 +6,6 @@ mod command; pub use command::*; mod iterators; pub use iterators::*; -mod nspawn; -pub use nspawn::*; mod path; pub use path::*; mod podman; From f2e94ca83ce0eae39bca23998df3a6d0ec2d8e02 Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Tue, 27 Jan 2026 15:31:55 +0100 Subject: [PATCH 13/15] try a bwrap approach --- crates/lib/src/bootloader.rs | 28 +++-- crates/utils/src/bwrap.rs | 214 +++++++++++++++++++++++++++++++++++ crates/utils/src/lib.rs | 4 + crates/utils/src/nspawn.rs | 212 ++++++++++++++++++++++++++++++++++ 4 files changed, 448 insertions(+), 10 deletions(-) create mode 100644 crates/utils/src/bwrap.rs create mode 100644 crates/utils/src/nspawn.rs diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index ab08ade58..cc04f73f6 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -2,7 +2,7 @@ use std::fs::create_dir_all; use std::process::Command; use anyhow::{anyhow, bail, Context, Result}; -use bootc_utils::{CommandRunExt, PodmanCmd}; +use bootc_utils::{BwrapCmd, CommandRunExt}; use camino::Utf8Path; use cap_std_ext::cap_std::fs::Dir; use cap_std_ext::dirext::CapStdExtDirExt; @@ -115,30 +115,38 @@ pub(crate) fn install_via_bootupd( } bootupd_args.extend(["--device", devpath.as_str(), rootfs_mount]); - // Run inside a podman container using --rootfs. Podman takes care of - // mounting and creating the necessary API filesystems in the target deployment. + // Run inside a bwrap container. Bwrap takes care of mounting and creating + // the necessary API filesystems in the target deployment. if let Some(deploy) = deployment_path { let target_root = rootfs.join(deploy); let boot_path = rootfs.join("boot"); - tracing::debug!("Running bootupctl via podman in {}", target_root); + tracing::debug!("Running bootupctl via bwrap in {}", target_root); - // Prepend "bootupctl" to the args for podman - let mut podman_args = vec!["bootupctl"]; - podman_args.extend(bootupd_args); + // Prepend "bootupctl" to the args for bwrap + let mut bwrap_args = vec!["bootupctl"]; + bwrap_args.extend(bootupd_args); - PodmanCmd::new(target_root.as_str()) + let mut cmd = BwrapCmd::new(target_root.as_str()) // Bind mount /boot from the physical target root so bootupctl can find // the boot partition and install the bootloader there .bind(boot_path.as_str(), "/boot") // Bind the target block device so bootupctl can access it - .bind_device(devpath.as_str()) + .bind_device(devpath.as_str()); + + // Also bind all partition devices (e.g., /dev/loop0p1, /dev/loop0p2, etc.) + // so that grub2-install and other tools can access them + for partition in &device.partitions { + cmd = cmd.bind_device(partition.node.as_str()); + } + + cmd // Set PATH so we find the required tools .setenv( "PATH", "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", ) - .run(podman_args) + .run(bwrap_args) } else { // Running directly without chroot Command::new("bootupctl") diff --git a/crates/utils/src/bwrap.rs b/crates/utils/src/bwrap.rs new file mode 100644 index 000000000..548192c0e --- /dev/null +++ b/crates/utils/src/bwrap.rs @@ -0,0 +1,214 @@ +//! Builder for running commands inside a container using bubblewrap (bwrap). +//! +//! This provides clean namespace isolation for running commands in a target +//! root filesystem, handling API filesystem setup (/dev, /proc, /sys) and +//! proper cleanup automatically. + +use std::ffi::OsStr; +use std::process::Command; + +use anyhow::Result; +use fn_error_context::context; + +use crate::CommandRunExt; + +/// Builder for running commands inside a container using bubblewrap (bwrap). +/// +/// Helps running commands in a target deployment, +/// handling API filesystem setup (/dev, /proc, /sys) and cleanup automatically. +/// +/// # Example +/// ```ignore +/// use bootc_utils::BwrapCmd; +/// +/// BwrapCmd::new("/path/to/rootfs") +/// .bind("/host/boot", "/boot") +/// .bind_device("/dev/sda") +/// .run(&["bootupctl", "backend", "install", ...])?; +/// ``` +#[derive(Debug)] +pub struct BwrapCmd<'a> { + /// The root directory for the container + chroot_path: &'a str, + /// Bind mounts in format (source, target) + bind_mounts: Vec<(&'a str, &'a str)>, + /// Read-only bind mounts in format (source, target) + bind_ro_mounts: Vec<(&'a str, &'a str)>, + /// Device nodes to bind into the container + devices: Vec<&'a str>, + /// Environment variables to set + env_vars: Vec<(&'a str, &'a str)>, + /// Whether to suppress output (no-op for bwrap, kept for API compatibility) + quiet: bool, +} + +impl<'a> BwrapCmd<'a> { + /// Create a new BwrapCmd builder with the given root directory. + pub fn new(path: &'a str) -> Self { + Self { + chroot_path: path, + bind_mounts: Vec::new(), + bind_ro_mounts: Vec::new(), + devices: Vec::new(), + env_vars: Vec::new(), + quiet: true, + } + } + + /// Add a bind mount from source to target inside the container. + /// If target is the same as source, you can pass the same value for both. + pub fn bind(mut self, source: &'a str, target: &'a str) -> Self { + self.bind_mounts.push((source, target)); + self + } + + /// Add a read-only bind mount from source to target inside the container. + #[allow(dead_code)] + pub fn bind_ro(mut self, source: &'a str, target: &'a str) -> Self { + self.bind_ro_mounts.push((source, target)); + self + } + + /// Bind a device node into the container. + /// This bind-mounts the device node so it's accessible inside. + pub fn bind_device(mut self, device: &'a str) -> Self { + self.devices.push(device); + self + } + + /// Set an environment variable for the command. + pub fn setenv(mut self, key: &'a str, value: &'a str) -> Self { + self.env_vars.push((key, value)); + self + } + + /// Whether to suppress output (no-op for bwrap, kept for API compatibility). + #[allow(dead_code)] + pub fn quiet(mut self, quiet: bool) -> Self { + self.quiet = quiet; + self + } + + /// Build the bwrap Command. + pub fn command>(&self, args: impl IntoIterator) -> Command { + let mut cmd = Command::new("bwrap"); + + // Bind the root filesystem + cmd.args(["--bind", self.chroot_path, "/"]); + + // Setup API filesystems automatically + // --proc creates a new procfs mount + cmd.args(["--proc", "/proc"]); + // --dev creates a minimal /dev with null, zero, full, random, urandom, tty + cmd.args(["--dev", "/dev"]); + // Bind /sys read-only for safety + cmd.args(["--ro-bind", "/sys", "/sys"]); + + // Add bind mounts + for (source, target) in &self.bind_mounts { + cmd.args(["--bind", source, target]); + } + + // Add read-only bind mounts + for (source, target) in &self.bind_ro_mounts { + cmd.args(["--ro-bind", source, target]); + } + + // Add device bind mounts using --dev-bind to preserve device node permissions + for device in &self.devices { + cmd.args(["--dev-bind", device, device]); + } + + // Add environment variables + for (key, value) in &self.env_vars { + cmd.args(["--setenv", key, value]); + } + + // Separator and command to run + cmd.arg("--"); + cmd.args(args); + + cmd + } + + /// Run the specified command inside the container. + #[context("Running command in bwrap container")] + pub fn run>(self, args: impl IntoIterator) -> Result<()> { + self.command(args) + .log_debug() + .run_inherited_with_cmd_context() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_bwrap_command_basic() { + let bwrap = BwrapCmd::new("/some/rootfs"); + let cmd = bwrap.command(["echo", "hello"]); + + let args: Vec<_> = cmd.get_args().collect(); + assert!(args.contains(&&std::ffi::OsStr::new("--bind"))); + assert!(args.contains(&&std::ffi::OsStr::new("/some/rootfs"))); + assert!(args.contains(&&std::ffi::OsStr::new("/"))); + assert!(args.contains(&&std::ffi::OsStr::new("--proc"))); + assert!(args.contains(&&std::ffi::OsStr::new("/proc"))); + assert!(args.contains(&&std::ffi::OsStr::new("--dev"))); + assert!(args.contains(&&std::ffi::OsStr::new("/dev"))); + assert!(args.contains(&&std::ffi::OsStr::new("--ro-bind"))); + assert!(args.contains(&&std::ffi::OsStr::new("/sys"))); + assert!(args.contains(&&std::ffi::OsStr::new("--"))); + assert!(args.contains(&&std::ffi::OsStr::new("echo"))); + assert!(args.contains(&&std::ffi::OsStr::new("hello"))); + } + + #[test] + fn test_bwrap_command_with_binds() { + let bwrap = BwrapCmd::new("/rootfs") + .bind("/host/boot", "/boot") + .bind_ro("/host/etc", "/etc") + .bind_device("/dev/sda"); + + let cmd = bwrap.command(["ls"]); + let args: Vec<_> = cmd.get_args().collect(); + + // Check bind mount - bwrap uses separate args, not colon-separated + assert!(args.contains(&&std::ffi::OsStr::new("/host/boot"))); + assert!(args.contains(&&std::ffi::OsStr::new("/boot"))); + assert!(args.contains(&&std::ffi::OsStr::new("/host/etc"))); + assert!(args.contains(&&std::ffi::OsStr::new("/etc"))); + // Device binds use --dev-bind + assert!(args.contains(&&std::ffi::OsStr::new("--dev-bind"))); + assert!(args.contains(&&std::ffi::OsStr::new("/dev/sda"))); + } + + #[test] + fn test_bwrap_command_same_source_target() { + let bwrap = BwrapCmd::new("/rootfs").bind("/same/path", "/same/path"); + + let cmd = bwrap.command(["ls"]); + let args: Vec<_> = cmd.get_args().collect(); + + // bwrap always uses separate source and target args + // Count occurrences of /same/path - should appear twice (source and target) + let count = args + .iter() + .filter(|a| *a == &std::ffi::OsStr::new("/same/path")) + .count(); + assert_eq!(count, 2); + } + + #[test] + fn test_bwrap_command_with_env() { + let bwrap = BwrapCmd::new("/rootfs").setenv("PATH", "/usr/bin:/bin"); + + let cmd = bwrap.command(["ls"]); + let args: Vec<_> = cmd.get_args().collect(); + + assert!(args.contains(&&std::ffi::OsStr::new("--setenv"))); + assert!(args.contains(&&std::ffi::OsStr::new("PATH"))); + assert!(args.contains(&&std::ffi::OsStr::new("/usr/bin:/bin"))); + } +} diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index 4fcaf8853..e8bd00e28 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -2,6 +2,8 @@ //! things here that only depend on the standard library and //! "core" crates. //! +mod bwrap; +pub use bwrap::*; mod command; pub use command::*; mod iterators; @@ -16,6 +18,8 @@ mod result_ext; pub use result_ext::*; mod timestamp; pub use timestamp::*; +mod nspawn; +pub use nspawn::*; mod tracing_util; pub use tracing_util::*; diff --git a/crates/utils/src/nspawn.rs b/crates/utils/src/nspawn.rs new file mode 100644 index 000000000..364fe8706 --- /dev/null +++ b/crates/utils/src/nspawn.rs @@ -0,0 +1,212 @@ +//! Builder for running commands inside a container using systemd-nspawn. +//! +//! This provides clean namespace isolation for running commands in a target +//! root filesystem, handling API filesystem setup (/dev, /proc, /sys) and +//! proper cleanup automatically. + +use std::ffi::OsStr; +use std::process::Command; + +use anyhow::Result; +use fn_error_context::context; + +use crate::CommandRunExt; + +/// Builder for running commands inside a container using systemd-nspawn. +/// +/// Helps running commands in a target deployment, +/// handling API filesystem setup (/dev, /proc, /sys) and cleanup automatically. +/// +/// # Example +/// ```ignore +/// use bootc_utils::NspawnCmd; +/// +/// NspawnCmd::new("/path/to/rootfs") +/// .bind("/host/boot", "/boot") +/// .bind_device("/dev/sda") +/// .run(&["bootupctl", "backend", "install", ...])?; +/// ``` +#[derive(Debug)] +pub struct NspawnCmd<'a> { + /// The root directory for the container + chroot_path: &'a str, + /// Bind mounts in format (source, target) + bind_mounts: Vec<(&'a str, &'a str)>, + /// Read-only bind mounts in format (source, target) + bind_ro_mounts: Vec<(&'a str, &'a str)>, + /// Device nodes to bind into the container + devices: Vec<&'a str>, + /// Environment variables to set + env_vars: Vec<(&'a str, &'a str)>, + /// Whether to suppress nspawn's own output + quiet: bool, +} + +impl<'a> NspawnCmd<'a> { + /// Create a new NspawnCmd builder with the given root directory. + pub fn new(path: &'a str) -> Self { + Self { + chroot_path: path, + bind_mounts: Vec::new(), + bind_ro_mounts: Vec::new(), + devices: Vec::new(), + env_vars: Vec::new(), + quiet: true, + } + } + + /// Add a bind mount from source to target inside the container. + /// If target is the same as source, you can pass the same value for both. + pub fn bind(mut self, source: &'a str, target: &'a str) -> Self { + self.bind_mounts.push((source, target)); + self + } + + /// Add a read-only bind mount from source to target inside the container. + #[allow(dead_code)] + pub fn bind_ro(mut self, source: &'a str, target: &'a str) -> Self { + self.bind_ro_mounts.push((source, target)); + self + } + + /// Bind a device node into the container. + /// This bind-mounts the device node so it's accessible inside. + pub fn bind_device(mut self, device: &'a str) -> Self { + self.devices.push(device); + self + } + + /// Set an environment variable for the command. + pub fn setenv(mut self, key: &'a str, value: &'a str) -> Self { + self.env_vars.push((key, value)); + self + } + + /// Whether to suppress nspawn's own output (default: true). + #[allow(dead_code)] + pub fn quiet(mut self, quiet: bool) -> Self { + self.quiet = quiet; + self + } + + /// Build the nspawn Command. + pub fn command>(&self, args: impl IntoIterator) -> Command { + let mut cmd = Command::new("/usr/sbin/systemd-nspawn"); + + // Basic options for non-interactive, isolated execution + cmd.args(["--directory", self.chroot_path]); + cmd.arg("--pipe"); // Non-interactive, pass stdio through + + if self.quiet { + cmd.arg("--quiet"); // Suppress nspawn's own output + } + + // Add bind mounts + for (source, target) in &self.bind_mounts { + if source == target { + cmd.args(["--bind", source]); + } else { + cmd.args(["--bind", &format!("{source}:{target}")]); + } + } + + // Add read-only bind mounts + for (source, target) in &self.bind_ro_mounts { + if source == target { + cmd.args(["--bind-ro", source]); + } else { + cmd.args(["--bind-ro", &format!("{source}:{target}")]); + } + } + + // Add device bind mounts - we bind-mount device nodes directly + for device in &self.devices { + cmd.args(["--bind", device]); + } + + // Add environment variables + for (key, value) in &self.env_vars { + cmd.args(["--setenv", &format!("{key}={value}")]); + } + + // don't register the service with machinectl + // as this is expected to be short-lived + cmd.arg("--register=no"); + + // Separator and command to run + cmd.arg("--"); + cmd.args(args); + + cmd + } + + /// Run the specified command inside the container. + #[context("Running command in nspawn container")] + pub fn run>(self, args: impl IntoIterator) -> Result<()> { + self.command(args) + .log_debug() + .run_inherited_with_cmd_context() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_nspawn_command_basic() { + let nspawn = NspawnCmd::new("/some/rootfs"); + let cmd = nspawn.command(["echo", "hello"]); + + let args: Vec<_> = cmd.get_args().collect(); + assert!(args.contains(&&std::ffi::OsStr::new("--directory"))); + assert!(args.contains(&&std::ffi::OsStr::new("/some/rootfs"))); + assert!(args.contains(&&std::ffi::OsStr::new("--pipe"))); + assert!(args.contains(&&std::ffi::OsStr::new("--quiet"))); + assert!(args.contains(&&std::ffi::OsStr::new("--register=no"))); + assert!(args.contains(&&std::ffi::OsStr::new("--"))); + assert!(args.contains(&&std::ffi::OsStr::new("echo"))); + assert!(args.contains(&&std::ffi::OsStr::new("hello"))); + } + + #[test] + fn test_nspawn_command_with_binds() { + let nspawn = NspawnCmd::new("/rootfs") + .bind("/host/boot", "/boot") + .bind_ro("/host/etc", "/etc") + .bind_device("/dev/sda"); + + let cmd = nspawn.command(["ls"]); + let args: Vec<_> = cmd.get_args().collect(); + + // Check bind mount format + assert!(args.contains(&&std::ffi::OsStr::new("--bind"))); + assert!(args.contains(&&std::ffi::OsStr::new("/host/boot:/boot"))); + assert!(args.contains(&&std::ffi::OsStr::new("--bind-ro"))); + assert!(args.contains(&&std::ffi::OsStr::new("/host/etc:/etc"))); + assert!(args.contains(&&std::ffi::OsStr::new("/dev/sda"))); + } + + #[test] + fn test_nspawn_command_same_source_target() { + let nspawn = NspawnCmd::new("/rootfs").bind("/same/path", "/same/path"); + + let cmd = nspawn.command(["ls"]); + let args: Vec<_> = cmd.get_args().collect(); + + // When source == target, should just use source without colon + assert!(args.contains(&&std::ffi::OsStr::new("/same/path"))); + assert!(!args.iter().any(|a| a.to_str().unwrap().contains(':'))); + } + + #[test] + fn test_nspawn_command_with_env() { + let nspawn = NspawnCmd::new("/rootfs").setenv("PATH", "/usr/bin:/bin"); + + let cmd = nspawn.command(["ls"]); + let args: Vec<_> = cmd.get_args().collect(); + + assert!(args.contains(&&std::ffi::OsStr::new("--setenv"))); + assert!(args.contains(&&std::ffi::OsStr::new("PATH=/usr/bin:/bin"))); + } +} From e94ab589a406d6c2efeec12eb1ac488c37985083 Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Tue, 27 Jan 2026 17:32:25 +0100 Subject: [PATCH 14/15] bwrap fixups --- crates/lib/src/bootloader.rs | 17 ++- crates/utils/src/bwrap.rs | 143 +++-------------------- crates/utils/src/lib.rs | 4 - crates/utils/src/nspawn.rs | 212 ----------------------------------- crates/utils/src/podman.rs | 211 ---------------------------------- 5 files changed, 21 insertions(+), 566 deletions(-) delete mode 100644 crates/utils/src/nspawn.rs delete mode 100644 crates/utils/src/podman.rs diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index cc04f73f6..c2f74f8a0 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -134,19 +134,16 @@ pub(crate) fn install_via_bootupd( // Bind the target block device so bootupctl can access it .bind_device(devpath.as_str()); - // Also bind all partition devices (e.g., /dev/loop0p1, /dev/loop0p2, etc.) - // so that grub2-install and other tools can access them + // Bind all partition devices so grub2-install can access them for partition in &device.partitions { - cmd = cmd.bind_device(partition.node.as_str()); + cmd = cmd.bind_device(&partition.node); } - cmd - // Set PATH so we find the required tools - .setenv( - "PATH", - "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", - ) - .run(bwrap_args) + cmd.setenv( + "PATH", + "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", + ) + .run(bwrap_args) } else { // Running directly without chroot Command::new("bootupctl") diff --git a/crates/utils/src/bwrap.rs b/crates/utils/src/bwrap.rs index 548192c0e..86dde78a4 100644 --- a/crates/utils/src/bwrap.rs +++ b/crates/utils/src/bwrap.rs @@ -1,8 +1,8 @@ //! Builder for running commands inside a container using bubblewrap (bwrap). //! -//! This provides clean namespace isolation for running commands in a target -//! root filesystem, handling API filesystem setup (/dev, /proc, /sys) and -//! proper cleanup automatically. +//! This provides namespace isolation for running commands in a target +//! root filesystem, handling API filesystem setup (/dev, /proc, /sys) +//! automatically. use std::ffi::OsStr; use std::process::Command; @@ -14,9 +14,6 @@ use crate::CommandRunExt; /// Builder for running commands inside a container using bubblewrap (bwrap). /// -/// Helps running commands in a target deployment, -/// handling API filesystem setup (/dev, /proc, /sys) and cleanup automatically. -/// /// # Example /// ```ignore /// use bootc_utils::BwrapCmd; @@ -24,22 +21,18 @@ use crate::CommandRunExt; /// BwrapCmd::new("/path/to/rootfs") /// .bind("/host/boot", "/boot") /// .bind_device("/dev/sda") -/// .run(&["bootupctl", "backend", "install", ...])?; +/// .run(&["grub2-install", "/dev/sda"])?; /// ``` -#[derive(Debug)] +#[derive(Debug, Default)] pub struct BwrapCmd<'a> { /// The root directory for the container chroot_path: &'a str, /// Bind mounts in format (source, target) bind_mounts: Vec<(&'a str, &'a str)>, - /// Read-only bind mounts in format (source, target) - bind_ro_mounts: Vec<(&'a str, &'a str)>, /// Device nodes to bind into the container devices: Vec<&'a str>, /// Environment variables to set env_vars: Vec<(&'a str, &'a str)>, - /// Whether to suppress output (no-op for bwrap, kept for API compatibility) - quiet: bool, } impl<'a> BwrapCmd<'a> { @@ -47,30 +40,17 @@ impl<'a> BwrapCmd<'a> { pub fn new(path: &'a str) -> Self { Self { chroot_path: path, - bind_mounts: Vec::new(), - bind_ro_mounts: Vec::new(), - devices: Vec::new(), - env_vars: Vec::new(), - quiet: true, + ..Default::default() } } /// Add a bind mount from source to target inside the container. - /// If target is the same as source, you can pass the same value for both. pub fn bind(mut self, source: &'a str, target: &'a str) -> Self { self.bind_mounts.push((source, target)); self } - /// Add a read-only bind mount from source to target inside the container. - #[allow(dead_code)] - pub fn bind_ro(mut self, source: &'a str, target: &'a str) -> Self { - self.bind_ro_mounts.push((source, target)); - self - } - /// Bind a device node into the container. - /// This bind-mounts the device node so it's accessible inside. pub fn bind_device(mut self, device: &'a str) -> Self { self.devices.push(device); self @@ -82,26 +62,17 @@ impl<'a> BwrapCmd<'a> { self } - /// Whether to suppress output (no-op for bwrap, kept for API compatibility). - #[allow(dead_code)] - pub fn quiet(mut self, quiet: bool) -> Self { - self.quiet = quiet; - self - } - - /// Build the bwrap Command. - pub fn command>(&self, args: impl IntoIterator) -> Command { + /// Run the specified command inside the container. + #[context("Running command in bwrap container")] + pub fn run>(self, args: impl IntoIterator) -> Result<()> { let mut cmd = Command::new("bwrap"); // Bind the root filesystem cmd.args(["--bind", self.chroot_path, "/"]); - // Setup API filesystems automatically - // --proc creates a new procfs mount + // Setup API filesystems cmd.args(["--proc", "/proc"]); - // --dev creates a minimal /dev with null, zero, full, random, urandom, tty cmd.args(["--dev", "/dev"]); - // Bind /sys read-only for safety cmd.args(["--ro-bind", "/sys", "/sys"]); // Add bind mounts @@ -109,13 +80,8 @@ impl<'a> BwrapCmd<'a> { cmd.args(["--bind", source, target]); } - // Add read-only bind mounts - for (source, target) in &self.bind_ro_mounts { - cmd.args(["--ro-bind", source, target]); - } - - // Add device bind mounts using --dev-bind to preserve device node permissions - for device in &self.devices { + // Add device bind mounts + for device in self.devices { cmd.args(["--dev-bind", device, device]); } @@ -124,91 +90,10 @@ impl<'a> BwrapCmd<'a> { cmd.args(["--setenv", key, value]); } - // Separator and command to run + // Command to run cmd.arg("--"); cmd.args(args); - cmd - } - - /// Run the specified command inside the container. - #[context("Running command in bwrap container")] - pub fn run>(self, args: impl IntoIterator) -> Result<()> { - self.command(args) - .log_debug() - .run_inherited_with_cmd_context() - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_bwrap_command_basic() { - let bwrap = BwrapCmd::new("/some/rootfs"); - let cmd = bwrap.command(["echo", "hello"]); - - let args: Vec<_> = cmd.get_args().collect(); - assert!(args.contains(&&std::ffi::OsStr::new("--bind"))); - assert!(args.contains(&&std::ffi::OsStr::new("/some/rootfs"))); - assert!(args.contains(&&std::ffi::OsStr::new("/"))); - assert!(args.contains(&&std::ffi::OsStr::new("--proc"))); - assert!(args.contains(&&std::ffi::OsStr::new("/proc"))); - assert!(args.contains(&&std::ffi::OsStr::new("--dev"))); - assert!(args.contains(&&std::ffi::OsStr::new("/dev"))); - assert!(args.contains(&&std::ffi::OsStr::new("--ro-bind"))); - assert!(args.contains(&&std::ffi::OsStr::new("/sys"))); - assert!(args.contains(&&std::ffi::OsStr::new("--"))); - assert!(args.contains(&&std::ffi::OsStr::new("echo"))); - assert!(args.contains(&&std::ffi::OsStr::new("hello"))); - } - - #[test] - fn test_bwrap_command_with_binds() { - let bwrap = BwrapCmd::new("/rootfs") - .bind("/host/boot", "/boot") - .bind_ro("/host/etc", "/etc") - .bind_device("/dev/sda"); - - let cmd = bwrap.command(["ls"]); - let args: Vec<_> = cmd.get_args().collect(); - - // Check bind mount - bwrap uses separate args, not colon-separated - assert!(args.contains(&&std::ffi::OsStr::new("/host/boot"))); - assert!(args.contains(&&std::ffi::OsStr::new("/boot"))); - assert!(args.contains(&&std::ffi::OsStr::new("/host/etc"))); - assert!(args.contains(&&std::ffi::OsStr::new("/etc"))); - // Device binds use --dev-bind - assert!(args.contains(&&std::ffi::OsStr::new("--dev-bind"))); - assert!(args.contains(&&std::ffi::OsStr::new("/dev/sda"))); - } - - #[test] - fn test_bwrap_command_same_source_target() { - let bwrap = BwrapCmd::new("/rootfs").bind("/same/path", "/same/path"); - - let cmd = bwrap.command(["ls"]); - let args: Vec<_> = cmd.get_args().collect(); - - // bwrap always uses separate source and target args - // Count occurrences of /same/path - should appear twice (source and target) - let count = args - .iter() - .filter(|a| *a == &std::ffi::OsStr::new("/same/path")) - .count(); - assert_eq!(count, 2); - } - - #[test] - fn test_bwrap_command_with_env() { - let bwrap = BwrapCmd::new("/rootfs").setenv("PATH", "/usr/bin:/bin"); - - let cmd = bwrap.command(["ls"]); - let args: Vec<_> = cmd.get_args().collect(); - - assert!(args.contains(&&std::ffi::OsStr::new("--setenv"))); - assert!(args.contains(&&std::ffi::OsStr::new("PATH"))); - assert!(args.contains(&&std::ffi::OsStr::new("/usr/bin:/bin"))); + cmd.log_debug().run_inherited_with_cmd_context() } } diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index e8bd00e28..39ae772c3 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -10,16 +10,12 @@ mod iterators; pub use iterators::*; mod path; pub use path::*; -mod podman; -pub use podman::*; /// Re-execute the current process pub mod reexec; mod result_ext; pub use result_ext::*; mod timestamp; pub use timestamp::*; -mod nspawn; -pub use nspawn::*; mod tracing_util; pub use tracing_util::*; diff --git a/crates/utils/src/nspawn.rs b/crates/utils/src/nspawn.rs deleted file mode 100644 index 364fe8706..000000000 --- a/crates/utils/src/nspawn.rs +++ /dev/null @@ -1,212 +0,0 @@ -//! Builder for running commands inside a container using systemd-nspawn. -//! -//! This provides clean namespace isolation for running commands in a target -//! root filesystem, handling API filesystem setup (/dev, /proc, /sys) and -//! proper cleanup automatically. - -use std::ffi::OsStr; -use std::process::Command; - -use anyhow::Result; -use fn_error_context::context; - -use crate::CommandRunExt; - -/// Builder for running commands inside a container using systemd-nspawn. -/// -/// Helps running commands in a target deployment, -/// handling API filesystem setup (/dev, /proc, /sys) and cleanup automatically. -/// -/// # Example -/// ```ignore -/// use bootc_utils::NspawnCmd; -/// -/// NspawnCmd::new("/path/to/rootfs") -/// .bind("/host/boot", "/boot") -/// .bind_device("/dev/sda") -/// .run(&["bootupctl", "backend", "install", ...])?; -/// ``` -#[derive(Debug)] -pub struct NspawnCmd<'a> { - /// The root directory for the container - chroot_path: &'a str, - /// Bind mounts in format (source, target) - bind_mounts: Vec<(&'a str, &'a str)>, - /// Read-only bind mounts in format (source, target) - bind_ro_mounts: Vec<(&'a str, &'a str)>, - /// Device nodes to bind into the container - devices: Vec<&'a str>, - /// Environment variables to set - env_vars: Vec<(&'a str, &'a str)>, - /// Whether to suppress nspawn's own output - quiet: bool, -} - -impl<'a> NspawnCmd<'a> { - /// Create a new NspawnCmd builder with the given root directory. - pub fn new(path: &'a str) -> Self { - Self { - chroot_path: path, - bind_mounts: Vec::new(), - bind_ro_mounts: Vec::new(), - devices: Vec::new(), - env_vars: Vec::new(), - quiet: true, - } - } - - /// Add a bind mount from source to target inside the container. - /// If target is the same as source, you can pass the same value for both. - pub fn bind(mut self, source: &'a str, target: &'a str) -> Self { - self.bind_mounts.push((source, target)); - self - } - - /// Add a read-only bind mount from source to target inside the container. - #[allow(dead_code)] - pub fn bind_ro(mut self, source: &'a str, target: &'a str) -> Self { - self.bind_ro_mounts.push((source, target)); - self - } - - /// Bind a device node into the container. - /// This bind-mounts the device node so it's accessible inside. - pub fn bind_device(mut self, device: &'a str) -> Self { - self.devices.push(device); - self - } - - /// Set an environment variable for the command. - pub fn setenv(mut self, key: &'a str, value: &'a str) -> Self { - self.env_vars.push((key, value)); - self - } - - /// Whether to suppress nspawn's own output (default: true). - #[allow(dead_code)] - pub fn quiet(mut self, quiet: bool) -> Self { - self.quiet = quiet; - self - } - - /// Build the nspawn Command. - pub fn command>(&self, args: impl IntoIterator) -> Command { - let mut cmd = Command::new("/usr/sbin/systemd-nspawn"); - - // Basic options for non-interactive, isolated execution - cmd.args(["--directory", self.chroot_path]); - cmd.arg("--pipe"); // Non-interactive, pass stdio through - - if self.quiet { - cmd.arg("--quiet"); // Suppress nspawn's own output - } - - // Add bind mounts - for (source, target) in &self.bind_mounts { - if source == target { - cmd.args(["--bind", source]); - } else { - cmd.args(["--bind", &format!("{source}:{target}")]); - } - } - - // Add read-only bind mounts - for (source, target) in &self.bind_ro_mounts { - if source == target { - cmd.args(["--bind-ro", source]); - } else { - cmd.args(["--bind-ro", &format!("{source}:{target}")]); - } - } - - // Add device bind mounts - we bind-mount device nodes directly - for device in &self.devices { - cmd.args(["--bind", device]); - } - - // Add environment variables - for (key, value) in &self.env_vars { - cmd.args(["--setenv", &format!("{key}={value}")]); - } - - // don't register the service with machinectl - // as this is expected to be short-lived - cmd.arg("--register=no"); - - // Separator and command to run - cmd.arg("--"); - cmd.args(args); - - cmd - } - - /// Run the specified command inside the container. - #[context("Running command in nspawn container")] - pub fn run>(self, args: impl IntoIterator) -> Result<()> { - self.command(args) - .log_debug() - .run_inherited_with_cmd_context() - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_nspawn_command_basic() { - let nspawn = NspawnCmd::new("/some/rootfs"); - let cmd = nspawn.command(["echo", "hello"]); - - let args: Vec<_> = cmd.get_args().collect(); - assert!(args.contains(&&std::ffi::OsStr::new("--directory"))); - assert!(args.contains(&&std::ffi::OsStr::new("/some/rootfs"))); - assert!(args.contains(&&std::ffi::OsStr::new("--pipe"))); - assert!(args.contains(&&std::ffi::OsStr::new("--quiet"))); - assert!(args.contains(&&std::ffi::OsStr::new("--register=no"))); - assert!(args.contains(&&std::ffi::OsStr::new("--"))); - assert!(args.contains(&&std::ffi::OsStr::new("echo"))); - assert!(args.contains(&&std::ffi::OsStr::new("hello"))); - } - - #[test] - fn test_nspawn_command_with_binds() { - let nspawn = NspawnCmd::new("/rootfs") - .bind("/host/boot", "/boot") - .bind_ro("/host/etc", "/etc") - .bind_device("/dev/sda"); - - let cmd = nspawn.command(["ls"]); - let args: Vec<_> = cmd.get_args().collect(); - - // Check bind mount format - assert!(args.contains(&&std::ffi::OsStr::new("--bind"))); - assert!(args.contains(&&std::ffi::OsStr::new("/host/boot:/boot"))); - assert!(args.contains(&&std::ffi::OsStr::new("--bind-ro"))); - assert!(args.contains(&&std::ffi::OsStr::new("/host/etc:/etc"))); - assert!(args.contains(&&std::ffi::OsStr::new("/dev/sda"))); - } - - #[test] - fn test_nspawn_command_same_source_target() { - let nspawn = NspawnCmd::new("/rootfs").bind("/same/path", "/same/path"); - - let cmd = nspawn.command(["ls"]); - let args: Vec<_> = cmd.get_args().collect(); - - // When source == target, should just use source without colon - assert!(args.contains(&&std::ffi::OsStr::new("/same/path"))); - assert!(!args.iter().any(|a| a.to_str().unwrap().contains(':'))); - } - - #[test] - fn test_nspawn_command_with_env() { - let nspawn = NspawnCmd::new("/rootfs").setenv("PATH", "/usr/bin:/bin"); - - let cmd = nspawn.command(["ls"]); - let args: Vec<_> = cmd.get_args().collect(); - - assert!(args.contains(&&std::ffi::OsStr::new("--setenv"))); - assert!(args.contains(&&std::ffi::OsStr::new("PATH=/usr/bin:/bin"))); - } -} diff --git a/crates/utils/src/podman.rs b/crates/utils/src/podman.rs deleted file mode 100644 index 66c9430db..000000000 --- a/crates/utils/src/podman.rs +++ /dev/null @@ -1,211 +0,0 @@ -//! Builder for running commands inside a container using podman with --rootfs. -//! -//! This provides clean namespace isolation for running commands in a target -//! root filesystem, handling API filesystem setup (/dev, /proc, /sys) and -//! proper cleanup automatically. - -use std::ffi::OsStr; -use std::process::Command; - -use anyhow::Result; -use fn_error_context::context; - -use crate::CommandRunExt; - -/// Builder for running commands inside a container using podman with --rootfs. -/// -/// Helps running commands in a target deployment, -/// handling API filesystem setup (/dev, /proc, /sys) and cleanup automatically. -/// -/// # Example -/// ```ignore -/// use bootc_utils::PodmanCmd; -/// -/// PodmanCmd::new("/path/to/rootfs") -/// .bind("/host/boot", "/boot") -/// .bind_device("/dev/sda") -/// .run(&["bootupctl", "backend", "install", ...])?; -/// ``` -#[derive(Debug)] -pub struct PodmanCmd<'a> { - /// The root directory for the container - rootfs_path: &'a str, - /// Bind mounts in format (source, target) - bind_mounts: Vec<(&'a str, &'a str)>, - /// Read-only bind mounts in format (source, target) - bind_ro_mounts: Vec<(&'a str, &'a str)>, - /// Device nodes to pass into the container - devices: Vec<&'a str>, - /// Environment variables to set - env_vars: Vec<(&'a str, &'a str)>, - /// Whether to run in privileged mode (default: true for device access) - privileged: bool, - /// Whether to remove the container after exit (default: true) - rm: bool, -} - -impl<'a> PodmanCmd<'a> { - /// Create a new PodmanCmd builder with the given root directory. - pub fn new(path: &'a str) -> Self { - Self { - rootfs_path: path, - bind_mounts: Vec::new(), - bind_ro_mounts: Vec::new(), - devices: Vec::new(), - env_vars: Vec::new(), - privileged: true, - rm: true, - } - } - - /// Add a bind mount from source to target inside the container. - /// If target is the same as source, you can pass the same value for both. - pub fn bind(mut self, source: &'a str, target: &'a str) -> Self { - self.bind_mounts.push((source, target)); - self - } - - /// Add a read-only bind mount from source to target inside the container. - #[allow(dead_code)] - pub fn bind_ro(mut self, source: &'a str, target: &'a str) -> Self { - self.bind_ro_mounts.push((source, target)); - self - } - - /// Pass a device node into the container. - pub fn bind_device(mut self, device: &'a str) -> Self { - self.devices.push(device); - self - } - - /// Set an environment variable for the command. - pub fn setenv(mut self, key: &'a str, value: &'a str) -> Self { - self.env_vars.push((key, value)); - self - } - - /// Whether to run in privileged mode (default: true). - #[allow(dead_code)] - pub fn privileged(mut self, privileged: bool) -> Self { - self.privileged = privileged; - self - } - - /// Whether to remove the container after exit (default: true). - #[allow(dead_code)] - pub fn rm(mut self, rm: bool) -> Self { - self.rm = rm; - self - } - - /// Build the podman Command. - pub fn command>(&self, args: impl IntoIterator) -> Command { - let mut cmd = Command::new("podman"); - - cmd.arg("run"); - - if self.rm { - cmd.arg("--rm"); - } - - if self.privileged { - cmd.arg("--privileged"); - } - - // Add bind mounts as volumes - for (source, target) in &self.bind_mounts { - cmd.args(["--volume", &format!("{source}:{target}")]); - } - - // Add read-only bind mounts - for (source, target) in &self.bind_ro_mounts { - cmd.args(["--volume", &format!("{source}:{target}:ro")]); - } - - // Add device nodes - for device in &self.devices { - cmd.args(["--device", device]); - } - - // Add environment variables - for (key, value) in &self.env_vars { - cmd.args(["--env", &format!("{key}={value}")]); - } - - // Use the rootfs as the container filesystem - cmd.args(["--rootfs", self.rootfs_path]); - - // Add the command to run - cmd.args(args); - - cmd - } - - /// Run the specified command inside the container. - #[context("Running command in podman container")] - pub fn run>(self, args: impl IntoIterator) -> Result<()> { - self.command(args) - .log_debug() - .run_inherited_with_cmd_context() - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_podman_command_basic() { - let podman = PodmanCmd::new("/some/rootfs"); - let cmd = podman.command(["echo", "hello"]); - - let args: Vec<_> = cmd.get_args().collect(); - assert!(args.contains(&&std::ffi::OsStr::new("run"))); - assert!(args.contains(&&std::ffi::OsStr::new("--rm"))); - assert!(args.contains(&&std::ffi::OsStr::new("--privileged"))); - assert!(args.contains(&&std::ffi::OsStr::new("--rootfs"))); - assert!(args.contains(&&std::ffi::OsStr::new("/some/rootfs"))); - assert!(args.contains(&&std::ffi::OsStr::new("echo"))); - assert!(args.contains(&&std::ffi::OsStr::new("hello"))); - } - - #[test] - fn test_podman_command_with_binds() { - let podman = PodmanCmd::new("/rootfs") - .bind("/host/boot", "/boot") - .bind_ro("/host/etc", "/etc") - .bind_device("/dev/sda"); - - let cmd = podman.command(["ls"]); - let args: Vec<_> = cmd.get_args().collect(); - - // Check volume mount format - assert!(args.contains(&&std::ffi::OsStr::new("--volume"))); - assert!(args.contains(&&std::ffi::OsStr::new("/host/boot:/boot"))); - assert!(args.contains(&&std::ffi::OsStr::new("/host/etc:/etc:ro"))); - assert!(args.contains(&&std::ffi::OsStr::new("--device"))); - assert!(args.contains(&&std::ffi::OsStr::new("/dev/sda"))); - } - - #[test] - fn test_podman_command_with_env() { - let podman = PodmanCmd::new("/rootfs").setenv("PATH", "/usr/bin:/bin"); - - let cmd = podman.command(["ls"]); - let args: Vec<_> = cmd.get_args().collect(); - - assert!(args.contains(&&std::ffi::OsStr::new("--env"))); - assert!(args.contains(&&std::ffi::OsStr::new("PATH=/usr/bin:/bin"))); - } - - #[test] - fn test_podman_command_not_privileged() { - let podman = PodmanCmd::new("/rootfs").privileged(false); - - let cmd = podman.command(["ls"]); - let args: Vec<_> = cmd.get_args().collect(); - - // Should NOT contain --privileged when privileged is false - assert!(!args.contains(&&std::ffi::OsStr::new("--privileged"))); - } -} From 7ccd821941942853f1efdd3d41557533b5f1a59f Mon Sep 17 00:00:00 2001 From: jbtrystram Date: Wed, 28 Jan 2026 15:11:14 +0100 Subject: [PATCH 15/15] fixup --- Cargo.lock | 1 - crates/lib/src/bootloader.rs | 37 ++++++++++++++++++------------------ crates/utils/Cargo.toml | 1 - crates/utils/src/bwrap.rs | 23 +++------------------- 4 files changed, 22 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a55db8a3f..45399dec0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -195,7 +195,6 @@ dependencies = [ "anstream", "anyhow", "chrono", - "fn-error-context", "owo-colors", "rustix", "serde", diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index c2f74f8a0..f6f050fb1 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -1,7 +1,7 @@ use std::fs::create_dir_all; use std::process::Command; -use anyhow::{anyhow, bail, Context, Result}; +use anyhow::{Context, Result, anyhow, bail}; use bootc_utils::{BwrapCmd, CommandRunExt}; use camino::Utf8Path; use cap_std_ext::cap_std::fs::Dir; @@ -11,7 +11,7 @@ use fn_error_context::context; use bootc_blockdev::{Partition, PartitionTable}; use bootc_mount as mount; -use crate::bootc_composefs::boot::{get_sysroot_parent_dev, mount_esp, SecurebootKeys}; +use crate::bootc_composefs::boot::{SecurebootKeys, get_sysroot_parent_dev, mount_esp}; use crate::{discoverable_partition_specification, utils}; /// The name of the mountpoint for efi (as a subdirectory of /boot, or at the toplevel) @@ -91,12 +91,11 @@ pub(crate) fn install_via_bootupd( // bootc defaults to only targeting the platform boot method. let bootupd_opts = (!configopts.generic_image).then_some(["--update-firmware", "--auto"]); - let devpath = device.path(); - // When not running inside the target container (through `--src-imgref`) we use - // systemd-nspawn to run bootupctl in the deployment. + // will bwrap as a chroot to run bootupctl from the deployment. // This makes sure we use binaries from the target image rather than the buildroot. - + // In that case, the target rootfs is replaced with `/` because this is just used by + // bootupd to find the backing device. let rootfs_mount = if deployment_path.is_none() { rootfs.as_str() } else { @@ -110,13 +109,15 @@ pub(crate) fn install_via_bootupd( if let Some(v) = verbose { bootupd_args.push(v); } + if let Some(ref opts) = bootupd_opts { bootupd_args.extend(opts.iter().copied()); } - bootupd_args.extend(["--device", devpath.as_str(), rootfs_mount]); + bootupd_args.extend(["--device", device.path().as_str(), rootfs_mount]); - // Run inside a bwrap container. Bwrap takes care of mounting and creating - // the necessary API filesystems in the target deployment. + // Run inside a bwrap container. It takes care of mounting and creating + // the necessary API filesystems in the target deployment and acts as + // a nicer `chroot`. if let Some(deploy) = deployment_path { let target_root = rootfs.join(deploy); let boot_path = rootfs.join("boot"); @@ -131,19 +132,19 @@ pub(crate) fn install_via_bootupd( // Bind mount /boot from the physical target root so bootupctl can find // the boot partition and install the bootloader there .bind(boot_path.as_str(), "/boot") - // Bind the target block device so bootupctl can access it - .bind_device(devpath.as_str()); + // Bind the target block device inside the bwrap container so bootupctl can access it + .bind_device(device.path().as_str()); - // Bind all partition devices so grub2-install can access them + // Also bind all partitions of the tafet block device for partition in &device.partitions { cmd = cmd.bind_device(&partition.node); } - - cmd.setenv( - "PATH", - "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", - ) - .run(bwrap_args) + // // TODO : is it needed ? + // cmd.setenv( + // "PATH", + // "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", + // ) + cmd.run(bwrap_args) } else { // Running directly without chroot Command::new("bootupctl") diff --git a/crates/utils/Cargo.toml b/crates/utils/Cargo.toml index a20d59b9f..4c3f4007c 100644 --- a/crates/utils/Cargo.toml +++ b/crates/utils/Cargo.toml @@ -11,7 +11,6 @@ repository = "https://github.com/bootc-dev/bootc" anstream = { workspace = true } anyhow = { workspace = true } chrono = { workspace = true, features = ["std"] } -fn-error-context = { workspace = true } owo-colors = { workspace = true } rustix = { workspace = true } serde = { workspace = true, features = ["derive"] } diff --git a/crates/utils/src/bwrap.rs b/crates/utils/src/bwrap.rs index 86dde78a4..ae6f469f5 100644 --- a/crates/utils/src/bwrap.rs +++ b/crates/utils/src/bwrap.rs @@ -1,31 +1,15 @@ -//! Builder for running commands inside a container using bubblewrap (bwrap). -//! -//! This provides namespace isolation for running commands in a target -//! root filesystem, handling API filesystem setup (/dev, /proc, /sys) -//! automatically. - +/// Builder for running commands inside a target os tree using bubblewrap (bwrap). use std::ffi::OsStr; use std::process::Command; use anyhow::Result; -use fn_error_context::context; use crate::CommandRunExt; -/// Builder for running commands inside a container using bubblewrap (bwrap). -/// -/// # Example -/// ```ignore -/// use bootc_utils::BwrapCmd; -/// -/// BwrapCmd::new("/path/to/rootfs") -/// .bind("/host/boot", "/boot") -/// .bind_device("/dev/sda") -/// .run(&["grub2-install", "/dev/sda"])?; -/// ``` +/// Builder for running commands inside a target directory using bwrap. #[derive(Debug, Default)] pub struct BwrapCmd<'a> { - /// The root directory for the container + /// The target directory to use as root for the container chroot_path: &'a str, /// Bind mounts in format (source, target) bind_mounts: Vec<(&'a str, &'a str)>, @@ -63,7 +47,6 @@ impl<'a> BwrapCmd<'a> { } /// Run the specified command inside the container. - #[context("Running command in bwrap container")] pub fn run>(self, args: impl IntoIterator) -> Result<()> { let mut cmd = Command::new("bwrap");