diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a9c3ab2f..b5be61c7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -161,4 +161,10 @@ jobs: run: | set -xeuo pipefail sudo ./scripts/test-uefi-vm-boot.sh "$IMG_NAME" "ostree" - sudo ./scripts/test-uefi-vm-boot.sh "$IMG_NAME" "composefs" + + # TODO: Skip composefs tests for C10S as bootc 1.16.3 uses composefs.digest= + # cmdline, but for some reason that cmdline is not mentioned in + # bootc-root-setup.service + if [[ "${{ matrix.stream }}" != "10" ]]; then + sudo ./scripts/test-uefi-vm-boot.sh "$IMG_NAME" "composefs" + fi diff --git a/src/aleph.rs b/src/aleph.rs index 783260a7..3ef8ffd4 100644 --- a/src/aleph.rs +++ b/src/aleph.rs @@ -12,6 +12,7 @@ use anyhow::{Context, Result}; use chrono::prelude::*; +use fn_error_context::context; use serde::{Deserialize, Serialize}; use std::fs::File; use std::path::Path; @@ -36,6 +37,7 @@ const ALEPH_PATHS: &[&str] = &[ "sysroot/.bootc-aleph.json", ]; +#[context("Getting aleph version")] pub(crate) fn get_aleph_version(root: &Path) -> Result> { for aleph_path in ALEPH_PATHS { let path = &root.join(aleph_path); diff --git a/src/cli/bootupctl.rs b/src/cli/bootupctl.rs index 7f2d7507..f462b2c8 100644 --- a/src/cli/bootupctl.rs +++ b/src/cli/bootupctl.rs @@ -49,10 +49,6 @@ impl CtlCommand { /// CLI sub-commands. #[derive(Debug, Parser)] pub enum CtlVerb { - // FIXME(lucab): drop this after refreshing - // https://github.com/coreos/fedora-coreos-config/pull/595 - #[clap(name = "backend", hide = true, subcommand)] - Backend(CtlBackend), #[clap(name = "status", about = "Show components status")] Status(StatusOpts), #[clap(name = "update", about = "Update all components")] @@ -61,6 +57,14 @@ pub enum CtlVerb { AdoptAndUpdate(AdoptAndUpdateOpts), #[clap(name = "validate", about = "Validate system state")] Validate, + /// Low-level commands for building and installing OS images + /// + /// These act on an OS tree or target root rather than on the running + /// system, and are intended for image build and installation tooling + /// (e.g. `bootc install` or disk image builders). To update the + /// bootloader of a booted system, use `update` or `adopt-and-update`. + #[clap(name = "backend", subcommand)] + Backend(super::bootupd::DVerb), #[clap( name = "migrate-static-grub-config", hide = true, @@ -69,17 +73,6 @@ pub enum CtlVerb { MigrateStaticGrubConfig, } -#[derive(Debug, Parser)] -pub enum CtlBackend { - #[clap(name = "generate-update-metadata", hide = true)] - Generate(super::bootupd::GenerateOpts), - #[clap(name = "install", hide = true)] - Install(super::bootupd::InstallOpts), - #[clap(hide = true)] - #[cfg(efi_arch)] - SetDefaultBootloader(super::bootupd::DefaultBootloaderOpts), -} - #[derive(Debug, Parser)] pub struct StatusOpts { /// If there are updates available, output `Updates available: ` to standard output; @@ -123,16 +116,7 @@ impl CtlCommand { CtlVerb::Update(opts) => Self::run_update(opts), CtlVerb::AdoptAndUpdate(opts) => Self::run_adopt_and_update(opts), CtlVerb::Validate => Self::run_validate(), - CtlVerb::Backend(CtlBackend::Generate(opts)) => { - super::bootupd::DCommand::run_generate_meta(opts) - } - CtlVerb::Backend(CtlBackend::Install(opts)) => { - super::bootupd::DCommand::run_install(opts) - } - #[cfg(efi_arch)] - CtlVerb::Backend(CtlBackend::SetDefaultBootloader(opts)) => { - super::bootupd::DCommand::set_default_bootloader(opts) - } + CtlVerb::Backend(verb) => verb.run(), CtlVerb::MigrateStaticGrubConfig => Self::run_migrate_static_grub_config(), } } diff --git a/src/cli/bootupd.rs b/src/cli/bootupd.rs index a9b8951b..4d57637f 100644 --- a/src/cli/bootupd.rs +++ b/src/cli/bootupd.rs @@ -36,10 +36,21 @@ impl DCommand { /// CLI sub-commands. #[derive(Debug, Parser)] pub enum DVerb { - #[clap(name = "generate-update-metadata", about = "Generate metadata")] + /// Generate update metadata from the bootloader files in an OS tree + /// + /// Run this at image build time. It records the bootloader component + /// versions (e.g. from the RPM database) under /usr/lib/bootupd/updates, + /// which `install` and `update` later use as their source. + #[clap(name = "generate-update-metadata")] GenerateUpdateMetadata(GenerateOpts), - #[clap(name = "install", about = "Install components")] + /// Install bootloader components to a target root + /// + /// Used when creating a disk image or installing a system: copies the + /// update payload from the source root into the target's ESP and/or + /// boot partition, and installs to the target block device(s) as needed. + #[clap(name = "install")] Install(InstallOpts), + /// Set the default bootloader on EFI systems #[cfg(efi_arch)] SetDefaultBootloader(DefaultBootloaderOpts), } @@ -94,7 +105,7 @@ pub(crate) struct InstallOpts { #[derive(Debug, Parser)] pub struct GenerateOpts { - /// Physical root mountpoint + /// Root of the OS tree (only `/` is currently supported) #[clap(value_parser)] sysroot: Option, } @@ -108,15 +119,22 @@ pub struct DefaultBootloaderOpts { pub(crate) bootloader: Bootloader, } +impl DVerb { + /// Run this verb; shared by `bootupd` and `bootupctl backend`. + pub(crate) fn run(self) -> Result<()> { + match self { + DVerb::Install(opts) => DCommand::run_install(opts), + DVerb::GenerateUpdateMetadata(opts) => DCommand::run_generate_meta(opts), + #[cfg(efi_arch)] + DVerb::SetDefaultBootloader(opts) => DCommand::set_default_bootloader(opts), + } + } +} + impl DCommand { /// Run CLI application. pub fn run(self) -> Result<()> { - match self.cmd { - DVerb::Install(opts) => Self::run_install(opts), - DVerb::GenerateUpdateMetadata(opts) => Self::run_generate_meta(opts), - #[cfg(efi_arch)] - DVerb::SetDefaultBootloader(opts) => Self::set_default_bootloader(opts), - } + self.cmd.run() } /// Runner for `generate-install-metadata` verb. diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 02212b56..6b505797 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -58,6 +58,23 @@ mod tests { bootupd::DCommand::command().debug_assert(); } + #[test] + fn backend_is_documented() { + use clap::CommandFactory; + let ctl = bootupctl::CtlCommand::command(); + let backend = ctl.find_subcommand("backend").unwrap(); + assert!(!backend.is_hide_set()); + assert!(backend.get_about().is_some()); + for verb in backend.get_subcommands() { + assert!(!verb.is_hide_set(), "{} is hidden", verb.get_name()); + assert!( + verb.get_about().is_some(), + "{} has no help", + verb.get_name() + ); + } + } + #[test] fn test_multicall_dispatch() { { diff --git a/src/grubconfigs.rs b/src/grubconfigs.rs index f06180e8..f76a2e4a 100644 --- a/src/grubconfigs.rs +++ b/src/grubconfigs.rs @@ -174,12 +174,19 @@ fn write_grubenv(grubdir: &Dir) -> Result<()> { .run_inherited_with_cmd_context() } -#[context("Ensure file permissions are 0600")] +#[context("Ensure file permissions are {GRUBCONFIG_FILE_MODE:04o}")] fn ensure_file_permissions(target_dir: &Dir, target_file: &str) -> Result<()> { - let metadata = match target_dir.metadata(target_file) { + use std::os::unix::fs::PermissionsExt; + + // grub.cfg may be a symlink pointing outside this Dir cap_std refuses to traverse + // such symlinks via openat, so resolve through /proc/self/fd + let absolute_path = format!("/proc/self/fd/{}/{target_file}", target_dir.as_raw_fd()); + + let metadata = match std::fs::metadata(&absolute_path) { Ok(metadata) => metadata, Err(e) if e.kind() == std::io::ErrorKind::NotFound => { // Silently return if file doesn't exist + log::debug!("File {target_file} not found"); return Ok(()); } Err(e) => return Err(e).context(format!("Reading {} metadata", target_file)), @@ -187,14 +194,21 @@ fn ensure_file_permissions(target_dir: &Dir, target_file: &str) -> Result<()> { let mode = metadata.permissions().mode() & 0o777; if mode != GRUBCONFIG_FILE_MODE { - target_dir - .set_permissions(target_file, Permissions::from_mode(GRUBCONFIG_FILE_MODE)) - .with_context(|| format!("Setting {} permissions to 0600", target_file))?; + std::fs::set_permissions( + absolute_path, + std::fs::Permissions::from_mode(GRUBCONFIG_FILE_MODE), + ) + .with_context(|| { + format!( + "Setting {} permissions to {GRUBCONFIG_FILE_MODE:04o}", + target_file + ) + })?; } Ok(()) } -#[context("Ensure grub files permissions are 0600")] +#[context("Ensure grub files permissions are {GRUBCONFIG_FILE_MODE:04o}")] pub(crate) fn ensure_grub_permissions(grub_dir: &Dir) -> Result<()> { for file_name in GRUB_FILES.iter() { ensure_file_permissions(&grub_dir, file_name) @@ -251,7 +265,7 @@ mod tests { { let metadata = td.metadata("grubenv")?; let mode = metadata.permissions().mode() & 0o777; - assert_eq!(mode, 0o600); + assert_eq!(mode, GRUBCONFIG_FILE_MODE); } Ok(()) } @@ -285,11 +299,29 @@ mod tests { // Verify grubenv was fixed to 0o600 let metadata = grub_dir.metadata("grubenv")?; - assert_eq!(metadata.permissions().mode() & 0o777, 0o600); + assert_eq!(metadata.permissions().mode() & 0o777, GRUBCONFIG_FILE_MODE); // Verify grub.cfg was fixed 0o600 let metadata = grub_dir.metadata("grub.cfg")?; - assert_eq!(metadata.permissions().mode() & 0o777, 0o600); + assert_eq!(metadata.permissions().mode() & 0o777, GRUBCONFIG_FILE_MODE); + + use std::os::unix::fs::PermissionsExt; + + // Test it with symlinks pointing outside the grub2 dir + // Create the symlink target outside the grub2 dir, with wrong perms + let external_target = tdp.join("external-grub.cfg"); + std::fs::write(&external_target, "external grub.cfg content")?; + std::fs::set_permissions(&external_target, std::fs::Permissions::from_mode(0o644))?; + + grub_dir.remove_file("grub.cfg")?; + std::os::unix::fs::symlink(&external_target, grub.join("grub.cfg"))?; + + ensure_grub_permissions(&grub_dir)?; + + // Verify grub.cfg was fixed 0o600 + let metadata = std::fs::metadata(&external_target)?; + assert_eq!(metadata.permissions().mode() & 0o777, GRUBCONFIG_FILE_MODE); + Ok(()) } }