install: Detect bootloader from target image instead of host

crates/lib/src/bootc_composefs/boot.rs

@@ -33,7 +33,6 @@ use rustix::{mount::MountFlags, path::Arg};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
-use crate::bootc_composefs::status::get_sorted_grub_uki_boot_entries;
 use crate::composefs_consts::{TYPE1_ENT_PATH, TYPE1_ENT_PATH_STAGED};
 use crate::parsers::bls_config::{BLSConfig, BLSConfigType};
 use crate::parsers::grub_menuconfig::MenuEntry;
@@ -46,6 +45,7 @@ use crate::{
     bootc_composefs::state::{get_booted_bls, write_composefs_state},
     bootloader::esp_in,
 };
+use crate::{bootc_composefs::status::get_sorted_grub_uki_boot_entries, install::PostFetchState};
 use crate::{
     composefs_consts::{
         BOOT_LOADER_ENTRIES, COMPOSEFS_CMDLINE, ORIGIN_KEY_BOOT, ORIGIN_KEY_BOOT_DIGEST,
@@ -77,7 +77,14 @@ pub(crate) const SYSTEMD_UKI_DIR: &str = "EFI/Linux/bootc";
 
 pub(crate) enum BootSetupType<'a> {
     /// For initial setup, i.e. install to-disk
-    Setup((&'a RootSetup, &'a State, &'a ComposefsFilesystem)),
+    Setup(
+        (
+            &'a RootSetup,
+            &'a State,
+            &'a PostFetchState,
+            &'a ComposefsFilesystem,
+        ),
+    ),
     /// For `bootc upgrade`
     Upgrade((&'a Storage, &'a ComposefsFilesystem, &'a Host)),
 }
@@ -378,7 +385,7 @@ pub(crate) fn setup_composefs_bls_boot(
     let id_hex = id.to_hex();
 
     let (root_path, esp_device, cmdline_refs, fs, bootloader) = match setup_type {
-        BootSetupType::Setup((root_setup, state, fs)) => {
+        BootSetupType::Setup((root_setup, state, postfetch, fs)) => {
             // root_setup.kargs has [root=UUID=<UUID>, "rw"]
             let mut cmdline_options = Cmdline::new();
 
@@ -400,7 +407,7 @@ pub(crate) fn setup_composefs_bls_boot(
                 esp_part.node.clone(),
                 cmdline_options,
                 fs,
-                state.detected_bootloader.clone(),
+                postfetch.detected_bootloader.clone(),
             )
         }
 
@@ -854,15 +861,15 @@ pub(crate) fn setup_composefs_uki_boot(
     entries: Vec<ComposefsBootEntry<Sha512HashValue>>,
 ) -> Result<()> {
     let (root_path, esp_device, bootloader, is_insecure_from_opts, uki_addons) = match setup_type {
-        BootSetupType::Setup((root_setup, state, ..)) => {
+        BootSetupType::Setup((root_setup, state, postfetch, ..)) => {
             state.require_no_kargs_for_uki()?;
 
             let esp_part = esp_in(&root_setup.device_info)?;
 
             (
                 root_setup.physical_root_path.clone(),
                 esp_part.node.clone(),
-                state.detected_bootloader.clone(),
+                postfetch.detected_bootloader.clone(),
                 state.composefs_options.insecure,
                 state.composefs_options.uki_addon.as_ref(),
             )
@@ -964,6 +971,18 @@ pub(crate) fn setup_composefs_boot(
     state: &State,
     image_id: &str,
 ) -> Result<()> {
+    let repo = open_composefs_repo(&root_setup.physical_root)?;
+    let mut fs = create_composefs_filesystem(&repo, image_id, None)?;
+    let entries = fs.transform_for_boot(&repo)?;
+    let id = fs.commit_image(&repo, None)?;
+    let mounted_fs = Dir::reopen_dir(
+        &repo
+            .mount(&id.to_hex())
+            .context("Failed to mount composefs image")?,
+    )?;
+
+    let postfetch = PostFetchState::new(state, &mounted_fs)?;
+
     let boot_uuid = root_setup
         .get_boot_uuid()?
         .or(root_setup.rootfs_uuid.as_deref())
@@ -972,7 +991,7 @@ pub(crate) fn setup_composefs_boot(
     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 state.detected_bootloader == Bootloader::Grub {
+    } else if postfetch.detected_bootloader == Bootloader::Grub {
         crate::bootloader::install_via_bootupd(
             &root_setup.device_info,
             &root_setup.physical_root_path,
@@ -988,13 +1007,6 @@ pub(crate) fn setup_composefs_boot(
         )?;
     }
 
-    let repo = open_composefs_repo(&root_setup.physical_root)?;
-
-    let mut fs = create_composefs_filesystem(&repo, image_id, None)?;
-
-    let entries = fs.transform_for_boot(&repo)?;
-    let id = fs.commit_image(&repo, None)?;
-
     let Some(entry) = entries.iter().next() else {
         anyhow::bail!("No boot entries!");
     };
@@ -1005,7 +1017,7 @@ pub(crate) fn setup_composefs_boot(
     match boot_type {
         BootType::Bls => {
             let digest = setup_composefs_bls_boot(
-                BootSetupType::Setup((&root_setup, &state, &fs)),
+                BootSetupType::Setup((&root_setup, &state, &postfetch, &fs)),
                 repo,
                 &id,
                 entry,
@@ -1014,7 +1026,7 @@ pub(crate) fn setup_composefs_boot(
             boot_digest = Some(digest);
         }
         BootType::Uki => setup_composefs_uki_boot(
-            BootSetupType::Setup((&root_setup, &state, &fs)),
+            BootSetupType::Setup((&root_setup, &state, &postfetch, &fs)),
             repo,
             &id,
             entries,

crates/lib/src/bootloader.rs

@@ -3,6 +3,7 @@ use std::process::Command;
 use anyhow::{anyhow, bail, Context, Result};
 use bootc_utils::CommandRunExt;
 use camino::Utf8Path;
+use cap_std_ext::cap_std::fs::Dir;
 use fn_error_context::context;
 
 use bootc_blockdev::{Partition, PartitionTable};
@@ -28,15 +29,12 @@ pub(crate) fn esp_in(device: &PartitionTable) -> Result<&Partition> {
 /// Determine if the invoking environment contains bootupd, and if there are bootupd-based
 /// updates in the target root.
 #[context("Querying for bootupd")]
-#[allow(dead_code)]
-pub(crate) fn supports_bootupd(deployment_path: Option<&str>) -> Result<bool> {
+pub(crate) fn supports_bootupd(root: &Dir) -> Result<bool> {
     if !utils::have_executable("bootupctl")? {
         tracing::trace!("No bootupctl binary found");
         return Ok(false);
     };
-    let deployment_path = Utf8Path::new(deployment_path.unwrap_or("/"));
-    let updates = deployment_path.join(BOOTUPD_UPDATES);
-    let r = updates.try_exists()?;
+    let r = root.try_exists(BOOTUPD_UPDATES)?;
     tracing::trace!("bootupd updates: {r}");
     Ok(r)
 }

crates/lib/src/install.rs

@@ -476,7 +476,11 @@ pub(crate) struct State {
 
     // If Some, then --composefs_native is passed
     pub(crate) composefs_options: InstallComposefsOpts,
+}
 
+// Shared read-only global state
+#[derive(Debug)]
+pub(crate) struct PostFetchState {
     /// Detected bootloader type for the target system
     pub(crate) detected_bootloader: crate::spec::Bootloader,
 }
@@ -1453,21 +1457,6 @@ async fn prepare_install(
         .map(|p| std::fs::read_to_string(p).with_context(|| format!("Reading {p}")))
         .transpose()?;
 
-    // Determine bootloader type for the target system
-    // Priority: user-specified > bootupd availability > systemd-boot fallback
-    let detected_bootloader = {
-        if let Some(bootloader) = composefs_options.bootloader.clone() {
-            bootloader
-        } else {
-            if crate::bootloader::supports_bootupd(None)? {
-                crate::spec::Bootloader::Grub
-            } else {
-                crate::spec::Bootloader::Systemd
-            }
-        }
-    };
-    println!("Bootloader: {detected_bootloader}");
-
     // Create our global (read-only) state which gets wrapped in an Arc
     // so we can pass it to worker threads too. Right now this just
     // combines our command line options along with some bind mounts from the host.
@@ -1483,13 +1472,35 @@ async fn prepare_install(
         tempdir,
         host_is_container,
         composefs_required,
-        detected_bootloader,
         composefs_options,
     });
 
     Ok(state)
 }
 
+impl PostFetchState {
+    pub(crate) fn new(state: &State, d: &Dir) -> Result<Self> {
+        // Determine bootloader type for the target system
+        // Priority: user-specified > bootupd availability > systemd-boot fallback
+        let detected_bootloader = {
+            if let Some(bootloader) = state.composefs_options.bootloader.clone() {
+                bootloader
+            } else {
+                if crate::bootloader::supports_bootupd(d)? {
+                    crate::spec::Bootloader::Grub
+                } else {
+                    crate::spec::Bootloader::Systemd
+                }
+            }
+        };
+        println!("Bootloader: {detected_bootloader}");
+        let r = Self {
+            detected_bootloader,
+        };
+        Ok(r)
+    }
+}
+
 /// Given a baseline root filesystem with an ostree sysroot initialized:
 /// - install the container to that root
 /// - install the bootloader
@@ -1513,11 +1524,17 @@ async fn install_with_sysroot(
 
     let deployment_path = ostree.deployment_dirpath(&deployment);
 
+    let deployment_dir = rootfs
+        .physical_root
+        .open_dir(&deployment_path)
+        .context("Opening deployment dir")?;
+    let postfetch = PostFetchState::new(state, &deployment_dir)?;
+
     if cfg!(target_arch = "s390x") {
         // TODO: Integrate s390x support into install_via_bootupd
         crate::bootloader::install_via_zipl(&rootfs.device_info, boot_uuid)?;
     } else {
-        match state.detected_bootloader {
+        match postfetch.detected_bootloader {
             Bootloader::Grub => {
                 crate::bootloader::install_via_bootupd(
                     &rootfs.device_info,
@@ -1660,6 +1677,7 @@ async fn install_to_filesystem_impl(
 
         let (id, verity) = initialize_composefs_repository(state, rootfs).await?;
         tracing::info!("id: {}, verity: {}", hex::encode(id), verity.to_hex());
+
         setup_composefs_boot(rootfs, state, &hex::encode(id))?;
     } else {
         ostree_install(state, rootfs, cleanup).await?;

tmt/tests/booted/test-install-outside-container.nu

@@ -1,14 +1,38 @@
 use std assert
 use tap.nu
 
+# In this test we install a generic image mainly because it keeps
+# this test in theory independent of starting from a bootc host,
+# but also because it's useful to test "skew" between the bootc binary
+# doing the install and the target image.
+let target_image = "docker://quay.io/centos-bootc/centos-bootc:stream10"
+
 # setup filesystem
 mkdir /var/mnt
-truncate -s 100M disk.img
+truncate -s 10G disk.img
 mkfs.ext4 disk.img
 mount -o loop disk.img /var/mnt
 
 # attempt to install to filesystem without specifying a source-imgref
 let result = bootc install to-filesystem /var/mnt e>| find "--source-imgref must be defined"
 assert not equal $result null
+umount /var/mnt
+
+# Mask off the bootupd state to reproduce https://github.com/bootc-dev/bootc/issues/1778
+# Also it turns out that installation outside of containers dies due to `error: Multiple commit objects found`
+# so we mask off /sysroot/ostree
+# And using systemd-run here breaks our install_t so we disable SELinux enforcement
+setenforce 0
+systemd-run -p MountFlags=slave -qdPG -- /bin/sh -c $"
+set -xeuo pipefail
+if test -d /sysroot/ostree; then mount --bind /usr/share/empty /sysroot/ostree; fi
+mkdir -p /tmp/ovl/{upper,work}
+mount -t overlay -olowerdir=/usr,workdir=/tmp/ovl/work,upperdir=/tmp/ovl/upper overlay /usr
+# Note we do keep the other bootupd state
+rm -vrf /usr/lib/bootupd/updates
+# Another bootc install bug, we should not look at this in outside-of-container flows
+rm -vrf /usr/lib/bootc/bound-images.d
+bootc install to-disk --disable-selinux --via-loopback --filesystem xfs --source-imgref ($target_image) ./disk.img
+"
 
 tap ok