From 3bbf7ee30809d033db6089887c4759997b5fdeb9 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 + lib/src/cli.rs | 76 +++++++++++++++++++++++- lib/src/deploy.rs | 120 ++++++++++++++++++++++++++++++++++++++ lib/src/generator.rs | 114 ++++++++++++++++++++++++++++++++++++ lib/src/lib.rs | 1 + lib/src/systemglue/mod.rs | 1 + 6 files changed, 312 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/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..19d55f61c 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,121 @@ 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 = parts.get(1); + let options = parts.get(3); + if let (Some(&path), Some(&options)) = (path, options) { + let options = options.split(',').collect::>(); + if options.iter().any(|&s| s == "ro") { + return Ok(false); + } + if path != "/" { + return Ok(false); + } + } else { + tracing::debug!("No path in entry: {line}"); + return Ok(false); + }; + + // SAFETY: we unpacked the options before. + // This adds `ro` to the option list + let options = format!("{},ro", options.unwrap()); + for (i, part) in parts.into_iter().enumerate() { + if i > 0 { + write!(w, " ")?; + } + if i == 3 { + write!(w, "{options}")?; + } else { + write!(w, "{part}")? + } + } + 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") +} + +#[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\ +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..559613364 --- /dev/null +++ b/lib/src/generator.rs @@ -0,0 +1,114 @@ +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 FSTAB_ANACONDA_STAMP: &str = "Created by anaconda"; + +/// 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(()); + } + + if let Some(fd) = root + .open_optional("etc/fstab") + .context("Opening /etc/fstab")? + .map(std::io::BufReader::new) + { + let mut from_anaconda = false; + // Only read 10 lines just because it should be near the start + for line in fd.lines().take(10) { + let line = line.context("Reading /etc/fstab")?; + if line.contains(FSTAB_ANACONDA_STAMP) { + from_anaconda = true; + break; + } + } + tracing::debug!("/etc/fstab from anaconda: {from_anaconda}"); + if from_anaconda { + generate_fstab_editor(unit_dir)?; + } + } + Ok(()) +} + +/// 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(()); + } + fstab_generator_impl(root, unit_dir) +} + +/// 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<()> { + let unit = "bootc-fstab-edit.service"; + unit_dir.atomic_write( + unit, + "[Unit]\n\ +Before=local-fs-pre.target\n\ +[Service]\n\ +ExecStart=bootc internals fixup-etc-fstab +", + )?; + let target = "local-fs-pre.target.wants"; + unit_dir.create_dir_all(target)?; + unit_dir.symlink(&format!("../{unit}"), &format!("{target}/{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(), 1); + + 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;