From da799198b2a2cebbd1e0880b300f1127947f88d5 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 8 Sep 2026 02:09:23 +0000 Subject: [PATCH] Document the completed Seriatim remediation --- docs/conventions.md | 9 ++ .../0001-minimal-module-driven-ansible.md | 6 +- .../0001-seriatim-source-installation.md | 70 ++++----- docs/roadmap/implementation.md | 57 ++++--- docs/roadmap/mvp.md | 16 +- docs/roadmap/remediation.md | 148 ++++++++++++++++-- docs/status.md | 8 +- 7 files changed, 225 insertions(+), 89 deletions(-) diff --git a/docs/conventions.md b/docs/conventions.md index 119dabf..8e1a294 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -37,6 +37,15 @@ - Trust Ansible's abstraction boundary. A module that manages state owns the checks necessary to manage that state. Inspect it beforehand only when the observed state is independently required. +- State inspection is justified when its result is necessary to decide whether + an imperative operation must run, such as checking whether an arbitrary + build command's output exists. +- Production role tasks establish required application state; they do not test + the role's own implementation. Put implementation verification in + repository-local tests unless the specification explicitly requires runtime + validation during convergence. +- Apply `become: true` to individual tasks that require elevated privileges. + Do not require callers to escalate an entire play merely to use a role. - Complexity requires a requirement. Every additional task, conditional, registered variable, or state probe must correspond to behavior the role is required to provide. diff --git a/docs/decisions/0001-minimal-module-driven-ansible.md b/docs/decisions/0001-minimal-module-driven-ansible.md index 618e30b..ac4312f 100644 --- a/docs/decisions/0001-minimal-module-driven-ansible.md +++ b/docs/decisions/0001-minimal-module-driven-ansible.md @@ -29,8 +29,10 @@ requirement falls outside that module contract. In particular: - Do not use `stat`, package queries, content comparisons, or account lookups merely to decide whether to invoke an idempotent module. - Gather state when it is genuinely needed to control application behavior, - such as determining whether a locally built executable is missing or has - the wrong application version. + such as determining whether an imperative build command's output is missing. +- Treat a successful module invocation as establishing its documented + postcondition. Verify the role's implementation in repository-local tests + rather than adding production tasks that immediately re-prove it. - Do not introduce fingerprints, checksums, sidecar state, staged replacement, custom drift recovery, or abstractions without an explicit acceptance criterion and rationale. diff --git a/docs/roadmap/features/0001-seriatim-source-installation.md b/docs/roadmap/features/0001-seriatim-source-installation.md index 29af37c..17e62ef 100644 --- a/docs/roadmap/features/0001-seriatim-source-installation.md +++ b/docs/roadmap/features/0001-seriatim-source-installation.md @@ -1,6 +1,6 @@ # Feature 0001: Seriatim Source Installation -Status: ready +Status: in_progress ## Summary @@ -35,10 +35,9 @@ the current design from becoming an accidental suite-wide pattern. `/usr/local/src/seriatim`. - Build Seriatim using the fixed upstream release build contract and install it at `/usr/local/bin/seriatim`. -- Rebuild when the source checkout changes, the binary is absent, or the - installed binary does not report the selected version. +- Rebuild when the source checkout changes or the binary is absent. - Converge the installed binary to `root:root` ownership and mode `0755`. -- Validate a newly built binary with `seriatim --version`. +- Verify the installed version in repository-local integration tests. - Keep the role interface, README, metadata, and focused local tests consistent with the simplified behavior. - Remove obsolete build-state files and implementation machinery from the @@ -52,9 +51,8 @@ the current design from becoming an accidental suite-wide pattern. - Installation or configuration of any other pipeline application. - Pipeline composition, campaign data, prompts, secrets, or scheduling. - Installation from prebuilt release artifacts or removal behavior. -- Detecting rebuild needs from Go compiler patch changes, fixed build-command - changes, or manually altered binary contents that still report the selected - version. +- Detecting rebuild needs from Go compiler changes, fixed build-command + changes, or out-of-band modification of an existing binary. - Build fingerprints, binary checksums, or persistent sidecar build state. - Transactional replacement that preserves a previous executable after a failed local build. @@ -72,9 +70,8 @@ under a root-owned `/usr/local/src/seriatim`, and installs a native executable a `/usr/local/bin/seriatim`. The executable is owned by `root:root`, has mode `0755`, and reports `seriatim version v1.5.0`. -An immediate second application makes no changes. A changed Git checkout, a -missing binary, or a binary reporting another version causes one build. The -following application is again unchanged. +An immediate second application makes no changes. A changed Git checkout or a +missing binary causes one build. The following application is again unchanged. The role trusts `apt`, `community.general.snap`, `file`, and `git` to provide their documented idempotence and change detection. It does not inspect state @@ -101,25 +98,28 @@ already manage. `1.26/stable`. - `community.general` is an approved dependency and provides Snap management; its version is pinned in the role's collection requirements. -- The build runs from the checked-out source using `/snap/bin/go`, - `GOWORK=off`, `GOTOOLCHAIN=local`, `CGO_ENABLED=0`, `-trimpath`, package - `./cmd/seriatim`, and the upstream linker flag that injects the selected - version into `internal/buildinfo.Version`. +- The build runs from the checked-out source using `seriatim_go_binary` + (defaulting to `/snap/bin/go`) with `GOWORK=off`, `GOTOOLCHAIN=local`, + `CGO_ENABLED=0`, `-trimpath`, package `./cmd/seriatim`, and the upstream + linker flag that injects the selected version into + `internal/buildinfo.Version`. - The selected version is both the Git revision and the version injected into the executable. Supporting independent source and build identities is not an MVP requirement. - The public interface is limited to meaningful deployment choices: `seriatim_version`, `seriatim_repository_url`, `seriatim_source_dir`, - `seriatim_binary_path`, `seriatim_manage_build_dependencies`, and - `seriatim_go_snap_channel`. -- Fixed executable paths, build flags, linker symbols, environment values, and - package paths are implementation details, not public variables. + `seriatim_binary_path`, `seriatim_manage_build_dependencies`, + `seriatim_go_snap_channel`, and `seriatim_go_binary`. +- Build flags, linker symbols, environment values, and package paths are + implementation details, not public variables. +- Tasks that install dependencies or manage root-owned application state use + task-level privilege escalation. - `docs/decisions/0001-minimal-module-driven-ansible.md` governs the role's implementation style. ## Simplest Viable Implementation -The intended Seriatim implementation is approximately the following ten +The intended Seriatim implementation is approximately the following eight ordinary tasks. 1. Assert the supported distribution, release, and architecture. @@ -132,19 +132,17 @@ ordinary tasks. 5. Check out the selected source with `ansible.builtin.git` and register its normal changed result. 6. Inspect whether the installed binary exists. -7. Read its version when it exists, without reporting a change. -8. Run the upstream Go build when the Git checkout changed, the binary is - absent, or its reported version differs from the selected version. -9. Converge source ownership and the installed binary's ownership and mode +7. Run the upstream Go build when the Git checkout changed or the binary is + absent. +8. Converge source ownership and the installed binary's ownership and mode with one `ansible.builtin.file` loop. -10. Validate the version after a build, without reporting a change. -For this fixed set of three rebuild triggers, the build remains an ordinary +For this fixed set of two rebuild triggers, the build remains an ordinary task with the conditions applied directly to its `when` expression. This keeps the complete decision visible at the action site and avoids deferred execution -or a play-wide handler flush. Permission convergence and post-build validation -then follow in their natural task order. Do not create an intermediate -`seriatim_build_required` fact. +or a play-wide handler flush. Permission convergence then follows in its +natural task order. Do not create an intermediate `seriatim_build_required` +fact. The implementation may combine or split a task when required for clear and correct Ansible, but it must not add preflight checks around idempotent modules @@ -154,10 +152,10 @@ file unless a split materially improves navigation. ## Rationale and Alternatives This design uses the Git module's changed result as the natural signal that -source changed. The installed application's version is the only additional -state needed to decide whether a binary must be built. That preserves useful -convergence without maintaining a second database of compiler and build -inputs. +source changed. A binary existence check is the only additional state needed +to decide whether the imperative build command must run. That preserves useful +convergence without adding unrequested drift detection or maintaining a second +database of compiler and build inputs. Building directly to the configured binary path is intentionally simpler than staging, checksumming, and recording a transactional installation. A failed @@ -176,16 +174,18 @@ failure mode is not an MVP requirement. - `/usr/local/bin/seriatim --version` reports `seriatim version v1.5.0`. - An immediate second role run reports no changes. -- A missing binary or a binary reporting the wrong version causes one rebuild - followed by an unchanged run. +- A missing binary causes one rebuild followed by an unchanged run. - A changed selected version causes the Git task and build task to change and then converges. - The implementation contains no build-input documents, fingerprints, binary checksums, sidecar build state, manual Git reset or clean commands, or preliminary checks for state already managed by `apt`, Snap, `file`, or `git`. -- Public defaults and argument specifications contain only the six variables +- Public defaults and argument specifications contain only the seven variables named in this roadmap. +- A caller may disable dependency management and select an existing Go + executable with `seriatim_go_binary`. +- Tasks requiring superuser access use task-level privilege escalation. - Syntax checking, ansible-lint, a focused local convergence test, and a clean repeated run pass. - Role documentation describes the actual concise behavior and does not claim diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index c4a682b..1744885 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Implementation Plan 0001: Simplify Seriatim Source Installation -Status: proposed +Status: done Target executor: GPT-5.6-Terra @@ -27,8 +27,10 @@ coverage. the immediately preceding stage. - Follow `docs/conventions.md` and the Feature 0001 task flow. Trust the idempotence and change detection of `apt`, Snap, `file`, and `git`. -- Keep the three rebuild triggers directly on the build task. Do not create an +- Keep the two rebuild triggers directly on the build task. Do not create an intermediate rebuild fact, build handlers, or a handler flush. +- Do not run the installed executable from production role tasks. Verify its + build output in repository-local integration tests. - Do not reintroduce fingerprints, checksums, sidecar build state, manual Git cleanup, staged replacement, compiler inspection, or malformed-path recovery. @@ -91,10 +93,11 @@ Retain exactly these public defaults: | `seriatim_binary_path` | `/usr/local/bin/seriatim` | Installed executable | | `seriatim_manage_build_dependencies` | `true` | Whether to install Git and Go | | `seriatim_go_snap_channel` | `1.26/stable` | Go Snap channel | +| `seriatim_go_binary` | `/snap/bin/go` | Go executable used for the build | -The Go executable, package, build flags, linker symbol, and build environment -are fixed implementation details. The selected version is both the Git ref and -the value injected into the binary. +The package, build flags, linker symbol, and build environment are fixed +implementation details. The selected version is both the Git ref and the value +injected into the binary. ## Stage 1: Replace the Role with the Minimal Implementation @@ -118,7 +121,7 @@ validates the default Seriatim release idempotently. ### Required Changes -1. Reduce `defaults/main.yml` and `meta/argument_specs.yml` to the six variables +1. Reduce `defaults/main.yml` and `meta/argument_specs.yml` to the seven variables in the Fixed Public Interface. Retain useful role and Galaxy metadata in `meta/main.yml`. 2. Replace the imported task structure with one readable `tasks/main.yml` and @@ -134,29 +137,27 @@ validates the default Seriatim release idempotently. `seriatim_repository_url`, `seriatim_source_dir`, and `force: true`, and register the module result; 6. inspect `seriatim_binary_path` with `ansible.builtin.stat`; - 7. run `seriatim_binary_path --version` only when the binary exists, - registering the result without reporting change or failing the play; - 8. build directly to `seriatim_binary_path` when the Git task changed, the - binary is absent, or the version command failed or returned anything - other than `seriatim version {{ seriatim_version }}`; - 9. converge recursive `root:root` source ownership and binary + 7. build directly to `seriatim_binary_path` when the Git task changed or the + binary is absent; + 8. converge recursive `root:root` source ownership and binary `root:root` mode `0755` with one item-driven `ansible.builtin.file` loop; - 10. validate the binary version after a build without reporting change. 3. Put the rebuild expression directly on the build command. Register the - build result for post-build validation and mark the command changed only - when it runs. -4. Invoke `/snap/bin/go build` with `argv`, the fixed package, flags, linker + Git and binary inspection results needed by its two conditions without + creating an intermediate rebuild fact. +4. Invoke `seriatim_go_binary` with `argv`, the fixed package, flags, linker value, output path, working directory, and environment from the upstream - contract. Do not turn those fixed values into defaults or facts. + contract. Do not turn the remaining fixed values into defaults or facts. + Apply task-level privilege escalation to dependency installation and tasks + that manage root-owned application state. 5. Remove all code for Git executable discovery, Go executable and compiler validation, manual Git status/reset/clean, resolved-commit facts, temporary build directories, build inputs, fingerprints, checksums, sidecar state, special path-object recovery, and transactional installation. -6. Do not add `handlers/main.yml`; the build, permission, and validation tasks - execute in normal order. -7. Rewrite the README to document the six-variable interface, privilege and +6. Do not add `handlers/main.yml`; the build and permission tasks execute in + normal order. +7. Rewrite the README to document the seven-variable interface, privilege and fact requirements, dependency opt-out behavior, authoritative Git checkout, - direct build contract, three rebuild triggers, installed metadata, and + direct build contract, two rebuild triggers, installed metadata, and validation commands. Remove every claim or example tied to deleted variables, state files, staging, compiler invalidation, or failure preservation. @@ -175,7 +176,7 @@ From `ansible-role-seriatim`, run: ```sh tests/integration ! rg -n \ - 'seriatim_(source_ref|build_version|build_state_dir|go_binary|go_build_flags|go_ldflags|go_package)|fingerprint|checksum' \ + 'seriatim_(source_ref|build_version|build_state_dir|go_build_flags|go_ldflags|go_package)|fingerprint|checksum' \ defaults meta tasks tests README.md git diff --check git status --short @@ -189,7 +190,7 @@ changes remain before commit. ### Completion Criteria -- The public interface contains exactly six defaults and matching argument +- The public interface contains exactly seven defaults and matching argument specifications. - The role's effective task flow matches the roadmap without intermediate rebuild state or handlers. @@ -230,8 +231,8 @@ default state. 2. Retain the baseline convergence, verification, and clean-repeat cases, then add focused cases for: - an absent binary causing one rebuild followed by an unchanged run; - - an executable that reports the wrong version causing one rebuild followed - by an unchanged run; + - dependency opt-out with a caller-supplied Go executable causing one build + followed by an unchanged run; - binary ownership or mode drift being repaired by the ordinary `file` task without requiring a rebuild, as shown by the captured Ansible task output; - selecting `v1.4.0`, confirming its commit and version output, and reaching @@ -312,4 +313,8 @@ push during out-of-band execution. Completion requires subsequent review. ## Progress Notes -No stages have been executed. +- Stage 1 completed in `ansible-role-seriatim` commit `325a5c7`. +- Stage 2 completed in `ansible-role-seriatim` commit `718030e`. +- The final manual-review remediation in `docs/roadmap/remediation.md` was + implemented and validated in `ansible-role-seriatim` commit `fb8dc41` on + 2026-09-08. Feature acceptance remains a separate review step. diff --git a/docs/roadmap/mvp.md b/docs/roadmap/mvp.md index 2168f3a..2a46241 100644 --- a/docs/roadmap/mvp.md +++ b/docs/roadmap/mvp.md @@ -56,15 +56,17 @@ role will manage the native configuration needed to connect to it. - Builds use the application repository's own build contract, including any application-specific version-injection linker flags. The common baseline is `GOWORK=off`, `GOTOOLCHAIN=local`, `CGO_ENABLED=0`, and `-trimpath`. -- Roles normally rebuild when the Git task reports a changed checkout, the - binary is absent, or an application-level version or smoke check shows that - the installed binary is unsuitable. They do not maintain fingerprints, - checksums, or sidecar build-state files for the MVP. +- Roles normally rebuild when the Git task reports a changed checkout or the + binary is absent. An application-level version or smoke check may add a + trigger only when its feature specification explicitly requires that + production behavior. Roles do not maintain fingerprints, checksums, or + sidecar build-state files for the MVP. - Each application role exposes variables only for meaningful consumer choices, normally including its repository, selected version or revision, - source directory, binary path, dependency management, and Go Snap channel. - Fixed details of the upstream build contract need not become public - variables. + source directory, binary path, dependency management, Go Snap channel, and a + build-tool path when dependency opt-out permits another provisioning + mechanism. Fixed details of the upstream build contract need not become + public variables. ### Go toolchain diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index cc3a302..e8a8b9b 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,19 +1,137 @@ -# Feature 0001 Prior Remediation Record +# Feature 0001 Remediation Review -Historical record: superseded +Status: resolved -The original Feature 0001 review required increasingly elaborate recovery for -build-state files, metadata drift, custom path shapes, and symbolic links. -Those findings were resolved in `ansible-role-seriatim` commits `c6c1907` -through `6fb1aa2`, but the resulting architecture was later rejected as -disproportionate to the MVP. +## Finding 1: Synchronize Implementation Progress Tracking -The authoritative replacement specification is -`docs/roadmap/features/0001-seriatim-source-installation.md`. It deliberately -removes persistent build state and treats unusual corruption, adversarial path -redirection, exact compiler-input tracking, and transactional replacement as -potential future hardening rather than current requirements. +Severity: low -This file is retained only to explain the historical commit sequence. It is -not an active remediation specification and must not be used as a template for -another role. +The two implementation stages are marked complete in +`docs/roadmap/implementation.md`, with commits `325a5c7` and `718030e`, but the +same document still has status `proposed` and says that no stages have been +executed. `docs/status.md` likewise still describes plan review as the current +focus and Feature 0001 as `ready` rather than reflecting the completed +implementation and pending review outcome. + +### Required Remediation + +- Update the implementation plan status and progress notes to agree with its + two recorded stage completions. +- Update workspace status to reflect that implementation is complete and the + feature is awaiting review acceptance. +- Preserve the prior remediation history in Git rather than carrying its + superseded contents in the active remediation document. + +### Validation + +- The implementation plan's status, stage completion annotations, and progress + notes describe one consistent state. +- Workspace status names implementation review as the current activity and + uses the corresponding feature state. +- `git diff --check` passes. + +## Finding 2: Remove Production Version Interrogation + +Severity: medium + +`ansible-role-seriatim/tasks/main.yml` runs the installed binary before a build +to detect unexpected version output and again after a build to validate the +result. Neither operation is part of the required production convergence +contract. The first adds unrequested drift detection, while the second turns a +role task into a test of the role's own build implementation. + +### Required Remediation + +- Remove the pre-build `--version` command and its registered result. +- Rebuild only when the Git checkout changed or the binary is absent. +- Retain the binary `ansible.builtin.stat`, because its result is independently + needed to decide whether the imperative build must run. +- Remove the post-build `--version` command, its registered build result, and + the associated lint suppression. +- Continue verifying the installed version in repository-local integration + tests. + +### Validation + +- The role contains no production invocation of `seriatim --version`. +- The build task has exactly the two required triggers: changed checkout or + absent binary. +- Missing-binary and changed-checkout integration cases each cause a build and + then converge to an unchanged repeat. +- The integration suite verifies the built binary's exact version output. + +## Finding 3: Complete the Declarative Dependency and Privilege Contract + +Severity: medium + +The build command hard-codes `/snap/bin/go`, which makes dependency opt-out +unnecessarily dependent on the Snap installation path. The Git package task +also refreshes the APT cache without a validity interval, and tasks that modify +system-owned paths or install system dependencies currently rely on privilege +escalation supplied by the calling play. + +### Required Remediation + +- Add `seriatim_go_binary`, defaulting to `/snap/bin/go`, to the public defaults + and argument specification, and use it as the build command executable. +- Add `cache_valid_time: 3600` to the Git `ansible.builtin.apt` task. +- Set `become: true` directly on tasks that install dependencies or modify + root-owned application state. Do not add privilege escalation to the + platform assertion or binary inspection merely for uniformity. +- Update the README's variable table, dependency opt-out description, and + privilege behavior to match the implemented contract. + +### Validation + +- Dependency opt-out can use a caller-supplied Go executable. +- The APT task retains declarative cache management without refreshing a valid + cache. +- A play connecting as an ordinary sudo-capable user can apply the role without + play-level `become: true`. +- Defaults, argument specifications, tasks, and README agree on the seven + public variables. + +## Finding 4: Align the Feature Record and Focused Tests + +Severity: medium + +The active feature roadmap and completed implementation plan still require +wrong-version drift detection, post-build validation as a production task, a +hard-coded Go executable, and a six-variable interface. The integration driver +also contains a wrong-version rebuild case that would preserve the superseded +contract. + +### Required Remediation + +- Revise the feature roadmap to record the two rebuild triggers, the + seven-variable interface, and the approximately eight-task target flow. +- Annotate or revise the completed implementation plan so it does not remain an + apparently authoritative description of the superseded target. +- Remove the wrong-version rebuild case from the integration driver. +- Retain focused coverage of default convergence and idempotence, a missing + binary, binary metadata convergence without a build, a changed selected + version, platform rejection, and final version output. +- Update the README to describe only the revised behavior. + +### Validation + +- Workspace roadmap, implementation record, role implementation, README, and + integration suite describe the same final architecture. +- No active document claims that unexpected version output triggers a rebuild + or that the production role validates a newly built executable by running + it. +- `tests/integration`, `bash -n tests/integration`, and `git diff --check` pass. + +## Resolution + +All four findings were resolved in `ansible-role-seriatim` commit `fb8dc41` on +2026-09-08. The role now uses an eight-task flow with two rebuild triggers, a +caller-selectable Go executable, bounded APT cache refreshes, and task-level +privilege escalation. Production tasks do not execute the installed +application; repository-local verification retains the version assertion. The +active roadmap, implementation record, status, README, interface, and focused +tests describe the same architecture. + +Validation completed successfully with `tests/integration`, +`bash -n tests/integration`, and `git diff --check` in both changed +repositories. diff --git a/docs/status.md b/docs/status.md index 529414e..184fc1a 100644 --- a/docs/status.md +++ b/docs/status.md @@ -4,8 +4,8 @@ Last updated: 2026-09-08 Current milestone: MVP standalone application roles -Current focus: Review the staged implementation plan for the Feature 0001 -Seriatim simplification. +Current focus: Review the completed Feature 0001 Seriatim simplification and +final remediation. | ID | Deliverable | Owner | State | Evidence or next step | | --- | --- | --- | --- | --- | @@ -15,10 +15,10 @@ Seriatim simplification. | M0-04 | Aggregate repository status | workspace | done | `scripts/status` created | | M0-05 | Suite-level role repository | ansible-role-dnd-pipeline | done | Repository created and added to `workspace.yml` | | M1-01 | Remaining MVP design decisions | workspace | ready | Resume after the revised Seriatim pattern is accepted | -| M1-02 | Standalone application roles | role repositories | in_progress | Simplify Seriatim before continuing to other roles | +| M1-02 | Standalone application roles | role repositories | in_progress | Accept the remediated Seriatim pattern before continuing | | M1-03 | Suite-level pipeline role | ansible-role-dnd-pipeline | proposed | Begin after standalone role interfaces stabilize | | M1-04 | Collection assembly | ansible-collection-dnd | proposed | Select and implement the pipeline-role incorporation mechanism | -| F0001 | Install Seriatim from pinned source | ansible-role-seriatim | ready | Review `docs/roadmap/implementation.md` | +| F0001 | Install Seriatim from pinned source | ansible-role-seriatim | in_progress | Remediation implemented in `fb8dc41` and validated; complete final review | | F0002 | Install and configure Audita | ansible-role-audita | proposed | Replan after the simplified Seriatim role is reviewed | ## Updating This File