Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/bootupd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,23 @@ pub(crate) fn client_run_migrate_static_grub_config() -> Result<()> {
Ok(())
}

/// Copy bootloader files from /usr/lib/efi to boot/ESP for package mode installations.
pub(crate) fn copy_to_boot() -> Result<()> {
let all_components = get_components_impl(false);
if all_components.is_empty() {
println!("No components available for this platform.");
return Ok(());
}

for component in all_components.values() {
component
.package_mode_copy_to_boot()
.with_context(|| format!("Failed to copy component {} to boot", component.name()))?;
}

Ok(())
}

/// Writes a stripped GRUB config to `stripped_config_name`, removing lines between
/// `### BEGIN /etc/grub.d/15_ostree ###` and `### END /etc/grub.d/15_ostree ###`.
fn strip_grub_config_file(
Expand Down
11 changes: 11 additions & 0 deletions src/cli/bootupd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ pub enum DVerb {
GenerateUpdateMetadata(GenerateOpts),
#[clap(name = "install", about = "Install components")]
Install(InstallOpts),
#[clap(
name = "copy-to-boot",
about = "Copy bootloader files from /usr/lib/efi to boot/ESP (package mode)"
)]
CopyToBoot,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look at more about this, this means to use like bootupctl backend copy-to-boot from root /, is this right? How about if we want to use like copying from specified directory, so I think maybe it would better to be under install, but need to confirm with @travier , WDYT?

}

#[derive(Debug, Parser)]
Expand Down Expand Up @@ -88,6 +93,7 @@ impl DCommand {
match self.cmd {
DVerb::Install(opts) => Self::run_install(opts),
DVerb::GenerateUpdateMetadata(opts) => Self::run_generate_meta(opts),
DVerb::CopyToBoot => Self::run_copy_to_boot(),
}
}

Expand Down Expand Up @@ -122,4 +128,9 @@ impl DCommand {
.context("boot data installation failed")?;
Ok(())
}

pub(crate) fn run_copy_to_boot() -> Result<()> {
bootupd::copy_to_boot().context("copying to boot failed")?;
Ok(())
}
}
5 changes: 5 additions & 0 deletions src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ pub(crate) trait Component {

/// Locating efi vendor dir
fn get_efi_vendor(&self, sysroot: &Path) -> Result<Option<String>>;

/// Copy from /usr/lib/efi to boot/ESP.
fn package_mode_copy_to_boot(&self) -> Result<()> {
Ok(())
}
}

/// Given a component name, create an implementation.
Expand Down
252 changes: 241 additions & 11 deletions src/efi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,89 @@ impl Efi {
clear_efi_target(&product_name)?;
create_efi_boot_entry(device, esp_part_num.trim(), &loader, &product_name)
}

/// Shared helper to copy EFI components to a single ESP
fn copy_efi_components_to_esp(
&self,
sysroot_dir: &openat::Dir,
esp_path: &Path,
efi_components: &[EFIComponent],
) -> Result<()> {
let dest_str = esp_path
.to_str()
.context("ESP path contains invalid UTF-8")?;

// Copy each component
for efi_comp in efi_components {
log::info!(
"Copying EFI component {} version {} to ESP at {}",
efi_comp.name,
efi_comp.version,
esp_path.display()
);

filetree::copy_dir_with_args(sysroot_dir, efi_comp.path.as_str(), dest_str, OPTIONS)
.with_context(|| {
format!(
"Failed to copy {} from {} to {}",
efi_comp.name, efi_comp.path, dest_str
)
})?;
}

// Sync filesystem
let efidir =
openat::Dir::open(&esp_path.join("EFI")).context("Opening EFI directory for sync")?;
fsfreeze_thaw_cycle(efidir.open_file(".")?)?;

Ok(())
}

