diff --git a/docs/execplans/4-1-1-git-identity-configuration.md b/docs/execplans/4-1-1-git-identity-configuration.md new file mode 100644 index 00000000..4ce7614d --- /dev/null +++ b/docs/execplans/4-1-1-git-identity-configuration.md @@ -0,0 +1,413 @@ +# Step 4.1.1: Git identity configuration + +This ExecPlan (execution plan) is a living document. The sections +`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises and discoveries`, +`Decision log`, and `Outcomes and retrospective` must be kept up to date as +work proceeds. + +Status: IN_PROGRESS + +No `PLANS.md` file exists in this repository as of 2026-04-07, so this ExecPlan +is the governing implementation document for this task. + +## Purpose and big picture + +Implement Step 4.1 (Git identity configuration) from +`docs/podbot-roadmap.md`. The goal is to read `user.name` and `user.email` +from the host Git configuration and apply them inside the container via +`git config --global`, so that Git commits made by agents carry the correct +identity. When the host has no Git identity configured, podbot must emit a +warning rather than failing. + +After this change, callers can configure Git identity within a container using +host settings. The behaviour is observable through unit tests and behaviour +tests that validate both happy paths (identity present) and unhappy paths +(identity missing or partially missing) without requiring a live daemon. + +This plan also covers required documentation updates: + +- Record the Git identity configuration design in `docs/podbot-design.md`. +- Update user-facing behaviour in `docs/users-guide.md`. +- Mark the relevant roadmap entry as done once acceptance criteria are met. + +## Constraints + +- Keep existing engine, configuration, and API behaviour unchanged. +- Do not add new third-party dependencies unless a blocker is documented in the + `Decision log` and approved. +- Keep module-level `//!` documentation in every Rust module touched. +- Avoid `unwrap` and `expect` outside test code. +- Use `rstest` fixtures for unit tests and `rstest-bdd` v0.5.0 for behavioural + tests. +- Keep files under 400 lines where practical; split modules when a file would + exceed this limit. +- Preserve current public configuration semantics unless a roadmap requirement + explicitly requires a change. +- Use en-GB-oxendict spelling in documentation and comments. +- Prefer Makefile targets for verification (`make check-fmt`, `make lint`, and + `make test`). +- Git identity reading and application must use dependency injection (trait + seams) so that unit tests run deterministically without a live Git + installation or container daemon. + +## Tolerances (exception triggers) + +- Scope: if implementation requires edits in more than 15 files or more than + 600 net lines, stop and confirm scope. +- Public API: if an existing public API must break compatibility, stop and + confirm the migration strategy. +- Dependencies: if a new dependency is required for Git configuration reading + or command execution mocking, stop and confirm before adding it. +- Iterations: if `make lint` or `make test` still fails after two fix passes, + stop and document the blocker with log evidence. + +## Risks + +- Risk: Reading host Git configuration may behave differently across platforms + (Linux, macOS, Windows) or when Git is not installed. Severity: medium + Likelihood: medium Mitigation: use `std::process::Command` to invoke + `git config --get` and handle non-zero exit codes as "not configured" rather + than fatal errors. Wrap this in a trait seam for deterministic testing. + +- Risk: Container exec for `git config --global` may fail if Git is not + installed in the container image. Severity: low Likelihood: low Mitigation: + map exec failures to warnings rather than hard errors, consistent with the + roadmap requirement that missing identity produces a warning but does not + block execution. + +- Risk: Behaviour tests may become flaky if they depend on the host Git state. + Severity: medium Likelihood: medium Mitigation: use dependency injection with + deterministic test doubles for the Git configuration reader and container + exec client. No test should depend on host Git configuration. + +## Progress + +- [x] (2026-04-07 UTC) Reviewed roadmap, design, testing guides, and current + engine code structure. +- [x] (2026-04-07 UTC) Drafted this ExecPlan at + `docs/execplans/4-1-1-git-identity-configuration.md`. +- [ ] Implement Git identity reader with trait seam. +- [ ] Implement container Git configuration applicator. +- [ ] Add `rstest` unit tests for happy, unhappy, and edge paths. +- [ ] Add `rstest-bdd` v0.5.0 scenarios for Git identity behaviour. +- [ ] Update design, user guide, and roadmap documentation. +- [ ] Run quality gates and capture logs. + +## Surprises and discoveries + +(To be populated as work proceeds.) + +## Decision log + +- Decision: introduce a `GitIdentityReader` trait and a `GitIdentity` data + type to abstract host Git configuration reading. Rationale: this follows the + dependency injection pattern established in + `docs/reliable-testing-in-rust-via-dependency-injection.md` and allows + deterministic testing without mutating host Git configuration. The trait will + have a `read_git_identity(&self) -> GitIdentity` method where `GitIdentity` + contains `Option` fields for `name` and `email`. + Date/Author: 2026-04-07 / DevBoxer. + +- Decision: place the Git identity module at + `src/engine/connection/git_identity/` rather than in a new top-level module. + Rationale: Git identity configuration is part of the container orchestration + lifecycle (Step 3 in the design document's execution flow) and belongs with + other container-lifecycle operations under `engine/connection`. The module + will contain the reader trait, the container applicator, and associated types. + Date/Author: 2026-04-07 / DevBoxer. + +- Decision: use `std::process::Command` for the production `GitIdentityReader` + implementation rather than parsing `.gitconfig` files directly. Rationale: + `git config --get` respects the full Git configuration precedence chain + (system, global, local, worktree, command-line) and handles includes, + conditional includes, and platform-specific paths. Direct file parsing would + need to replicate all of this. The command approach is simpler, more correct, + and aligns with the existing pattern of using command execution for host + interactions. Date/Author: 2026-04-07 / DevBoxer. + +- Decision: treat missing Git identity as a partial result rather than an + error. Rationale: the roadmap explicitly requires "Handle missing Git + identity with a warning rather than failure." The `GitIdentity` struct will + use `Option` for both `name` and `email`, and the container + application function will skip missing fields with a log warning rather than + returning an error. Date/Author: 2026-04-07 / DevBoxer. + +- Decision: reuse the existing `ContainerExecClient` trait for executing + `git config --global` within the container rather than introducing a new + abstraction. Rationale: the exec infrastructure already provides deterministic + test seams, error mapping, and async/sync bridging. Adding another layer would + increase complexity without benefit. + Date/Author: 2026-04-07 / DevBoxer. + +## Outcomes and retrospective + +(To be populated on completion.) + +## Context and orientation + +The Git identity configuration step is Step 3 in the design document's +execution flow (see `docs/podbot-design.md`, "Execution flow"): + +> 3. **Configure Git identity** by reading `user.name` and `user.email` from +> the host and executing `git config --global` within the container. + +Current engine code is organised under `src/engine/connection/` and supports: + +- socket resolution from config, environment, and defaults; +- connection establishment via Bollard; +- health-check verification with timeout handling; +- container creation with configurable security options; +- credential injection via tar archive upload; +- interactive and protocol-safe command execution. + +Relevant files for this work: + +- `src/engine/mod.rs` — engine module re-exports +- `src/engine/connection/mod.rs` — connection module and `EngineConnector` +- `src/engine/connection/exec/mod.rs` — `ContainerExecClient` trait and exec + types +- `src/error.rs` — semantic error types (`ContainerError::ExecFailed`) +- `src/api/mod.rs` — orchestration API stubs +- `docs/podbot-design.md` — execution flow description +- `docs/users-guide.md` — user documentation +- `docs/podbot-roadmap.md` — roadmap with Step 4.1 + +Key requirement translation for Step 4.1.1: + +- Read `user.name` from host Git configuration. +- Read `user.email` from host Git configuration. +- Execute `git config --global user.name` within the container. +- Execute `git config --global user.email` within the container. +- Handle missing Git identity with a warning rather than failure. + +## Plan of work + +### Stage A: Introduce Git identity module and types + +Create a dedicated submodule at `src/engine/connection/git_identity/` with: + +- `GitIdentity` struct: holds `Option` for `name` and `email`, with + predicate methods (`has_name`, `has_email`, `is_empty`, `is_complete`). +- `GitIdentityReader` trait: `fn read_git_identity(&self) -> GitIdentity`. +- `SystemGitIdentityReader`: production implementation using + `std::process::Command` to run `git config --get user.name` and + `git config --get user.email`. +- `configure_git_identity_async`: async function that reads host identity and + applies it to the container via `ContainerExecClient` exec calls, logging + warnings for missing fields. + +Target outcome: a testable Git identity module that compiles and is wired into +the engine re-exports. + +Validation gate: + +- new module compiles; +- no behavioural changes to existing tests. + +### Stage B: Implement host Git identity reading + +Implement `SystemGitIdentityReader`: + +- Run `git config --get user.name` via `std::process::Command`. +- Run `git config --get user.email` via `std::process::Command`. +- Non-zero exit codes (key not set) → `None` for that field. +- Command execution failures (Git not installed) → `None` for both fields. +- Trim whitespace from output. + +Target outcome: production reader that gracefully handles all host states. + +Validation gate: + +- unit tests validate reader behaviour with mock command executor. + +### Stage C: Implement container Git identity application + +Implement `configure_git_identity_async`: + +- Accept `ContainerExecClient`, container ID, and `GitIdentity`. +- For each present field, execute `git config --global user.name ` or + `git config --global user.email ` in the container. +- Log a warning (to stderr via `eprintln!` or a structured log) for each + missing field. +- If both fields are missing, log a single consolidated warning. +- Container exec failures for individual config commands are logged as warnings + and do not abort the overall operation. +- Return a `GitIdentityResult` indicating which fields were applied. + +Target outcome: deterministic, testable application of Git identity to a +container. + +Validation gate: + +- unit tests validate generated exec commands and warning behaviour. + +### Stage D: Add unit tests with rstest + +Implement module-local unit tests using `rstest` fixtures and parameterised +cases. Coverage must include: + +- happy path: both `user.name` and `user.email` present on host; +- happy path: identity applied to container via two exec calls; +- edge path: only `user.name` present (email missing); +- edge path: only `user.email` present (name missing); +- unhappy path: neither field present → warnings emitted, no exec calls; +- unhappy path: host Git not installed → graceful degradation; +- unhappy path: container exec fails for one field → other field still applied. + +Use mock implementations of `GitIdentityReader` and `ContainerExecClient` +for determinism. + +Validation gate: + +- `cargo test` for the targeted module passes before proceeding. + +### Stage E: Add behavioural tests with rstest-bdd v0.5.0 + +Add a dedicated feature file at `tests/features/git_identity.feature` and +scenario bindings covering observable behaviour in Given-When-Then form. + +Proposed scenarios: + +- Git identity configured on host → both fields applied to container; +- Only user name configured → name applied, warning for missing email; +- Only user email configured → email applied, warning for missing name; +- No Git identity configured → warning emitted, no failure; +- Container exec fails → graceful degradation with warning. + +Use state fixtures and step helpers in a new helper module mirroring the +existing engine BDD structure: + +- `tests/bdd_git_identity.rs` +- `tests/bdd_git_identity_helpers/mod.rs` +- `tests/bdd_git_identity_helpers/state.rs` +- `tests/bdd_git_identity_helpers/steps.rs` +- `tests/bdd_git_identity_helpers/assertions.rs` + +Validation gate: + +- new behavioural tests pass under `make test` with `rstest-bdd` macros. + +### Stage F: Documentation and roadmap updates + +Update `docs/podbot-design.md` with a concise subsection documenting: + +- Git identity reading strategy (command-based, not file parsing); +- container application approach (exec-based); +- warning-not-failure behaviour for missing identity. + +Update `docs/users-guide.md` with user-visible behaviour for Git identity +configuration: + +- when identity is applied; +- what happens when identity is missing; +- relationship to host `git config` settings. + +Update `docs/podbot-roadmap.md` by marking Step 4.1 entries as done once tests +and quality gates pass. + +Validation gate: + +- documentation is accurate to implemented behaviour; +- roadmap checkbox state matches delivered scope. + +### Stage G: Quality gates and commit + +Run required quality gates and retain logs, then commit atomically with a +message that explains what changed and why. + +Validation gate: + +- `make check-fmt`, `make lint`, and `make test` all exit zero; +- commit includes only intended files. + +## Concrete steps + +All commands run from repository root: `/home/user/project`. + +1. Create `src/engine/connection/git_identity/mod.rs` with types and traits. +2. Implement `SystemGitIdentityReader` using `std::process::Command`. +3. Implement `configure_git_identity_async` using `ContainerExecClient`. +4. Wire module into `src/engine/connection/mod.rs` and `src/engine/mod.rs`. +5. Add `rstest` unit tests for reader, applicator, and integration paths. +6. Create `tests/features/git_identity.feature` with BDD scenarios. +7. Create `tests/bdd_git_identity.rs` and helper modules. +8. Update `docs/podbot-design.md` with Git identity design. +9. Update `docs/users-guide.md` with user-facing behaviour. +10. Mark roadmap entry complete. +11. Run verification with log capture: + + ```sh + set -o pipefail + make check-fmt 2>&1 | tee /tmp/check-fmt-podbot-$(git branch --show).out + + set -o pipefail + make lint 2>&1 | tee /tmp/lint-podbot-$(git branch --show).out + + set -o pipefail + make test 2>&1 | tee /tmp/test-podbot-$(git branch --show).out + ``` + +12. Review diffs and commit. + +## Validation and acceptance + +Implementation is complete when all conditions below are true: + +- A callable `configure_git_identity_async` path exists and applies host Git + identity to a container. +- Host Git identity reading is abstracted behind a `GitIdentityReader` trait + with a production implementation and a mock for testing. +- Both `user.name` and `user.email` are read from the host and applied to + the container. +- Missing Git identity fields produce warnings rather than errors. +- Container exec failures for identity commands produce warnings rather than + aborting the operation. +- Unit tests cover happy, unhappy, and edge paths with deterministic mocks. +- Behavioural tests implemented with `rstest-bdd` v0.5.0 pass. +- `docs/podbot-design.md` records the Git identity configuration approach. +- `docs/users-guide.md` describes user-visible behaviour changes. +- `docs/podbot-roadmap.md` marks Step 4.1 entries as done. +- `make check-fmt`, `make lint`, and `make test` all pass. + +## Idempotence and recovery + +- Code-edit stages are safe to rerun; repeated edits should converge on the + same end state. +- If test failures occur, capture output logs, fix one class of failure at a + time, and rerun the specific failing command. +- If a tolerance trigger is hit, stop implementation, update `Decision log`, + and wait for direction before expanding scope. + +## Artefacts and notes + +Expected verification artefacts: + +- `/tmp/check-fmt-podbot-.out` +- `/tmp/lint-podbot-.out` +- `/tmp/test-podbot-.out` + +Keep these paths in the final implementation summary for traceability. + +## Interfaces and dependencies + +Planned interfaces (names may be refined during implementation, but intent is +fixed): + +- `GitIdentity`: data struct holding `Option` for `name` and `email`. +- `GitIdentityReader` trait: abstracts host Git configuration reading. +- `SystemGitIdentityReader`: production implementation using + `std::process::Command`. +- `configure_git_identity_async`: applies Git identity + to a container. +- `GitIdentityResult`: outcome struct reporting which fields were applied and + which were skipped. + +Dependencies remain unchanged. The implementation uses only `std::process` for +host Git reading and the existing `ContainerExecClient` trait for container +exec. + +## Revision note + +Initial draft created on 2026-04-07 to plan Step 4.1.1 Git identity +configuration work, including implementation, testing, documentation, and +roadmap-completion tasks. diff --git a/docs/podbot-design.md b/docs/podbot-design.md index efd58cc0..c1af326c 100644 --- a/docs/podbot-design.md +++ b/docs/podbot-design.md @@ -611,6 +611,48 @@ or whitespace-only, the operation fails with a semantic resolved layered configuration (`AppConfig.image`) at request-construction time, so this validation happens before any engine create call is attempted. +## Git identity configuration + +After container creation and credential injection, podbot configures Git +identity within the container so that commits made by agents carry the +correct author information. This corresponds to Step 3 in the execution +flow above. + +### Reading host identity + +The host Git identity is read by invoking `git config --get user.name` and +`git config --get user.email` as subprocesses. This approach respects the +full Git configuration precedence chain (system, global, local, worktree, +command-line, includes, and conditional includes) without reimplementing +Git's configuration resolution logic. + +The reading is abstracted behind a `GitIdentityReader` trait so that tests +can inject deterministic values without depending on the host Git +installation. + +### Applying identity to the container + +For each present field, podbot executes `git config --global user. +` within the container using the existing `ContainerExecClient` +trait seam. The commands run in detached mode with `tty = false`. + +### Graceful degradation + +Missing identity fields produce warnings rather than errors. The following +cases are handled: + +- **Neither field configured**: a single warning is emitted and no exec + calls are made. Agent execution continues normally. +- **One field missing**: the present field is applied; a warning is emitted + for the absent field. +- **Container exec failure**: each field is applied independently. If one + exec fails, the other is still attempted. Failures are reported as + warnings rather than aborting orchestration. + +This design ensures that operators without Git configuration (such as CI +environments where identity is managed externally) are not blocked from +using podbot. + ## Error handling Podbot defines semantic error enums in `src/error.rs` for configuration, diff --git a/docs/podbot-roadmap.md b/docs/podbot-roadmap.md index cdb2e13b..f6a04b35 100644 --- a/docs/podbot-roadmap.md +++ b/docs/podbot-roadmap.md @@ -266,17 +266,17 @@ Operations continue working after token refresh without intervention. Wire together the complete execution flow from container creation through agent startup. -### Step 4.1: Git identity configuration +### Step 4.1: Git identity configuration ✅ Configure Git identity within the container using host settings. **Tasks:** -- [ ] Read user.name from host Git configuration. -- [ ] Read user.email from host Git configuration. -- [ ] Execute git config --global user.name within the container. -- [ ] Execute git config --global user.email within the container. -- [ ] Handle missing Git identity with a warning rather than failure. +- [x] Read user.name from host Git configuration. +- [x] Read user.email from host Git configuration. +- [x] Execute git config --global user.name within the container. +- [x] Execute git config --global user.email within the container. +- [x] Handle missing Git identity with a warning rather than failure. **Completion criteria:** Git commits within the container use the configured identity. Missing identity produces a warning but does not block execution. diff --git a/docs/users-guide.md b/docs/users-guide.md index e870b9e1..e51151ab 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -546,6 +546,38 @@ Verification notes: 3. Compare permission bits for a representative file between host and container, for example, with `stat` on each side. +### Git identity configuration + +After creating the container and injecting credentials, podbot configures +Git identity inside the container by reading the host's `user.name` and +`user.email` from Git configuration and executing `git config --global` +within the container for each present field. + +**Behaviour:** + +- If both `user.name` and `user.email` are configured on the host, both + are applied to the container. +- If only one field is configured, the present field is applied and a + warning is emitted for the missing field. +- If neither field is configured, a warning is emitted and the container + uses its default Git identity. Agent execution continues normally. +- If the `git config --global` command fails inside the container (for + example, because `git` is not installed), a warning is emitted. The + failure does not prevent agent execution from proceeding. + +This step requires no user configuration. Podbot reads the identity from +the host's Git configuration automatically using `git config --get`. + +**Possible warning messages:** + +| Warning | Cause | +| ------------------------------------------------------ | ------------------------------------------ | +| "host Git user.name is not configured; skipping" | No `user.name` in host Git configuration | +| "host Git user.email is not configured; skipping" | No `user.email` in host Git configuration | +| "no Git identity configured on the host" | Neither `user.name` nor `user.email` found | +| "failed to create exec for git config user.name: ..." | Container exec failed for name field | +| "failed to create exec for git config user.email: ..." | Container exec failed for email field | + ## Security model Podbot's security model is based on capability-based containment: diff --git a/src/engine/connection/git_identity/mod.rs b/src/engine/connection/git_identity/mod.rs new file mode 100644 index 00000000..2068dd0b --- /dev/null +++ b/src/engine/connection/git_identity/mod.rs @@ -0,0 +1,417 @@ +//! Git identity reading and container configuration. +//! +//! This module reads `user.name` and `user.email` from the host Git +//! configuration and applies them inside a container via +//! `git config --global`. Missing identity fields produce warnings rather +//! than errors, following the roadmap requirement that absent identity must +//! not block agent execution. +//! +//! Host Git reading is abstracted behind the [`GitIdentityReader`] trait so +//! tests can inject deterministic values without depending on the host Git +//! installation. Container application reuses the existing +//! [`super::exec::ContainerExecClient`] trait seam. + +use std::process::Command; + +use super::exec::ContainerExecClient; +use super::EngineConnector; +use crate::error::{ContainerError, PodbotError}; + +// ============================================================================= +// GitIdentity data type +// ============================================================================= + +/// Git identity fields read from the host configuration. +/// +/// Both fields are optional because the host may have neither, one, or both +/// configured. Consumers should check individual fields rather than treating +/// the struct as all-or-nothing. +/// +/// # Examples +/// +/// ``` +/// use podbot::engine::GitIdentity; +/// +/// let identity = GitIdentity::new(Some("Alice".into()), Some("alice@example.com".into())); +/// assert!(identity.is_complete()); +/// assert!(!identity.is_empty()); +/// ``` +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct GitIdentity { + name: Option, + email: Option, +} + +impl GitIdentity { + /// Create a new Git identity from optional name and email values. + #[must_use] + pub fn new(name: Option, email: Option) -> Self { + Self { name, email } + } + + /// Return the configured user name, if any. + #[must_use] + pub fn name(&self) -> Option<&str> { + self.name.as_deref() + } + + /// Return the configured user email, if any. + #[must_use] + pub fn email(&self) -> Option<&str> { + self.email.as_deref() + } + + /// Return true when a user name is configured. + #[must_use] + pub fn has_name(&self) -> bool { + self.name.is_some() + } + + /// Return true when a user email is configured. + #[must_use] + pub fn has_email(&self) -> bool { + self.email.is_some() + } + + /// Return true when neither name nor email is configured. + #[must_use] + pub fn is_empty(&self) -> bool { + self.name.is_none() && self.email.is_none() + } + + /// Return true when both name and email are configured. + #[must_use] + pub fn is_complete(&self) -> bool { + self.name.is_some() && self.email.is_some() + } +} + +impl Default for GitIdentity { + fn default() -> Self { + Self { + name: None, + email: None, + } + } +} + +// ============================================================================= +// GitIdentityReader trait +// ============================================================================= + +/// Reads Git identity from the host configuration. +/// +/// Implementations query the host environment for `user.name` and +/// `user.email`. The trait enables dependency injection so tests can +/// provide deterministic values without depending on the host Git +/// installation. +pub trait GitIdentityReader { + /// Read the Git identity from the host configuration. + fn read_git_identity(&self) -> GitIdentity; +} + +// ============================================================================= +// SystemGitIdentityReader +// ============================================================================= + +/// Production implementation that reads Git identity using `git config --get`. +/// +/// This reader invokes `git config --get user.name` and +/// `git config --get user.email` as subprocesses. Non-zero exit codes +/// (indicating the key is not set) and command failures (Git not installed) +/// both result in `None` for the affected field. +pub struct SystemGitIdentityReader; + +impl SystemGitIdentityReader { + /// Create a new system Git identity reader. + #[must_use] + pub const fn new() -> Self { + Self + } +} + +impl Default for SystemGitIdentityReader { + fn default() -> Self { + Self::new() + } +} + +impl GitIdentityReader for SystemGitIdentityReader { + fn read_git_identity(&self) -> GitIdentity { + let name = read_git_config_value("user.name"); + let email = read_git_config_value("user.email"); + GitIdentity::new(name, email) + } +} + +/// Run `git config --get ` and return the trimmed output on success. +/// +/// Returns `None` when the command exits with a non-zero code (key not set) +/// or cannot be executed (Git not installed). +fn read_git_config_value(key: &str) -> Option { + let output = Command::new("git") + .args(["config", "--get", key]) + .output() + .ok()?; + + if !output.status.success() { + return None; + } + + let value = String::from_utf8_lossy(&output.stdout).trim().to_owned(); + if value.is_empty() { + return None; + } + + Some(value) +} + +// ============================================================================= +// GitIdentityResult +// ============================================================================= + +/// Outcome of applying Git identity to a container. +/// +/// Reports which fields were successfully applied and which were skipped. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct GitIdentityResult { + name_applied: bool, + email_applied: bool, + warnings: Vec, +} + +impl GitIdentityResult { + /// Return true if the user name was applied to the container. + #[must_use] + pub const fn name_applied(&self) -> bool { + self.name_applied + } + + /// Return true if the user email was applied to the container. + #[must_use] + pub const fn email_applied(&self) -> bool { + self.email_applied + } + + /// Return warnings emitted during identity configuration. + #[must_use] + pub fn warnings(&self) -> &[String] { + &self.warnings + } + + /// Return true if no fields were applied. + #[must_use] + pub const fn is_empty(&self) -> bool { + !self.name_applied && !self.email_applied + } +} + +// ============================================================================= +// Container Git identity application +// ============================================================================= + +impl EngineConnector { + /// Read host Git identity and apply it within a container. + /// + /// Reads `user.name` and `user.email` from the host using the provided + /// reader, then executes `git config --global` for each present field + /// inside the container. Missing fields and exec failures produce + /// warnings rather than errors. + /// + /// # Errors + /// + /// This function does not return errors for missing identity or exec + /// failures. It returns `ContainerError::ExecFailed` only when the + /// container ID is invalid (empty). + pub async fn configure_git_identity_async( + client: &C, + container_id: &str, + reader: &R, + ) -> Result + where + C: ContainerExecClient, + R: GitIdentityReader, + { + if container_id.trim().is_empty() { + return Err(PodbotError::from(ContainerError::ExecFailed { + container_id: String::from(container_id), + message: String::from("container ID must not be empty"), + })); + } + + let identity = reader.read_git_identity(); + apply_git_identity_async(client, container_id, &identity).await + } + + /// Apply a pre-read Git identity within a container. + /// + /// This variant accepts an already-read [`GitIdentity`] for callers + /// that have obtained the identity separately. + /// + /// # Errors + /// + /// Returns `ContainerError::ExecFailed` only when the container ID is + /// invalid (empty). + pub async fn apply_git_identity_async( + client: &C, + container_id: &str, + identity: &GitIdentity, + ) -> Result { + if container_id.trim().is_empty() { + return Err(PodbotError::from(ContainerError::ExecFailed { + container_id: String::from(container_id), + message: String::from("container ID must not be empty"), + })); + } + + apply_git_identity_async(client, container_id, identity).await + } +} + +/// Apply Git identity fields to a container, collecting warnings. +async fn apply_git_identity_async( + client: &C, + container_id: &str, + identity: &GitIdentity, +) -> Result { + let mut warnings = Vec::new(); + let mut name_applied = false; + let mut email_applied = false; + + if identity.is_empty() { + warnings.push(String::from( + "no Git identity configured on the host; \ + commits in the container will use the container default", + )); + return Ok(GitIdentityResult { + name_applied, + email_applied, + warnings, + }); + } + + if let Some(name) = identity.name() { + name_applied = + apply_single_config(client, container_id, "user.name", name, &mut warnings).await; + } else { + warnings.push(String::from( + "host Git user.name is not configured; skipping", + )); + } + + if let Some(email) = identity.email() { + email_applied = + apply_single_config(client, container_id, "user.email", email, &mut warnings).await; + } else { + warnings.push(String::from( + "host Git user.email is not configured; skipping", + )); + } + + Ok(GitIdentityResult { + name_applied, + email_applied, + warnings, + }) +} + +/// Execute a single `git config --global ` in the container. +/// +/// Returns `true` on success. On failure, appends a warning and returns +/// `false`. +async fn apply_single_config( + client: &C, + container_id: &str, + key: &str, + value: &str, + warnings: &mut Vec, +) -> bool { + let options = bollard::exec::CreateExecOptions:: { + attach_stdout: Some(false), + attach_stderr: Some(false), + attach_stdin: Some(false), + tty: Some(false), + cmd: Some(vec![ + String::from("git"), + String::from("config"), + String::from("--global"), + String::from(key), + String::from(value), + ]), + ..bollard::exec::CreateExecOptions::default() + }; + + let create_result = client.create_exec(container_id, options).await; + let exec_id = match create_result { + Ok(result) => result.id, + Err(error) => { + warnings.push(format!( + "failed to create exec for git config {key}: {error}" + )); + return false; + } + }; + + let start_result = client + .start_exec( + &exec_id, + Some(bollard::exec::StartExecOptions { + detach: true, + tty: false, + output_capacity: None, + }), + ) + .await; + + if let Err(error) = start_result { + warnings.push(format!( + "failed to start exec for git config {key}: {error}" + )); + return false; + } + + // Poll for completion. + if let Err(error) = wait_for_exec_completion(client, &exec_id).await { + warnings.push(format!( + "failed to inspect exec for git config {key}: {error}" + )); + return false; + } + + true +} + +/// Wait for a detached exec to complete by polling inspect. +async fn wait_for_exec_completion( + client: &C, + exec_id: &str, +) -> Result<(), String> { + loop { + let inspect = client + .inspect_exec(exec_id) + .await + .map_err(|e| format!("{e}"))?; + + if let Some(running) = inspect.running { + if !running { + // Check exit code for non-zero (Git config failure). + if let Some(code) = inspect.exit_code { + if code != 0 { + return Err(format!( + "git config exited with code {code}" + )); + } + } + return Ok(()); + } + } + + tokio::time::sleep(std::time::Duration::from_millis( + super::exec::EXEC_INSPECT_POLL_INTERVAL_MS, + )) + .await; + } +} + +#[cfg(test)] +mod tests; diff --git a/src/engine/connection/git_identity/tests.rs b/src/engine/connection/git_identity/tests.rs new file mode 100644 index 00000000..efc57c34 --- /dev/null +++ b/src/engine/connection/git_identity/tests.rs @@ -0,0 +1,540 @@ +//! Unit tests for Git identity reading and container configuration. + +use std::sync::{Arc, Mutex}; + +use bollard::exec::{ + CreateExecOptions, CreateExecResults, ResizeExecOptions, StartExecOptions, StartExecResults, +}; +use bollard::models::ExecInspectResponse; +use rstest::{fixture, rstest}; + +use super::*; +use crate::engine::{ + ContainerExecClient, CreateExecFuture, InspectExecFuture, ResizeExecFuture, StartExecFuture, +}; + +// ============================================================================= +// Mock GitIdentityReader +// ============================================================================= + +/// Deterministic reader that returns pre-configured identity values. +struct MockGitIdentityReader { + identity: GitIdentity, +} + +impl MockGitIdentityReader { + fn new(name: Option<&str>, email: Option<&str>) -> Self { + Self { + identity: GitIdentity::new(name.map(String::from), email.map(String::from)), + } + } +} + +impl GitIdentityReader for MockGitIdentityReader { + fn read_git_identity(&self) -> GitIdentity { + self.identity.clone() + } +} + +// ============================================================================= +// Mock ContainerExecClient +// ============================================================================= + +/// Recorded exec invocation for assertion. +#[derive(Debug, Clone)] +struct ExecCall { + container_id: String, + cmd: Vec, +} + +/// Mock exec client that records calls and returns configurable results. +struct MockExecClient { + calls: Arc>>, + create_fail: bool, + start_fail: bool, + inspect_fail: bool, + exit_code: i64, +} + +impl MockExecClient { + fn new() -> Self { + Self { + calls: Arc::new(Mutex::new(Vec::new())), + create_fail: false, + start_fail: false, + inspect_fail: false, + exit_code: 0, + } + } + + fn with_create_failure(mut self) -> Self { + self.create_fail = true; + self + } + + fn with_start_failure(mut self) -> Self { + self.start_fail = true; + self + } + + fn with_inspect_failure(mut self) -> Self { + self.inspect_fail = true; + self + } + + fn with_exit_code(mut self, code: i64) -> Self { + self.exit_code = code; + self + } + + fn calls(&self) -> Vec { + self.calls.lock().expect("lock poisoned").clone() + } +} + +impl ContainerExecClient for MockExecClient { + fn create_exec( + &self, + container_id: &str, + options: CreateExecOptions, + ) -> CreateExecFuture<'_> { + let cid = String::from(container_id); + let cmd = options.cmd.clone().unwrap_or_default(); + self.calls + .lock() + .expect("lock poisoned") + .push(ExecCall { + container_id: cid, + cmd, + }); + + let fail = self.create_fail; + Box::pin(async move { + if fail { + Err(bollard::errors::Error::DockerResponseServerError { + status_code: 500, + message: String::from("mock create failure"), + }) + } else { + Ok(CreateExecResults { + id: String::from("mock-exec-id"), + }) + } + }) + } + + fn start_exec( + &self, + _exec_id: &str, + _options: Option, + ) -> StartExecFuture<'_> { + let fail = self.start_fail; + Box::pin(async move { + if fail { + Err(bollard::errors::Error::DockerResponseServerError { + status_code: 500, + message: String::from("mock start failure"), + }) + } else { + Ok(StartExecResults::Detached) + } + }) + } + + fn inspect_exec(&self, _exec_id: &str) -> InspectExecFuture<'_> { + let fail = self.inspect_fail; + let exit_code = self.exit_code; + Box::pin(async move { + if fail { + Err(bollard::errors::Error::DockerResponseServerError { + status_code: 500, + message: String::from("mock inspect failure"), + }) + } else { + Ok(ExecInspectResponse { + running: Some(false), + exit_code: Some(exit_code), + ..ExecInspectResponse::default() + }) + } + }) + } + + fn resize_exec( + &self, + _exec_id: &str, + _options: ResizeExecOptions, + ) -> ResizeExecFuture<'_> { + Box::pin(async { Ok(()) }) + } +} + +// ============================================================================= +// Fixtures +// ============================================================================= + +#[fixture] +fn container_id() -> &'static str { + "test-container-abc123" +} + +#[fixture] +fn mock_client() -> MockExecClient { + MockExecClient::new() +} + +// ============================================================================= +// GitIdentity tests +// ============================================================================= + +#[rstest] +fn identity_with_both_fields_is_complete() { + let identity = GitIdentity::new( + Some(String::from("Alice")), + Some(String::from("alice@example.com")), + ); + assert!(identity.is_complete()); + assert!(!identity.is_empty()); + assert!(identity.has_name()); + assert!(identity.has_email()); + assert_eq!(identity.name(), Some("Alice")); + assert_eq!(identity.email(), Some("alice@example.com")); +} + +#[rstest] +fn identity_with_name_only_is_partial() { + let identity = GitIdentity::new(Some(String::from("Alice")), None); + assert!(!identity.is_complete()); + assert!(!identity.is_empty()); + assert!(identity.has_name()); + assert!(!identity.has_email()); +} + +#[rstest] +fn identity_with_email_only_is_partial() { + let identity = GitIdentity::new(None, Some(String::from("alice@example.com"))); + assert!(!identity.is_complete()); + assert!(!identity.is_empty()); + assert!(!identity.has_name()); + assert!(identity.has_email()); +} + +#[rstest] +fn empty_identity_reports_correctly() { + let identity = GitIdentity::default(); + assert!(identity.is_empty()); + assert!(!identity.is_complete()); + assert!(!identity.has_name()); + assert!(!identity.has_email()); + assert_eq!(identity.name(), None); + assert_eq!(identity.email(), None); +} + +// ============================================================================= +// MockGitIdentityReader tests +// ============================================================================= + +#[rstest] +fn mock_reader_returns_configured_identity() { + let reader = MockGitIdentityReader::new(Some("Bob"), Some("bob@example.com")); + let identity = reader.read_git_identity(); + assert_eq!(identity.name(), Some("Bob")); + assert_eq!(identity.email(), Some("bob@example.com")); +} + +#[rstest] +fn mock_reader_returns_empty_identity() { + let reader = MockGitIdentityReader::new(None, None); + let identity = reader.read_git_identity(); + assert!(identity.is_empty()); +} + +// ============================================================================= +// configure_git_identity_async tests +// ============================================================================= + +#[rstest] +#[tokio::test] +async fn apply_both_fields_executes_two_config_commands( + container_id: &str, + mock_client: MockExecClient, +) { + let identity = GitIdentity::new( + Some(String::from("Alice")), + Some(String::from("alice@example.com")), + ); + let result = EngineConnector::apply_git_identity_async( + &mock_client, + container_id, + &identity, + ) + .await + .expect("should succeed"); + + assert!(result.name_applied()); + assert!(result.email_applied()); + assert!(result.warnings().is_empty()); + + let calls = mock_client.calls(); + assert_eq!(calls.len(), 2, "expected two exec calls"); + assert_eq!( + calls[0].cmd, + vec!["git", "config", "--global", "user.name", "Alice"] + ); + assert_eq!( + calls[1].cmd, + vec!["git", "config", "--global", "user.email", "alice@example.com"] + ); +} + +#[rstest] +#[tokio::test] +async fn apply_name_only_warns_about_missing_email( + container_id: &str, + mock_client: MockExecClient, +) { + let identity = GitIdentity::new(Some(String::from("Alice")), None); + let result = EngineConnector::apply_git_identity_async( + &mock_client, + container_id, + &identity, + ) + .await + .expect("should succeed"); + + assert!(result.name_applied()); + assert!(!result.email_applied()); + assert_eq!(result.warnings().len(), 1); + assert!(result.warnings()[0].contains("user.email")); + + let calls = mock_client.calls(); + assert_eq!(calls.len(), 1, "expected one exec call for name only"); +} + +#[rstest] +#[tokio::test] +async fn apply_email_only_warns_about_missing_name( + container_id: &str, + mock_client: MockExecClient, +) { + let identity = GitIdentity::new(None, Some(String::from("alice@example.com"))); + let result = EngineConnector::apply_git_identity_async( + &mock_client, + container_id, + &identity, + ) + .await + .expect("should succeed"); + + assert!(!result.name_applied()); + assert!(result.email_applied()); + assert_eq!(result.warnings().len(), 1); + assert!(result.warnings()[0].contains("user.name")); + + let calls = mock_client.calls(); + assert_eq!(calls.len(), 1, "expected one exec call for email only"); +} + +#[rstest] +#[tokio::test] +async fn apply_empty_identity_warns_and_makes_no_exec_calls( + container_id: &str, + mock_client: MockExecClient, +) { + let identity = GitIdentity::default(); + let result = EngineConnector::apply_git_identity_async( + &mock_client, + container_id, + &identity, + ) + .await + .expect("should succeed"); + + assert!(!result.name_applied()); + assert!(!result.email_applied()); + assert!(result.is_empty()); + assert_eq!(result.warnings().len(), 1); + assert!(result.warnings()[0].contains("no Git identity configured")); + + let calls = mock_client.calls(); + assert!(calls.is_empty(), "expected no exec calls for empty identity"); +} + +#[rstest] +#[tokio::test] +async fn configure_reads_identity_and_applies_it(container_id: &str) { + let client = MockExecClient::new(); + let reader = MockGitIdentityReader::new(Some("Carol"), Some("carol@example.com")); + + let result = EngineConnector::configure_git_identity_async( + &client, + container_id, + &reader, + ) + .await + .expect("should succeed"); + + assert!(result.name_applied()); + assert!(result.email_applied()); + assert!(result.warnings().is_empty()); +} + +#[rstest] +#[tokio::test] +async fn configure_with_empty_reader_warns(container_id: &str) { + let client = MockExecClient::new(); + let reader = MockGitIdentityReader::new(None, None); + + let result = EngineConnector::configure_git_identity_async( + &client, + container_id, + &reader, + ) + .await + .expect("should succeed"); + + assert!(result.is_empty()); + assert!(!result.warnings().is_empty()); +} + +#[rstest] +#[tokio::test] +async fn empty_container_id_returns_error() { + let client = MockExecClient::new(); + let reader = MockGitIdentityReader::new(Some("Alice"), Some("alice@example.com")); + + let err = EngineConnector::configure_git_identity_async(&client, "", &reader) + .await + .expect_err("should fail with empty container ID"); + + assert!(err.to_string().contains("container ID must not be empty")); +} + +#[rstest] +#[tokio::test] +async fn whitespace_container_id_returns_error() { + let client = MockExecClient::new(); + let identity = GitIdentity::new( + Some(String::from("Alice")), + Some(String::from("alice@example.com")), + ); + + let err = EngineConnector::apply_git_identity_async(&client, " ", &identity) + .await + .expect_err("should fail with whitespace container ID"); + + assert!(err.to_string().contains("container ID must not be empty")); +} + +#[rstest] +#[tokio::test] +async fn create_exec_failure_produces_warning_not_error(container_id: &str) { + let client = MockExecClient::new().with_create_failure(); + let identity = GitIdentity::new( + Some(String::from("Alice")), + Some(String::from("alice@example.com")), + ); + + let result = EngineConnector::apply_git_identity_async( + &client, + container_id, + &identity, + ) + .await + .expect("should succeed despite exec failures"); + + assert!(!result.name_applied()); + assert!(!result.email_applied()); + assert!(result.warnings().len() >= 2); + assert!(result.warnings()[0].contains("failed to create exec")); +} + +#[rstest] +#[tokio::test] +async fn start_exec_failure_produces_warning_not_error(container_id: &str) { + let client = MockExecClient::new().with_start_failure(); + let identity = GitIdentity::new( + Some(String::from("Alice")), + Some(String::from("alice@example.com")), + ); + + let result = EngineConnector::apply_git_identity_async( + &client, + container_id, + &identity, + ) + .await + .expect("should succeed despite exec failures"); + + assert!(!result.name_applied()); + assert!(!result.email_applied()); + assert!(result.warnings().len() >= 2); + assert!(result.warnings()[0].contains("failed to start exec")); +} + +#[rstest] +#[tokio::test] +async fn inspect_exec_failure_produces_warning_not_error(container_id: &str) { + let client = MockExecClient::new().with_inspect_failure(); + let identity = GitIdentity::new( + Some(String::from("Alice")), + Some(String::from("alice@example.com")), + ); + + let result = EngineConnector::apply_git_identity_async( + &client, + container_id, + &identity, + ) + .await + .expect("should succeed despite inspect failures"); + + assert!(!result.name_applied()); + assert!(!result.email_applied()); + assert!(result.warnings().len() >= 2); + assert!(result.warnings()[0].contains("failed to inspect exec")); +} + +#[rstest] +#[tokio::test] +async fn nonzero_exit_code_produces_warning_not_error(container_id: &str) { + let client = MockExecClient::new().with_exit_code(1); + let identity = GitIdentity::new( + Some(String::from("Alice")), + Some(String::from("alice@example.com")), + ); + + let result = EngineConnector::apply_git_identity_async( + &client, + container_id, + &identity, + ) + .await + .expect("should succeed despite non-zero exit code"); + + assert!(!result.name_applied()); + assert!(!result.email_applied()); + assert!(result.warnings().len() >= 2); + assert!(result.warnings()[0].contains("exited with code 1")); +} + +#[rstest] +#[tokio::test] +async fn container_id_is_passed_to_exec_calls(mock_client: MockExecClient) { + let identity = GitIdentity::new( + Some(String::from("Alice")), + Some(String::from("alice@example.com")), + ); + + let _ = EngineConnector::apply_git_identity_async( + &mock_client, + "my-special-container", + &identity, + ) + .await; + + let calls = mock_client.calls(); + for call in &calls { + assert_eq!(call.container_id, "my-special-container"); + } +} diff --git a/src/engine/connection/mod.rs b/src/engine/connection/mod.rs index 989142ba..dd6c55f9 100644 --- a/src/engine/connection/mod.rs +++ b/src/engine/connection/mod.rs @@ -7,6 +7,7 @@ mod create_container; mod error_classification; mod exec; +mod git_identity; mod health_check; mod upload_credentials; @@ -25,6 +26,9 @@ pub use exec::{ ContainerExecClient, CreateExecFuture, ExecMode, ExecRequest, ExecResult, InspectExecFuture, ResizeExecFuture, StartExecFuture, }; +pub use git_identity::{ + GitIdentity, GitIdentityReader, GitIdentityResult, SystemGitIdentityReader, +}; pub use upload_credentials::{ ContainerUploader, CredentialUploadRequest, CredentialUploadResult, UploadToContainerFuture, }; diff --git a/src/engine/mod.rs b/src/engine/mod.rs index b00ca32e..fb2aecc0 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -17,6 +17,7 @@ mod connection; pub use connection::{ ContainerCreator, ContainerExecClient, ContainerSecurityOptions, ContainerUploader, CreateContainerFuture, CreateContainerRequest, CreateExecFuture, CredentialUploadRequest, - CredentialUploadResult, EngineConnector, ExecMode, ExecRequest, ExecResult, InspectExecFuture, - ResizeExecFuture, SelinuxLabelMode, SocketResolver, StartExecFuture, UploadToContainerFuture, + CredentialUploadResult, EngineConnector, ExecMode, ExecRequest, ExecResult, GitIdentity, + GitIdentityReader, GitIdentityResult, InspectExecFuture, ResizeExecFuture, SelinuxLabelMode, + SocketResolver, StartExecFuture, SystemGitIdentityReader, UploadToContainerFuture, }; diff --git a/tests/bdd_git_identity.rs b/tests/bdd_git_identity.rs new file mode 100644 index 00000000..fc955443 --- /dev/null +++ b/tests/bdd_git_identity.rs @@ -0,0 +1,46 @@ +//! Behavioural tests for Git identity configuration. + +mod bdd_git_identity_helpers; + +pub use bdd_git_identity_helpers::{GitIdentityState, git_identity_state}; +use rstest_bdd_macros::scenario; + +#[scenario( + path = "tests/features/git_identity.feature", + name = "Both name and email configured on host" +)] +fn both_name_and_email_configured(git_identity_state: GitIdentityState) { + let _ = git_identity_state; +} + +#[scenario( + path = "tests/features/git_identity.feature", + name = "Only user name configured on host" +)] +fn only_user_name_configured(git_identity_state: GitIdentityState) { + let _ = git_identity_state; +} + +#[scenario( + path = "tests/features/git_identity.feature", + name = "Only user email configured on host" +)] +fn only_user_email_configured(git_identity_state: GitIdentityState) { + let _ = git_identity_state; +} + +#[scenario( + path = "tests/features/git_identity.feature", + name = "No Git identity configured on host" +)] +fn no_git_identity_configured(git_identity_state: GitIdentityState) { + let _ = git_identity_state; +} + +#[scenario( + path = "tests/features/git_identity.feature", + name = "Container exec fails for one field" +)] +fn container_exec_fails_for_one_field(git_identity_state: GitIdentityState) { + let _ = git_identity_state; +} diff --git a/tests/bdd_git_identity_helpers/assertions.rs b/tests/bdd_git_identity_helpers/assertions.rs new file mode 100644 index 00000000..29b7915b --- /dev/null +++ b/tests/bdd_git_identity_helpers/assertions.rs @@ -0,0 +1,115 @@ +//! Then step implementations for Git identity scenarios. + +use rstest_bdd_macros::then; + +use super::state::{GitIdentityState, StepResult}; + +// ============================================================================= +// Helper functions +// ============================================================================= + +/// Retrieve the outcome from state or return an error. +fn get_outcome(state: &GitIdentityState) -> StepResult { + state + .outcome + .get() + .and_then(|opt| opt.clone()) + .ok_or_else(|| String::from("outcome not captured")) +} + +// ============================================================================= +// Then steps +// ============================================================================= + +#[then("Git identity configuration succeeds")] +fn git_identity_configuration_succeeds( + git_identity_state: &GitIdentityState, +) -> StepResult<()> { + let _outcome = get_outcome(git_identity_state)?; + // If we have an outcome, the operation succeeded (errors return Err in When). + Ok(()) +} + +#[then("user.name is applied to the container")] +fn user_name_is_applied( + git_identity_state: &GitIdentityState, +) -> StepResult<()> { + let outcome = get_outcome(git_identity_state)?; + if outcome.name_applied() { + Ok(()) + } else { + Err(String::from("expected user.name to be applied")) + } +} + +#[then("user.name is not applied to the container")] +fn user_name_is_not_applied( + git_identity_state: &GitIdentityState, +) -> StepResult<()> { + let outcome = get_outcome(git_identity_state)?; + if !outcome.name_applied() { + Ok(()) + } else { + Err(String::from("expected user.name to not be applied")) + } +} + +#[then("user.email is applied to the container")] +fn user_email_is_applied( + git_identity_state: &GitIdentityState, +) -> StepResult<()> { + let outcome = get_outcome(git_identity_state)?; + if outcome.email_applied() { + Ok(()) + } else { + Err(String::from("expected user.email to be applied")) + } +} + +#[then("user.email is not applied to the container")] +fn user_email_is_not_applied( + git_identity_state: &GitIdentityState, +) -> StepResult<()> { + let outcome = get_outcome(git_identity_state)?; + if !outcome.email_applied() { + Ok(()) + } else { + Err(String::from("expected user.email to not be applied")) + } +} + +#[then("no warnings are emitted")] +fn no_warnings_are_emitted( + git_identity_state: &GitIdentityState, +) -> StepResult<()> { + let outcome = get_outcome(git_identity_state)?; + if outcome.warnings().is_empty() { + Ok(()) + } else { + Err(format!( + "expected no warnings, got: {:?}", + outcome.warnings() + )) + } +} + +#[then("a warning mentions {text}")] +fn a_warning_mentions( + git_identity_state: &GitIdentityState, + text: String, +) -> StepResult<()> { + let outcome = get_outcome(git_identity_state)?; + let found = outcome + .warnings() + .iter() + .any(|w| w.contains(&text)); + + if found { + Ok(()) + } else { + Err(format!( + "expected a warning containing '{text}', got: {:?}", + outcome.warnings() + )) + } +} diff --git a/tests/bdd_git_identity_helpers/mod.rs b/tests/bdd_git_identity_helpers/mod.rs new file mode 100644 index 00000000..1c36137b --- /dev/null +++ b/tests/bdd_git_identity_helpers/mod.rs @@ -0,0 +1,7 @@ +//! Behavioural step helpers for Git identity configuration scenarios. + +mod assertions; +mod state; +mod steps; + +pub use state::{GitIdentityState, git_identity_state}; diff --git a/tests/bdd_git_identity_helpers/state.rs b/tests/bdd_git_identity_helpers/state.rs new file mode 100644 index 00000000..46a93d10 --- /dev/null +++ b/tests/bdd_git_identity_helpers/state.rs @@ -0,0 +1,39 @@ +//! Shared behavioural-test state for Git identity configuration scenarios. + +use podbot::engine::GitIdentityResult; +use rstest::fixture; +use rstest_bdd::Slot; +use rstest_bdd_macros::ScenarioState; + +/// Step result type for Git identity BDD tests. +pub type StepResult = Result; + +/// Shared scenario state for Git identity behavioural tests. +#[derive(Default, ScenarioState)] +pub struct GitIdentityState { + /// Host Git user.name value (None = not configured). + pub(crate) host_name: Slot>, + + /// Host Git user.email value (None = not configured). + pub(crate) host_email: Slot>, + + /// Whether the mocked container exec should fail. + pub(crate) should_fail_exec: Slot, + + /// Outcome of the most recent Git identity configuration attempt. + pub(crate) outcome: Slot>, + + /// Captured exec commands forwarded to the container. + pub(crate) captured_commands: Slot>>, +} + +/// Fixture providing fresh state for each Git identity scenario. +#[fixture] +pub fn git_identity_state() -> GitIdentityState { + let state = GitIdentityState::default(); + state.host_name.set(None); + state.host_email.set(None); + state.should_fail_exec.set(false); + state.captured_commands.set(Vec::new()); + state +} diff --git a/tests/bdd_git_identity_helpers/steps.rs b/tests/bdd_git_identity_helpers/steps.rs new file mode 100644 index 00000000..5f57b00e --- /dev/null +++ b/tests/bdd_git_identity_helpers/steps.rs @@ -0,0 +1,175 @@ +//! Given and When step implementations for Git identity scenarios. + +use std::sync::{Arc, Mutex}; + +use bollard::exec::{ + CreateExecOptions, CreateExecResults, ResizeExecOptions, StartExecOptions, StartExecResults, +}; +use bollard::models::ExecInspectResponse; +use podbot::engine::{ + ContainerExecClient, CreateExecFuture, EngineConnector, GitIdentity, GitIdentityReader, + InspectExecFuture, ResizeExecFuture, StartExecFuture, +}; +use rstest_bdd_macros::{given, when}; + +use super::state::{GitIdentityState, StepResult}; + +// ============================================================================= +// Mock types +// ============================================================================= + +/// Deterministic reader that returns state-configured identity values. +struct MockReader { + identity: GitIdentity, +} + +impl GitIdentityReader for MockReader { + fn read_git_identity(&self) -> GitIdentity { + self.identity.clone() + } +} + +/// Mock exec client that records commands and optionally fails. +struct MockExecClient { + commands: Arc>>>, + should_fail: bool, +} + +impl ContainerExecClient for MockExecClient { + fn create_exec( + &self, + _container_id: &str, + options: CreateExecOptions, + ) -> CreateExecFuture<'_> { + let cmd = options.cmd.clone().unwrap_or_default(); + self.commands + .lock() + .expect("lock poisoned") + .push(cmd); + + let fail = self.should_fail; + Box::pin(async move { + if fail { + Err(bollard::errors::Error::DockerResponseServerError { + status_code: 500, + message: String::from("mock exec failure"), + }) + } else { + Ok(CreateExecResults { + id: String::from("bdd-exec-id"), + }) + } + }) + } + + fn start_exec( + &self, + _exec_id: &str, + _options: Option, + ) -> StartExecFuture<'_> { + Box::pin(async { Ok(StartExecResults::Detached) }) + } + + fn inspect_exec(&self, _exec_id: &str) -> InspectExecFuture<'_> { + Box::pin(async { + Ok(ExecInspectResponse { + running: Some(false), + exit_code: Some(0), + ..ExecInspectResponse::default() + }) + }) + } + + fn resize_exec( + &self, + _exec_id: &str, + _options: ResizeExecOptions, + ) -> ResizeExecFuture<'_> { + Box::pin(async { Ok(()) }) + } +} + +// ============================================================================= +// Given steps +// ============================================================================= + +#[given("host Git user name is configured as {name}")] +fn host_git_user_name_configured( + git_identity_state: &GitIdentityState, + name: String, +) { + git_identity_state.host_name.set(Some(name)); +} + +#[given("host Git user name is absent")] +fn host_git_user_name_absent(git_identity_state: &GitIdentityState) { + git_identity_state.host_name.set(None); +} + +#[given("host Git user email is configured as {email}")] +fn host_git_user_email_configured( + git_identity_state: &GitIdentityState, + email: String, +) { + git_identity_state.host_email.set(Some(email)); +} + +#[given("host Git user email is absent")] +fn host_git_user_email_absent(git_identity_state: &GitIdentityState) { + git_identity_state.host_email.set(None); +} + +#[given("container exec will fail")] +fn container_exec_will_fail(git_identity_state: &GitIdentityState) { + git_identity_state.should_fail_exec.set(true); +} + +// ============================================================================= +// When steps +// ============================================================================= + +#[when("Git identity is applied to the container")] +fn git_identity_is_applied( + git_identity_state: &GitIdentityState, +) -> StepResult<()> { + let name = git_identity_state.host_name.get().flatten(); + let email = git_identity_state.host_email.get().flatten(); + + let should_fail = git_identity_state + .should_fail_exec + .get() + .unwrap_or(false); + + let commands = Arc::new(Mutex::new(Vec::new())); + let client = MockExecClient { + commands: Arc::clone(&commands), + should_fail, + }; + + let reader = MockReader { + identity: GitIdentity::new(name, email), + }; + + let runtime = tokio::runtime::Runtime::new() + .map_err(|e| format!("failed to create runtime: {e}"))?; + + let result = runtime.block_on(EngineConnector::configure_git_identity_async( + &client, + "bdd-container-id", + &reader, + )); + + match result { + Ok(identity_result) => { + git_identity_state.outcome.set(Some(identity_result)); + } + Err(error) => { + return Err(format!("unexpected error: {error}")); + } + } + + let captured = commands.lock().expect("lock poisoned").clone(); + git_identity_state.captured_commands.set(captured); + + Ok(()) +} diff --git a/tests/features/git_identity.feature b/tests/features/git_identity.feature new file mode 100644 index 00000000..0deceae0 --- /dev/null +++ b/tests/features/git_identity.feature @@ -0,0 +1,48 @@ +Feature: Git identity configuration + + Configure Git identity within the container using host settings. + Missing identity fields produce warnings rather than failures. + + Scenario: Both name and email configured on host + Given host Git user name is configured as Alice + And host Git user email is configured as alice@example.com + When Git identity is applied to the container + Then Git identity configuration succeeds + And user.name is applied to the container + And user.email is applied to the container + And no warnings are emitted + + Scenario: Only user name configured on host + Given host Git user name is configured as Bob + And host Git user email is absent + When Git identity is applied to the container + Then Git identity configuration succeeds + And user.name is applied to the container + And user.email is not applied to the container + And a warning mentions user.email + + Scenario: Only user email configured on host + Given host Git user name is absent + And host Git user email is configured as carol@example.com + When Git identity is applied to the container + Then Git identity configuration succeeds + And user.name is not applied to the container + And user.email is applied to the container + And a warning mentions user.name + + Scenario: No Git identity configured on host + Given host Git user name is absent + And host Git user email is absent + When Git identity is applied to the container + Then Git identity configuration succeeds + And user.name is not applied to the container + And user.email is not applied to the container + And a warning mentions no Git identity configured + + Scenario: Container exec fails for one field + Given host Git user name is configured as Dave + And host Git user email is configured as dave@example.com + And container exec will fail + When Git identity is applied to the container + Then Git identity configuration succeeds + And a warning mentions failed