Skip to content
Merged
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
24 changes: 24 additions & 0 deletions crates/integration-tests/src/tests/to_disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,30 @@ fn test_to_disk_different_imgref_same_digest() -> TestResult {
}
integration_test!(test_to_disk_different_imgref_same_digest);

/// Test that --bootc-install-podman-arg passes an innocuous extra arg to the inner podman run
fn test_to_disk_bootc_install_podman_arg() -> TestResult {
let sh = shell()?;
let bck = get_bck_command()?;
let label = INTEGRATION_TEST_LABEL;
let image = get_test_image();

let temp_dir = TempDir::new().expect("Failed to create temp directory");
let disk_path = Utf8PathBuf::try_from(temp_dir.path().join("test-disk.img"))
.expect("temp path is not UTF-8");

// Pass an innocuous podman label arg - this exercises the new flag without
// affecting the installation outcome.
let output = cmd!(
sh,
"{bck} to-disk --label {label} --bootc-install-podman-arg=--label=bcvk-test=1 {image} {disk_path}"
)
.output()?;

validate_disk_image(&disk_path, &output, "test_to_disk_bootc_install_podman_arg")?;
Ok(())
}
integration_test!(test_to_disk_bootc_install_podman_arg);

/// Test to-disk with various bootc images to ensure compatibility
///
/// This parameterized test runs to-disk with multiple container images,
Expand Down
47 changes: 47 additions & 0 deletions crates/kit/src/to_disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,15 @@ pub struct ToDiskAdditionalOpts {
/// Check if the disk would be regenerated without actually creating it
#[clap(long)]
pub dry_run: bool,

/// Pass an extra argument to the inner `podman run` that executes `bootc
/// install`. May be specified multiple times. Useful for testing edge
/// cases; for example `--bootc-install-podman-arg=--read-only` stresses
/// the install path by making the container rootfs read-only, which
/// exercises bootloader code paths that avoid writing to the host's
/// read-only /boot (similar to osbuild sandbox environments).
#[clap(long = "bootc-install-podman-arg")]
Comment thread
cgwalters marked this conversation as resolved.
pub bootc_install_podman_args: Vec<String>,
}

/// Configuration options for installing a bootc container image to disk
Expand Down Expand Up @@ -222,6 +231,19 @@ impl ToDiskOpts {
.map(|v| format!("--env=RUST_LOG={v}"))
.unwrap_or_default();

// Build extra podman args as a shell-safe string
let extra_podman_args = self
.additional
.bootc_install_podman_args
.iter()
.map(|a| {
shlex::try_quote(a)
.map(|q| q.to_string())
.map_err(|e| eyre!("Failed to quote podman arg '{}': {}", a, e))
})
.collect::<Result<Vec<_>>>()?
.join(" ");
Comment on lines +235 to +245
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The logic for building extra_podman_args correctly uses shlex::try_quote for shell safety, which is essential since these arguments are later embedded into a bash script. However, the current implementation results in a string that, when empty, still leaves a trailing backslash in the generated script (e.g., at line 296). While bash handles this, it would be cleaner to handle the empty case explicitly or ensure the template doesn't end up with an empty line continuation.


// Size /var/tmp tmpfs to match swap size (disk_size)
// This avoids duplicating size calculation logic
let tmpfs_size_str = format!("size={}k", disk_size / 1024);
Expand Down Expand Up @@ -271,6 +293,7 @@ impl ToDiskOpts {
--security-opt label=type:unconfined_t \
--env=STORAGE_OPTS \
{INSTALL_LOG} \
{EXTRA_PODMAN_ARGS} \
{SOURCE_IMGREF} \
bootc install to-disk \
--generic-image \
Expand Down Expand Up @@ -309,6 +332,7 @@ EOF
--security-opt label=type:unconfined_t \
--env=STORAGE_OPTS \
{INSTALL_LOG} \
{EXTRA_PODMAN_ARGS} \
containers-storage:{SOURCE_IMAGE} \
bootc install to-disk \
--generic-image \
Expand All @@ -330,6 +354,7 @@ EOF
.replace("{SOURCE_IMGREF}", &quoted_source_imgref)
.replace("{SOURCE_IMAGE}", &quoted_source_image)
.replace("{INSTALL_LOG}", &install_log)
.replace("{EXTRA_PODMAN_ARGS}", &extra_podman_args)
.replace("{BOOTC_ARGS}", &bootc_args);

Ok(vec!["/bin/bash".to_string(), "-c".to_string(), script])
Expand Down Expand Up @@ -684,4 +709,26 @@ mod tests {

Ok(())
}

/// Clap parses `--flag=--value` by treating everything after `=` as the raw
/// value, so `--bootc-install-podman-arg=--read-only` works without needing
/// `allow_hyphen_values = true`. This test documents and locks that behaviour.
#[test]
fn test_podman_arg_equals_form_accepts_hyphen_values() {
use clap::Parser;

let opts = ToDiskOpts::try_parse_from([
"bcvk",
"test:latest",
"/tmp/test.img",
"--bootc-install-podman-arg=--read-only",
"--bootc-install-podman-arg=--env=FOO=bar",
])
.expect("--flag=--value form must parse without allow_hyphen_values");

assert_eq!(
opts.additional.bootc_install_podman_args,
["--read-only", "--env=FOO=bar"]
);
}
}
8 changes: 8 additions & 0 deletions docs/src/man/bcvk-libvirt-run.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ Run a bootable container as a persistent VM

Disable TPM 2.0 support (enabled by default)

**--firmware-log**

Enable firmware debug log (captures OVMF/EDK2 DEBUG output via isa-debugcon)

**--secure-boot-keys**=*SECURE_BOOT_KEYS*

Directory containing secure boot keys (required for uefi-secure)
Expand All @@ -150,6 +154,10 @@ Run a bootable container as a persistent VM

Create a transient VM that disappears on shutdown/reboot

**--ignition**=*IGNITION_CONFIG*

Path to Ignition config file (JSON format) for first-boot provisioning

<!-- END GENERATED OPTIONS -->

# EXAMPLES
Expand Down
4 changes: 4 additions & 0 deletions docs/src/man/bcvk-to-disk.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ The installation process:

Check if the disk would be regenerated without actually creating it

**--bootc-install-podman-arg**=*BOOTC_INSTALL_PODMAN_ARGS*

Pass an extra argument to the inner `podman run` that executes `bootc install`. May be specified multiple times. Useful for testing edge cases; for example `--bootc-install-podman-arg=--read-only` stresses the install path by making the container rootfs read-only, which exercises bootloader code paths that avoid writing to the host's read-only /boot (similar to osbuild sandbox environments)

<!-- END GENERATED OPTIONS -->

# ARGUMENTS
Expand Down
Loading