/// Copy from /usr/lib/efi to boot/ESP.
fn package_mode_copy_to_boot_impl(&self) -> Result<()> {
let sysroot = Path::new("/");
let sysroot_path =
Utf8Path::from_path(sysroot).context("Sysroot path is not valid UTF-8")?;

let efi_comps = match get_efi_component_from_usr(sysroot_path, EFILIB)? {
Some(comps) if !comps.is_empty() => comps,
_ => {
log::debug!("No EFI components found in /usr/lib/efi");
return Ok(());
}
};

let sysroot_dir = openat::Dir::open(sysroot).context("Opening sysroot for reading")?;

// First try to use an already mounted ESP
let esp_path = if let Some(mounted_esp) = self.get_mounted_esp(sysroot)? {
mounted_esp
} else {
// If not mounted, find ESP from devices
let devices = blockdev::get_devices(sysroot)?;
let Some(esp_devices) = blockdev::find_colocated_esps(&devices)? else {
anyhow::bail!("No ESP found");
};

let esp_device = esp_devices
.first()
.ok_or_else(|| anyhow::anyhow!("No ESP device found"))?;
Comment on lines +250 to +252

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The .ok_or_else() call here is on a value that is guaranteed to be Some. The find_colocated_esps function returns None if no ESPs are found, which is handled by the let-else statement on line 246. If it returns Some(vec), the vector is guaranteed to be non-empty. Therefore, esp_devices.first() will never be None, and this error-handling code is unreachable.

You can simplify this by using .unwrap(), which is safe in this context. This also removes a potentially confusing error message.

Suggested change
let esp_device = esp_devices
.first()
.ok_or_else(|| anyhow::anyhow!("No ESP device found"))?;
let esp_device = esp_devices.first().unwrap();

self.ensure_mounted_esp(sysroot, Path::new(esp_device))?
};

let esp_dir = openat::Dir::open(&esp_path)
.with_context(|| format!("Opening ESP at {}", esp_path.display()))?;
validate_esp_fstype(&esp_dir)?;

self.copy_efi_components_to_esp(&sysroot_dir, &esp_path, &efi_comps)?;

log::info!(
"Successfully copied {} EFI component(s) to ESP at {}",
efi_comps.len(),
esp_path.display()
);
Ok(())
}
}

#[context("Get product name")]
Expand Down Expand Up @@ -414,23 +497,22 @@ impl Component for Efi {
} else {
None
};
let dest = destpath.to_str().with_context(|| {
format!(
"Include invalid UTF-8 characters in dest {}",
destpath.display()
)
})?;

let efi_path = if let Some(efi_components) = efi_comps {
for efi in efi_components {
filetree::copy_dir_with_args(&src_dir, efi.path.as_str(), dest, OPTIONS)?;
}
// Use shared helper to copy components from /usr/lib/efi
self.copy_efi_components_to_esp(&src_dir, &destpath, &efi_components)?;
EFILIB
} else {
let updates = component_updatedirname(self);
let src = updates
.to_str()
.context("Include invalid UTF-8 characters in path")?;
let dest = destpath.to_str().with_context(|| {
format!(
"Include invalid UTF-8 characters in dest {}",
destpath.display()
)
})?;
filetree::copy_dir_with_args(&src_dir, src, dest, OPTIONS)?;
&src.to_owned()
};
Expand Down Expand Up @@ -622,6 +704,11 @@ impl Component for Efi {
anyhow::bail!("Failed to find {SHIM} in the image")
}
}

/// Package mode copy: Simple copy from /usr/lib/efi to boot/ESP.
fn package_mode_copy_to_boot(&self) -> Result<()> {
self.package_mode_copy_to_boot_impl()
}
}

