Merge pull request #417 from cgwalters/generator-reconcile

Add a systemd generator to fixup Anaconda's /etc/fstab

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,

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*

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;
@@ -138,6 +139,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<Utf8PathBuf>,
+        #[allow(dead_code)]
+        late_dir: Option<Utf8PathBuf>,
+    },
+    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 {
@@ -229,6 +248,9 @@ pub(crate) enum Opt {
         #[clap(trailing_var_arg = true, allow_hyphen_values = true)]
         args: Vec<OsString>,
     },
+    #[clap(subcommand)]
+    #[clap(hide = true)]
+    Internals(InternalsOpts),
     /// Internal integration testing helpers.
     #[clap(hide(true), subcommand)]
     #[cfg(feature = "internal-testing-api")]
@@ -528,11 +550,39 @@ where
     I: IntoIterator,
     I::Item: Into<OsString> + 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<I>(args: I) -> Self
+    where
+        I: IntoIterator,
+        I::Item: Into<OsString> + 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,
@@ -554,6 +604,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")]
@@ -583,10 +644,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 { .. })
+    ));
+}

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<bool> {
+        if line.starts_with("#") {
+            return Ok(false);
+        }
+        let parts = line.split_ascii_whitespace().collect::<Vec<_>>();
+
+        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(())
+}