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::{
@@ -214,6 +212,17 @@ fi
     )
 }
 
+fn open_target_root(root_setup: &RootSetup) -> Result<Dir> {
+    if let Some(target_root) = root_setup.target_root_path.as_ref() {
+        Dir::open_ambient_dir(target_root, ambient_authority()).context("Opening target root path")
+    } else {
+        root_setup
+            .physical_root
+            .try_clone()
+            .context("Cloning target root handle")
+    }
+}
+
 pub fn get_esp_partition(device: &str) -> Result<(String, Option<String>)> {
     let device_info = bootc_blockdev::partitions_of(Utf8Path::new(device))?;
     let esp = crate::bootloader::esp_in(&device_info)?;
@@ -521,11 +530,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 = open_target_root(root_setup)?;
+            let esp_part = if root_setup.require_esp_mount {
+                crate::bootloader::require_boot_efi_mount(&esp_root)?
+            } else {
+                match crate::bootloader::require_boot_efi_mount(&esp_root) {
+                    Ok(p) => p,
+                    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_part,
                 cmdline_options,
                 fs,
                 postfetch.detected_bootloader.clone(),
@@ -1063,11 +1086,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 = open_target_root(root_setup)?;
+
+            let esp_part = if root_setup.require_esp_mount {
+                crate::bootloader::require_boot_efi_mount(&esp_root)?
+            } else {
+                match crate::bootloader::require_boot_efi_mount(&esp_root) {
+                    Ok(p) => p,
+                    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_part,
                 postfetch.detected_bootloader.clone(),
                 state.composefs_options.insecure,
                 state.composefs_options.uki_addon.as_ref(),
@@ -1231,18 +1269,23 @@ 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 = open_target_root(root_setup)?;
+
     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,
@@ -1406,4 +1449,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

@@ -11,7 +11,7 @@ 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)
@@ -31,40 +31,45 @@ pub(crate) fn esp_in(device: &PartitionTable) -> Result<&Partition> {
         .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()))
+fn normalize_esp_source(source: String) -> String {
+    if let Some(uuid) = source.strip_prefix("UUID=") {
+        format!("/dev/disk/by-uuid/{uuid}")
+    } else if let Some(label) = source.strip_prefix("LABEL=") {
+        format!("/dev/disk/by-label/{label}")
+    } else {
+        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
+pub(crate) fn require_boot_efi_mount(root: &Dir) -> Result<String> {
+    let efi_path = Utf8Path::new("boot").join(EFI_DIR);
+    let esp_fd = root
         .open_dir_optional(&efi_path)
         .context("Opening /boot/efi")?
-    else {
-        return Ok(());
-    };
+        .ok_or_else(|| anyhow::anyhow!("/boot/efi does not exist"))?;
 
-    let Some(false) = esp_fd.is_mountpoint(".")? else {
-        return Ok(());
-    };
+    if !esp_fd.is_mountpoint(".")?.unwrap_or(false) {
+        anyhow::bail!("/boot/efi is not a mountpoint. This is required for installation.");
+    }
 
-    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")?
-    } 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");
+    let fs = bootc_mount::inspect_filesystem_of_dir(&esp_fd)?;
+    if fs.fstype != "vfat" && fs.fstype != "fat" {
+        anyhow::bail!(
+            "/boot/efi is mounted but is not vfat/fat (found {}). This is required for installation.",
+            fs.fstype
+        );
     }
-    Ok(())
+
+    let source = normalize_esp_source(fs.source);
+
+    let source_path = Utf8Path::new(&source);
+    if !source_path.try_exists()? {
+        anyhow::bail!(
+            "/boot/efi source device does not exist: {source}. This is required for installation."
+        );
+    }
+
+    Ok(source)
 }
 
 /// Determine if the invoking environment contains bootupd, and if there are bootupd-based
@@ -83,10 +88,19 @@ 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<()> {
+    if require_mount {
+        // We require /boot/efi to be mounted; finding the device is just a sanity check that it is.
+        // The device argument is currently used by bootupctl as a fallback if it can't find the ESP.
+        // But we want to fail fast if our own check fails.
+        let _esp_device = require_boot_efi_mount(root)?;
+    }
+
     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"]);
@@ -111,17 +125,16 @@ pub(crate) fn install_via_bootupd(
 
 #[context("Installing bootloader")]
 pub(crate) fn install_systemd_boot(
-    device: &PartitionTable,
+    root: &Dir,
+    _device: &PartitionTable,
     _rootfs: &Utf8Path,
     _configopts: &crate::install::InstallConfigOpts,
     _deployment_path: Option<&str>,
     autoenroll: Option<SecurebootKeys>,
 ) -> 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 = require_boot_efi_mount(root)?;
 
-    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"))?;
 
@@ -237,3 +250,26 @@ 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 normalize_esp_source_uuid() {
+        let normalized = normalize_esp_source("UUID=abcd-1234".to_string());
+        assert_eq!(normalized, "/dev/disk/by-uuid/abcd-1234");
+    }
+
+    #[test]
+    fn normalize_esp_source_label() {
+        let normalized = normalize_esp_source("LABEL=EFI".to_string());
+        assert_eq!(normalized, "/dev/disk/by-label/EFI");
+    }
+
+    #[test]
+    fn normalize_esp_source_passthrough() {
+        let normalized = normalize_esp_source("/dev/sda1".to_string());
+        assert_eq!(normalized, "/dev/sda1");
+    }
+}

crates/lib/src/install.rs

@@ -1160,6 +1160,7 @@ pub(crate) struct RootSetup {
     skip_finalize: bool,
     boot: Option<MountSpec>,
     pub(crate) kargs: CmdlineOwned,
+    pub(crate) require_esp_mount: bool,
 }
 
 fn require_boot_uuid(spec: &MountSpec) -> Result<&str> {
@@ -1606,6 +1607,13 @@ async fn install_with_sysroot(
         .context("Opening deployment dir")?;
     let postfetch = PostFetchState::new(state, &deployment_dir)?;
 
+    let target_root_path = rootfs
+        .target_root_path
+        .clone()
+        .unwrap_or(rootfs.physical_root_path.clone());
+    let target_root = Dir::open_ambient_dir(&target_root_path, cap_std::ambient_authority())
+        .with_context(|| format!("Opening target root directory {target_root_path}"))?;
+
     if cfg!(target_arch = "s390x") {
         // TODO: Integrate s390x support into install_via_bootupd
         crate::bootloader::install_via_zipl(&rootfs.device_info, boot_uuid)?;
@@ -1614,12 +1622,11 @@ async fn install_with_sysroot(
             Bootloader::Grub => {
                 crate::bootloader::install_via_bootupd(
                     &rootfs.device_info,
-                    &rootfs
-                        .target_root_path
-                        .clone()
-                        .unwrap_or(rootfs.physical_root_path.clone()),
+                    &target_root,
+                    &target_root_path,
                     &state.config_opts,
                     Some(&deployment_path.as_str()),
+                    rootfs.require_esp_mount,
                 )?;
             }
             Bootloader::Systemd => {
@@ -2007,14 +2014,13 @@ fn remove_all_except_loader_dirs(bootdir: &Dir, is_ostree: bool) -> Result<()> {
 }
 
 #[context("Removing boot directory content")]
-fn clean_boot_directories(rootfs: &Dir, rootfs_path: &Utf8Path, is_ostree: bool) -> Result<()> {
+fn clean_boot_directories(rootfs: &Dir, is_ostree: bool, strict_esp: bool) -> Result<()> {
     let bootdir =
         crate::utils::open_dir_remount_rw(rootfs, BOOT.into()).context("Opening /boot")?;
 
-    if ARCH_USES_EFI {
-        // On booted FCOS, esp is not mounted by default
-        // Mount ESP part at /boot/efi before clean
-        crate::bootloader::mount_esp_part(&rootfs, &rootfs_path, is_ostree)?;
+    if ARCH_USES_EFI && strict_esp {
+        // Require an explicit /boot/efi mount to avoid cleaning the wrong ESP.
+        crate::bootloader::require_boot_efi_mount(rootfs)?;
     }
 
     // This should not remove /boot/efi note.
@@ -2228,7 +2234,7 @@ pub(crate) async fn install_to_filesystem(
                 .await??;
         }
         Some(ReplaceMode::Alongside) => {
-            clean_boot_directories(&target_rootfs_fd, &target_root_path, is_already_ostree)?
+            clean_boot_directories(&target_rootfs_fd, is_already_ostree, !targeting_host_root)?
         }
         None => require_empty_rootdir(&rootfs_fd)?,
     }
@@ -2390,6 +2396,7 @@ pub(crate) async fn install_to_filesystem(
         boot,
         kargs,
         skip_finalize,
+        require_esp_mount: !targeting_host_root,
     };
 
     install_to_filesystem_impl(&state, &mut rootfs, cleanup).await?;

crates/lib/src/install/baseline.rs

@@ -496,5 +496,6 @@ pub(crate) fn install_create_rootfs(
         boot,
         kargs,
         skip_finalize: false,
+        require_esp_mount: true,
     })
 }