impl Drop for Efi {
Expand Down Expand Up @@ -917,7 +1004,6 @@ Boot0003* test";
);
Ok(())
}
#[cfg(test)]
fn fixture() -> Result<cap_std_ext::cap_tempfile::TempDir> {
let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?;
tempdir.create_dir("etc")?;
Expand Down Expand Up @@ -947,7 +1033,7 @@ Boot0003* test";
{
tmpd.atomic_write(
"etc/system-release",
"Red Hat Enterprise Linux CoreOS release 4
r"Red Hat Enterprise Linux CoreOS release 4
",
)?;
let name = get_product_name(&tmpd)?;
Expand Down Expand Up @@ -993,4 +1079,148 @@ Boot0003* test";
assert_eq!(efi_comps, None);
Ok(())
}

#[test]
fn test_package_mode_copy_to_boot_discovery() -> Result<()> {
// Test that we can discover components from /usr/lib/efi
let tmpdir: &tempfile::TempDir = &tempfile::tempdir()?;
let tpath = tmpdir.path();
let efi_path = tpath.join("usr/lib/efi");

// Create mock EFI components
std::fs::create_dir_all(efi_path.join("shim/15.8-3/EFI/fedora"))?;
std::fs::create_dir_all(efi_path.join("grub2/2.12-28/EFI/fedora"))?;

// Write some test files
std::fs::write(
efi_path.join("shim/15.8-3/EFI/fedora/shimx64.efi"),
b"shim content",
)?;
std::fs::write(
efi_path.join("grub2/2.12-28/EFI/fedora/grubx64.efi"),
b"grub content",
)?;

let utf8_tpath =
Utf8Path::from_path(tpath).ok_or_else(|| anyhow::anyhow!("Path is not valid UTF-8"))?;

// Test component discovery
let efi_comps = match get_efi_component_from_usr(utf8_tpath, EFILIB)? {
Some(comps) if !comps.is_empty() => comps,
_ => {
anyhow::bail!("Should have found components");
}
};

// Verify we found the expected components
assert_eq!(efi_comps.len(), 2);
let names: Vec<_> = efi_comps.iter().map(|c| c.name.as_str()).collect();
assert!(names.contains(&"shim"));
assert!(names.contains(&"grub2"));

// Verify paths are correct
for comp in &efi_comps {
assert!(comp.path.starts_with("usr/lib/efi"));
assert!(comp.path.ends_with("EFI"));
}

Ok(())
}

#[test]
fn test_package_mode_shim_installation() -> Result<()> {
// Test that shim can be installed from /usr/lib/efi to ESP
let tmpdir: &tempfile::TempDir = &tempfile::tempdir()?;
let tpath = tmpdir.path();

// Create mock /usr/lib/efi structure with shim
let efi_path = tpath.join("usr/lib/efi");
let shim_path = efi_path.join("shim/15.8-3/EFI/fedora");
std::fs::create_dir_all(&shim_path)?;

// Write shim binary
let shim_content = b"mock shim binary content";
std::fs::write(shim_path.join(SHIM), shim_content)?;

// Create additional shim files that might be present
std::fs::write(shim_path.join("MokManager.efi"), b"mok manager content")?;
std::fs::write(shim_path.join("fbx64.efi"), b"fallback content")?;

// Create mock ESP directory structure (simulating /boot/efi in container)
let esp_path = tpath.join("boot/efi");
std::fs::create_dir_all(&esp_path)?;

// Create EFI directory in ESP
let esp_efi_path = esp_path.join("EFI");
std::fs::create_dir_all(&esp_efi_path)?;

// Set up sysroot directory
let sysroot_dir = openat::Dir::open(tpath)?;

// Get EFI components from usr/lib/efi
let utf8_tpath =
Utf8Path::from_path(tpath).ok_or_else(|| anyhow::anyhow!("Path is not valid UTF-8"))?;
let efi_comps = get_efi_component_from_usr(utf8_tpath, EFILIB)?;
assert!(efi_comps.is_some(), "Should find shim component");
let efi_comps = efi_comps.unwrap();
assert_eq!(efi_comps.len(), 1, "Should find exactly one component");
assert_eq!(efi_comps[0].name, "shim");
assert_eq!(efi_comps[0].version, "15.8-3");

// Create Efi instance and copy components to ESP
let efi = Efi::default();
efi.copy_efi_components_to_esp(&sysroot_dir, &esp_path, &efi_comps)?;

// Expected path: /boot/efi/EFI/fedora/shimx64.efi (or shimaa64.efi, etc.)
let copied_shim_path = esp_path.join("EFI/fedora").join(SHIM);
assert!(
copied_shim_path.exists(),
"Shim should be copied to ESP at {}",
copied_shim_path.display()
);

// Verify the shim file is actually a file, not a directory
assert!(
copied_shim_path.is_file(),
"Shim should be a file at {}",
copied_shim_path.display()
);

// Verify the content matches exactly
let copied_content = std::fs::read(&copied_shim_path)?;
assert_eq!(
copied_content, shim_content,
"Shim content should match exactly"
);

// Verify the directory structure is correct
assert!(
esp_path.join("EFI").exists(),
"EFI directory should exist in ESP at {}",
esp_path.join("EFI").display()
);
assert!(esp_path.join("EFI").is_dir(), "EFI should be a directory");

assert!(
esp_path.join("EFI/fedora").exists(),
"Vendor directory (fedora) should exist in ESP at {}",
esp_path.join("EFI/fedora").display()
);
assert!(
esp_path.join("EFI/fedora").is_dir(),
"EFI/fedora should be a directory"
);

// Verify the path structure matches expected package mode layout
// Source: /usr/lib/efi/shim/15.8-3/EFI/fedora/shimx64.efi
// Dest: /boot/efi/EFI/fedora/shimx64.efi
let expected_base = esp_path.join("EFI/fedora");
assert_eq!(
copied_shim_path.parent(),
Some(expected_base.as_path()),
"Shim should be directly under EFI/fedora/, not in a subdirectory"
);

Ok(())
}
}
Loading