Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions contrib/packaging/bootc.spec
Original file line number Diff line number Diff line change
Expand Up @@ -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*
Expand Down
76 changes: 74 additions & 2 deletions lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<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 {
Expand Down Expand Up @@ -226,6 +245,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")]
Expand Down Expand Up @@ -525,11 +547,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,
Expand All @@ -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")]
Expand Down Expand Up @@ -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 { .. })
));
}
136 changes: 136 additions & 0 deletions lib/src/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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<()> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to say one really nice thing about pervasively using cap-std is that it makes it trivial to unit-test things like this in a super reliable fashion without accidentally trying to edit the system global /etc/fstab when running cargo test and also without going all the way to running containers. (Which we should do too of course...)

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(())
}
Loading
Loading