fix(linting): stop the lint checks failing open on Compose output - #89
Merged
Merged
Conversation
Prep for moving the hosted runners to ubuntu-26.04. The self-hosted runners already run Compose v5.5.1, so lint (hosted, 2.38.2) and deploy (self-hosted, 5.5.1) validate with different engines today; the migration closes that gap. These are the parsers that had to be sound first - and two of them were not. validate-image-platforms.sh: `config --images` was piped into the read loop with `2>/dev/null`, discarding both the diagnostics and the exit status. Any failure to resolve the compose file produced an empty image list, which the zero-images branch then reported as a PASS - a check that verified nothing, reporting success. Verified before/after on an unparseable fixture: exit 0 (silent pass) -> exit 1 with the parse error surfaced. Zero images is still a legitimate pass when the file declares no services, so the empty case is discriminated with `config --services` rather than failed outright. validate-stack.sh: under `set -e`, a bare `wait` on a failing child aborted the script before `DOCKER_EXIT=$?` and before the whole formatted report. A stack that failed validation printed raw output and died - no "Issues found", no fix hint, no status line. CI verdicts were always correct (the job still exited non-zero), which is why it went unnoticed; only the diagnostics were lost. This also made the filter block below it dead code on every path that uses it. validate-stack.sh + lint-summary.sh: the env-var warning filters matched `WARNING` case-sensitively. Compose 5.x emits `level=warning msg="..."` instead, so every missing-variable warning would leak into the displayed error block. Matching the bare token `warning` case-insensitively covers both spellings. Verified against Compose v5.5.1: shellcheck clean; broken stack -> 1, healthy stack -> 0, image platform check passes 2/2 against a real digest-pinned stack; filter drops both warning formats while keeping real errors and unrelated warnings (`WARNING: Found orphan containers` is retained, so it is not a blanket suppressor).
Reviewer's GuideThe lint scripts now fail closed on unparseable or structurally unexpected Compose output, preserve the complete formatted failure report under Sequence diagram for preserved Compose validation failure reportingsequenceDiagram
participant Script as validate-stack.sh
participant YAML as YAML validator
participant Compose as docker compose
participant Output as Formatted report
Script->>YAML: validate stack YAML
Script->>Compose: config
Compose-->>Output: tee raw output
Script->>YAML: wait YAML_PID
YAML-->>Script: exit status
Script->>Compose: wait DOCKER_PID
Compose-->>Script: exit status
Script->>Script: grep -viE warning filters
Script->>Output: print issues, fix hint, overall status
Flow diagram for fail-closed Compose image validationflowchart TD
A["Run docker compose config --images"] --> B{Command succeeds?}
B -- No --> C["Print Compose diagnostics"]
C --> D["Exit 1"]
B -- Yes --> E["Collect unique image references"]
E --> F{Images resolved?}
F -- Yes --> G["Verify image platforms"]
G --> H["Report validation result"]
F -- No --> I["Run config --services"]
I --> J{Services declared?}
J -- Yes --> K["Reject unexpected empty output"]
K --> D
J -- No --> L["Pass: no services to check"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
owine
added a commit
that referenced
this pull request
Sep 19, 2026
The hold's exit condition was already met when it was written. It said to remove the rule "once the SELF-HOSTED runners have been upgraded to a Compose 5.x image" - they run v5.5.1, verified on piwine, and have for some time. So the premise was inverted. The hold was written to stop lint and deploy validating with different Compose engines, but that split is the CURRENT state: every Compose-touching job in deploy.yml is self-hosted on 5.5.1 (:67, :258, :647, :742, :760), while lint runs hosted on ubuntu-24.04 with 2.38.2. Keeping the hold perpetuates the mismatch; moving the hosted runners to 26.04 closes it. A detail that shows how invisible this was: the 2 literal `runs-on:` values Renovate could already see are deploy.yml's notify job and workflow-lint.yml - neither runs Compose at all. The 3 labels that do feed `docker compose config` were the ones it could not see, until #88. Compose 5.x compatibility was checked directly rather than assumed: `config --images` still emits one resolved ref per line on stdout; `ps -a --format json` is still NDJSON, so deploy.yml's `jq -s` health gate parses correctly; verdicts remain exit-code driven. The parsers that were genuinely fragile are fixed in #89. With #88 in place Renovate sees all 5 labels, so this produces one PR that moves them together rather than the partial migration #87 was guarding against.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why now
The self-hosted runners already run Compose v5.5.1 (verified on
piwine). Every Compose-touching job indeploy.ymlis self-hosted —:67,:258,:647,:742,:760— while lint runs hosted onubuntu-24.04with 2.38.2. So the two sides already validate with different engines; moving the hosted runners to 26.04 closes that gap rather than opening one.Before doing that, the output parsers had to be sound. Two were not.
1.
validate-image-platforms.shfailed open2>/dev/nulldiscarded the diagnostics and the exit status. Any failure to resolve the compose file produced an empty list, which the zero-images branch reported as a pass — a check that verified nothing, reporting success. This was the one place a Compose change could break things silently rather than loudly.Measured on an unparseable fixture:
No images resolved — nothing to check.✗ 'docker compose config --images' failed+ the parse errorZero images is still legitimately a pass for a build-only or service-less file, so the empty case is discriminated with
config --servicesinstead of being failed outright.2.
validate-stack.sh—set -eswallowed the entire reportUnder
set -euo pipefail(line 14) a barewaiton a failing child aborts beforeDOCKER_EXIT=$?, and before every formatted block below it. A stack that failed validation printed its rawtee'd output and then died — no "Issues found", no fix hint, no overall status.CI verdicts were always correct (the job still exited non-zero), which is why this went unnoticed — only the diagnostics were lost. It also meant the warning-filter block was dead code on every path that uses it, since it only runs when Compose fails.
After, on the same fixture:
3. Warning filters didn't match Compose 5.x
The filters matched
WARNINGcase-sensitively, so under 5.x every missing-variable warning would leak into the displayed error block. Replaced the three chained greps with one case-insensitive match on the bare tokenwarning— a substring of both spellings, so there's no alternation to keep in sync as formats drift again. The|| cpfallback behaviour is unchanged.Still narrow, not a blanket suppressor —
WARNING: Found orphan containersis correctly retained, since it matches none of the three phrases.Not changed, and why
A missing stack initially looked like a third fail-open:
lint-summary.shprintedERROR: Stack file not foundand still reportedALL STACKS PASSED. It isn't one.lint-summary.shtakes--lint-resultas an input from the upstream per-stack matrix job; it reports, it doesn't decide. Passing it--lint-result successfor a nonexistent stack feeds it a false premise.validate-stack.sh, which actually decides, fails correctly on a missing stack. Left alone.Validation
Against Compose v5.5.1 locally and on
piwine:shellcheckclean on all three scripts.validate-stack.sh: broken stack → 1, healthy stack (dozzle) → 0.validate-image-platforms.sh: real digest-pinned stack → PASSED (2 verified, 0 skipped); unparseable fixture → 1.test-classify-rollback-scope.shandtest-detect-stack-changes.shpass. (test-workflow.shexits 1 printing usage when given no args, onmaintoo — not a regression.)Next
With these sound, the
ubuntu-24.04hold from #87 can be lifted — its stated exit condition, "once the self-hosted runners have been upgraded", turned out to have been satisfied before it was written. That's a separate PR.Summary by Sourcery
Harden Compose linting so parser failures are reported clearly and validation checks cannot pass without verifying their inputs.
Bug Fixes: