engineering: Propagate flexible ESP mount path

.vscode/settings.json

@@ -26,4 +26,7 @@
     "protobuf.includes": [
         "${workspaceFolder}/proto"
     ],
+    "chat.tools.terminal.autoApprove": {
+        "cargo fmt": true
+    },
 }

crates/trident/src/engine/ab_update.rs

@@ -4,7 +4,7 @@ use log::{debug, info, warn};
 
 use osutils::{chroot, container};
 use trident_api::{
-    constants::{internal_params::NO_TRANSITION, ESP_MOUNT_POINT_PATH, ROOT_MOUNT_POINT_PATH},
+    constants::{internal_params::NO_TRANSITION, ROOT_MOUNT_POINT_PATH},
     error::{InternalError, ReportError, ServicingError, TridentError, TridentResultExt},
     status::{HostStatus, ServicingState, ServicingType},
 };
@@ -154,7 +154,7 @@ pub(crate) fn finalize_update(
     })?;
 
     let root_path = container::get_host_relative_path(PathBuf::from(ROOT_MOUNT_POINT_PATH))?;
-    let esp_path = container::get_host_relative_path(PathBuf::from(ESP_MOUNT_POINT_PATH))?;
+    let esp_path = container::get_host_relative_path(ctx.esp_mount_path.as_path().into())?;
     bootentries::create_and_update_boot_variables(&ctx, &esp_path)?;
     // Analogous to how UEFI variables are configured, finalize must start configuring
     // UEFI fallback, and a successful commit will finish it.

crates/trident/src/engine/boot/grub.rs

@@ -15,7 +15,7 @@ use osutils::{
 use trident_api::{
     config::Selinux,
     constants::{
-        BOOT_MOUNT_POINT_PATH, ESP_EFI_DIRECTORY, ESP_MOUNT_POINT_PATH, GRUB2_CONFIG_FILENAME,
+        BOOT_MOUNT_POINT_PATH, ESP_EFI_DIRECTORY, GRUB2_CONFIG_FILENAME,
         GRUB2_CONFIG_RELATIVE_PATH, ROOT_MOUNT_POINT_PATH, TRIDENT_OVERLAY_LOWER_RELATIVE_PATH,
         TRIDENT_OVERLAY_UPPER_RELATIVE_PATH, TRIDENT_OVERLAY_WORK_RELATIVE_PATH,
     },
@@ -81,7 +81,9 @@ pub(super) fn update_configs(ctx: &EngineContext, os_modifier_path: &Path) -> Re
     }
 
     // Update GRUB config on the ESP
-    let bootentry_config_path = Path::new(ESP_MOUNT_POINT_PATH)
+    let bootentry_config_path = ctx
+        .esp_mount_path
+        .as_path()
         .join(ESP_EFI_DIRECTORY)
         .join(super::get_update_esp_dir_name(ctx).context("Failed to get update install ID")?)
         .join(GRUB2_CONFIG_FILENAME);
@@ -285,6 +287,7 @@ pub(crate) mod functional_test {
             self, AbUpdate, AbVolumePair, Disk, FileSystem, FileSystemSource, HostConfiguration,
             MountOptions, MountPoint, Partition, PartitionType, RaidLevel, SoftwareRaidArray,
         },
+        constants::ESP_MOUNT_POINT_PATH,
         status::ServicingType,
     };
 

crates/trident/src/engine/bootentries.rs

