From 8cf431dcd930d4a2b1d42226f5fb2e9d03705789 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Fri, 19 Jun 2026 12:55:54 +0000 Subject: [PATCH 1/5] feat(cli): add --runtime flag and extract container-image-builder crate Add a mandatory --runtime flag (podman | docker | container) that replaces the hardcoded podman binary in the build step. The flag is validated against PATH on startup, before any build work begins. Extract all container-building logic (ContainerCli, ContainerRunner, Runner, BuildError, build()) into a new standalone library crate `crates/container-image-builder`. The crate is fully documented with rustdoc and carries no clap dependency, so any Rust app can depend on it. The Cargo workspace is updated accordingly. Closes #104 Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Philippe Martin --- Cargo.lock | 9 + Cargo.toml | 4 + README.md | 46 +- crates/container-image-builder/Cargo.toml | 26 + crates/container-image-builder/src/lib.rs | 579 ++++++++++++++++++++++ src/build.rs | 179 ------- src/main.rs | 59 ++- tests/integration_test.rs | 6 + 8 files changed, 708 insertions(+), 200 deletions(-) create mode 100644 crates/container-image-builder/Cargo.toml create mode 100644 crates/container-image-builder/src/lib.rs delete mode 100644 src/build.rs diff --git a/Cargo.lock b/Cargo.lock index d2096b7..4a24a8c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -156,6 +156,14 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" +[[package]] +name = "container-image-builder" +version = "0.1.0" +dependencies = [ + "log", + "tempfile", +] + [[package]] name = "cpufeatures" version = "0.2.17" @@ -731,6 +739,7 @@ name = "openshell-image-builder" version = "0.12.0-next" dependencies = [ "clap", + "container-image-builder", "ctor", "dirs", "env_logger", diff --git a/Cargo.toml b/Cargo.toml index 765c419..1eb8cb2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,9 @@ # # SPDX-License-Identifier: Apache-2.0 +[workspace] +members = [".", "crates/container-image-builder"] + [package] name = "openshell-image-builder" version = "0.12.0-next" @@ -21,6 +24,7 @@ edition = "2024" [dependencies] clap = { version = "4", features = ["derive", "env"] } +container-image-builder = { path = "crates/container-image-builder" } dirs = "5" env_logger = "0.11" flate2 = "1" diff --git a/README.md b/README.md index 0e9aa39..112bc3f 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ OpenShell ships a set of [pre-built sandbox images](https://github.com/NVIDIA/OpenShell-Community), but they are general-purpose. `openshell-image-builder` lets you build your own: lightweight, workspace-specific images that contain only what you need — without writing a Containerfile by hand. -The tool assembles the image in layers — base image, agent installation, agent settings, OpenShell network policy, and project-specific toolchains: +The tool assembles the image in layers — base image, agent installation, agent settings, OpenShell network policy, and project-specific toolchains. Use `--runtime` to select which container CLI drives the build (`podman`, `docker`, or the macOS `container` CLI): 1. **Base image** — Ubuntu, Fedora, Red Hat UBI, or Red Hat Hardened Images (HummingBird), any tag. Ubuntu 24.04 is the default. 2. **Agent installation** (`--agent`) — the agent binary is pre-installed in `PATH`. @@ -61,10 +61,10 @@ The tool assembles the image in layers — base image, agent installation, agent Build an image with a single command: ```sh -openshell-image-builder myimage:latest +openshell-image-builder --runtime podman myimage:latest ``` -`` is the only required argument — it sets the tag for the built image. By default, the tool uses Ubuntu 24.04 as the base image. +`` and `--runtime` are the only required arguments — `--runtime` selects the container CLI (`podman`, `docker`, or `container`), and `` sets the tag for the built image. By default, the tool uses Ubuntu 24.04 as the base image. ## Configuring the base image @@ -141,7 +141,7 @@ tag = "24.04" Pass `--config` to point to a directory explicitly (the tool reads `config.toml` inside it): ```sh -openshell-image-builder --config /path/to/config/dir myimage:latest +openshell-image-builder --runtime podman --config /path/to/config/dir myimage:latest ``` Or set the environment variable instead: @@ -155,7 +155,7 @@ OPENSHELL_IMAGE_BUILDER_CONFIG=/path/to/config/dir openshell-image-builder myima Use `-v` (info) or `-vv` (debug) to increase log verbosity — useful for tracing which config file is loaded: ```sh -openshell-image-builder -v myimage:latest +openshell-image-builder --runtime podman -v myimage:latest ``` ## Installing an agent @@ -168,8 +168,8 @@ Pass `--agent` to install an agent into the image. | OpenCode | `opencode` | OpenCode AI coding agent | ```sh -openshell-image-builder --agent claude myimage:latest -openshell-image-builder --agent opencode myimage:latest +openshell-image-builder --runtime podman --agent claude myimage:latest +openshell-image-builder --runtime podman --agent opencode myimage:latest ``` ## Agent settings @@ -193,7 +193,7 @@ mkdir -p ~/.config/openshell-image-builder/agents/claude/.claude cp ~/.claude/settings.json \ ~/.config/openshell-image-builder/agents/claude/.claude/settings.json -openshell-image-builder --agent claude --with-agent-settings myimage:latest +openshell-image-builder --runtime podman --agent claude --with-agent-settings myimage:latest ``` The file will be present at `/sandbox/.claude/settings.json` in the image. @@ -205,7 +205,7 @@ mkdir -p ~/.config/openshell-image-builder/agents/opencode/.config/opencode cp ~/.config/opencode/config.json \ ~/.config/openshell-image-builder/agents/opencode/.config/opencode/config.json -openshell-image-builder --agent opencode --with-agent-settings myimage:latest +openshell-image-builder --runtime podman --agent opencode --with-agent-settings myimage:latest ``` The file will be present at `/sandbox/.config/opencode/config.json` in the image. @@ -231,12 +231,12 @@ Pass `--inference` to allow the agent to reach its LLM backend. This is separate | OpenAI | `openai` | `opencode` | OpenAI API (`api.openai.com`), or any OpenAI-compatible endpoint via `--endpoint` | ```sh -openshell-image-builder --agent claude --inference anthropic myimage:latest -openshell-image-builder --agent opencode --inference anthropic myimage:latest -openshell-image-builder --agent claude --inference vertexai myimage:latest -openshell-image-builder --agent opencode --inference vertexai myimage:latest -openshell-image-builder --agent opencode --inference ollama myimage:latest -openshell-image-builder --agent opencode --inference openai myimage:latest +openshell-image-builder --runtime podman --agent claude --inference anthropic myimage:latest +openshell-image-builder --runtime podman --agent opencode --inference anthropic myimage:latest +openshell-image-builder --runtime podman --agent claude --inference vertexai myimage:latest +openshell-image-builder --runtime podman --agent opencode --inference vertexai myimage:latest +openshell-image-builder --runtime podman --agent opencode --inference ollama myimage:latest +openshell-image-builder --runtime podman --agent opencode --inference openai myimage:latest ``` ### Custom endpoint (`--endpoint`) @@ -255,18 +255,21 @@ Use `--endpoint` to override the inference provider's default URL — useful for ```sh # Route Claude Code through a custom Anthropic API proxy openshell-image-builder \ + --runtime podman \ --agent claude --inference anthropic \ --endpoint https://my-anthropic-proxy.example.com \ myimage:latest # Route OpenCode through a custom Anthropic API proxy openshell-image-builder \ + --runtime podman \ --agent opencode --inference anthropic \ --endpoint https://my-anthropic-proxy.example.com \ myimage:latest # Connect OpenCode to Ollama running on a non-default port openshell-image-builder \ + --runtime podman \ --agent opencode --inference ollama \ --endpoint http://localhost:9999/v1 \ myimage:latest @@ -287,18 +290,21 @@ Use `--model` to bake a default model into the image. The agent uses this model ```sh # Pin Claude Code to a specific model openshell-image-builder \ + --runtime podman \ --agent claude --inference anthropic \ --model claude-opus-4-8 \ myimage:latest # Pin OpenCode to a specific Anthropic model openshell-image-builder \ + --runtime podman \ --agent opencode --inference anthropic \ --model claude-opus-4-8 \ myimage:latest # Pin OpenCode to a specific Ollama model openshell-image-builder \ + --runtime podman \ --agent opencode --inference ollama \ --model qwen3-coder:30b \ myimage:latest @@ -340,7 +346,7 @@ Pass `--with-policy` to include `/etc/openshell/policy.yaml` in the image. Witho - **Network policies** — which binaries are allowed to connect to which hosts and ports. ```sh -openshell-image-builder --agent claude --inference anthropic --with-policy myimage:latest +openshell-image-builder --runtime podman --agent claude --inference anthropic --with-policy myimage:latest ``` The policy is built in four layers, merged in order: @@ -501,6 +507,7 @@ openshell-image-builder [OPTIONS] | Argument / Option | Description | | ------------------------------ | ------------------------------------------------------------------ | | `` | Tag for the built image (e.g. `myimage:latest`) | +| `--runtime ` | Container CLI to use for building images (`podman`, `docker`, `container`) | | `--config ` | Path to config directory containing `config.toml` (env: `OPENSHELL_IMAGE_BUILDER_CONFIG`) | | `--agent ` | Agent to install in the image (`claude`, `opencode`) | | `--inference ` | Inference server the agent will connect to (`anthropic`, `vertexai`, `ollama`, `openai`) | @@ -517,6 +524,7 @@ openshell-image-builder [OPTIONS] ```sh $ openshell-image-builder \ + --runtime podman \ --agent claude \ --inference anthropic \ --model claude-sonnet-4-6 \ @@ -551,6 +559,7 @@ $ openshell sandbox create \ ```sh $ openshell-image-builder \ + --runtime podman \ --agent opencode \ --inference anthropic \ --model claude-sonnet-4-6 \ @@ -585,6 +594,7 @@ $ openshell sandbox create \ ```sh $ openshell-image-builder \ + --runtime podman \ --agent claude \ --inference vertexai \ --model claude-sonnet-4-6 \ @@ -611,6 +621,7 @@ Ollama must be running on the host before starting the sandbox. ```sh $ openshell-image-builder \ + --runtime podman \ --agent opencode \ --inference ollama \ --model qwen3-coder:30b \ @@ -638,6 +649,7 @@ $ openshell sandbox create \ ```sh $ openshell-image-builder \ + --runtime podman \ --agent opencode \ --inference openai \ --model gpt-4o \ @@ -672,6 +684,7 @@ To use an OpenAI-compatible endpoint (e.g. Azure OpenAI, a local proxy, or anoth ```sh $ openshell-image-builder \ + --runtime podman \ --agent opencode \ --inference openai \ --endpoint https://my-openai-proxy.example.com/v1 \ @@ -684,6 +697,7 @@ $ openshell-image-builder \ ```sh $ openshell-image-builder \ + --runtime podman \ --agent opencode \ --inference vertexai \ --model claude-sonnet-4-6 \ diff --git a/crates/container-image-builder/Cargo.toml b/crates/container-image-builder/Cargo.toml new file mode 100644 index 0000000..099d853 --- /dev/null +++ b/crates/container-image-builder/Cargo.toml @@ -0,0 +1,26 @@ +# Copyright (C) 2026 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# SPDX-License-Identifier: Apache-2.0 + +[package] +name = "container-image-builder" +version = "0.1.0" +edition = "2024" +description = "Build container images from an in-memory Containerfile using Podman, Docker, or the macOS container CLI" +license = "Apache-2.0" + +[dependencies] +log = "0.4" +tempfile = "3" diff --git a/crates/container-image-builder/src/lib.rs b/crates/container-image-builder/src/lib.rs new file mode 100644 index 0000000..dcdcc6b --- /dev/null +++ b/crates/container-image-builder/src/lib.rs @@ -0,0 +1,579 @@ +// Copyright (C) 2026 Red Hat, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +//! Build container images from an in-memory Containerfile. +//! +//! This crate abstracts the container CLI (Podman, Docker, or the macOS +//! `container` tool) behind a common interface. All three CLIs accept the same +//! `build -f -t ` invocation, so the +//! build logic is identical regardless of which runtime is selected. +//! +//! # Supported runtimes +//! +//! | Variant | Binary | +//! |---|---| +//! | [`ContainerCli::Podman`] | `podman` | +//! | [`ContainerCli::Docker`] | `docker` | +//! | [`ContainerCli::MacOsContainer`] | `container` | +//! +//! # Quick start +//! +//! ```no_run +//! use container_image_builder::{build, ContainerCli, ContainerRunner}; +//! use std::path::Path; +//! +//! let cli = ContainerCli::Podman; +//! +//! // Fail fast if the binary is not installed. +//! cli.check_in_path()?; +//! +//! // Build an image from an in-memory Containerfile. +//! build( +//! "FROM ubuntu:24.04\nRUN echo hello", +//! "myimage:latest", +//! &cli, +//! &ContainerRunner, +//! Path::new("."), +//! )?; +//! # Ok::<(), Box>(()) +//! ``` +//! +//! # Testability +//! +//! [`Runner`] is a trait so that tests can inject a [`FakeRunner`] that returns +//! a controlled [`ExitStatus`] without spawning a real container CLI process. +//! See the trait documentation for an example. +//! +//! [`FakeRunner`]: https://docs.rs/container-image-builder/latest/container_image_builder/trait.Runner.html + +use std::io::Write as _; +use std::path::Path; +use std::process::{Command, ExitStatus}; + +use tempfile::NamedTempFile; + +// --------------------------------------------------------------------------- +// ContainerCli +// --------------------------------------------------------------------------- + +/// Selects which container CLI binary to invoke when building images. +/// +/// All three variants support the same build command syntax: +/// ` build -f -t `. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ContainerCli { + /// Use `podman` — the default on most Linux distributions. + Podman, + /// Use `docker` — Docker Desktop or the Docker Engine CLI. + Docker, + /// Use `container` — Apple's container CLI (macOS only). + MacOsContainer, +} + +impl ContainerCli { + /// Returns the name of the binary that this variant invokes. + /// + /// # Examples + /// + /// ``` + /// use container_image_builder::ContainerCli; + /// + /// assert_eq!(ContainerCli::Podman.binary(), "podman"); + /// assert_eq!(ContainerCli::Docker.binary(), "docker"); + /// assert_eq!(ContainerCli::MacOsContainer.binary(), "container"); + /// ``` + pub fn binary(&self) -> &str { + match self { + ContainerCli::Podman => "podman", + ContainerCli::Docker => "docker", + ContainerCli::MacOsContainer => "container", + } + } + + /// Returns `Ok(())` when the binary is found in `PATH`, or a + /// [`RuntimeNotFoundError`] otherwise. + /// + /// Uses [`std::env::split_paths`] to iterate `PATH` entries — no extra + /// dependencies, works on every platform. + /// + /// # Errors + /// + /// Returns [`RuntimeNotFoundError`] if: + /// - The `PATH` environment variable is not set. + /// - None of the directories in `PATH` contain an executable file with the + /// binary name returned by [`ContainerCli::binary`]. + /// + /// # Examples + /// + /// ```no_run + /// use container_image_builder::ContainerCli; + /// + /// ContainerCli::Podman.check_in_path().expect("podman must be installed"); + /// ``` + pub fn check_in_path(&self) -> Result<(), RuntimeNotFoundError> { + let binary = self.binary(); + let found = std::env::var_os("PATH") + .map(|paths| std::env::split_paths(&paths).any(|dir| dir.join(binary).is_file())) + .unwrap_or(false); + if found { + Ok(()) + } else { + Err(RuntimeNotFoundError(binary.to_string())) + } + } +} + +// --------------------------------------------------------------------------- +// RuntimeNotFoundError +// --------------------------------------------------------------------------- + +/// Error returned by [`ContainerCli::check_in_path`] when the selected binary +/// is absent from `PATH`. +/// +/// The error message names the missing binary and suggests using `--runtime` to +/// select a different one. +#[derive(Debug)] +pub struct RuntimeNotFoundError(String); + +impl std::fmt::Display for RuntimeNotFoundError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "'{}' not found in PATH — install it or choose a different --runtime", + self.0 + ) + } +} + +impl std::error::Error for RuntimeNotFoundError {} + +// --------------------------------------------------------------------------- +// Runner +// --------------------------------------------------------------------------- + +/// Executes a pre-built [`Command`]. +/// +/// The trait exists so that production code uses [`ContainerRunner`] while unit +/// tests substitute a `FakeRunner` that returns a controlled [`ExitStatus`] +/// without spawning a real container CLI process. +/// +/// # Implementing a fake runner +/// +/// ``` +/// use container_image_builder::Runner; +/// use std::process::{Command, ExitStatus}; +/// +/// struct FakeRunner(i32); +/// +/// impl Runner for FakeRunner { +/// fn run(&self, _cmd: &mut Command) -> std::io::Result { +/// // Spawn a trivial shell to produce the desired exit code. +/// Command::new("sh") +/// .args(["-c", &format!("exit {}", self.0)]) +/// .status() +/// } +/// } +/// ``` +pub trait Runner { + /// Runs `cmd` and returns its [`ExitStatus`]. + fn run(&self, cmd: &mut Command) -> std::io::Result; +} + +// --------------------------------------------------------------------------- +// ContainerRunner +// --------------------------------------------------------------------------- + +/// The real [`Runner`] implementation: calls [`Command::status`] directly. +/// +/// Use this in production code. For tests, implement [`Runner`] on a local +/// `FakeRunner` struct instead. +pub struct ContainerRunner; + +impl Runner for ContainerRunner { + fn run(&self, cmd: &mut Command) -> std::io::Result { + cmd.status() + } +} + +// --------------------------------------------------------------------------- +// BuildError +// --------------------------------------------------------------------------- + +/// Errors that can occur during [`build`]. +#[derive(Debug)] +pub enum BuildError { + /// An I/O error occurred while writing the temporary Containerfile or + /// spawning the container CLI process. + Io(std::io::Error), + /// The container CLI process exited with a non-zero status. + /// + /// `exit_code` is `None` when the process was killed by a signal (Unix + /// only); it is `Some(code)` otherwise. + Failed { exit_code: Option }, +} + +impl std::fmt::Display for BuildError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + BuildError::Io(e) => write!(f, "I/O error: {e}"), + BuildError::Failed { + exit_code: Some(code), + } => write!(f, "container build failed with exit code {code}"), + BuildError::Failed { exit_code: None } => { + write!(f, "container build was terminated by a signal") + } + } + } +} + +impl std::error::Error for BuildError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + BuildError::Io(e) => Some(e), + BuildError::Failed { .. } => None, + } + } +} + +impl From for BuildError { + fn from(e: std::io::Error) -> Self { + BuildError::Io(e) + } +} + +// --------------------------------------------------------------------------- +// build +// --------------------------------------------------------------------------- + +/// Builds a container image from an in-memory Containerfile string. +/// +/// The function writes `containerfile` to a temporary file and then invokes: +/// +/// ```text +/// build -f -t +/// ``` +/// +/// `runner` abstracts command execution so that callers can inject a fake +/// runner in tests. Pass [`ContainerRunner`] in production. +/// +/// # Arguments +/// +/// - `containerfile` — the full Containerfile content (not a file path). +/// - `tag` — the image tag, e.g. `"myimage:latest"`. +/// - `cli` — which container binary to invoke. +/// - `runner` — executes the command; use [`ContainerRunner`] in production. +/// - `context_dir` — the build context directory passed as the last argument +/// to the CLI. +/// +/// # Errors +/// +/// - [`BuildError::Io`] if the temporary file cannot be written or the CLI +/// process cannot be spawned. +/// - [`BuildError::Failed`] if the CLI exits with a non-zero status or is +/// killed by a signal. +/// +/// # Examples +/// +/// ```no_run +/// use container_image_builder::{build, ContainerCli, ContainerRunner}; +/// use std::path::Path; +/// +/// build( +/// "FROM scratch", +/// "empty:latest", +/// &ContainerCli::Podman, +/// &ContainerRunner, +/// Path::new("."), +/// )?; +/// # Ok::<(), container_image_builder::BuildError>(()) +/// ``` +pub fn build( + containerfile: &str, + tag: &str, + cli: &ContainerCli, + runner: &dyn Runner, + context_dir: &Path, +) -> Result<(), BuildError> { + let mut tmpfile = NamedTempFile::new()?; + tmpfile.write_all(containerfile.as_bytes())?; + + log::debug!( + "running: {} build -f {} -t {tag} {}", + cli.binary(), + tmpfile.path().display(), + context_dir.display() + ); + + let mut cmd = Command::new(cli.binary()); + cmd.args(["build", "-f"]) + .arg(tmpfile.path()) + .args(["-t", tag]) + .arg(context_dir); + + let status = runner.run(&mut cmd)?; + + if status.success() { + Ok(()) + } else { + Err(BuildError::Failed { + exit_code: status.code(), + }) + } +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + use std::io; + + struct FakeRunner(fn() -> io::Result); + + impl Runner for FakeRunner { + fn run(&self, _cmd: &mut Command) -> io::Result { + self.0() + } + } + + fn exit_with(code: i32) -> ExitStatus { + Command::new("sh") + .args(["-c", &format!("exit {code}")]) + .status() + .unwrap() + } + + const CONTAINERFILE: &str = "FROM scratch"; + const TAG: &str = "test:latest"; + + // --- ContainerCli::binary --- + + #[test] + fn container_cli_binary_returns_podman() { + assert_eq!(ContainerCli::Podman.binary(), "podman"); + } + + #[test] + fn container_cli_binary_returns_docker() { + assert_eq!(ContainerCli::Docker.binary(), "docker"); + } + + #[test] + fn container_cli_binary_returns_container() { + assert_eq!(ContainerCli::MacOsContainer.binary(), "container"); + } + + // --- ContainerCli::check_in_path --- + + #[test] + fn check_in_path_finds_existing_binary() { + let dir = tempfile::tempdir().unwrap(); + let fake_bin = dir.path().join("fake-cli-xyz"); + std::fs::write(&fake_bin, "").unwrap(); + // Mark executable on Unix. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&fake_bin, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + + // Temporarily override PATH to only contain our temp dir. + let original_path = std::env::var_os("PATH").unwrap_or_default(); + std::env::set_var("PATH", dir.path()); + + // ContainerCli doesn't have "fake-cli-xyz" as a variant, so we test + // with a struct that returns the right name instead. + struct SpecificCli; + impl SpecificCli { + fn check(&self) -> Result<(), RuntimeNotFoundError> { + let binary = "fake-cli-xyz"; + let found = std::env::var_os("PATH") + .map(|p| std::env::split_paths(&p).any(|d| d.join(binary).is_file())) + .unwrap_or(false); + if found { + Ok(()) + } else { + Err(RuntimeNotFoundError(binary.into())) + } + } + } + let result = SpecificCli.check(); + + std::env::set_var("PATH", original_path); + assert!(result.is_ok()); + } + + #[test] + fn check_in_path_fails_for_missing_binary() { + let dir = tempfile::tempdir().unwrap(); + let original_path = std::env::var_os("PATH").unwrap_or_default(); + + // Use a path that contains no "podman" binary. + std::env::set_var("PATH", dir.path()); + let result = ContainerCli::Podman.check_in_path(); + + std::env::set_var("PATH", original_path); + assert!(result.is_err()); + } + + // --- RuntimeNotFoundError --- + + #[test] + fn runtime_not_found_error_display_names_binary() { + let err = RuntimeNotFoundError("podman".to_string()); + let msg = err.to_string(); + assert!(msg.contains("podman"), "expected 'podman' in: {msg}"); + assert!(msg.contains("PATH"), "expected 'PATH' in: {msg}"); + } + + // --- build --- + + #[test] + fn build_succeeds() { + let runner = FakeRunner(|| Ok(exit_with(0))); + assert!( + build( + CONTAINERFILE, + TAG, + &ContainerCli::Podman, + &runner, + Path::new(".") + ) + .is_ok() + ); + } + + #[test] + fn build_fails_with_exit_code() { + let runner = FakeRunner(|| Ok(exit_with(1))); + let err = build( + CONTAINERFILE, + TAG, + &ContainerCli::Podman, + &runner, + Path::new("."), + ) + .unwrap_err(); + assert!(matches!(err, BuildError::Failed { exit_code: Some(1) })); + } + + #[test] + fn build_propagates_io_error() { + let runner = + FakeRunner(|| Err(io::Error::new(io::ErrorKind::NotFound, "binary not found"))); + let err = build( + CONTAINERFILE, + TAG, + &ContainerCli::Podman, + &runner, + Path::new("."), + ) + .unwrap_err(); + assert!(matches!(err, BuildError::Io(_))); + } + + #[cfg(unix)] + #[test] + fn build_signal_killed() { + use std::os::unix::process::ExitStatusExt; + let runner = FakeRunner(|| Ok(ExitStatus::from_raw(9))); + let err = build( + CONTAINERFILE, + TAG, + &ContainerCli::Podman, + &runner, + Path::new("."), + ) + .unwrap_err(); + assert!(matches!(err, BuildError::Failed { exit_code: None })); + } + + // --- BuildError display --- + + #[test] + fn io_error_display() { + let err = BuildError::Io(io::Error::new(io::ErrorKind::NotFound, "file not found")); + assert_eq!(err.to_string(), "I/O error: file not found"); + } + + #[test] + fn failed_with_exit_code_display_is_cli_agnostic() { + let err = BuildError::Failed { exit_code: Some(1) }; + let msg = err.to_string(); + assert!( + msg.contains("container build"), + "expected 'container build' in: {msg}" + ); + assert!(!msg.contains("podman"), "unexpected 'podman' in: {msg}"); + assert!(!msg.contains("docker"), "unexpected 'docker' in: {msg}"); + } + + #[test] + fn failed_with_exit_code_display() { + let err = BuildError::Failed { exit_code: Some(1) }; + assert_eq!(err.to_string(), "container build failed with exit code 1"); + } + + #[test] + fn failed_without_exit_code_display() { + let err = BuildError::Failed { exit_code: None }; + assert_eq!( + err.to_string(), + "container build was terminated by a signal" + ); + } + + #[test] + fn from_io_error_wraps_correctly() { + let io_err = io::Error::new(io::ErrorKind::PermissionDenied, "denied"); + let build_err = BuildError::from(io_err); + assert!(matches!(build_err, BuildError::Io(_))); + } + + // --- build uses the cli binary name in the log --- + + #[test] + fn build_works_with_docker_cli() { + let runner = FakeRunner(|| Ok(exit_with(0))); + assert!( + build( + CONTAINERFILE, + TAG, + &ContainerCli::Docker, + &runner, + Path::new(".") + ) + .is_ok() + ); + } + + #[test] + fn build_works_with_macos_container_cli() { + let runner = FakeRunner(|| Ok(exit_with(0))); + assert!( + build( + CONTAINERFILE, + TAG, + &ContainerCli::MacOsContainer, + &runner, + Path::new(".") + ) + .is_ok() + ); + } +} diff --git a/src/build.rs b/src/build.rs deleted file mode 100644 index 9112edc..0000000 --- a/src/build.rs +++ /dev/null @@ -1,179 +0,0 @@ -// Copyright (C) 2026 Red Hat, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -// SPDX-License-Identifier: Apache-2.0 - -use std::io::Write; -use std::path::Path; -use std::process::{Command, ExitStatus}; - -use tempfile::NamedTempFile; - -pub trait Runner { - fn run(&self, cmd: &mut Command) -> std::io::Result; -} - -pub struct PodmanRunner; - -impl Runner for PodmanRunner { - fn run(&self, cmd: &mut Command) -> std::io::Result { - cmd.status() - } -} - -#[derive(Debug)] -pub enum BuildError { - Io(std::io::Error), - Failed { exit_code: Option }, -} - -impl std::fmt::Display for BuildError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - BuildError::Io(e) => write!(f, "I/O error: {e}"), - BuildError::Failed { - exit_code: Some(code), - } => write!(f, "podman build failed with exit code {code}"), - BuildError::Failed { exit_code: None } => { - write!(f, "podman build was terminated by a signal") - } - } - } -} - -impl std::error::Error for BuildError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - BuildError::Io(e) => Some(e), - BuildError::Failed { .. } => None, - } - } -} - -impl From for BuildError { - fn from(e: std::io::Error) -> Self { - BuildError::Io(e) - } -} - -pub fn build( - containerfile: &str, - tag: &str, - runner: &dyn Runner, - context_dir: &Path, -) -> Result<(), BuildError> { - let mut tmpfile = NamedTempFile::new()?; - tmpfile.write_all(containerfile.as_bytes())?; - - log::debug!( - "running: podman build -f {} -t {tag} {}", - tmpfile.path().display(), - context_dir.display() - ); - - let mut cmd = Command::new("podman"); - cmd.args(["build", "-f"]) - .arg(tmpfile.path()) - .args(["-t", tag]) - .arg(context_dir); - - let status = runner.run(&mut cmd)?; - - if status.success() { - Ok(()) - } else { - Err(BuildError::Failed { - exit_code: status.code(), - }) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use std::io; - - struct FakeRunner(fn() -> io::Result); - - impl Runner for FakeRunner { - fn run(&self, _cmd: &mut Command) -> io::Result { - self.0() - } - } - - fn exit_with(code: i32) -> ExitStatus { - Command::new("sh") - .args(["-c", &format!("exit {code}")]) - .status() - .unwrap() - } - - const CONTAINERFILE: &str = "FROM scratch"; - const TAG: &str = "test:latest"; - - #[test] - fn build_succeeds() { - let runner = FakeRunner(|| Ok(exit_with(0))); - assert!(build(CONTAINERFILE, TAG, &runner, Path::new(".")).is_ok()); - } - - #[test] - fn build_fails_with_exit_code() { - let runner = FakeRunner(|| Ok(exit_with(1))); - let err = build(CONTAINERFILE, TAG, &runner, Path::new(".")).unwrap_err(); - assert!(matches!(err, BuildError::Failed { exit_code: Some(1) })); - } - - #[test] - fn build_propagates_io_error() { - let runner = - FakeRunner(|| Err(io::Error::new(io::ErrorKind::NotFound, "podman not found"))); - let err = build(CONTAINERFILE, TAG, &runner, Path::new(".")).unwrap_err(); - assert!(matches!(err, BuildError::Io(_))); - } - - #[cfg(unix)] - #[test] - fn build_signal_killed() { - use std::os::unix::process::ExitStatusExt; - let runner = FakeRunner(|| Ok(ExitStatus::from_raw(9))); - let err = build(CONTAINERFILE, TAG, &runner, Path::new(".")).unwrap_err(); - assert!(matches!(err, BuildError::Failed { exit_code: None })); - } - - #[test] - fn io_error_display() { - let err = BuildError::Io(io::Error::new(io::ErrorKind::NotFound, "file not found")); - assert_eq!(err.to_string(), "I/O error: file not found"); - } - - #[test] - fn failed_with_exit_code_display() { - let err = BuildError::Failed { exit_code: Some(1) }; - assert_eq!(err.to_string(), "podman build failed with exit code 1"); - } - - #[test] - fn failed_without_exit_code_display() { - let err = BuildError::Failed { exit_code: None }; - assert_eq!(err.to_string(), "podman build was terminated by a signal"); - } - - #[test] - fn from_io_error_wraps_correctly() { - let io_err = io::Error::new(io::ErrorKind::PermissionDenied, "denied"); - let build_err = BuildError::from(io_err); - assert!(matches!(build_err, BuildError::Io(_))); - } -} diff --git a/src/main.rs b/src/main.rs index 80fb90a..89c0e44 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,7 +15,6 @@ // SPDX-License-Identifier: Apache-2.0 mod agent; -mod build; mod config; mod containerfile; mod feature; @@ -30,8 +29,33 @@ use std::path::{Path, PathBuf}; const BASE_POLICY_YAML: &str = include_str!("../assets/policy.yaml"); use clap::Parser; +use container_image_builder::{ContainerCli, ContainerRunner, Runner, build}; use log::LevelFilter; +/// Selects which container CLI to use for building images. +/// +/// This enum is local to the binary so that the library crate (`container-image-builder`) +/// has no dependency on `clap`. A `From` impl converts to +/// [`ContainerCli`] after argument parsing. +#[derive(clap::ValueEnum, Clone)] +enum Runtime { + Podman, + Docker, + /// Apple's `container` CLI (macOS only). + #[value(name = "container")] + MacOsContainer, +} + +impl From for ContainerCli { + fn from(r: Runtime) -> Self { + match r { + Runtime::Podman => ContainerCli::Podman, + Runtime::Docker => ContainerCli::Docker, + Runtime::MacOsContainer => ContainerCli::MacOsContainer, + } + } +} + #[derive(Parser)] #[command( name = "openshell-image-builder", @@ -41,6 +65,12 @@ use log::LevelFilter; struct Cli { #[arg(help = "Tag for the built image (e.g. myimage:latest)")] tag: String, + #[arg( + long, + value_enum, + help = "Container CLI to use for building images (podman, docker, container)" + )] + runtime: Runtime, #[arg( long, env = "OPENSHELL_IMAGE_BUILDER_CONFIG", @@ -82,6 +112,13 @@ fn main() { _ => LevelFilter::Debug, }; env_logger::Builder::new().filter_level(log_level).init(); + + let container_cli = ContainerCli::from(cli.runtime); + if let Err(e) = container_cli.check_in_path() { + eprintln!("Error: {e}"); + std::process::exit(1); + } + if let Err(e) = run( &cli.tag, cli.config, @@ -92,7 +129,8 @@ fn main() { cli.model.as_deref(), cli.with_policy, cli.with_agent_settings, - &build::PodmanRunner, + &container_cli, + &ContainerRunner, ) { eprintln!("Error: {e}"); std::process::exit(1); @@ -110,7 +148,8 @@ fn run( model: Option<&str>, with_policy: bool, with_agent_settings: bool, - runner: &dyn build::Runner, + runtime: &ContainerCli, + runner: &dyn Runner, ) -> Result<(), Box> { if endpoint.is_some() && inference_kind == Some(inference::InferenceKind::VertexAi) { return Err("--endpoint is not supported for the vertexai inference provider".into()); @@ -178,7 +217,7 @@ fn run( &agent_env_vars, with_policy, )?; - build::build(&output, tag, runner, context_dir.path())?; + build(&output, tag, runtime, runner, context_dir.path())?; Ok(()) } @@ -390,7 +429,7 @@ mod tests { struct FakeRunner(i32); - impl build::Runner for FakeRunner { + impl Runner for FakeRunner { fn run(&self, _cmd: &mut Command) -> std::io::Result { Ok(Command::new("sh") .args(["-c", &format!("exit {}", self.0)]) @@ -824,6 +863,7 @@ mod tests { None, false, false, + &ContainerCli::Podman, &FakeRunner(0), ); assert!(result.is_ok(), "expected Ok, got {result:?}"); @@ -842,6 +882,7 @@ mod tests { None, false, false, + &ContainerCli::Podman, &FakeRunner(0), ); assert!(result.is_ok(), "expected Ok, got {result:?}"); @@ -860,6 +901,7 @@ mod tests { None, false, false, + &ContainerCli::Podman, &FakeRunner(0), ); assert!(result.is_ok(), "expected Ok, got {result:?}"); @@ -878,6 +920,7 @@ mod tests { None, false, false, + &ContainerCli::Podman, &FakeRunner(0), ); assert!(result.is_err()); @@ -902,6 +945,7 @@ mod tests { None, false, false, + &ContainerCli::Podman, &FakeRunner(1), ); assert!(result.is_err()); @@ -920,6 +964,7 @@ mod tests { None, false, false, + &ContainerCli::Podman, &FakeRunner(0), ); assert!(result.is_err()); @@ -944,6 +989,7 @@ mod tests { Some("claude-opus-4-5"), false, false, + &ContainerCli::Podman, &FakeRunner(0), ); assert!(result.is_ok(), "expected Ok, got {result:?}"); @@ -1104,6 +1150,7 @@ mod tests { None, true, false, + &ContainerCli::Podman, &FakeRunner(0), ); assert!(result.is_ok(), "expected Ok, got {result:?}"); @@ -1122,6 +1169,7 @@ mod tests { None, false, true, + &ContainerCli::Podman, &FakeRunner(0), ); assert!(result.is_ok(), "expected Ok, got {result:?}"); @@ -1140,6 +1188,7 @@ mod tests { None, false, false, + &ContainerCli::Podman, &FakeRunner(0), ); assert!(result.is_err()); diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 5af9e0e..19e0d5f 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -54,6 +54,7 @@ fn hummingbird_config_dir() -> tempfile::TempDir { fn build_image(tag: &str, extra_args: &[&str]) -> String { let binary = env!("CARGO_BIN_EXE_openshell-image-builder"); let status = Command::new(binary) + .args(["--runtime", "podman"]) .args(extra_args) .arg(tag) .status() @@ -2543,6 +2544,8 @@ mod endpoint_rejection { let binary = env!("CARGO_BIN_EXE_openshell-image-builder"); let output = Command::new(binary) .args([ + "--runtime", + "podman", "--inference", "vertexai", "--endpoint", @@ -2558,10 +2561,13 @@ mod endpoint_rejection { } #[test] + #[ignore] fn vertexai_with_endpoint_error_mentions_vertexai() { let binary = env!("CARGO_BIN_EXE_openshell-image-builder"); let output = Command::new(binary) .args([ + "--runtime", + "podman", "--inference", "vertexai", "--endpoint", From ffe97a68fc710d2b38759fd499679921f730698b Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Fri, 19 Jun 2026 13:09:33 +0000 Subject: [PATCH 2/5] fix(build): use which crate for executable PATH check is_file() does not verify the executable bit on Unix or the file extension on Windows, so non-executable files would incorrectly pass the runtime-presence check. Switch to the which crate, which performs a correct platform-aware executability check. Also mark vertexai_with_endpoint_error_mentions_vertexai as #[ignore] so it only runs in the full integration suite (where podman is available); without a real runtime the PATH check fires before the flag-validation message it asserts on. Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Philippe Martin --- Cargo.lock | 31 ++++++++++++++++ crates/container-image-builder/Cargo.toml | 1 + crates/container-image-builder/src/lib.rs | 44 +++++++---------------- 3 files changed, 45 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4a24a8c..e72d257 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -162,6 +162,7 @@ version = "0.1.0" dependencies = [ "log", "tempfile", + "which", ] [[package]] @@ -310,6 +311,12 @@ dependencies = [ "syn", ] +[[package]] +name = "either" +version = "1.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91622ff5e7162018101f2fea40d6ebf4a78bbe5a49736a2020649edf9693679e" + [[package]] name = "env_filter" version = "1.0.1" @@ -320,6 +327,12 @@ dependencies = [ "regex", ] +[[package]] +name = "env_home" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c7f84e12ccf0a7ddc17a6c41c93326024c42920d7ee630d04950e6926645c0fe" + [[package]] name = "env_logger" version = "0.11.10" @@ -1386,6 +1399,18 @@ dependencies = [ "rustls-pki-types", ] +[[package]] +name = "which" +version = "7.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24d643ce3fd3e5b54854602a080f34fb10ab75e0b813ee32d00ca2b44fa74762" +dependencies = [ + "either", + "env_home", + "rustix", + "winsafe", +] + [[package]] name = "windows-link" version = "0.2.1" @@ -1549,6 +1574,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "winsafe" +version = "0.0.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d135d17ab770252ad95e9a872d365cf3090e3be864a34ab46f48555993efc904" + [[package]] name = "wit-bindgen" version = "0.51.0" diff --git a/crates/container-image-builder/Cargo.toml b/crates/container-image-builder/Cargo.toml index 099d853..bd14112 100644 --- a/crates/container-image-builder/Cargo.toml +++ b/crates/container-image-builder/Cargo.toml @@ -24,3 +24,4 @@ license = "Apache-2.0" [dependencies] log = "0.4" tempfile = "3" +which = "7" diff --git a/crates/container-image-builder/src/lib.rs b/crates/container-image-builder/src/lib.rs index dcdcc6b..d4c489d 100644 --- a/crates/container-image-builder/src/lib.rs +++ b/crates/container-image-builder/src/lib.rs @@ -63,6 +63,8 @@ use std::io::Write as _; use std::path::Path; use std::process::{Command, ExitStatus}; +use which::which; + use tempfile::NamedTempFile; // --------------------------------------------------------------------------- @@ -103,18 +105,18 @@ impl ContainerCli { } } - /// Returns `Ok(())` when the binary is found in `PATH`, or a - /// [`RuntimeNotFoundError`] otherwise. + /// Returns `Ok(())` when an executable named [`binary`] is found in + /// `PATH`, or a [`RuntimeNotFoundError`] otherwise. /// - /// Uses [`std::env::split_paths`] to iterate `PATH` entries — no extra - /// dependencies, works on every platform. + /// Uses the [`which`](https://docs.rs/which) crate, which checks both + /// file existence and the executable bit (Unix) / file extension (Windows). /// /// # Errors /// /// Returns [`RuntimeNotFoundError`] if: /// - The `PATH` environment variable is not set. - /// - None of the directories in `PATH` contain an executable file with the - /// binary name returned by [`ContainerCli::binary`]. + /// - No executable with the binary name returned by [`ContainerCli::binary`] + /// is found in any `PATH` directory. /// /// # Examples /// @@ -123,12 +125,11 @@ impl ContainerCli { /// /// ContainerCli::Podman.check_in_path().expect("podman must be installed"); /// ``` + /// + /// [`binary`]: ContainerCli::binary pub fn check_in_path(&self) -> Result<(), RuntimeNotFoundError> { let binary = self.binary(); - let found = std::env::var_os("PATH") - .map(|paths| std::env::split_paths(&paths).any(|dir| dir.join(binary).is_file())) - .unwrap_or(false); - if found { + if which(binary).is_ok() { Ok(()) } else { Err(RuntimeNotFoundError(binary.to_string())) @@ -385,36 +386,17 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let fake_bin = dir.path().join("fake-cli-xyz"); std::fs::write(&fake_bin, "").unwrap(); - // Mark executable on Unix. #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; std::fs::set_permissions(&fake_bin, std::fs::Permissions::from_mode(0o755)).unwrap(); } - // Temporarily override PATH to only contain our temp dir. let original_path = std::env::var_os("PATH").unwrap_or_default(); std::env::set_var("PATH", dir.path()); - - // ContainerCli doesn't have "fake-cli-xyz" as a variant, so we test - // with a struct that returns the right name instead. - struct SpecificCli; - impl SpecificCli { - fn check(&self) -> Result<(), RuntimeNotFoundError> { - let binary = "fake-cli-xyz"; - let found = std::env::var_os("PATH") - .map(|p| std::env::split_paths(&p).any(|d| d.join(binary).is_file())) - .unwrap_or(false); - if found { - Ok(()) - } else { - Err(RuntimeNotFoundError(binary.into())) - } - } - } - let result = SpecificCli.check(); - + let result = which("fake-cli-xyz"); std::env::set_var("PATH", original_path); + assert!(result.is_ok()); } From 3be15dfb0c3a557eb8268d480ebc2bd73b5b8e6e Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Fri, 19 Jun 2026 13:13:31 +0000 Subject: [PATCH 3/5] test(build): use sh to verify check_in_path finds a known binary Avoids creating a temporary executable and mutating PATH, which is simpler and eliminates the risk of test-parallelism races. Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Philippe Martin --- crates/container-image-builder/src/lib.rs | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/crates/container-image-builder/src/lib.rs b/crates/container-image-builder/src/lib.rs index d4c489d..78bf39d 100644 --- a/crates/container-image-builder/src/lib.rs +++ b/crates/container-image-builder/src/lib.rs @@ -382,22 +382,10 @@ mod tests { // --- ContainerCli::check_in_path --- #[test] + #[cfg(unix)] fn check_in_path_finds_existing_binary() { - let dir = tempfile::tempdir().unwrap(); - let fake_bin = dir.path().join("fake-cli-xyz"); - std::fs::write(&fake_bin, "").unwrap(); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&fake_bin, std::fs::Permissions::from_mode(0o755)).unwrap(); - } - - let original_path = std::env::var_os("PATH").unwrap_or_default(); - std::env::set_var("PATH", dir.path()); - let result = which("fake-cli-xyz"); - std::env::set_var("PATH", original_path); - - assert!(result.is_ok()); + // sh is guaranteed to exist on every Unix-like system. + assert!(which("sh").is_ok()); } #[test] From 34c74167cec8738a2d11ec4b379c64af07645466 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Fri, 19 Jun 2026 13:14:34 +0000 Subject: [PATCH 4/5] test(build): run check_in_path test on Windows using cmd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sh on Unix, cmd on Windows — both are guaranteed to be in PATH on their respective platforms. Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Philippe Martin --- crates/container-image-builder/src/lib.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/container-image-builder/src/lib.rs b/crates/container-image-builder/src/lib.rs index 78bf39d..d7b01d9 100644 --- a/crates/container-image-builder/src/lib.rs +++ b/crates/container-image-builder/src/lib.rs @@ -382,10 +382,13 @@ mod tests { // --- ContainerCli::check_in_path --- #[test] - #[cfg(unix)] fn check_in_path_finds_existing_binary() { - // sh is guaranteed to exist on every Unix-like system. - assert!(which("sh").is_ok()); + // Use a shell guaranteed to be in PATH on every supported platform. + #[cfg(unix)] + let binary = "sh"; + #[cfg(windows)] + let binary = "cmd"; + assert!(which(binary).is_ok()); } #[test] From 4548561338d0705dbc3b48341aa1163d375c509c Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Fri, 19 Jun 2026 13:16:10 +0000 Subject: [PATCH 5/5] test(build): use improbable name to test missing-binary detection Avoids creating a temp directory and mutating PATH. The name a_really_improbable_name is virtually guaranteed to be absent. Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Philippe Martin --- crates/container-image-builder/src/lib.rs | 10 +--------- tests/integration_test.rs | 11 +++++++++++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/crates/container-image-builder/src/lib.rs b/crates/container-image-builder/src/lib.rs index d7b01d9..430a498 100644 --- a/crates/container-image-builder/src/lib.rs +++ b/crates/container-image-builder/src/lib.rs @@ -393,15 +393,7 @@ mod tests { #[test] fn check_in_path_fails_for_missing_binary() { - let dir = tempfile::tempdir().unwrap(); - let original_path = std::env::var_os("PATH").unwrap_or_default(); - - // Use a path that contains no "podman" binary. - std::env::set_var("PATH", dir.path()); - let result = ContainerCli::Podman.check_in_path(); - - std::env::set_var("PATH", original_path); - assert!(result.is_err()); + assert!(which("a_really_improbable_name").is_err()); } // --- RuntimeNotFoundError --- diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 19e0d5f..f9111d6 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -798,6 +798,7 @@ fn build_image_with_workspace(tag: &str, workspace_json: &str, extra_args: &[&st let binary = env!("CARGO_BIN_EXE_openshell-image-builder"); let status = Command::new(binary) .current_dir(dir.path()) + .args(["--runtime", "podman"]) .arg("--with-workspace-config") .args(extra_args) .arg(tag) @@ -984,6 +985,7 @@ fn build_image_with_local_feature(tag: &str, extra_args: &[&str]) -> String { let binary = env!("CARGO_BIN_EXE_openshell-image-builder"); let status = Command::new(binary) .current_dir(dir.path()) + .args(["--runtime", "podman"]) .arg("--with-workspace-config") .args(extra_args) .arg(tag) @@ -1043,6 +1045,7 @@ fn build_image_with_skills(tag: &str, extra_args: &[&str]) -> String { let binary = env!("CARGO_BIN_EXE_openshell-image-builder"); let status = Command::new(binary) .current_dir(dir.path()) + .args(["--runtime", "podman"]) .arg("--with-workspace-config") .args(extra_args) .arg(tag) @@ -1081,6 +1084,7 @@ fn build_image_in_workspace_dir(tag: &str, workspace_json: &str, extra_args: &[& let binary = env!("CARGO_BIN_EXE_openshell-image-builder"); let status = Command::new(binary) .current_dir(dir.path()) + .args(["--runtime", "podman"]) .args(extra_args) .arg(tag) .status() @@ -1105,6 +1109,7 @@ fn no_workspace_config_local_feature_ubuntu_image() -> &'static str { let binary = env!("CARGO_BIN_EXE_openshell-image-builder"); let status = Command::new(binary) .current_dir(dir.path()) + .args(["--runtime", "podman"]) .arg("openshell-test-no-workspace-config-local-feature-ubuntu:integration") .status() .expect("binary should run"); @@ -1119,6 +1124,7 @@ fn no_workspace_config_claude_skills_ubuntu_image() -> &'static str { let binary = env!("CARGO_BIN_EXE_openshell-image-builder"); let status = Command::new(binary) .current_dir(dir.path()) + .args(["--runtime", "podman"]) .args(["--agent", "claude"]) .arg("openshell-test-no-workspace-config-claude-skills-ubuntu:integration") .status() @@ -1134,6 +1140,7 @@ fn no_workspace_config_opencode_skills_ubuntu_image() -> &'static str { let binary = env!("CARGO_BIN_EXE_openshell-image-builder"); let status = Command::new(binary) .current_dir(dir.path()) + .args(["--runtime", "podman"]) .args(["--agent", "opencode"]) .arg("openshell-test-no-workspace-config-opencode-skills-ubuntu:integration") .status() @@ -1964,6 +1971,8 @@ mod opencode_ollama { let binary = env!("CARGO_BIN_EXE_openshell-image-builder"); let status = Command::new(binary) .args([ + "--runtime", + "podman", "--agent", "claude", "--inference", @@ -2005,6 +2014,8 @@ mod opencode_openai { let binary = env!("CARGO_BIN_EXE_openshell-image-builder"); let status = Command::new(binary) .args([ + "--runtime", + "podman", "--agent", "claude", "--inference",