Skip to content

step1: Extend generate-update-metadata() to read from /usr #938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cgwalters merged 3 commits into from
Jul 25, 2025
Merged

step1: Extend generate-update-metadata() to read from /usr #938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
811c1f0
efi: extend `generate-update-metadata()` to read from `/usr`
HuijingHei Jul 16, 2025
1ec371a
efi: change timestamp to `now` as workaround of
HuijingHei Jul 25, 2025
3ac4491
ci: add fake `/usr/lib/efi` to test `generate-update-metadata()`
HuijingHei Jul 25, 2025
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
33 changes: 33 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,36 @@ jobs:
# Verify we injected static configs
jq -re '.["static-configs"].version' /boot/bootupd-state.json
[ $(sudo stat -c "%a" /boot/grub2/grub.cfg) == "600" ]

- name: bootupctl generate-update-metadata
run: |
set -xeuo pipefail
# This will be helpful when we extend /usr
sudo podman run --rm -it --privileged localhost/bootupd:latest \
Copy link
Member

@cgwalters cgwalters Jul 25, 2025

Choose a reason for hiding this comment

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

I've been trying harder to move load-bearing test logic out of GHA yaml and into something that's easier to run outside of GHA.

In this case this script could live in e.g. tests/move-content-to-usr.sh or something.

bash -c '
updates=/usr/lib/bootupd/updates
rm -fv ${updates}/{BIOS,EFI}.json
cp -r ${updates}/EFI /usr/lib/ostree-boot/efi
# prepare /usr/lib/efi/<grub2|shim>/<ver>
if [ ! -d "/usr/lib/efi" ]; then
arch="$(uname --machine)"
if [[ "${arch}" == "x86_64" ]]; then
surfix="x64"
else
# Assume aarch64 for now
surfix="aa64"
fi

grub_ver=$(rpm -qa grub2-efi-${surfix} --queryformat '%{VERSION}-%{RELEASE}')
mkdir -p /usr/lib/efi/grub2/${grub_ver}/EFI/centos
mv ${updates}/EFI/centos/grub${surfix}.efi /usr/lib/efi/grub2/${grub_ver}/EFI/centos/

shim_ver=$(rpm -qa shim-${surfix} --queryformat '%{VERSION}-%{RELEASE}')
mkdir -p /usr/lib/efi/shim/${shim_ver}/EFI/
mv ${updates}/EFI /usr/lib/efi/shim/${shim_ver}/
else
rm -rf ${updates}/EFI
fi
bootupctl backend generate-update-metadata -vvv
cat ${updates}/EFI.json | jq
'
136 changes: 111 additions & 25 deletions src/efi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ use std::process::Command;

use anyhow::{bail, Context, Result};
use bootc_internal_utils::CommandRunExt;
use camino::{Utf8Path, Utf8PathBuf};
use cap_std::fs::Dir;
use cap_std_ext::cap_std;
use chrono::prelude::*;
use fn_error_context::context;
use openat_ext::OpenatDirExt;
use os_release::OsRelease;
Expand All @@ -31,6 +33,9 @@ use crate::{component::*, packagesystem};
/// Well-known paths to the ESP that may have been mounted external to us.
pub(crate) const ESP_MOUNTS: &[&str] = &["boot/efi", "efi", "boot"];

/// New efi dir under usr/lib
const EFILIB: &str = "usr/lib/efi";

/// The binary to change EFI boot ordering
const EFIBOOTMGR: &str = "efibootmgr";
#[cfg(target_arch = "aarch64")]
Expand Down Expand Up @@ -427,38 +432,77 @@ impl Component for Efi {
})
}