@@ -12,7 +12,7 @@ use osutils::{
 
 use trident_api::{
     config::RaidLevel,
-    constants::{self, internal_params::VIRTDEPLOY_BOOT_ORDER_WORKAROUND, ESP_MOUNT_POINT_PATH},
+    constants::{self, internal_params::VIRTDEPLOY_BOOT_ORDER_WORKAROUND},
     error::{InternalError, ReportError, ServicingError, TridentError, TridentResultExt},
     status::{AbVolumeSelection, ServicingType},
     BlockDeviceId,
@@ -286,7 +286,7 @@ fn create_boot_entries_for_rebuilt_esp_partitions(
                                 ctx,
                                 esp_device.clone(),
                                 entry_label.clone(),
-                                Path::new(ESP_MOUNT_POINT_PATH),
+                                ctx.esp_mount_path.as_path(),
                                 bootloader_path,
                                 true,
                             )
@@ -1024,9 +1024,12 @@ mod functional_test {
         testutils::repart::{OS_DISK_DEVICE_PATH, TEST_DISK_DEVICE_PATH},
     };
     use pytest_gen::functional_test;
-    use trident_api::config::{
-        self, Disk, HostConfiguration, ImageSha384, MountOptions, MountPoint, OsImage, Partition,
-        PartitionSize, PartitionType,
+    use trident_api::{
+        config::{
+            self, Disk, HostConfiguration, ImageSha384, MountOptions, MountPoint, OsImage,
+            Partition, PartitionSize, PartitionType,
+        },
+        constants::ESP_MOUNT_POINT_PATH,
     };
 
     use crate::engine::{storage::partitioning, EngineContext};

crates/trident/src/engine/clean_install.rs

@@ -18,7 +18,7 @@ use trident_api::{
         internal_params::{
             DISABLE_MEDIA_EJECTION, ENABLE_UKI_SUPPORT, NO_TRANSITION, RAW_COSI_STORAGE,
         },
-        ESP_MOUNT_POINT_PATH, ROOT_MOUNT_POINT_PATH, UPDATE_ROOT_PATH,
+        ROOT_MOUNT_POINT_PATH, UPDATE_ROOT_PATH,
     },
     error::{
         InitializationError, InternalError, InvalidInputError, ReportError, ServicingError,
@@ -214,7 +214,8 @@ fn stage_clean_install(
         &ctx.partition_paths,
         AbVolumeSelection::VolumeA,
     )?;
-    ctx.install_index = install_index::next_install_index(newroot_mount.path())?;
+    ctx.install_index =
+        install_index::next_install_index(newroot_mount.path(), ctx.esp_mount_path.as_path())?;
 
     engine::provision(subsystems, &ctx, newroot_mount.path())?;
 
@@ -305,7 +306,7 @@ pub(crate) fn finalize_clean_install(
     };
 
     // On clean install, need to verify that AZLA entry exists in /mnt/newroot/boot/efi
-    let esp_path = join_relative(new_root.path(), ESP_MOUNT_POINT_PATH);
+    let esp_path = join_relative(new_root.path(), ctx.esp_mount_path.as_path());
 
     if !ctx.spec.internal_params.get_flag(RAW_COSI_STORAGE) {
         bootentries::create_and_update_boot_variables(&ctx, &esp_path)?;

crates/trident/src/engine/context/filesystem.rs

@@ -8,7 +8,7 @@ use derive_more::From;
 use sysdefs::filesystems::{KernelFilesystemType, NodevFilesystemType, RealFilesystemType};
 use trident_api::{
     config::{FileSystem, FileSystemSource, MountPoint, NewFileSystemType},
-    constants::{ESP_MOUNT_POINT_PATH, MOUNT_OPTION_READ_ONLY, ROOT_MOUNT_POINT_PATH},
+    constants::{MOUNT_OPTION_READ_ONLY, ROOT_MOUNT_POINT_PATH},
     error::{InternalError, ReportError, TridentError, TridentResultExt},
     BlockDeviceId,
 };
@@ -171,13 +171,6 @@ impl FileSystemData {
             .is_some_and(|mpp| mpp == Path::new(ROOT_MOUNT_POINT_PATH))
     }
 
-    /// Returns whether the filesystem is the EFI System Partition (ESP), as
-    /// determined by its mount point path.
-    pub fn is_esp(&self) -> bool {
-        self.mount_point_path()
-            .is_some_and(|mpp| mpp == Path::new(ESP_MOUNT_POINT_PATH))
-    }
-
     /// Returns the path of the mount point, if it exists.
     pub fn mount_point_path(&self) -> Option<&Path> {
         match self {
@@ -254,10 +247,6 @@ impl FileSystemDataImage {
         self.mount_point.options.contains(MOUNT_OPTION_READ_ONLY)
     }
 
-    pub fn is_esp(&self) -> bool {
-        self.mount_point_path() == Path::new(ESP_MOUNT_POINT_PATH)
-    }
-
     pub fn is_root(&self) -> bool {
         self.mount_point_path() == Path::new(ROOT_MOUNT_POINT_PATH)
     }
@@ -354,7 +343,7 @@ impl EngineContext {
     pub fn esp_filesystem(&self) -> Option<FileSystemDataImage> {
         self.filesystems()
             .filter_map(|fs| fs.as_image().cloned())
-            .find(|img_fs| img_fs.mount_point.path == Path::new(ESP_MOUNT_POINT_PATH))
+            .find(|img_fs| img_fs.mount_point.path == self.esp_mount_path.as_path())
     }
 }
 

crates/trident/src/engine/context/mod.rs

@@ -99,6 +99,9 @@ pub struct EngineContext {
 
     /// Whether the image will use a UKI or not.
     pub is_uki: Option<bool>,
+
+    /// The mount path of the ESP partition.
+    pub esp_mount_path: PathBuf,
 }
 
 impl EngineContext {
@@ -108,6 +111,16 @@ impl EngineContext {
     pub fn new(params: EngineContextParams) -> Result<Self, TridentError> {
         let graph = super::build_storage_graph(&params.spec.storage)?;
 
+        // Get the ESP mount path from the storage graph. This should be
+        // guaranteed to exist in validated Host configurations.
+        let esp_mount_path =
+            graph
+                .esp_mount_path()
+                .ok_or(TridentError::new(InternalError::Internal(
+                    "Storage graph does not contain an ESP mount path, the Host Configuration was not validated properly.",
+                )))?
+                .to_path_buf();
+
         Ok(Self {
             spec: params.spec,
             spec_old: params.spec_old,
@@ -120,6 +133,7 @@ impl EngineContext {
             storage_graph: graph,
             filesystems: Vec::new(),
             is_uki: params.is_uki,
+            esp_mount_path,
         })
     }
 

crates/trident/src/engine/install_index.rs

@@ -2,18 +2,31 @@ use std::path::Path;
 
 use log::{debug, trace};
 
+use osutils::path;
 use trident_api::{
-    constants::{ESP_EFI_DIRECTORY, ESP_RELATIVE_MOUNT_POINT_PATH},
+    constants::ESP_EFI_DIRECTORY,
     error::{ReportError, TridentError, TridentResultExt, UnsupportedConfigurationError},
 };
 
 use crate::engine::boot;
 
 /// Returns the next available install index for the current install.
-pub fn next_install_index(mount_point: &Path) -> Result<usize, TridentError> {
-    let esp_efi_path = mount_point
-        .join(ESP_RELATIVE_MOUNT_POINT_PATH)
-        .join(ESP_EFI_DIRECTORY);
+///
+/// The install index is determined by looking at the existing ESP directory
+/// names in the ESP EFI path and finding the first available index that doesn't
+/// have any corresponding ESP directory names. This ensures that the new
+/// install will not overwrite any existing installs, even if some of them have
+/// missing ESP directories.
+///
+/// The function takes the root mount point and the ESP mount path (with or
+/// without leading `/`) as parameters to construct the full path to the ESP EFI
+/// directory where the install directories are located.
+pub fn next_install_index(
+    root_mount_point: &Path,
+    esp_mount_path: &Path,
+) -> Result<usize, TridentError> {
+    let esp_efi_path =
+        path::join_relative(root_mount_point, esp_mount_path).join(ESP_EFI_DIRECTORY);
 
     debug!(
         "Looking for next available install index in '{}'",
@@ -55,41 +68,48 @@ mod tests {
 
     use tempfile::TempDir;
 
-    use trident_api::error::ErrorKind;
+    use trident_api::{constants::ESP_MOUNT_POINT_PATH, error::ErrorKind};
 
     use crate::engine::boot::make_esp_dir_name_candidates;
 
-    // Helper that constructs the ESP EFI path, creates the directories, and returns the path.
-    fn setup_esp_efi_path(mount_point: &Path) -> PathBuf {
-        let esp_efi_path = mount_point
-            .join(ESP_RELATIVE_MOUNT_POINT_PATH)
-            .join(ESP_EFI_DIRECTORY);
+    // Helper that constructs the ESP EFI path, creates the directories, and returns
+    // (full_esp_efi_path, esp_mount_path). The esp_mount_path is what next_install_index expects.
+    fn setup_esp_efi_path(mount_point: &Path) -> (PathBuf, PathBuf) {
+        let esp_mount_path = PathBuf::from(ESP_MOUNT_POINT_PATH);
+        let esp_efi_path =
+            path::join_relative(mount_point, &esp_mount_path).join(ESP_EFI_DIRECTORY);
         fs::create_dir_all(&esp_efi_path).unwrap();
-        esp_efi_path
+        (esp_efi_path, esp_mount_path)
     }
 
     #[test]
     fn test_install_index_variants() {
         // Test case #0: No existing install directories.
         let test_dir = TempDir::new().unwrap();
-        let esp_efi_path = setup_esp_efi_path(test_dir.path());
-        assert_eq!(next_install_index(test_dir.path()).unwrap(), 0);
+        let (esp_efi_path, relative_esp_efi_path) = setup_esp_efi_path(test_dir.path());
+        assert_eq!(
+            next_install_index(test_dir.path(), &relative_esp_efi_path).unwrap(),
+            0
+        );
         assert_eq!(
             find_first_available_install_index(&esp_efi_path).unwrap(),
             0
         );
 
         // Test case #1: Install directories 0-9 exist.
         let test_dir = TempDir::new().unwrap();
-        let esp_efi_path = setup_esp_efi_path(test_dir.path());
+        let (esp_efi_path, relative_esp_efi_path) = setup_esp_efi_path(test_dir.path());
         make_esp_dir_name_candidates()
             .take(10)
             .for_each(|(_, dir_names)| {
                 for dir_name in dir_names {
                     fs::create_dir(esp_efi_path.join(dir_name)).unwrap();
                 }
             });
-        assert_eq!(next_install_index(test_dir.path()).unwrap(), 10);
+        assert_eq!(
+            next_install_index(test_dir.path(), &relative_esp_efi_path).unwrap(),
+            10
+        );
         assert_eq!(
             find_first_available_install_index(&esp_efi_path).unwrap(),
             10
@@ -98,13 +118,16 @@ mod tests {
         // Test case #2: Install directories 0-9 exist. Func will skip unavailable indices, even
         // when only the A volume IDs are present.
         let test_dir = TempDir::new().unwrap();
-        let esp_efi_path = setup_esp_efi_path(test_dir.path());
+        let (esp_efi_path, relative_esp_efi_path) = setup_esp_efi_path(test_dir.path());
         make_esp_dir_name_candidates()
             .take(10)
             .for_each(|(_, dir_names)| {
                 fs::create_dir(esp_efi_path.join(&dir_names[0])).unwrap();
             });
-        assert_eq!(next_install_index(test_dir.path()).unwrap(), 10);
+        assert_eq!(
+            next_install_index(test_dir.path(), &relative_esp_efi_path).unwrap(),
+            10
+        );
         assert_eq!(
             find_first_available_install_index(&esp_efi_path).unwrap(),
             10
@@ -113,13 +136,16 @@ mod tests {
         // Test case #3: Install directories 0-9 exist. Func will skip unavailable indices, even
         // when only the A volume IDs are present.
         let test_dir = TempDir::new().unwrap();
-        let esp_efi_path = setup_esp_efi_path(test_dir.path());
+        let (esp_efi_path, relative_esp_efi_path) = setup_esp_efi_path(test_dir.path());
         make_esp_dir_name_candidates()
             .take(10)
             .for_each(|(_, dir_names)| {
                 fs::create_dir(esp_efi_path.join(&dir_names[1])).unwrap();
             });
-        assert_eq!(next_install_index(test_dir.path()).unwrap(), 10);
+        assert_eq!(
+            next_install_index(test_dir.path(), &relative_esp_efi_path).unwrap(),
+            10
+        );
         assert_eq!(
             find_first_available_install_index(&esp_efi_path).unwrap(),
             10
@@ -128,22 +154,25 @@ mod tests {
         // Test case #4: Install directories 0-9 exist. Func will skip unavailable indices, even
         // when only one ID is present per install.
         let test_dir = TempDir::new().unwrap();
-        let esp_efi_path = setup_esp_efi_path(test_dir.path());
+        let (esp_efi_path, relative_esp_efi_path) = setup_esp_efi_path(test_dir.path());
         let mut volume_selector = (0..=1).cycle();
         make_esp_dir_name_candidates()
             .take(10)
             .for_each(|(_, dir_names)| {
                 fs::create_dir(esp_efi_path.join(&dir_names[volume_selector.next().unwrap()]))
                     .unwrap();
             });
-        assert_eq!(next_install_index(test_dir.path()).unwrap(), 10);
+        assert_eq!(
+            next_install_index(test_dir.path(), &relative_esp_efi_path).unwrap(),
+            10
+        );
         assert_eq!(
             find_first_available_install_index(&esp_efi_path).unwrap(),
             10
         );
 
         let test_dir = TempDir::new().unwrap();
-        let esp_efi_path = setup_esp_efi_path(test_dir.path());
+        let (esp_efi_path, relative_esp_efi_path) = setup_esp_efi_path(test_dir.path());
         let mut volume_selector = (0..=1).cycle();
         volume_selector.next(); // Advance to start with B
         make_esp_dir_name_candidates()
@@ -152,7 +181,10 @@ mod tests {
                 fs::create_dir(esp_efi_path.join(&dir_names[volume_selector.next().unwrap()]))
                     .unwrap();
             });
-        assert_eq!(next_install_index(test_dir.path()).unwrap(), 10);
+        assert_eq!(
+            next_install_index(test_dir.path(), &relative_esp_efi_path).unwrap(),
+            10
+        );
         assert_eq!(
             find_first_available_install_index(&esp_efi_path).unwrap(),
             10
@@ -162,7 +194,7 @@ mod tests {
     #[test]
     fn test_no_available_install_index() {
         let test_dir = tempfile::TempDir::new().unwrap();
-        let esp_efi_path = setup_esp_efi_path(test_dir.path());
+        let (esp_efi_path, relative_esp_efi_path) = setup_esp_efi_path(test_dir.path());
 
         // Exhaust all possible indices (up to 1000)
         crate::engine::boot::make_esp_dir_name_candidates()
@@ -183,7 +215,9 @@ mod tests {
         );
 
         assert_eq!(
-            next_install_index(test_dir.path()).unwrap_err().kind(),
+            next_install_index(test_dir.path(), &relative_esp_efi_path)
+                .unwrap_err()
+                .kind(),
             &ErrorKind::UnsupportedConfiguration(
                 UnsupportedConfigurationError::NoAvailableInstallIndex
             )