Skip to content

Conversation

@mvo5
Copy link
Contributor

@mvo5 mvo5 commented Dec 3, 2025

[draft as this is potentially controversial, also build on top f #1820 mostly to avoid conflicts]

We would like to only inspect a container image with
{bootc,}image-builder and not actually run the container.

One reason is (too much?) paranoia, i.e. we want to eventually
support bootc containers coming from the users that get passed
into the service and not having to run anything on the container
initially to generate the osbuild manifest minimizes the risk.

So to still get the require parameters like preferred rootfs
or kargs we would like to run our own bootc and then pass

bootc install print-configuration --root-dir /path/to/mounted/cnt

to generate the manifest. The actual build will still run
the boots install to-filesystem from the container. But that
step happens in an isolated instance that is destroyed after
each build (we already do this for package based image builds
as users can also inject custom content/rpms here).

This PR implements this new "--root-dir" option.

It also tweaks print_configuration to make it easier to
test. With that we could drop parts of the tests for
PR#1820 from the container.rs and move them in here.

(c.f. osbuild/images#1988)

@github-actions github-actions bot added the area/install Issues related to `bootc install` label Dec 3, 2025
@bootc-bot bootc-bot bot requested a review from jmarrero December 3, 2025 12:16
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Sure, seems fine to me. That said:

We would like to only inspect a container image with
{bootc,}image-builder and not actually run the container. One reason is (too much?) paranoia, i.e. we want to eventually
support bootc containers coming from the users that get passed
into the service and not having to run anything on the container
initially to generate the osbuild manifest minimizes the risk.

And in this model the bootc binary being invoked would come from the bootc-image-builder container, and not the users' container so would be theoretically more predictable? I guess but...I think doing this via the equivalent of RUN --network=none bootc install print-configuration would constrain unpredictability enough too.

@mvo5
Copy link
Contributor Author

mvo5 commented Dec 4, 2025

[..]

And in this model the bootc binary being invoked would come from the bootc-image-builder container, and not the users' container so would be theoretically more predictable? I guess but...I think doing this via the equivalent of RUN --network=none bootc install print-configuration would constrain unpredictability enough too.

I will talk to the rest of the team about this, honestly I'm not sure myself, I would like to minimize the attack surface in the service as much as possible and and ideally run no-user provided code at the manifest generation time (where we are much less isolated than at build time). But maybe I'm just overly paranoid, this is run inside containers and as you pointed out, there are ways like "network=none" to heavily restrict what the container can do. Therefore I leave this as "draft" until we discussed this internally again. But thanks a lot for your excellent review, even though its a tiny amount of code I enjoyed the codebase (and rust).

@mvo5 mvo5 force-pushed the print-config-rootdir branch 2 times, most recently from bf04398 to c78fa5c Compare December 4, 2025 10:05
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

LGTM but needs a rebase and DCO


/// Set an alternative rootdir
#[clap(long, default_value = "/")]
pub(crate) root_dir: Option<String>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

To have default_value work just drop the Option

We would like to only inspect a container image with
{bootc,}image-builder and not actually run the container.

One reason is (too much?) paranoia, i.e. we want to eventually
support bootc containers coming from the users that get passed
into the service and not having to run anything on the container
initially to generate the osbuild manifest minimizes the risk.

So to still get the require parameters like preferred rootfs
or kargs we would like to run our own bootc and then pass
```
bootc install print-configuration --sysroot /path/to/mounted/cnt
```
to generate the manifest. The actual build will still run
the `boots install to-filesystem` from the container. But that
step happens in an isolated instance that is destroyed after
each build (we already do this for package based image builds
as users can also inject custom content/rpms here).

It also tweaks print_configuration to make it easier to
test. With that we could drop parts of the tests for
PR#1820 from the container.rs and move them in here.

Signed-off-by: Michael Vogt <michael.vogt@gmail.com>
Co-authored-by: Colin Walters <walters@verbum.org>
@cgwalters cgwalters force-pushed the print-config-rootdir branch from c78fa5c to d1e50d9 Compare December 15, 2025 23:00
@cgwalters cgwalters marked this pull request as ready for review December 15, 2025 23:00
@github-actions github-actions bot added the area/documentation Updates to the documentation label Dec 15, 2025
@bootc-bot bootc-bot bot requested a review from ckyrouac December 15, 2025 23:01
@cgwalters
Copy link
Collaborator

I tweaked this

  • Rebased on git main 🏄
  • dropped the Option
  • Use PathBuf instead of String
  • Name the option --sysroot (there's not a real standard here, but ostree uses that for example. systemd uses `--root)
  • Run cargo xtask update-generated

@mvo5
Copy link
Contributor Author

mvo5 commented Dec 17, 2025

I tweaked this

* Rebased on git main 🏄

* dropped the Option

* Use PathBuf instead of String

* Name the option `--sysroot` (there's not a real standard here, but ostree uses that for example. systemd uses `--root)

* Run `cargo xtask update-generated`

Thanks a lot for these tweaks/fixes! When I talked in my team there was a bit of uncertainty if/when we need this so I am hesitant to push for it. I would feel bad to push for this and then its not being used in the "images" library :/ So I'm inclined to close it (for now) but I have the suspicion that this will come back (but its up to the image-builder team to decide that).

@cgwalters
Copy link
Collaborator

OK sure. For posterity here's the current diff so it survives even if you delete your fork.

diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs
index 1e948ac9e..7a08598ad 100644
--- a/crates/lib/src/install.rs
+++ b/crates/lib/src/install.rs
@@ -18,7 +18,7 @@ use std::collections::HashMap;
 use std::io::Write;
 use std::os::fd::{AsFd, AsRawFd};
 use std::os::unix::process::CommandExt;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::process;
 use std::process::Command;
 use std::str::FromStr;
@@ -453,6 +453,13 @@ pub(crate) struct InstallPrintConfigurationOpts {
     /// Print configuration that is usually handled internally, like kargs.
     #[clap(long)]
     pub(crate) all: bool,
+
+    /// Set an alternative root filesystem.
+    ///
+    /// This can be used as part of a multi-stage build operation to detect
+    /// the desired configuration before executing code from a target container image.
+    #[clap(long, default_value = "/")]
+    pub(crate) sysroot: PathBuf,
 }
 
 /// Global state captured from the container.
@@ -742,12 +749,19 @@ impl SourceInfo {
 }
 
 pub(crate) fn print_configuration(opts: InstallPrintConfigurationOpts) -> Result<()> {
-    let mut install_config = config::load_config()?.unwrap_or_default();
+    let stdout = std::io::stdout().lock();
+    print_configuration_to_writer(opts, stdout)
+}
+
+fn print_configuration_to_writer<W: Write>(
+    opts: InstallPrintConfigurationOpts,
+    writer: W,
+) -> Result<()> {
+    let mut install_config = config::load_config_at(&opts.sysroot)?.unwrap_or_default();
     if !opts.all {
         install_config.filter_to_external();
     }
-    let stdout = std::io::stdout().lock();
-    anyhow::Ok(install_config.to_canon_json_writer(stdout)?)
+    anyhow::Ok(install_config.to_canon_json_writer(writer)?)
 }
 
 #[context("Creating ostree deployment")]
@@ -2517,6 +2531,8 @@ pub(crate) async fn install_finalize(target: &Utf8Path) -> Result<()> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::fs;
+    use tempfile::tempdir;
 
     #[test]
     fn install_opts_serializable() {
@@ -2721,4 +2737,46 @@ UUID=boot-uuid /boot ext4 defaults 0 0
 
         Ok(())
     }
+
+    #[test]
+    fn test_print_configuration_with_root_dir() -> Result<()> {
+        use crate::install::config::{
+            Filesystem, InstallConfiguration, InstallConfigurationToplevel,
+        };
+
+        let temp_dir = tempdir()?;
+        let root_path = temp_dir.path();
+
+        let config_dir = root_path.join("etc/bootc/install");
+        fs::create_dir_all(&config_dir)?;
+        let config_path = config_dir.join("10-install.toml");
+
+        let test_config = InstallConfigurationToplevel {
+            install: Some(InstallConfiguration {
+                root_fs_type: Some(Filesystem::Xfs),
+                kargs: Some(vec!["quiet".to_string(), "karg2=2".to_string()]),
+                ..Default::default()
+            }),
+        };
+        let toml_content = toml::to_string(&test_config)?;
+        fs::write(config_path, toml_content)?;
+
+        let opts = InstallPrintConfigurationOpts {
+            sysroot: root_path.to_owned(),
+            all: true,
+        };
+
+        let mut buffer = Vec::new();
+        print_configuration_to_writer(opts, &mut buffer)?;
+
+        let output_json = String::from_utf8(buffer)?;
+        let output_config: crate::install::config::InstallConfiguration =
+            serde_json::from_str(&output_json)?;
+
+        let install_config = test_config.install.unwrap();
+        assert_eq!(install_config.kargs, output_config.kargs);
+        assert_eq!(install_config.root_fs_type, output_config.root_fs_type);
+
+        Ok(())
+    }
 }
diff --git a/crates/lib/src/install/config.rs b/crates/lib/src/install/config.rs
index bdeecb459..633b4886e 100644
--- a/crates/lib/src/install/config.rs
+++ b/crates/lib/src/install/config.rs
@@ -6,6 +6,7 @@ use anyhow::{Context, Result};
 use clap::ValueEnum;
 use fn_error_context::context;
 use serde::{Deserialize, Serialize};
+use std::path::Path;
 
 #[cfg(feature = "install-to-disk")]
 use super::baseline::BlockSetup;
@@ -191,11 +192,20 @@ impl InstallConfiguration {
 #[context("Loading configuration")]
 /// Load the install configuration, merging all found configuration files.
 pub(crate) fn load_config() -> Result<Option<InstallConfiguration>> {
+    load_config_at("/")
+}
+
+pub(crate) fn load_config_at(root_dir: impl AsRef<Path>) -> Result<Option<InstallConfiguration>> {
     let env = EnvProperties {
         sys_arch: std::env::consts::ARCH.to_string(),
     };
-    const SYSTEMD_CONVENTIONAL_BASES: &[&str] = &["/usr/lib", "/usr/local/lib", "/etc", "/run"];
-    let fragments = liboverdrop::scan(SYSTEMD_CONVENTIONAL_BASES, "bootc/install", &["toml"], true);
+    let root_dir = root_dir.as_ref();
+    const SYSTEMD_CONVENTIONAL_BASES: &[&str] = &["usr/lib", "usr/local/lib", "etc", "run"];
+    let systemd_conventional_bases = SYSTEMD_CONVENTIONAL_BASES
+        .iter()
+        .map(|v| root_dir.join(v))
+        .collect::<Vec<_>>();
+    let fragments = liboverdrop::scan(systemd_conventional_bases, "bootc/install", &["toml"], true);
     let mut config: Option<InstallConfiguration> = None;
     for (_name, path) in fragments {
         let buf = std::fs::read_to_string(&path)?;
diff --git a/docs/src/man/bootc-install-print-configuration.8.md b/docs/src/man/bootc-install-print-configuration.8.md
index 67928c9f1..20b58fd7d 100644
--- a/docs/src/man/bootc-install-print-configuration.8.md
+++ b/docs/src/man/bootc-install-print-configuration.8.md
@@ -26,6 +26,12 @@ string-valued filesystem name suitable for passing to `mkfs.\$type`.
 
     Print all configuration
 
+**--sysroot**=*SYSROOT*
+
+    Set an alternative root filesystem
+
+    Default: /
+
 <!-- END GENERATED OPTIONS -->
 
 # VERSION

@cgwalters cgwalters closed this Dec 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/documentation Updates to the documentation area/install Issues related to `bootc install`

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants