From 1d81d1dc2760525c334b97b67797d276f684438f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 23 Jul 2024 16:41:22 -0400 Subject: [PATCH 1/3] install: Allocate a global tmpdir We allocate temporary things in a few places, and it's handy to have a pre-created single directory for the whole install process to use instead of creating individual tempfiles. Signed-off-by: Colin Walters --- lib/src/install.rs | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/lib/src/install.rs b/lib/src/install.rs index cd7e068d..77b7e74d 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -26,6 +26,7 @@ use camino::Utf8PathBuf; use cap_std::fs::{Dir, MetadataExt}; use cap_std_ext::cap_std; use cap_std_ext::cap_std::fs_utf8::DirEntry as DirEntryUtf8; +use cap_std_ext::cap_tempfile::TempDir; use cap_std_ext::cmdext::CapStdExtCommandExt; use cap_std_ext::prelude::CapStdExtDirExt; use chrono::prelude::*; @@ -322,6 +323,7 @@ pub(crate) struct State { pub(crate) root_ssh_authorized_keys: Option, /// The root filesystem of the running container pub(crate) container_root: Dir, + pub(crate) tempdir: TempDir, } impl State { @@ -342,6 +344,7 @@ impl State { #[context("Finalizing state")] pub(crate) fn consume(self) -> Result<()> { + self.tempdir.close()?; // If we had invoked `setenforce 0`, then let's re-enable it. if let SELinuxFinalState::Enabled(Some(guard)) = self.selinux_state { guard.consume()?; @@ -1001,7 +1004,7 @@ fn ensure_var() -> Result<()> { /// via a custom bwrap container today) and work around it by /// mounting a writable transient overlayfs. #[context("Ensuring writable /etc")] -fn ensure_writable_etc_containers() -> Result<()> { +fn ensure_writable_etc_containers(tempdir: &Dir) -> Result<()> { let etc_containers = Utf8Path::new("/etc/containers"); // If there's no /etc/containers, nothing to do if !etc_containers.try_exists()? { @@ -1010,24 +1013,18 @@ fn ensure_writable_etc_containers() -> Result<()> { if rustix::fs::access(etc_containers.as_std_path(), rustix::fs::Access::WRITE_OK).is_ok() { return Ok(()); } - // Create a tempdir for the overlayfs upper; right now this is leaked, - // but in the case we care about it's into a tmpfs allocated only while - // we're running (equivalent to PrivateTmp=yes), so it's not - // really a leak. - let td = tempfile::tempdir_in("/tmp")?.into_path(); - let td: &Utf8Path = (td.as_path()).try_into()?; - let upper = &td.join("upper"); - let work = &td.join("work"); - std::fs::create_dir(upper)?; - std::fs::create_dir(work)?; - let opts = format!("lowerdir={etc_containers},workdir={work},upperdir={upper}"); - Task::new( + // Create dirs for the overlayfs upper and work in the install-global tmpdir. + tempdir.create_dir_all("etc-ovl/upper")?; + tempdir.create_dir("etc-ovl/work")?; + let opts = format!("lowerdir={etc_containers},workdir=etc-ovl/work,upperdir=etc-ovl/upper"); + let mut t = Task::new( &format!("Mount transient overlayfs for {etc_containers}"), "mount", ) .args(["-t", "overlay", "overlay", "-o", opts.as_str()]) - .arg(etc_containers) - .run()?; + .arg(etc_containers); + t.cmd.cwd_dir(tempdir.try_clone()?); + t.run()?; Ok(()) } @@ -1214,9 +1211,14 @@ async fn prepare_install( verify_target_fetch(&target_imgref).await?; } + // A bit of basic global state setup ensure_var()?; setup_tmp_mounts()?; - ensure_writable_etc_containers()?; + // Allocate a temporary directory we can use in various places to avoid + // creating multiple. + let tempdir = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + // And continue to init global state + ensure_writable_etc_containers(&tempdir)?; // Even though we require running in a container, the mounts we create should be specific // to this process, so let's enter a private mountns to avoid leaking them. @@ -1260,6 +1262,7 @@ async fn prepare_install( install_config, root_ssh_authorized_keys, container_root: rootfs, + tempdir, }); Ok(state) From e2fc38a8f6ba953893344805618aa5868a962b3a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 25 Jul 2024 13:57:40 -0400 Subject: [PATCH 2/3] install: Use tmpdir for target fetch verification We create a transient ostree repo, to do so use the global install tmpdir. Signed-off-by: Colin Walters --- lib/src/install.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/src/install.rs b/lib/src/install.rs index 77b7e74d..633819d6 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -1120,11 +1120,12 @@ pub(crate) fn setup_sys_mount(fstype: &str, fspath: &str) -> Result<()> { /// Verify that we can load the manifest of the target image #[context("Verifying fetch")] -async fn verify_target_fetch(imgref: &ostree_container::OstreeImageReference) -> Result<()> { - let tmpdir = tempfile::tempdir()?; - let tmprepo = &ostree::Repo::new_for_path(tmpdir.path()); - tmprepo - .create(ostree::RepoMode::Bare, ostree::gio::Cancellable::NONE) +async fn verify_target_fetch( + tmpdir: &Dir, + imgref: &ostree_container::OstreeImageReference, +) -> Result<()> { + let tmpdir = &TempDir::new_in(&tmpdir)?; + let tmprepo = &ostree::Repo::create_at_dir(tmpdir.as_fd(), ".", ostree::RepoMode::Bare, None) .context("Init tmp repo")?; tracing::trace!("Verifying fetch for {imgref}"); @@ -1207,10 +1208,6 @@ async fn prepare_install( }; tracing::debug!("Target image reference: {target_imgref}"); - if !target_opts.skip_fetch_check { - verify_target_fetch(&target_imgref).await?; - } - // A bit of basic global state setup ensure_var()?; setup_tmp_mounts()?; @@ -1220,6 +1217,10 @@ async fn prepare_install( // And continue to init global state ensure_writable_etc_containers(&tempdir)?; + if !target_opts.skip_fetch_check { + verify_target_fetch(&tempdir, &target_imgref).await?; + } + // Even though we require running in a container, the mounts we create should be specific // to this process, so let's enter a private mountns to avoid leaking them. if !external_source && std::env::var_os("BOOTC_SKIP_UNSHARE").is_none() { From 37d19c3cb581127b321a1dbace1548f25a50f0b7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 25 Jul 2024 14:18:34 -0400 Subject: [PATCH 3/3] Initialize a containers-storage: owned by bootc Initial work for: https://github.com/containers/bootc/issues/721 - Initialize a containers-storage: instance at install time (that defaults to empty) - "Open" it (but do nothing with it) as part of the core CLI operations Further APIs and work will build on top of this. Signed-off-by: Colin Walters --- Makefile | 1 + lib/src/cli.rs | 4 +- lib/src/imgstorage.rs | 85 +++++++++++++++++++ lib/src/install.rs | 15 +++- lib/src/lib.rs | 1 + lib/src/store/mod.rs | 14 ++- tests-integration/src/install.rs | 11 +++ .../booted/010-test-bootc-container-store.nu | 8 ++ 8 files changed, 135 insertions(+), 4 deletions(-) create mode 100644 lib/src/imgstorage.rs create mode 100644 tests/booted/010-test-bootc-container-store.nu diff --git a/Makefile b/Makefile index d3e82e0c..0b8a8630 100644 --- a/Makefile +++ b/Makefile @@ -11,6 +11,7 @@ install: install -D -m 0755 -t $(DESTDIR)$(prefix)/bin target/release/bootc install -d -m 0755 $(DESTDIR)$(prefix)/lib/bootc/bound-images.d install -d -m 0755 $(DESTDIR)$(prefix)/lib/bootc/kargs.d + ln -s /sysroot/ostree/bootc/storage $(DESTDIR)$(prefix)/lib/bootc/storage install -d -m 0755 $(DESTDIR)$(prefix)/lib/systemd/system-generators/ ln -f $(DESTDIR)$(prefix)/bin/bootc $(DESTDIR)$(prefix)/lib/systemd/system-generators/bootc-systemd-generator install -d $(DESTDIR)$(prefix)/lib/bootc/install diff --git a/lib/src/cli.rs b/lib/src/cli.rs index 31707cfe..6627f804 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -430,10 +430,12 @@ pub(crate) async fn get_locked_sysroot() -> Result Result { + let global_run = Dir::open_ambient_dir("/run", cap_std::ambient_authority())?; let sysroot = get_locked_sysroot().await?; - crate::store::Storage::new(sysroot) + crate::store::Storage::new(sysroot, &global_run) } #[context("Querying root privilege")] diff --git a/lib/src/imgstorage.rs b/lib/src/imgstorage.rs new file mode 100644 index 00000000..8cabb30f --- /dev/null +++ b/lib/src/imgstorage.rs @@ -0,0 +1,85 @@ +//! # bootc-managed container storage +//! +//! The default storage for this project uses ostree, canonically storing all of its state in +//! `/sysroot/ostree`. +//! +//! This containers-storage: which canonically lives in `/sysroot/ostree/bootc`. + +use std::sync::Arc; + +use anyhow::{Context, Result}; +use camino::Utf8Path; +use cap_std_ext::cap_std::fs::Dir; +use cap_std_ext::cmdext::CapStdExtCommandExt; +use cap_std_ext::dirext::CapStdExtDirExt; +use fn_error_context::context; +use std::os::fd::OwnedFd; + +use crate::task::Task; + +/// The path to the storage, relative to the physical system root. +pub(crate) const SUBPATH: &str = "ostree/bootc/storage"; +/// The path to the "runroot" with transient runtime state; this is +/// relative to the /run directory +const RUNROOT: &str = "bootc/storage"; +pub(crate) struct Storage { + root: Dir, + #[allow(dead_code)] + run: Dir, +} + +impl Storage { + fn podman_task_in(sysroot: OwnedFd, run: OwnedFd) -> Result { + let mut t = Task::new_quiet("podman"); + // podman expects absolute paths for these, so use /proc/self/fd + { + let sysroot_fd: Arc = Arc::new(sysroot); + t.cmd.take_fd_n(sysroot_fd, 3); + } + { + let run_fd: Arc = Arc::new(run); + t.cmd.take_fd_n(run_fd, 4); + } + t = t.args(["--root=/proc/self/fd/3", "--runroot=/proc/self/fd/4"]); + Ok(t) + } + + #[allow(dead_code)] + fn podman_task(&self) -> Result { + let sysroot = self.root.try_clone()?.into_std_file().into(); + let run = self.run.try_clone()?.into_std_file().into(); + Self::podman_task_in(sysroot, run) + } + + #[context("Creating imgstorage")] + pub(crate) fn create(sysroot: &Dir, run: &Dir) -> Result { + let subpath = Utf8Path::new(SUBPATH); + // SAFETY: We know there's a parent + let parent = subpath.parent().unwrap(); + if !sysroot.try_exists(subpath)? { + let tmp = format!("{SUBPATH}.tmp"); + sysroot.remove_all_optional(&tmp)?; + sysroot.create_dir_all(parent)?; + sysroot.create_dir_all(&tmp).context("Creating tmpdir")?; + // There's no explicit API to initialize a containers-storage: + // root, simply passing a path will attempt to auto-create it. + // We run "podman images" in the new root. + Self::podman_task_in(sysroot.open_dir(&tmp)?.into(), run.try_clone()?.into())? + .arg("images") + .run()?; + sysroot + .rename(&tmp, sysroot, subpath) + .context("Renaming tmpdir")?; + } + Self::open(sysroot, run) + } + + #[context("Opening imgstorage")] + pub(crate) fn open(sysroot: &Dir, run: &Dir) -> Result { + let root = sysroot.open_dir(SUBPATH).context(SUBPATH)?; + // Always auto-create this if missing + run.create_dir_all(RUNROOT)?; + let run = run.open_dir(RUNROOT).context(RUNROOT)?; + Ok(Self { root, run }) + } +} diff --git a/lib/src/install.rs b/lib/src/install.rs index 633819d6..f2b8b4d1 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -598,6 +598,19 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result .cwd(rootfs_dir)? .run()?; + let sysroot = ostree::Sysroot::new(Some(&gio::File::for_path(rootfs))); + sysroot.load(cancellable)?; + let sysroot_dir = Dir::reopen_dir(&crate::utils::sysroot_fd(&sysroot))?; + + state.tempdir.create_dir("temp-run")?; + let temp_run = state.tempdir.open_dir("temp-run")?; + sysroot_dir + .create_dir_all(Utf8Path::new(crate::imgstorage::SUBPATH).parent().unwrap()) + .context("creating bootc dir")?; + let imgstore = crate::imgstorage::Storage::create(&sysroot_dir, &temp_run)?; + // And drop it again - we'll reopen it after this + drop(imgstore); + // Bootstrap the initial labeling of the /ostree directory as usr_t if let Some(policy) = sepolicy { let ostree_dir = rootfs_dir.open_dir("ostree")?; @@ -613,7 +626,7 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result let sysroot = ostree::Sysroot::new(Some(&gio::File::for_path(rootfs))); sysroot.load(cancellable)?; let sysroot = SysrootLock::new_from_sysroot(&sysroot).await?; - Storage::new(sysroot) + Storage::new(sysroot, &temp_run) } #[context("Creating ostree deployment")] diff --git a/lib/src/lib.rs b/lib/src/lib.rs index 6d05aa64..cf03412e 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -44,3 +44,4 @@ pub mod spec; #[cfg(feature = "docgen")] mod docgen; +mod imgstorage; diff --git a/lib/src/store/mod.rs b/lib/src/store/mod.rs index 71334177..b501347a 100644 --- a/lib/src/store/mod.rs +++ b/lib/src/store/mod.rs @@ -2,6 +2,7 @@ use std::env; use std::ops::Deref; use anyhow::Result; +use cap_std_ext::cap_std::fs::Dir; use clap::ValueEnum; use ostree_ext::container::OstreeImageReference; @@ -15,6 +16,8 @@ mod ostree_container; pub(crate) struct Storage { pub sysroot: SysrootLock, + #[allow(dead_code)] + pub imgstore: crate::imgstorage::Storage, pub store: Box, } @@ -48,7 +51,7 @@ impl Deref for Storage { } impl Storage { - pub fn new(sysroot: SysrootLock) -> Result { + pub fn new(sysroot: SysrootLock, run: &Dir) -> Result { let store = match env::var("BOOTC_STORAGE") { Ok(val) => crate::spec::Store::from_str(&val, true).unwrap_or_else(|_| { let default = crate::spec::Store::default(); @@ -58,9 +61,16 @@ impl Storage { Err(_) => crate::spec::Store::default(), }; + let sysroot_dir = Dir::reopen_dir(&crate::utils::sysroot_fd(&sysroot))?; + let imgstore = crate::imgstorage::Storage::open(&sysroot_dir, run)?; + let store = load(store); - Ok(Self { sysroot, store }) + Ok(Self { + sysroot, + store, + imgstore, + }) } } diff --git a/tests-integration/src/install.rs b/tests-integration/src/install.rs index 73b9f30e..7955528d 100644 --- a/tests-integration/src/install.rs +++ b/tests-integration/src/install.rs @@ -2,6 +2,7 @@ use std::path::Path; use std::{os::fd::AsRawFd, path::PathBuf}; use anyhow::Result; +use camino::Utf8Path; use cap_std_ext::cap_std; use cap_std_ext::cap_std::fs::Dir; use fn_error_context::context; @@ -53,6 +54,12 @@ fn find_deployment_root() -> Result { anyhow::bail!("Failed to find deployment root") } +// Hook relatively cheap post-install tests here +fn generic_post_install_verification() -> Result<()> { + assert!(Utf8Path::new("/ostree/bootc/storage/overlay").try_exists()?); + Ok(()) +} + #[context("Install tests")] pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments) -> Result<()> { // Force all of these tests to be serial because they mutate global state @@ -88,6 +95,8 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments) std::fs::write(&tmp_keys, b"ssh-ed25519 ABC0123 testcase@example.com")?; cmd!(sh, "sudo {BASE_ARGS...} {target_args...} -v {tmp_keys}:/test_authorized_keys {image} bootc install to-filesystem {generic_inst_args...} --acknowledge-destructive --karg=foo=bar --replace=alongside --root-ssh-authorized-keys=/test_authorized_keys /target").run()?; + generic_post_install_verification()?; + // Test kargs injected via CLI cmd!( sh, @@ -120,6 +129,7 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments) let sh = &xshell::Shell::new()?; reset_root(sh)?; cmd!(sh, "sudo {BASE_ARGS...} {target_args...} {image} bootc install to-existing-root --acknowledge-destructive {generic_inst_args...}").run()?; + generic_post_install_verification()?; let root = &Dir::open_ambient_dir("/ostree", cap_std::ambient_authority()).unwrap(); let mut path = PathBuf::from("."); crate::selinux::verify_selinux_recurse(root, &mut path, false)?; @@ -131,6 +141,7 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments) let empty = sh.create_temp_dir()?; let empty = empty.path().to_str().unwrap(); cmd!(sh, "sudo {BASE_ARGS...} {target_args...} -v {empty}:/usr/lib/bootc/install {image} bootc install to-existing-root {generic_inst_args...}").run()?; + generic_post_install_verification()?; Ok(()) }), ]; diff --git a/tests/booted/010-test-bootc-container-store.nu b/tests/booted/010-test-bootc-container-store.nu new file mode 100644 index 00000000..036c5904 --- /dev/null +++ b/tests/booted/010-test-bootc-container-store.nu @@ -0,0 +1,8 @@ +use std assert +use tap.nu + +tap begin "verify bootc-owned container storage" + +# This should currently be empty by default... +podman --storage-opt=additionalimagestore=/usr/lib/bootc/storage images +tap ok