From 313d701a6337eb0591d02a1b1770623cd7f79ef2 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 7 Apr 2026 01:38:06 +0000 Subject: [PATCH] feat(engine/connection/git_identity): add Git identity configuration from host to container Implement Step 4.1: read user.name and user.email from host Git config and apply inside container via git config --global. Missing fields emit warnings, do not fail. Introduce GitIdentityReader trait with SystemGitIdentityReader implementation using git config commands. Apply identity to container using ContainerExecClient exec calls. Add comprehensive unit and behavioral tests covering all cases, including partial and missing identity and container exec failures. Update design and user docs to reflect Git identity configuration behaviour. Mark Step 4.1 as done in roadmap. This feature ensures commits inside containers carry correct author info derived from host configuration, enabling smoother agent operation without requiring manual identity configuration or causing failures when identity is missing. Co-authored-by: devboxerhub[bot] --- .../4-1-1-git-identity-configuration.md | 413 ++++++++++++++ docs/podbot-design.md | 42 ++ docs/podbot-roadmap.md | 12 +- docs/users-guide.md | 32 ++ src/engine/connection/git_identity/mod.rs | 417 ++++++++++++++ src/engine/connection/git_identity/tests.rs | 540 ++++++++++++++++++ src/engine/connection/mod.rs | 4 + src/engine/mod.rs | 5 +- tests/bdd_git_identity.rs | 46 ++ tests/bdd_git_identity_helpers/assertions.rs | 115 ++++ tests/bdd_git_identity_helpers/mod.rs | 7 + tests/bdd_git_identity_helpers/state.rs | 39 ++ tests/bdd_git_identity_helpers/steps.rs | 175 ++++++ tests/features/git_identity.feature | 48 ++ 14 files changed, 1887 insertions(+), 8 deletions(-) create mode 100644 docs/execplans/4-1-1-git-identity-configuration.md create mode 100644 src/engine/connection/git_identity/mod.rs create mode 100644 src/engine/connection/git_identity/tests.rs create mode 100644 tests/bdd_git_identity.rs create mode 100644 tests/bdd_git_identity_helpers/assertions.rs create mode 100644 tests/bdd_git_identity_helpers/mod.rs create mode 100644 tests/bdd_git_identity_helpers/state.rs create mode 100644 tests/bdd_git_identity_helpers/steps.rs create mode 100644 tests/features/git_identity.feature 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