From d46b072cd218ab114a01d604592b5c42cc1b8866 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 19 Mar 2024 08:59:52 -0400 Subject: [PATCH] Add a systemd generator to fixup Anaconda's /etc/fstab This is a giant and hacky workaround for https://github.com/ostreedev/ostree/issues/3193 The better real fix is probably in either systemd or anaconda (more realistically both) but let's paper over things here for now. Having code to run as a generator will likely be useful in the future anyways. Signed-off-by: Colin Walters --- Makefile | 2 + contrib/packaging/bootc.spec | 1 + lib/src/cli.rs | 76 ++++++++++++++++- lib/src/deploy.rs | 136 ++++++++++++++++++++++++++++++ lib/src/generator.rs | 157 +++++++++++++++++++++++++++++++++++ lib/src/lib.rs | 1 + lib/src/systemglue/mod.rs | 1 + 7 files changed, 372 insertions(+), 2 deletions(-) create mode 100644 lib/src/generator.rs create mode 100644 lib/src/systemglue/mod.rs diff --git a/Makefile b/Makefile index bf98298b2..f760dac0e 100644 --- a/Makefile +++ b/Makefile @@ -8,6 +8,8 @@ all-test: install: install -D -m 0755 -t $(DESTDIR)$(prefix)/bin target/release/bootc + 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 # Support installing pre-generated man pages shipped in source tarball, to avoid # a dependency on pandoc downstream. But in local builds these end up in target/man, diff --git a/contrib/packaging/bootc.spec b/contrib/packaging/bootc.spec index dbd01c225..73695f646 100644 --- a/contrib/packaging/bootc.spec +++ b/contrib/packaging/bootc.spec @@ -28,6 +28,7 @@ BuildRequires: systemd-devel %license LICENSE-APACHE LICENSE-MIT %doc README.md %{_bindir}/bootc +%{_prefix}/lib/systemd/system-generators/* %{_prefix}/lib/bootc %{_unitdir}/* %{_mandir}/man*/bootc* diff --git a/lib/src/cli.rs b/lib/src/cli.rs index 1e9366ff9..5adef689b 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -5,6 +5,7 @@ use anyhow::{Context, Result}; use camino::Utf8PathBuf; use cap_std_ext::cap_std; +use cap_std_ext::cap_std::fs::Dir; use clap::Parser; use fn_error_context::context; use ostree::gio; @@ -136,6 +137,24 @@ pub(crate) struct ManOpts { pub(crate) directory: Utf8PathBuf, } +/// Hidden, internal only options +#[derive(Debug, clap::Subcommand, PartialEq, Eq)] +pub(crate) enum InternalsOpts { + SystemdGenerator { + normal_dir: Utf8PathBuf, + #[allow(dead_code)] + early_dir: Option, + #[allow(dead_code)] + late_dir: Option, + }, + FixupEtcFstab, +} + +impl InternalsOpts { + /// The name of the binary we inject into /usr/lib/systemd/system-generators + const GENERATOR_BIN: &'static str = "bootc-systemd-generator"; +} + /// Options for internal testing #[derive(Debug, clap::Subcommand, PartialEq, Eq)] pub(crate) enum TestingOpts { @@ -226,6 +245,9 @@ pub(crate) enum Opt { #[clap(trailing_var_arg = true, allow_hyphen_values = true)] args: Vec, }, + #[clap(subcommand)] + #[clap(hide = true)] + Internals(InternalsOpts), /// Internal integration testing helpers. #[clap(hide(true), subcommand)] #[cfg(feature = "internal-testing-api")] @@ -525,11 +547,39 @@ where I: IntoIterator, I::Item: Into + Clone, { - run_from_opt(Opt::parse_from(args)).await + run_from_opt(Opt::parse_including_static(args)).await +} + +impl Opt { + /// In some cases (e.g. systemd generator) we dispatch specifically on argv0. This + /// requires some special handling in clap. + fn parse_including_static(args: I) -> Self + where + I: IntoIterator, + I::Item: Into + Clone, + { + let mut args = args.into_iter(); + let first = if let Some(first) = args.next() { + let first: OsString = first.into(); + let argv0 = first.to_str().and_then(|s| s.rsplit_once('/')).map(|s| s.1); + tracing::debug!("argv0={argv0:?}"); + if matches!(argv0, Some(InternalsOpts::GENERATOR_BIN)) { + let base_args = ["bootc", "internals", "systemd-generator"] + .into_iter() + .map(OsString::from); + return Opt::parse_from(base_args.chain(args.map(|i| i.into()))); + } + Some(first) + } else { + None + }; + Opt::parse_from(first.into_iter().chain(args.map(|i| i.into()))) + } } /// Internal (non-generic/monomorphized) primary CLI entrypoint async fn run_from_opt(opt: Opt) -> Result<()> { + let root = &Dir::open_ambient_dir("/", cap_std::ambient_authority())?; match opt { Opt::Upgrade(opts) => upgrade(opts).await, Opt::Switch(opts) => switch(opts).await, @@ -551,6 +601,17 @@ async fn run_from_opt(opt: Opt) -> Result<()> { crate::install::exec_in_host_mountns(args.as_slice()) } Opt::Status(opts) => super::status::status(opts).await, + Opt::Internals(opts) => match opts { + InternalsOpts::SystemdGenerator { + normal_dir, + early_dir: _, + late_dir: _, + } => { + let unit_dir = &Dir::open_ambient_dir(normal_dir, cap_std::ambient_authority())?; + crate::generator::generator(root, unit_dir) + } + InternalsOpts::FixupEtcFstab => crate::deploy::fixup_etc_fstab(&root), + }, #[cfg(feature = "internal-testing-api")] Opt::InternalTests(opts) => crate::privtests::run(opts).await, #[cfg(feature = "docgen")] @@ -580,10 +641,21 @@ fn test_parse_install_args() { #[test] fn test_parse_opts() { assert!(matches!( - Opt::parse_from(["bootc", "status"]), + Opt::parse_including_static(["bootc", "status"]), Opt::Status(StatusOpts { json: false, booted: false }) )); } + +#[test] +fn test_parse_generator() { + assert!(matches!( + Opt::parse_including_static([ + "/usr/lib/systemd/system/bootc-systemd-generator", + "/run/systemd/system" + ]), + Opt::Internals(InternalsOpts::SystemdGenerator { .. }) + )); +} diff --git a/lib/src/deploy.rs b/lib/src/deploy.rs index 11aca2a76..020f48020 100644 --- a/lib/src/deploy.rs +++ b/lib/src/deploy.rs @@ -2,6 +2,8 @@ //! //! Create a merged filesystem tree with the image and mounted configmaps. +use std::io::{BufRead, Write}; + use anyhow::Ok; use anyhow::{Context, Result}; @@ -377,3 +379,137 @@ fn test_switch_inplace() -> Result<()> { assert_eq!(replaced, target_deployment); Ok(()) } + +/// A workaround for https://github.com/ostreedev/ostree/issues/3193 +/// as generated by anaconda. +#[context("Updating /etc/fstab for anaconda+composefs")] +pub(crate) fn fixup_etc_fstab(root: &Dir) -> Result<()> { + let fstab_path = "etc/fstab"; + // Read the old file + let fd = root + .open(fstab_path) + .with_context(|| format!("Opening {fstab_path}")) + .map(std::io::BufReader::new)?; + + // Helper function to possibly change a line from /etc/fstab. + // Returns Ok(true) if we made a change (and we wrote the modified line) + // otherwise returns Ok(false) and the caller should write the original line. + fn edit_fstab_line(line: &str, mut w: impl Write) -> Result { + if line.starts_with("#") { + return Ok(false); + } + let parts = line.split_ascii_whitespace().collect::>(); + + let path_idx = 1; + let options_idx = 3; + let (&path, &options) = match (parts.get(path_idx), parts.get(options_idx)) { + (None, _) => { + tracing::debug!("No path in entry: {line}"); + return Ok(false); + } + (_, None) => { + tracing::debug!("No options in entry: {line}"); + return Ok(false); + } + (Some(p), Some(o)) => (p, o), + }; + // If this is not the root, we're not matching on it + if path != "/" { + return Ok(false); + } + // If options already contains `ro`, nothing to do + if options.split(',').any(|s| s == "ro") { + return Ok(false); + } + + writeln!(w, "# {}", crate::generator::BOOTC_EDITED_STAMP)?; + + // SAFETY: we unpacked the options before. + // This adds `ro` to the option list + assert!(!options.is_empty()); // Split wouldn't have turned this up if it was empty + let options = format!("{options},ro"); + for (i, part) in parts.into_iter().enumerate() { + // TODO: would obviously be nicer to preserve whitespace...but...eh. + if i > 0 { + write!(w, " ")?; + } + if i == options_idx { + write!(w, "{options}")?; + } else { + write!(w, "{part}")? + } + } + // And add the trailing newline + writeln!(w)?; + Ok(true) + } + + // Read the input, and atomically write a modified version + root.atomic_replace_with(fstab_path, move |mut w| { + for line in fd.lines() { + let line = line?; + if !edit_fstab_line(&line, &mut w)? { + writeln!(w, "{line}")?; + } + } + Ok(()) + }) + .context("Replacing /etc/fstab")?; + + println!("Updated /etc/fstab to add `ro` for `/`"); + Ok(()) +} + +#[test] +fn test_fixup_etc_fstab_default() -> Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + let default = "UUID=f7436547-20ac-43cb-aa2f-eac9632183f6 /boot auto ro 0 0\n"; + tempdir.create_dir_all("etc")?; + tempdir.atomic_write("etc/fstab", default)?; + fixup_etc_fstab(&tempdir).unwrap(); + assert_eq!(tempdir.read_to_string("etc/fstab")?, default); + Ok(()) +} + +#[test] +fn test_fixup_etc_fstab_multi() -> Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + let default = "UUID=f7436547-20ac-43cb-aa2f-eac9632183f6 /boot auto ro 0 0\n\ +UUID=6907-17CA /boot/efi vfat umask=0077,shortname=winnt 0 2\n"; + tempdir.create_dir_all("etc")?; + tempdir.atomic_write("etc/fstab", default)?; + fixup_etc_fstab(&tempdir).unwrap(); + assert_eq!(tempdir.read_to_string("etc/fstab")?, default); + Ok(()) +} + +#[test] +fn test_fixup_etc_fstab_ro() -> Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + let default = "UUID=f7436547-20ac-43cb-aa2f-eac9632183f6 /boot auto ro 0 0\n\ +UUID=1eef9f42-40e3-4bd8-ae20-e9f2325f8b52 / xfs ro 0 0\n\ +UUID=6907-17CA /boot/efi vfat umask=0077,shortname=winnt 0 2\n"; + tempdir.create_dir_all("etc")?; + tempdir.atomic_write("etc/fstab", default)?; + fixup_etc_fstab(&tempdir).unwrap(); + assert_eq!(tempdir.read_to_string("etc/fstab")?, default); + Ok(()) +} + +#[test] +fn test_fixup_etc_fstab_rw() -> Result<()> { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + // This case uses `defaults` + let default = "UUID=f7436547-20ac-43cb-aa2f-eac9632183f6 /boot auto ro 0 0\n\ +UUID=1eef9f42-40e3-4bd8-ae20-e9f2325f8b52 / xfs defaults 0 0\n\ +UUID=6907-17CA /boot/efi vfat umask=0077,shortname=winnt 0 2\n"; + let modified = "UUID=f7436547-20ac-43cb-aa2f-eac9632183f6 /boot auto ro 0 0\n\ +# Updated by bootc-fstab-edit.service\n\ +UUID=1eef9f42-40e3-4bd8-ae20-e9f2325f8b52 / xfs defaults,ro 0 0\n\ +UUID=6907-17CA /boot/efi vfat umask=0077,shortname=winnt 0 2\n"; + tempdir.create_dir_all("etc")?; + tempdir.atomic_write("etc/fstab", default)?; + fixup_etc_fstab(&tempdir).unwrap(); + assert_eq!(tempdir.read_to_string("etc/fstab")?, modified); + Ok(()) +} diff --git a/lib/src/generator.rs b/lib/src/generator.rs new file mode 100644 index 000000000..78ef3647e --- /dev/null +++ b/lib/src/generator.rs @@ -0,0 +1,157 @@ +use std::io::BufRead; + +use anyhow::{Context, Result}; +use cap_std::fs::Dir; +use cap_std_ext::{cap_std, dirext::CapStdExtDirExt}; +use fn_error_context::context; +use rustix::{fd::AsFd, fs::StatVfsMountFlags}; + +const EDIT_UNIT: &str = "bootc-fstab-edit.service"; +const FSTAB_ANACONDA_STAMP: &str = "Created by anaconda"; +pub(crate) const BOOTC_EDITED_STAMP: &str = "Updated by bootc-fstab-edit.service"; + +/// Called when the root is read-only composefs to reconcile /etc/fstab +#[context("bootc generator")] +pub(crate) fn fstab_generator_impl(root: &Dir, unit_dir: &Dir) -> Result { + // Do nothing if not ostree-booted + if !root.try_exists("run/ostree-booted")? { + return Ok(false); + } + + if let Some(fd) = root + .open_optional("etc/fstab") + .context("Opening /etc/fstab")? + .map(std::io::BufReader::new) + { + let mut from_anaconda = false; + for line in fd.lines() { + let line = line.context("Reading /etc/fstab")?; + if line.contains(BOOTC_EDITED_STAMP) { + // We're done + return Ok(false); + } + if line.contains(FSTAB_ANACONDA_STAMP) { + from_anaconda = true; + } + } + if !from_anaconda { + return Ok(false); + } + tracing::debug!("/etc/fstab from anaconda: {from_anaconda}"); + if from_anaconda { + generate_fstab_editor(unit_dir)?; + return Ok(true); + } + } + Ok(false) +} + +/// Main entrypoint for the generator +pub(crate) fn generator(root: &Dir, unit_dir: &Dir) -> Result<()> { + // Right now we only do something if the root is a read-only overlayfs (a composefs really) + let st = rustix::fs::fstatfs(root.as_fd())?; + if st.f_type != libc::OVERLAYFS_SUPER_MAGIC { + tracing::trace!("Root is not overlayfs"); + return Ok(()); + } + let st = rustix::fs::fstatvfs(root.as_fd())?; + if !st.f_flag.contains(StatVfsMountFlags::RDONLY) { + tracing::trace!("Root is writable"); + return Ok(()); + } + let updated = fstab_generator_impl(root, unit_dir)?; + tracing::trace!("Generated fstab: {updated}"); + Ok(()) +} + +/// Parse /etc/fstab and check if the root mount is out of sync with the composefs +/// state, and if so, fix it. +fn generate_fstab_editor(unit_dir: &Dir) -> Result<()> { + unit_dir.atomic_write( + EDIT_UNIT, + "[Unit]\n\ +DefaultDependencies=no\n\ +After=systemd-fsck-root.service\n\ +Before=local-fs-pre.target local-fs.target shutdown.target systemd-remount-fs.service\n\ +\n\ +[Service]\n\ +Type=oneshot\n\ +RemainAfterExit=yes\n\ +ExecStart=bootc internals fixup-etc-fstab\n\ +", + )?; + let target = "local-fs-pre.target.wants"; + unit_dir.create_dir_all(target)?; + unit_dir.symlink(&format!("../{EDIT_UNIT}"), &format!("{target}/{EDIT_UNIT}"))?; + Ok(()) +} + +#[cfg(test)] +fn fixture() -> Result { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + tempdir.create_dir("etc")?; + tempdir.create_dir("run")?; + tempdir.create_dir_all("run/systemd/system")?; + Ok(tempdir) +} + +#[test] +fn test_generator_no_fstab() -> Result<()> { + let tempdir = fixture()?; + let unit_dir = &tempdir.open_dir("run/systemd/system")?; + fstab_generator_impl(&tempdir, &unit_dir).unwrap(); + + assert_eq!(unit_dir.entries()?.count(), 0); + Ok(()) +} + +#[test] +fn test_generator_fstab() -> Result<()> { + let tempdir = fixture()?; + let unit_dir = &tempdir.open_dir("run/systemd/system")?; + // Should still be a no-op + tempdir.atomic_write("etc/fstab", "# Some dummy fstab")?; + fstab_generator_impl(&tempdir, &unit_dir).unwrap(); + assert_eq!(unit_dir.entries()?.count(), 0); + + // Also a no-op, not booted via ostree + tempdir.atomic_write("etc/fstab", &format!("# {FSTAB_ANACONDA_STAMP}"))?; + fstab_generator_impl(&tempdir, &unit_dir).unwrap(); + assert_eq!(unit_dir.entries()?.count(), 0); + + // Now it should generate + tempdir.atomic_write("run/ostree-booted", "ostree booted")?; + fstab_generator_impl(&tempdir, &unit_dir).unwrap(); + assert_eq!(unit_dir.entries()?.count(), 2); + + Ok(()) +} + +#[test] +fn test_generator_fstab_idempotent() -> Result<()> { + let anaconda_fstab = "\n\ +#\n\ +# /etc/fstab\n\ +# Created by anaconda on Tue Mar 19 12:24:29 2024\n\ +#\n\ +# Accessible filesystems, by reference, are maintained under '/dev/disk/'.\n\ +# See man pages fstab(5), findfs(8), mount(8) and/or blkid(8) for more info.\n\ +#\n\ +# After editing this file, run 'systemctl daemon-reload' to update systemd\n\ +# units generated from this file.\n\ +#\n\ +# Updated by bootc-fstab-edit.service\n\ +UUID=715be2b7-c458-49f2-acec-b2fdb53d9089 / xfs ro 0 0\n\ +UUID=341c4712-54e8-4839-8020-d94073b1dc8b /boot xfs defaults 0 0\n\ +"; + let tempdir = fixture()?; + let unit_dir = &tempdir.open_dir("run/systemd/system")?; + + tempdir.atomic_write("etc/fstab", anaconda_fstab)?; + tempdir.atomic_write("run/ostree-booted", "ostree booted")?; + let updated = fstab_generator_impl(&tempdir, &unit_dir).unwrap(); + assert!(!updated); + assert_eq!(unit_dir.entries()?.count(), 0); + + Ok(()) +} diff --git a/lib/src/lib.rs b/lib/src/lib.rs index 5b416f71a..b04d33642 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -19,6 +19,7 @@ pub mod cli; pub(crate) mod deploy; +pub(crate) mod generator; pub(crate) mod journal; mod lsm; pub(crate) mod metadata; diff --git a/lib/src/systemglue/mod.rs b/lib/src/systemglue/mod.rs new file mode 100644 index 000000000..abd9b55a4 --- /dev/null +++ b/lib/src/systemglue/mod.rs @@ -0,0 +1 @@ +pub(crate) mod generator;