Complete the Notarius feature
This commit is contained in:
@@ -1,84 +1,64 @@
|
||||
# Feature 0003 Remediation Review
|
||||
|
||||
Status: open
|
||||
Status: resolved
|
||||
|
||||
Reviewed implementation commits:
|
||||
|
||||
- `287fc4a Install and configure Notarius`
|
||||
- `733a194 Add focused Notarius integration coverage`
|
||||
|
||||
The role's complete integration suite passes. The production architecture is
|
||||
substantially complete and follows the selected nine-operation flow, but the
|
||||
following issues should be corrected before Feature 0003 is marked done.
|
||||
Remediation commit:
|
||||
|
||||
- `eb4f511 Correct Notarius metadata and dependency coverage`
|
||||
|
||||
## Finding 1: Correct the Galaxy License Metadata
|
||||
|
||||
Severity: medium
|
||||
|
||||
`ansible-role-notarius/meta/main.yml` declares `license: BSD-3-Clause`, while
|
||||
the repository's retained `LICENSE` file is CC0 1.0 Universal. Package metadata
|
||||
must not misstate the terms under which the repository is distributed.
|
||||
`ansible-role-notarius/meta/main.yml` declared `license: BSD-3-Clause`, while
|
||||
the retained repository license is CC0 1.0 Universal.
|
||||
|
||||
### Required Remediation
|
||||
### Resolution
|
||||
|
||||
- Change the Galaxy license identifier to `CC0-1.0`, consistent with the
|
||||
repository license and the other standalone roles that use the same license.
|
||||
- Run metadata lint as part of the complete integration suite.
|
||||
The Galaxy license identifier is now `CC0-1.0`, consistent with the repository
|
||||
license.
|
||||
|
||||
## Finding 2: Remove the Redundant Build Change Override
|
||||
|
||||
Severity: low
|
||||
Status: withdrawn
|
||||
|
||||
The `Build Notarius` task sets `changed_when: true`. The
|
||||
`ansible.builtin.command` module already reports a successful executed command
|
||||
as changed, and the task's `when` expression already determines exactly when a
|
||||
build occurs. The override adds no behavior and conflicts with the convention
|
||||
to trust module behavior and require a reason for every additional directive.
|
||||
The initial review treated `changed_when: true` on the conditionally executed
|
||||
build command as redundant. Validation after removing it failed ansible-lint's
|
||||
`no-changed-when` production rule.
|
||||
|
||||
### Required Remediation
|
||||
### Resolution
|
||||
|
||||
- Remove `changed_when: true` from the production build task.
|
||||
- Retain the existing rebuild expression unchanged:
|
||||
`notarius_checkout.changed or not notarius_binary.stat.exists`.
|
||||
- Run the complete integration suite to confirm that build runs still report
|
||||
changed and immediate repeats remain clean.
|
||||
The directive remains in place. Unlike a desired-state module, the imperative
|
||||
build command must explicitly declare its change semantics for lint. The
|
||||
existing `when` expression still determines whether the command runs, and
|
||||
`changed_when: true` accurately reports that every executed build changes the
|
||||
binary. The distinction is now documented in `docs/conventions.md`.
|
||||
|
||||
## Finding 3: Make the Dependency-Opt-Out Test Prove the Selected Go Binary
|
||||
|
||||
Severity: low
|
||||
|
||||
The integration suite sets `notarius_go_binary` to a symlink whose target is
|
||||
`/snap/bin/go`. The test succeeds with the current implementation, but it would
|
||||
also succeed if the build task ignored `notarius_go_binary` and hard-coded
|
||||
`/snap/bin/go`. It therefore does not establish the acceptance criterion that
|
||||
a caller-supplied Go executable is actually used.
|
||||
The original integration test pointed `notarius_go_binary` at a symlink to
|
||||
`/snap/bin/go`, so it did not prove that the configured path was actually
|
||||
invoked.
|
||||
|
||||
### Required Remediation
|
||||
### Resolution
|
||||
|
||||
- Replace the symlink with a small executable wrapper in the suite's private
|
||||
temporary directory.
|
||||
- Have the wrapper record its invocation and then delegate to `/snap/bin/go`.
|
||||
- Assert that the invocation record was written during the dependency-opt-out
|
||||
build, while retaining the existing clean-repeat assertion.
|
||||
- Keep the wrapper and its record in the existing private temporary directory;
|
||||
do not add production behavior.
|
||||
The test now supplies a wrapper in its private temporary directory. The
|
||||
wrapper records its invocation before delegating to `/snap/bin/go`, and the
|
||||
suite asserts that the record exists after the dependency-opt-out build.
|
||||
|
||||
## Review Evidence
|
||||
## Final Validation
|
||||
|
||||
The review ran from `ansible-role-notarius`:
|
||||
The complete `tests/integration` suite passed after remediation with Ansible
|
||||
Core 2.21.3 and ansible-lint 26.8.0. Production-profile lint passed, every
|
||||
selected build and configuration behavior remained valid, all required repeat
|
||||
runs were clean, and the suite restored Notarius `v0.6.0` with the minimal
|
||||
`version: 4` configuration.
|
||||
|
||||
```sh
|
||||
tests/integration
|
||||
git diff --check
|
||||
```
|
||||
|
||||
The complete suite passed with Ansible Core 2.21.3 and ansible-lint 26.8.0. It
|
||||
covered default convergence, the selected source commit, executable version,
|
||||
native configuration validation, clean repeats, missing-binary and checkout
|
||||
rebuilds, metadata repair without rebuilding, nullable field rendering,
|
||||
PromptKit source alternatives, intentional falsey values, caller-side
|
||||
pipeline-map composition, dependency opt-out, and platform rejection. The
|
||||
suite restored Notarius `v0.6.0` and the minimal `version: 4` configuration.
|
||||
|
||||
No additional production tasks, configuration probes, runtime validation,
|
||||
private state, or unrequested hardening were found.
|
||||
`git diff --check` also passed. No unresolved Feature 0003 findings remain.
|
||||
|
||||
Reference in New Issue
Block a user