fn generate_update_metadata(&self, sysroot_path: &str) -> Result<ContentMetadata> {
let ostreebootdir = Path::new(sysroot_path).join(ostreeutil::BOOT_PREFIX);
let dest_efidir = component_updatedir(sysroot_path, self);

if ostreebootdir.exists() {
let cruft = ["loader", "grub2"];
for p in cruft.iter() {
let p = ostreebootdir.join(p);
if p.exists() {
std::fs::remove_dir_all(&p)?;
fn generate_update_metadata(&self, sysroot: &str) -> Result<ContentMetadata> {
let sysroot_path = Utf8Path::new(sysroot);

// copy EFI files to updates dir from usr/lib/efi
let meta = if sysroot_path.join(EFILIB).exists() {
let mut packages = Vec::new();
for efi_dir in get_efi_path_from_usr(&sysroot_path)? {
let efi_dir = efi_dir.context("Invalid EFI directory path")?;

let sysroot_dir =
Dir::open_ambient_dir(sysroot_path, cap_std::ambient_authority())?;
Command::new("cp")
.args(["-rp", "--reflink=auto"])
.arg(&efi_dir)
.arg(crate::model::BOOTUPD_UPDATES_DIR)
.current_dir(format!("/proc/self/fd/{}", sysroot_dir.as_raw_fd()))
.run()?;

if let Ok(relative) = efi_dir.strip_prefix(EFILIB) {
let mut components = relative.components();

if let (Some(pkg), Some(ver)) = (components.next(), components.next()) {
packages.push(format!("{}-{}", pkg.as_str(), ver.as_str()));
}
}
}

let efisrc = ostreebootdir.join("efi/EFI");
if !efisrc.exists() {
bail!("Failed to find {:?}", &efisrc);
// change to now to workaround https://github.com/coreos/bootupd/issues/933
let timestamp = std::time::SystemTime::now();
ContentMetadata {
timestamp: chrono::DateTime::<Utc>::from(timestamp),
version: packages.join(","),
}
} else {
let ostreebootdir = sysroot_path.join(ostreeutil::BOOT_PREFIX);

// move EFI files to updates dir from /usr/lib/ostree-boot
if ostreebootdir.exists() {
let cruft = ["loader", "grub2"];
for p in cruft.iter() {
let p = ostreebootdir.join(p);
if p.exists() {
std::fs::remove_dir_all(&p)?;
}
}

// Fork off mv() because on overlayfs one can't rename() a lower level
// directory today, and this will handle the copy fallback.
Command::new("mv").args([&efisrc, &dest_efidir]).run()?;
}
let efisrc = ostreebootdir.join("efi/EFI");
if !efisrc.exists() {
bail!("Failed to find {:?}", &efisrc);
}

let efidir = openat::Dir::open(&dest_efidir)
.with_context(|| format!("Opening {}", dest_efidir.display()))?;
let files = crate::util::filenames(&efidir)?.into_iter().map(|mut f| {
f.insert_str(0, "/boot/efi/EFI/");
f
});
let dest_efidir = component_updatedir(sysroot, self);
let dest_efidir =
Utf8PathBuf::from_path_buf(dest_efidir).expect("Path is invalid UTF-8");
// Fork off mv() because on overlayfs one can't rename() a lower level
// directory today, and this will handle the copy fallback.
Command::new("mv").args([&efisrc, &dest_efidir]).run()?;

let efidir = openat::Dir::open(dest_efidir.as_std_path())
.with_context(|| format!("Opening {}", dest_efidir))?;
let files = crate::util::filenames(&efidir)?.into_iter().map(|mut f| {
f.insert_str(0, "/boot/efi/EFI/");
f
});
packagesystem::query_files(sysroot, files)?
} else {
anyhow::bail!("Failed to find {ostreebootdir}");
}
};

let meta = packagesystem::query_files(sysroot_path, files)?;
write_update_metadata(sysroot_path, self, &meta)?;
write_update_metadata(sysroot, self, &meta)?;
Ok(meta)
}

Expand Down Expand Up @@ -654,6 +698,29 @@ fn find_file_recursive<P: AsRef<Path>>(dir: P, target_file: &str) -> Result<Vec<
Ok(result)
}

/// Get EFI path under usr/lib/efi, eg. usr/lib/efi/<package>/<version>/EFI
fn get_efi_path_from_usr(
sysroot: &Utf8Path,
) -> Result<impl Iterator<Item = Result<Utf8PathBuf>> + use<'_>> {
Copy link
Member

@cgwalters cgwalters Jul 25, 2025

Choose a reason for hiding this comment

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

OK as is but I think this might be cleaner if we returned something like a

struct EFIComponent {
  name: String,
  version: String,
  path: Utf8PathBuf
}

or so

Copy link
Member Author

@HuijingHei HuijingHei Jul 30, 2025

Choose a reason for hiding this comment

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

Nice suggestion, will update in later PR.

let efilib_path = sysroot.join(EFILIB);

Ok(WalkDir::new(efilib_path)
.min_depth(3)
.max_depth(3)
.into_iter()
.filter_entry(|e| e.file_type().is_dir() && e.file_name() == "EFI")
.map(move |entry| {
let abs_path = entry.context("Failed to read directory entry")?.into_path();

let rel_path = abs_path
.strip_prefix(sysroot)
.context("Failed to strip sysroot prefix")?;

Utf8PathBuf::from_path_buf(rel_path.to_path_buf())
.map_err(|e| anyhow::anyhow!("Invalid UTF-8 path : {}", e.display()))
}))
}

#[cfg(test)]
mod tests {
use cap_std_ext::dirext::CapStdExtDirExt;
Expand Down Expand Up @@ -774,4 +841,23 @@ Boot0003* test";
}
Ok(())
}

#[test]
fn test_get_efi_path_from_usr() -> Result<()> {
let tmpdir: &tempfile::TempDir = &tempfile::tempdir()?;
let tpath = tmpdir.path();
let efi_path = tpath.join("usr/lib/efi");
std::fs::create_dir_all(efi_path.join("FOO/1.1/EFI"))?;
std::fs::create_dir_all(efi_path.join("BAR/1.1/EFI"))?;
std::fs::create_dir_all(efi_path.join("FOOBAR/1.1/test"))?;
let utf8_tpath =
Utf8Path::from_path(tpath).ok_or_else(|| anyhow::anyhow!("Path is not valid UTF-8"))?;
let efi_path_iter = get_efi_path_from_usr(utf8_tpath)?;
let paths: Vec<Utf8PathBuf> = efi_path_iter.filter_map(Result::ok).collect();
assert_eq!(
paths,
["usr/lib/efi/FOO/1.1/EFI", "usr/lib/efi/BAR/1.1/EFI"]
);
Ok(())
}
}
Loading