diff --git a/crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs b/crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs index 9f3e9be36..bf203c0d4 100644 --- a/crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs +++ b/crates/lib/src/bootc_composefs/backwards_compat/bcompat_boot.rs @@ -16,7 +16,7 @@ use crate::{ ORIGIN_KEY_BOOT, ORIGIN_KEY_BOOT_TYPE, STATE_DIR_RELATIVE, TYPE1_BOOT_DIR_PREFIX, TYPE1_ENT_PATH_STAGED, UKI_NAME_PREFIX, USER_CFG_STAGED, }, - parsers::bls_config::{BLSConfig, BLSConfigType}, + parsers::bls_config::{BLSConfig, BLSConfigType, EFIKey}, spec::BootloaderKind, store::Storage, }; @@ -214,12 +214,16 @@ fn stage_bls_entry_changes( .collect(); } - BLSConfigType::EFI { efi, .. } => { + BLSConfigType::EFI { key, .. } => { // boot_dir in case of UKI is the ESP plan_efi_binary_renames(&boot_dir, &digest, &mut rename_transaction)?; - *efi = Utf8PathBuf::from("/") + let new_path = Utf8PathBuf::from("/") .join(BOOTC_UKI_DIR) .join(get_uki_name(&digest)); + *key = match key { + EFIKey::Efi(_) => EFIKey::Efi(new_path), + EFIKey::Uki(_) => EFIKey::Uki(new_path), + }; } _ => anyhow::bail!("Unknown BLS config type"), diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index d632383e8..0ed919731 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -95,7 +95,7 @@ use crate::bootc_composefs::state::{get_booted_bls, write_composefs_state}; use crate::bootc_composefs::status::ComposefsCmdline; use crate::bootc_kargs::compute_new_kargs; use crate::composefs_consts::{TYPE1_BOOT_DIR_PREFIX, TYPE1_ENT_PATH, TYPE1_ENT_PATH_STAGED}; -use crate::parsers::bls_config::{BLSConfig, BLSConfigType}; +use crate::parsers::bls_config::{BLSConfig, BLSConfigType, EFIKey}; use crate::spec::BootloaderKind; use crate::task::Task; use crate::{bootc_composefs::repo::open_composefs_repo, store::Storage}; @@ -997,6 +997,7 @@ fn write_systemd_uki_config( setup_type: &BootSetupType, boot_label: UKIInfo, id: &Sha512HashValue, + bootloader: &Bootloader, ) -> Result<()> { let os_id = boot_label.os_id.as_deref().unwrap_or("bootc"); let primary_sort_key = primary_sort_key(os_id); @@ -1005,7 +1006,10 @@ fn write_systemd_uki_config( bls_conf .with_title(boot_label.boot_label) .with_cfg(BLSConfigType::EFI { - efi: format!("/{BOOTC_UKI_DIR}/{}", get_uki_name(&id.to_hex())).into(), + key: EFIKey::for_bootloader( + format!("/{BOOTC_UKI_DIR}/{}", get_uki_name(&id.to_hex())).into(), + bootloader, + ), }) .with_sort_key(primary_sort_key.clone()) .with_version(boot_label.version.unwrap_or_else(|| id.to_hex())); @@ -1169,7 +1173,7 @@ pub(crate) fn setup_composefs_uki_boot( } BootloaderKind::BLSCompatible => { - write_systemd_uki_config(&esp_mount.fd, &setup_type, uki_info, id)? + write_systemd_uki_config(&esp_mount.fd, &setup_type, uki_info, id, &bootloader)? } }; diff --git a/crates/lib/src/bootc_composefs/delete.rs b/crates/lib/src/bootc_composefs/delete.rs index 999afaa7c..97295f389 100644 --- a/crates/lib/src/bootc_composefs/delete.rs +++ b/crates/lib/src/bootc_composefs/delete.rs @@ -14,7 +14,7 @@ use crate::{ COMPOSEFS_STAGED_DEPLOYMENT_FNAME, COMPOSEFS_TRANSIENT_STATE_DIR, STATE_DIR_RELATIVE, TYPE1_ENT_PATH, TYPE1_ENT_PATH_STAGED, USER_CFG_STAGED, }, - parsers::bls_config::{BLSConfigType, parse_bls_config}, + parsers::bls_config::{BLSConfigType, EFIKey, parse_bls_config}, spec::{BootEntry, BootloaderKind, DeploymentEntry}, status::Slot, store::{BootedComposefs, Storage}, @@ -54,8 +54,11 @@ fn delete_type1_conf_file( let bls_config = parse_bls_config(&cfg)?; match &bls_config.cfg_type { - BLSConfigType::EFI { efi } => { - if !efi.as_str().contains(&depl.deployment.verity) { + BLSConfigType::EFI { key } => { + let path = match key { + EFIKey::Efi(path) | EFIKey::Uki(path) => path, + }; + if !path.as_str().contains(&depl.deployment.verity) { continue; } diff --git a/crates/lib/src/bootc_composefs/state.rs b/crates/lib/src/bootc_composefs/state.rs index a5e65cd10..019662559 100644 --- a/crates/lib/src/bootc_composefs/state.rs +++ b/crates/lib/src/bootc_composefs/state.rs @@ -29,7 +29,7 @@ use crate::bootc_composefs::boot::BootType; use crate::bootc_composefs::status::{ ComposefsCmdline, StagedDeployment, get_sorted_type1_boot_entries, }; -use crate::parsers::bls_config::BLSConfigType; +use crate::parsers::bls_config::{BLSConfigType, EFIKey}; use crate::store::{BootedComposefs, Storage}; use crate::{ composefs_consts::{ @@ -69,8 +69,11 @@ pub(crate) fn get_booted_bls(boot_dir: &Dir, booted_cfs: &BootedComposefs) -> Re for entry in sorted_entries { match &entry.cfg_type { - BLSConfigType::EFI { efi } => { - if efi.as_str().contains(&*booted_cfs.cmdline.digest) { + BLSConfigType::EFI { key } => { + let path = match key { + EFIKey::Efi(path) | EFIKey::Uki(path) => path, + }; + if path.as_str().contains(&*booted_cfs.cmdline.digest) { return Ok(entry); } } diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index 634aa9225..c8e7851c3 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -22,7 +22,7 @@ use crate::{ }, install::EFI_LOADER_INFO, parsers::{ - bls_config::{BLSConfig, BLSConfigType, parse_bls_config}, + bls_config::{BLSConfig, BLSConfigType, EFIKey, parse_bls_config}, grub_menuconfig::{MenuEntry, parse_grub_menuentry_file}, }, spec::{BootEntry, BootOrder, BootloaderKind, Host, HostSpec, ImageStatus}, @@ -980,8 +980,11 @@ async fn composefs_deployment_status_from( let is_rollback_queued = match &bls_config.cfg_type { // For UKI boot - BLSConfigType::EFI { efi } => { - efi.as_str().contains(booted_composefs_digest.as_ref()) + BLSConfigType::EFI { key } => { + let path = match key { + EFIKey::Efi(path) | EFIKey::Uki(path) => path, + }; + path.as_str().contains(booted_composefs_digest.as_ref()) } // For boot entry Type1 diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index f59f51f81..4109e4aff 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -1,5 +1,6 @@ use std::fs::create_dir_all; use std::process::Command; +use std::sync::OnceLock; use anyhow::{Context, Result, anyhow, bail}; use bootc_utils::{ChrootCmd, CommandRunExt}; @@ -326,9 +327,19 @@ pub(crate) fn install_systemd_boot( } #[context("Querying bootctl version")] -fn bootctl_systemd_version() -> Result { +pub(crate) fn bootctl_systemd_version() -> Result { + static VERSION: OnceLock = OnceLock::new(); + + if let Some(v) = VERSION.get() { + return Ok(*v); + }; + let out = Command::new("bootctl").arg("--version").run_get_string()?; - parse_systemd_version(&out) + let v = parse_systemd_version(&out).context("Failed to parse version to integer")?; + + let version = VERSION.get_or_init(|| v); + + Ok(*version) } /// Parse the systemd major version from `bootctl --version` output, whose first diff --git a/crates/lib/src/parsers/bls_config.rs b/crates/lib/src/parsers/bls_config.rs index c72674a08..6cb499427 100644 --- a/crates/lib/src/parsers/bls_config.rs +++ b/crates/lib/src/parsers/bls_config.rs @@ -13,13 +13,46 @@ use std::fmt::Display; use uapi_version::Version; use crate::bootc_composefs::status::ComposefsCmdline; +use crate::bootloader::bootctl_systemd_version; use crate::composefs_consts::{TYPE1_BOOT_DIR_PREFIX, UKI_NAME_PREFIX}; +use crate::spec::Bootloader; + +/// First systemd release that supports UKI without the `efi` prefix +const SYSTEMD_UKI_MIN_VERSION: u32 = 258; + +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum EFIKey { + /// Relates to 'efi' key in BLSConfig file + Efi(Utf8PathBuf), + /// Relates to 'uki' key in BLSConfig file + Uki(Utf8PathBuf), +} + +impl EFIKey { + /// Create an EFIKey with the appropriate variant based on bootloader and systemd version + /// + /// For GrubCC: always use "uki" + /// For systemd version >= SYSTEMD_UKI_MIN_VERSION use "uki" otherwise uses "efi" + pub(crate) fn for_bootloader(path: Utf8PathBuf, bootloader: &Bootloader) -> EFIKey { + if *bootloader == Bootloader::GrubCC { + // GrubCC doesn't support 'uki' key right now + // See: https://github.com/bootc-dev/bootc/issues/2268 + EFIKey::Efi(path) + } else { + // Check systemd version for non-GrubCC bootloaders + match bootctl_systemd_version() { + Ok(version) if version >= SYSTEMD_UKI_MIN_VERSION => EFIKey::Uki(path), + _ => EFIKey::Efi(path), + } + } + } +} #[derive(Debug, PartialEq, Eq, Default, Clone)] pub enum BLSConfigType { EFI { /// The path to the EFI binary, usually a UKI - efi: Utf8PathBuf, + key: EFIKey, }, NonEFI { /// The path to the linux kernel to boot. @@ -102,9 +135,10 @@ impl Display for BLSConfig { writeln!(f, "version {}", self.version)?; match &self.cfg_type { - BLSConfigType::EFI { efi } => { - writeln!(f, "efi {}", efi)?; - } + BLSConfigType::EFI { key } => match key { + EFIKey::Efi(path) => writeln!(f, "efi {}", path)?, + EFIKey::Uki(path) => writeln!(f, "uki {}", path)?, + }, BLSConfigType::NonEFI { linux, @@ -176,8 +210,11 @@ impl BLSConfig { /// For Non-EFI BLS entries, this returns the fs-verity digest in the "options" field pub(crate) fn get_verity(&self) -> Result { match &self.cfg_type { - BLSConfigType::EFI { efi } => { - let name = efi + BLSConfigType::EFI { key } => { + let path = match key { + EFIKey::Efi(path) | EFIKey::Uki(path) => path, + }; + let name = path .components() .last() .ok_or(anyhow::anyhow!("Empty efi field"))? @@ -224,10 +261,13 @@ impl BLSConfig { /// The second value is a boolean indicating whether it found our custom prefix or not pub(crate) fn boot_artifact_info(&self) -> Result<(&str, bool)> { match &self.cfg_type { - BLSConfigType::EFI { efi } => { - let file_name = efi + BLSConfigType::EFI { key } => { + let path = match key { + EFIKey::Efi(path) | EFIKey::Uki(path) => path, + }; + let file_name = path .file_name() - .ok_or_else(|| anyhow::anyhow!("EFI path missing file name: {}", efi))?; + .ok_or_else(|| anyhow::anyhow!("EFI path missing file name: {}", path))?; let without_suffix = file_name.strip_suffix(EFI_EXT).ok_or_else(|| { anyhow::anyhow!( @@ -288,7 +328,7 @@ pub(crate) fn parse_bls_config(input: &str) -> Result { let mut title = None; let mut version = None; let mut linux = None; - let mut efi = None; + let mut efi_key = None; let mut initrd = Vec::new(); let mut options = None; let mut machine_id = None; @@ -311,7 +351,8 @@ pub(crate) fn parse_bls_config(input: &str) -> Result { "options" => options = Some(CmdlineOwned::from(value)), "machine-id" => machine_id = Some(value), "sort-key" => sort_key = Some(value), - "efi" => efi = Some(Utf8PathBuf::from(value)), + "efi" => efi_key = Some(EFIKey::Efi(Utf8PathBuf::from(value))), + "uki" => efi_key = Some(EFIKey::Uki(Utf8PathBuf::from(value))), _ => { extra.insert(key.to_string(), value); } @@ -321,8 +362,8 @@ pub(crate) fn parse_bls_config(input: &str) -> Result { let version = version.ok_or_else(|| anyhow!("Missing 'version' value"))?; - let cfg_type = match (linux, efi) { - (None, Some(efi)) => BLSConfigType::EFI { efi }, + let cfg_type = match (linux, efi_key) { + (None, Some(key)) => BLSConfigType::EFI { key }, (Some(linux), None) => BLSConfigType::NonEFI { linux, @@ -331,9 +372,9 @@ pub(crate) fn parse_bls_config(input: &str) -> Result { }, // The spec makes no mention of whether both can be present or not - // Fow now, for us, we won't have both at the same time - (Some(_), Some(_)) => anyhow::bail!("'linux' and 'efi' values present"), - (None, None) => anyhow::bail!("Missing 'linux' or 'efi' value"), + // For now, for us, we won't have both at the same time + (Some(_), Some(_)) => anyhow::bail!("'linux' and 'efi'/'uki' values present"), + (None, None) => anyhow::bail!("Missing 'linux', 'efi', or 'uki' value"), }; Ok(BLSConfig { @@ -653,7 +694,9 @@ mod tests { let efi_path = Utf8PathBuf::from("bootc_composefs-abcd1234.efi"); let config = BLSConfig { - cfg_type: BLSConfigType::EFI { efi: efi_path }, + cfg_type: BLSConfigType::EFI { + key: EFIKey::Efi(efi_path), + }, version: "1".to_string(), ..Default::default() }; @@ -689,7 +732,9 @@ mod tests { let efi_path = Utf8PathBuf::from("/EFI/Linux/abcd1234.efi"); let config = BLSConfig { - cfg_type: BLSConfigType::EFI { efi: efi_path }, + cfg_type: BLSConfigType::EFI { + key: EFIKey::Efi(efi_path), + }, version: "1".to_string(), ..Default::default() }; @@ -706,7 +751,9 @@ mod tests { let efi_path = Utf8PathBuf::from("bootc_composefs-abcd1234"); let config = BLSConfig { - cfg_type: BLSConfigType::EFI { efi: efi_path }, + cfg_type: BLSConfigType::EFI { + key: EFIKey::Efi(efi_path), + }, version: "1".to_string(), ..Default::default() }; @@ -727,7 +774,9 @@ mod tests { let efi_path = Utf8PathBuf::from("/"); let config = BLSConfig { - cfg_type: BLSConfigType::EFI { efi: efi_path }, + cfg_type: BLSConfigType::EFI { + key: EFIKey::Efi(efi_path), + }, version: "1".to_string(), ..Default::default() }; @@ -763,7 +812,9 @@ mod tests { fn test_boot_artifact_name_efi_nested_path() -> Result<()> { let efi_path = Utf8PathBuf::from("/EFI/Linux/bootc/bootc_composefs-deadbeef01234567.efi"); let config = BLSConfig { - cfg_type: BLSConfigType::EFI { efi: efi_path }, + cfg_type: BLSConfigType::EFI { + key: EFIKey::Efi(efi_path), + }, version: "1".to_string(), ..Default::default() }; @@ -795,6 +846,7 @@ mod tests { #[test] fn test_boot_artifact_name_from_parsed_efi_config() -> Result<()> { let digest = "f7415d75017a12a387a39d2281e033a288fc15775108250ef70a01dcadb93346"; + // Test 'efi' key let input = format!( r#" title Fedora UKI @@ -807,6 +859,12 @@ mod tests { let config = parse_bls_config(&input)?; assert_eq!(config.boot_artifact_name()?, digest); assert_eq!(config.get_verity()?, digest); + + let uki_input = input.replace("efi", "uki"); + let config = parse_bls_config(&uki_input)?; + assert_eq!(config.boot_artifact_name()?, digest); + assert_eq!(config.get_verity()?, digest); + Ok(()) } @@ -826,4 +884,66 @@ mod tests { let result = config.boot_artifact_name(); assert!(result.is_err()); } + + #[test] + fn test_efi_key_variants_and_display() -> Result<()> { + use camino::Utf8PathBuf; + + // Test EFI variant displays "efi" + let efi_config = BLSConfig { + title: Some("Test EFI Entry".to_string()), + version: "1.0".to_string(), + cfg_type: BLSConfigType::EFI { + key: EFIKey::Efi(Utf8PathBuf::from("/EFI/Linux/test.efi")), + }, + sort_key: Some("test-key".to_string()), + ..Default::default() + }; + + let result = efi_config.to_string(); + assert!(result.contains("efi /EFI/Linux/test.efi")); + assert!(result.contains("title Test EFI Entry")); + assert!(result.contains("version 1.0")); + assert!(result.contains("sort-key test-key")); + assert!(!result.contains("uki")); + + // Test UKI variant displays "uki" + let uki_config = BLSConfig { + title: Some("Test UKI Entry".to_string()), + version: "1.0".to_string(), + cfg_type: BLSConfigType::EFI { + key: EFIKey::Uki(Utf8PathBuf::from("/EFI/Linux/test.efi")), + }, + sort_key: Some("test-key".to_string()), + ..Default::default() + }; + + let result = uki_config.to_string(); + assert!(result.contains("uki /EFI/Linux/test.efi")); + assert!(result.contains("title Test UKI Entry")); + assert!(result.contains("version 1.0")); + assert!(result.contains("sort-key test-key")); + // Don't check for absence of "efi" since we're looking for the key, not anywhere in the path + assert!(!result.starts_with("efi ") && !result.contains("\nefi ")); + + // Test that Non-EFI config is unaffected + let non_efi_config = BLSConfig { + version: "1.0".to_string(), + cfg_type: BLSConfigType::NonEFI { + linux: Utf8PathBuf::from("/boot/vmlinuz"), + initrd: vec![Utf8PathBuf::from("/boot/initrd.img")], + options: Some("root=UUID=abc123".into()), + }, + ..Default::default() + }; + + let result = non_efi_config.to_string(); + assert!(result.contains("linux /boot/vmlinuz")); + assert!(result.contains("initrd /boot/initrd.img")); + assert!(result.contains("options root=UUID=abc123")); + assert!(!result.contains("efi")); + assert!(!result.contains("uki")); + + Ok(()) + } } diff --git a/tmt/tests/booted/readonly/053-test-efi-uki-key.nu b/tmt/tests/booted/readonly/053-test-efi-uki-key.nu new file mode 100644 index 000000000..eb16ba2d5 --- /dev/null +++ b/tmt/tests/booted/readonly/053-test-efi-uki-key.nu @@ -0,0 +1,53 @@ +use std assert +use tap.nu + +tap begin "Test EFI/UKI key" + +let st = bootc status --json | from json + +if not (tap is_compoesfs) { + exit 0 +} + +let bootloader = $st.status.booted.composefs.bootloader + +# Only for UKIs +if ($st.status.booted.composefs.bootType | str downcase) != "uki" { + exit 0 +} + +let systemd_version = systemctl --version | lines | first | awk '{print $2}' | into int + +echo $"($systemd_version)" + +echo "BLS entry" +echo (cat /boot/loader/entries/*) + +# Both bootloaders support BLI so EFI should be mounted at /boot +let efi_line = ( + open /boot/loader/entries/* + | lines + | where $it =~ '^efi' +) + +let uki_line = ( + open /boot/loader/entries/* + | lines + | where $it =~ '^uki' +) + +if $bootloader == "grub-cc" { + assert equal ($efi_line | length) 1 + assert equal ($uki_line | length) 0 +} else { + + if $systemd_version >= 258 { + assert equal ($efi_line | length) 0 + assert equal ($uki_line | length) 1 + } else { + assert equal ($efi_line | length) 1 + assert equal ($uki_line | length) 0 + } +} + +tap ok