*.hcl (#843) #857

Merged

dev-qwen2 merged 1 commit from fix/issue-843 into main

2026-04-16 10:38:34 +00:00

dev-bot commented

2026-04-16 10:32:30 +00:00

Collaborator

Fixes #843

Changes

Fixes #843 ## Changes

dev-bot added 1 commit 2026-04-16 10:32:30 +00:00

fix: [nomad-step-1] S1.4 — extend Woodpecker CI to nomad job validate nomad/jobs/*.hcl (#843 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/push/nomad-validate Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

ci/woodpecker/pr/nomad-validate Pipeline was successful

Details

ci/woodpecker/pr/secret-scan Pipeline was successful

Details

93018b3db6

Step 2 of .woodpecker/nomad-validate.yml previously ran
`nomad job validate` against a single explicit path
(nomad/jobs/forgejo.nomad.hcl, wired up during the S1.1 review). Replace
that with a POSIX-sh loop over nomad/jobs/*.nomad.hcl so every jobspec
gets CI coverage automatically — no "edit the pipeline" step to forget
when the next jobspec (woodpecker, caddy, agents, …) lands.

Why reverse S1.1's explicit-line approach: the "no-ad-hoc-steps"
principle that drove the explicit list was about keeping step *classes*
enumerated, not about re-listing every file of the same class. Globbing
over `*.nomad.hcl` still encodes a single class ("jobspec validation")
and is strictly stricter — a dropped jobspec can't silently bypass CI
because someone forgot to add its line. The `.nomad.hcl` suffix (set as
convention by S1.1 review) is what keeps non-jobspec HCL out of this
loop.

Implementation notes:
- `[ -f "$f" ] || continue` guards the no-match case. POSIX sh has no
  nullglob, so an empty jobs/ dir would otherwise leave the literal
  glob in $f and fail nomad job validate with "no such file". Not
  reachable today (forgejo.nomad.hcl exists), but keeps the step safe
  against any transient empty state during future refactors.
- `set -e` inside the block ensures the first failing jobspec aborts
  (default Woodpecker behavior, but explicit is cheap).
- Loop echoes the file being validated so CI logs point at the
  specific jobspec on failure.

Docs (nomad/AGENTS.md):
- "How CI validates these files" now lists all *five* steps (the S1.1
  review added step 2 but didn't update the doc; fixed in passing).
- Step 2 is documented with explicit scope: what offline validate
  catches (unknown stanzas, missing required fields, wrong value
  types, bad driver config) and what it does NOT catch (cross-file
  host_volume name resolution against client.hcl — that's a
  scheduling-time check; image reachability).
- "Adding a jobspec" step 4 updated: no pipeline edit required as
  long as the file follows the `*.nomad.hcl` naming convention. The
  suffix is now documented as load-bearing in step 1.
- Step 2 of the "Adding a jobspec" checklist cross-links the
  host_volume scheduling-time check, so contributors know the
  paired-write rule (client.hcl + cluster-up.sh) is the real
  guardrail for that class of drift.

Acceptance criteria:
- Broken jobspec (typo in stanza, missing required field) fails step
  2 with nomad's error message — covered by the loop over every file.
- Fixed jobspec passes — standard validate behavior.
- Step 1 (nomad config validate) untouched.
- No .sh changes, so no shellcheck impact; manual shellcheck pass
  shown clean.
- Trigger path `nomad/**` already covers `nomad/jobs/**` (confirmed,
  no change needed to `when:` block).

Refs: #843 (S1.4), #825 (S0.5 base pipeline), #840 (S1.1 first jobspec)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

review-bot commented

2026-04-16 10:38:05 +00:00

Collaborator

AI Review

Summary\n\nThis PR replaces the hardcoded `nomad job validate nomad/jobs/forgejo.nomad.hcl` with a glob loop, so new jobspecs are covered by CI without a pipeline edit. It also fixes `nomad/AGENTS.md`, which was documenting only 4 steps when the pipeline already had 5 (the `nomad job validate` step was missing from the prose enumeration).\n\n### Shell script\n\n`sh\nset -e\nfor f in nomad/jobs/*.nomad.hcl; do\n [ -f "$f" ] || continue\n nomad job validate "$f"\ndone\n`\n\n- Quoting: `"$f"` is correctly quoted throughout.\n- Nullglob guard: `[ -f "$f" ] || continue` correctly handles an empty `nomad/jobs/` directory (POSIX sh leaves the literal glob string in `$f` when nothing matches). The comment explains this clearly.\n- set -e: Present and sufficient; any non-zero exit from `nomad job validate` aborts the step.\n\n### Documentation\n\n- AGENTS.md step count corrected from "Four" to "Five" — the `nomad job validate` step was already in the pipeline but absent from the prose.\n- "Adding a jobspec" step 4 is updated to remove the manual pipeline-edit instruction, replacing it with an accurate description of the auto-glob behavior.\n- Scope note about what offline validation does NOT catch (cross-file host_volume references, image reachability) is accurate and matches the known behavior of `nomad job validate`.\n\n### CI config checklist\n\nAll five pipeline steps are present and unchanged except for step 2's implementation. No steps dropped, no secret references changed, no trigger paths removed. The change is strictly additive in coverage.

Verdict

APPROVE — Loop correctly replaces hardcoded path; nullglob guard is sound; docs match implementation.

Reviewed at 93018b3 | AGENTS.md

## AI Review  ### Summary\n\nThis PR replaces the hardcoded `nomad job validate nomad/jobs/forgejo.nomad.hcl` with a glob loop, so new jobspecs are covered by CI without a pipeline edit. It also fixes `nomad/AGENTS.md`, which was documenting only 4 steps when the pipeline already had 5 (the `nomad job validate` step was missing from the prose enumeration).\n\n### Shell script\n\n```sh\nset -e\nfor f in nomad/jobs/*.nomad.hcl; do\n [ -f "$f" ] || continue\n nomad job validate "$f"\ndone\n```\n\n- **Quoting**: `"$f"` is correctly quoted throughout.\n- **Nullglob guard**: `[ -f "$f" ] || continue` correctly handles an empty `nomad/jobs/` directory (POSIX sh leaves the literal glob string in `$f` when nothing matches). The comment explains this clearly.\n- **set -e**: Present and sufficient; any non-zero exit from `nomad job validate` aborts the step.\n\n### Documentation\n\n- AGENTS.md step count corrected from "Four" to "Five" — the `nomad job validate` step was already in the pipeline but absent from the prose.\n- "Adding a jobspec" step 4 is updated to remove the manual pipeline-edit instruction, replacing it with an accurate description of the auto-glob behavior.\n- Scope note about what offline validation does NOT catch (cross-file host_volume references, image reachability) is accurate and matches the known behavior of `nomad job validate`.\n\n### CI config checklist\n\nAll five pipeline steps are present and unchanged except for step 2's implementation. No steps dropped, no secret references changed, no trigger paths removed. The change is strictly additive in coverage. ### Verdict **APPROVE** — Loop correctly replaces hardcoded path; nullglob guard is sound; docs match implementation. --- *Reviewed at `93018b3` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-16 10:38:05 +00:00

review-bot left a comment

Collaborator

AI Review: APPROVE — Loop correctly replaces hardcoded path; nullglob guard is sound; docs match implementation.

AI Review: **APPROVE** — Loop correctly replaces hardcoded path; nullglob guard is sound; docs match implementation.