lib/install: Use mounted ESP for install to-filesystem

crates/lib/src/bootc_composefs/boot.rs

@@ -106,10 +106,8 @@ use crate::{
 };
 use crate::{
     bootc_composefs::state::{get_booted_bls, write_composefs_state},
-    bootloader::esp_in,
-};
-use crate::{
-    bootc_composefs::status::get_container_manifest_and_config, bootc_kargs::compute_new_kargs,
+    bootc_composefs::status::get_container_manifest_and_config,
+    bootc_kargs::compute_new_kargs,
 };
 use crate::{bootc_composefs::status::get_sorted_grub_uki_boot_entries, install::PostFetchState};
 use crate::{
@@ -521,11 +519,25 @@ pub(crate) fn setup_composefs_bls_boot(
             cmdline_options.extend(&Cmdline::from(&composefs_cmdline));
 
             // Locate ESP partition device
-            let esp_part = esp_in(&root_setup.device_info)?;
+            let esp_root = root_setup.open_target_root()?;
+            let esp_device = if root_setup.require_esp_mount {
+                crate::bootloader::find_esp_mount(&esp_root)?.device
+            } else {
+                match crate::bootloader::find_esp_mount(&esp_root) {
+                    Ok(esp) => esp.device,
+                    Err(e) => {
+                        tracing::debug!(
+                            "ESP mount check failed in permissive mode: {e}; falling back to partition table scan"
+                        );
+                        let esp = crate::bootloader::esp_in(&root_setup.device_info)?;
+                        esp.node.clone()
+                    }
+                }
+            };
 
             (
                 root_setup.physical_root_path.clone(),
-                esp_part.node.clone(),
+                esp_device,
                 cmdline_options,
                 fs,
                 postfetch.detected_bootloader.clone(),
@@ -1063,11 +1075,26 @@ pub(crate) fn setup_composefs_uki_boot(
         BootSetupType::Setup((root_setup, state, postfetch, ..)) => {
             state.require_no_kargs_for_uki()?;
 
-            let esp_part = esp_in(&root_setup.device_info)?;
+            let esp_root = root_setup.open_target_root()?;
+
+            let esp_device = if root_setup.require_esp_mount {
+                crate::bootloader::find_esp_mount(&esp_root)?.device
+            } else {
+                match crate::bootloader::find_esp_mount(&esp_root) {
+                    Ok(esp) => esp.device,
+                    Err(e) => {
+                        tracing::debug!(
+                            "ESP mount check failed in permissive mode: {e}; falling back to partition table scan"
+                        );
+                        let esp = crate::bootloader::esp_in(&root_setup.device_info)?;
+                        esp.node.clone()
+                    }
+                }
+            };
 
             (
                 root_setup.physical_root_path.clone(),
-                esp_part.node.clone(),
+                esp_device,
                 postfetch.detected_bootloader.clone(),
                 state.composefs_options.insecure,
                 state.composefs_options.uki_addon.as_ref(),
@@ -1231,23 +1258,29 @@ pub(crate) async fn setup_composefs_boot(
         .or(root_setup.rootfs_uuid.as_deref())
         .ok_or_else(|| anyhow!("No uuid for boot/root"))?;
 
+    let target_root = root_setup.open_target_root()?;
+
     if cfg!(target_arch = "s390x") {
         // TODO: Integrate s390x support into install_via_bootupd
         crate::bootloader::install_via_zipl(&root_setup.device_info, boot_uuid)?;
     } else if postfetch.detected_bootloader == Bootloader::Grub {
         crate::bootloader::install_via_bootupd(
             &root_setup.device_info,
+            &target_root,
             &root_setup.physical_root_path,
             &state.config_opts,
             None,
+            root_setup.require_esp_mount,
         )?;
     } else {
         crate::bootloader::install_systemd_boot(
+            &target_root,
             &root_setup.device_info,
             &root_setup.physical_root_path,
             &state.config_opts,
             None,
             get_secureboot_keys(&mounted_fs, BOOTC_AUTOENROLL_PATH)?,
+            root_setup.require_esp_mount,
         )?;
     }
 
@@ -1406,4 +1439,11 @@ mod tests {
             "RHEL should sort before Fedora in descending order"
         );
     }
+
+    #[test]
+    fn test_efi_uuid_source_formatting() {
+        let source = get_efi_uuid_source();
+        assert!(source.contains("${config_directory}/"));
+        assert!(source.contains(EFI_UUID_FILE));
+    }
 }

crates/lib/src/bootloader.rs

@@ -1,21 +1,23 @@
+use std::borrow::Cow;
 use std::fs::create_dir_all;
 use std::process::Command;
 
 use anyhow::{Context, Result, anyhow, bail};
 use bootc_utils::{BwrapCmd, CommandRunExt};
-use camino::Utf8Path;
+use camino::{Utf8Path, Utf8PathBuf};
 use cap_std_ext::cap_std::fs::Dir;
 use cap_std_ext::dirext::CapStdExtDirExt;
 use fn_error_context::context;
 
 use bootc_blockdev::{Partition, PartitionTable};
 use bootc_mount as mount;
 
-use crate::bootc_composefs::boot::{SecurebootKeys, get_sysroot_parent_dev, mount_esp};
+use crate::bootc_composefs::boot::{SecurebootKeys, mount_esp};
 use crate::{discoverable_partition_specification, utils};
 
 /// The name of the mountpoint for efi (as a subdirectory of /boot, or at the toplevel)
 pub(crate) const EFI_DIR: &str = "efi";
+
 /// The EFI system partition GUID
 /// Path to the bootupd update payload
 #[allow(dead_code)]
@@ -24,47 +26,114 @@ const BOOTUPD_UPDATES: &str = "usr/lib/bootupd/updates";
 // from: https://github.com/systemd/systemd/blob/26b2085d54ebbfca8637362eafcb4a8e3faf832f/man/systemd-boot.xml#L392
 const SYSTEMD_KEY_DIR: &str = "loader/keys";
 
+/// Represents a found EFI System Partition (ESP) mount
+pub(crate) struct Esp {
+    /// The backing device path
+    pub(crate) device: String,
+    /// The mountpoint path relative to the root
+    pub(crate) path: Utf8PathBuf,
+}
+
 #[allow(dead_code)]
 pub(crate) fn esp_in(device: &PartitionTable) -> Result<&Partition> {
     device
         .find_partition_of_type(discoverable_partition_specification::ESP)
         .ok_or(anyhow::anyhow!("ESP not found in partition table"))
 }
 
-/// Get esp partition node based on the root dir
-pub(crate) fn get_esp_partition_node(root: &Dir) -> Result<Option<String>> {
-    let device = get_sysroot_parent_dev(&root)?;
-    let base_partitions = bootc_blockdev::partitions_of(Utf8Path::new(&device))?;
-    let esp = base_partitions.find_partition_of_esp()?;
-    Ok(esp.map(|v| v.node.clone()))
+/// Convert a source specification like UUID=... or LABEL=... into a
+/// canonical /dev/disk/by-... path. If the source is already a
+/// path, it is returned unchanged.
+fn normalize_esp_source(source: &str) -> Cow<'_, str> {
+    if let Some(uuid) = source.strip_prefix("UUID=") {
+        Cow::Owned(format!("/dev/disk/by-uuid/{uuid}"))
+    } else if let Some(label) = source.strip_prefix("LABEL=") {
+        Cow::Owned(format!("/dev/disk/by-label/{label}"))
+    } else {
+        Cow::Borrowed(source)
+    }
 }
 
-/// Mount ESP part at /boot/efi
-pub(crate) fn mount_esp_part(root: &Dir, root_path: &Utf8Path, is_ostree: bool) -> Result<()> {
-    let efi_path = Utf8Path::new("boot").join(crate::bootloader::EFI_DIR);
-    let Some(esp_fd) = root
-        .open_dir_optional(&efi_path)
-        .context("Opening /boot/efi")?
-    else {
-        return Ok(());
+/// Helper to check if a path is a valid ESP mount
+fn inspect_esp_at(root: &Dir, path: impl AsRef<Utf8Path>) -> Result<Option<String>> {
+    let path = path.as_ref();
+    let esp_fd = match root.open_dir_optional(path)? {
+        Some(fd) => fd,
+        None => return Ok(None),
     };
 
-    let Some(false) = esp_fd.is_mountpoint(".")? else {
-        return Ok(());
-    };
+    if !esp_fd.is_mountpoint(".")?.unwrap_or(false) {
+        return Ok(None);
+    }
+
+    let fs = bootc_mount::inspect_filesystem_of_dir(&esp_fd)?;
+    if fs.fstype != "vfat" && fs.fstype != "fat" {
+        return Ok(None);
+    }
 
-    tracing::debug!("Not a mountpoint: /boot/efi");
-    // On ostree env with enabled composefs, should be /target/sysroot
-    let physical_root = if is_ostree {
-        &root.open_dir("sysroot").context("Opening /sysroot")?
+    let source = normalize_esp_source(&fs.source);
+
+    let source_path = Utf8Path::new(source.as_ref());
+    if !source_path.try_exists()? {
+        return Ok(None);
+    }
+
+    Ok(Some(source.into_owned()))
+}
+
+/// Find the ESP mountpoint by searching common locations (/efi, /boot/efi, /boot)
+pub(crate) fn find_esp_mount(root: &Dir) -> Result<Esp> {
+    // Possible paths for ESP, in order of preference.
+    // We check /efi first as per DPS, then /boot/efi (distro standard),
+    // and finally /boot itself (if it is the ESP).
+    if let Some(device) = inspect_esp_at(root, EFI_DIR)? {
+        tracing::debug!("Found ESP at {EFI_DIR} (device: {device})");
+        return Ok(Esp {
+            device,
+            path: Utf8PathBuf::from(EFI_DIR),
+        });
+    }
+
+    let boot_efi = Utf8Path::new("boot").join(EFI_DIR);
+    if let Some(device) = inspect_esp_at(root, &boot_efi)? {
+        tracing::debug!("Found ESP at {boot_efi} (device: {device})");
+        return Ok(Esp {
+            device,
+            path: boot_efi,
+        });
+    }
+
+    if let Some(device) = inspect_esp_at(root, "boot")? {
+        tracing::debug!("Found ESP at boot (device: {device})");
+        return Ok(Esp {
+            device,
+            path: Utf8PathBuf::from("boot"),
+        });
+    }
+
+    anyhow::bail!("No ESP found at /efi, /boot/efi or /boot. This is required for installation.")
+}
+
+/// Helper to find the ESP device, either via mount or partition table scan
+pub(crate) fn get_esp_device<'a>(
+    root: &Dir,
+    device_info: &'a PartitionTable,
+    require_mount: bool,
+) -> Result<Cow<'a, str>> {
+    if require_mount {
+        Ok(Cow::Owned(find_esp_mount(root)?.device))
     } else {
-        root
-    };
-    if let Some(esp_part) = get_esp_partition_node(physical_root)? {
-        bootc_mount::mount(&esp_part, &root_path.join(&efi_path))?;
-        tracing::debug!("Mounted {esp_part} at /boot/efi");
+        match find_esp_mount(root) {
+            Ok(esp) => Ok(Cow::Owned(esp.device)),
+            Err(e) => {
+                tracing::debug!(
+                    "ESP mount check failed in permissive mode: {e}; falling back to partition table scan"
+                );
+                let esp = esp_in(device_info)?;
+                Ok(Cow::Borrowed(&esp.node))
+            }
+        }
     }
-    Ok(())
 }
 
 /// Determine if the invoking environment contains bootupd, and if there are bootupd-based
@@ -83,10 +152,18 @@ pub(crate) fn supports_bootupd(root: &Dir) -> Result<bool> {
 #[context("Installing bootloader")]
 pub(crate) fn install_via_bootupd(
     device: &PartitionTable,
+    root: &Dir,
     rootfs: &Utf8Path,
     configopts: &crate::install::InstallConfigOpts,
     deployment_path: Option<&str>,
+    require_mount: bool,
 ) -> Result<()> {
+    let esp = if require_mount {
+        Some(find_esp_mount(root)?)
+    } else {
+        find_esp_mount(root).ok()
+    };
+
     let verbose = std::env::var_os("BOOTC_BOOTLOADER_DEBUG").map(|_| "-vvvv");
     // bootc defaults to only targeting the platform boot method.
     let bootupd_opts = (!configopts.generic_image).then_some(["--update-firmware", "--auto"]);
@@ -126,6 +203,8 @@ pub(crate) fn install_via_bootupd(
     if let Some(deploy) = deployment_path {
         let target_root = rootfs.join(deploy);
         let boot_path = rootfs.join("boot");
+        let efi_path = rootfs.join(EFI_DIR);
+        let efi_target = format!("/{EFI_DIR}");
 
         tracing::debug!("Running bootupctl via bwrap in {}", target_root);
 
@@ -140,6 +219,15 @@ pub(crate) fn install_via_bootupd(
             // Bind the target block device inside the bwrap container so bootupctl can access it
             .bind_device(device.path().as_str());
 
+        // If we found an ESP, ensure it's bound.
+        // If it's at /boot or /boot/efi, it's already bound via /boot.
+        // If it's at /efi, we need to bind it explicitly.
+        if let Some(esp) = esp {
+            if esp.path == EFI_DIR {
+                cmd = cmd.bind(&efi_path, &efi_target);
+            }
+        }
+
         // Also bind all partitions of the tafet block device
         for partition in &device.partitions {
             cmd = cmd.bind_device(&partition.node);
@@ -165,17 +253,17 @@ pub(crate) fn install_via_bootupd(
 
 #[context("Installing bootloader")]
 pub(crate) fn install_systemd_boot(
+    root: &Dir,
     device: &PartitionTable,
     _rootfs: &Utf8Path,
     _configopts: &crate::install::InstallConfigOpts,
     _deployment_path: Option<&str>,
     autoenroll: Option<SecurebootKeys>,
+    require_mount: bool,
 ) -> Result<()> {
-    let esp_part = device
-        .find_partition_of_type(discoverable_partition_specification::ESP)
-        .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?;
+    let esp_device = get_esp_device(root, device, require_mount)?;
 
-    let esp_mount = mount_esp(&esp_part.node).context("Mounting ESP")?;
+    let esp_mount = mount_esp(&esp_device).context("Mounting ESP")?;
     let esp_path = Utf8Path::from_path(esp_mount.dir.path())
         .ok_or_else(|| anyhow::anyhow!("Failed to convert ESP mount path to UTF-8"))?;
 
@@ -291,3 +379,30 @@ pub(crate) fn install_via_zipl(device: &PartitionTable, boot_uuid: &str) -> Resu
         .log_debug()
         .run_inherited_with_cmd_context()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_normalize_esp_source_uuid() {
+        let normalized = normalize_esp_source("UUID=abcd-1234");
+        assert_eq!(normalized, "/dev/disk/by-uuid/abcd-1234");
+        assert!(matches!(normalized, Cow::Owned(_)));
+    }
+
+    #[test]
+    fn test_normalize_esp_source_label() {
+        let normalized = normalize_esp_source("LABEL=EFI");
+        assert_eq!(normalized, "/dev/disk/by-label/EFI");
+        assert!(matches!(normalized, Cow::Owned(_)));
+    }
+
+    #[test]
+    fn test_normalize_esp_source_passthrough() {
+        let path = "/dev/sda1";
+        let normalized = normalize_esp_source(path);
+        assert_eq!(normalized, path);
+        assert!(matches!(normalized, Cow::Borrowed(_)));
+    }
+}