fix(lint-compose): validate against current compose, not the runner's - #420
Conversation
The job ran `docker compose config` with whatever version the runner image happens to ship. That is v2.38 today, while compose is at v5.5.1, and the gap is not cosmetic: v2.38 rejects `pre_start` with "additional properties not allowed", although it is the documented way to model init containers and current compose accepts it. So the lint reported a valid file as broken, and the repository it fired in worked around the lint by removing the construct - the check made the code worse rather than better. Validating against an older parser than the one deployments actually run is the wrong way round. docker/setup-compose-action now pins the version being validated against, defaulting to latest, with a `compose-version` input for callers that want a fixed one. The input is forwarded through docker-image-ci.yml. Blast radius measured before changing this: two repositories reference the workflow, plus callers of the docker-image-ci meta. A newer compose accepts a superset of what the old one did, so files that pass today keep passing. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_01FRHeDxbgUnv868eNhVCCsr Agent-Host: 32116e Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe reusable workflows now define a Changes
Priority: ⬇️ Low Change: Bug fix Merge Risk: ⚪ Minimal · up to The workflows now select and install the requested Compose version before linting, with no concrete merge-blocking risk established. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The Copilot reviewer is out of quota account-wide, so no bot review can land here. CodeRabbit left no inline findings and SonarCloud's quality gate passed. I reviewed the diff myself. It is additive: a One deliberate choice worth stating: the default is Self-review: 95f7c46 |
Both compose-using jobs inherited whatever version the runner image ships. That version rejects `pre_start`, so the syntax check failed on a file that is valid, and the stack test would have failed to start it at all. docker/setup-compose-action, SHA-pinned, installs the current release in both jobs. The org-wide lint got the same treatment in netresearch/.github#420. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_01FRHeDxbgUnv868eNhVCCsr Agent-Host: 32116e Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
…t hook (#79) Merging this removes a container that existed only to be idle, and puts the database bootstrap back where compose says it belongs. The stack runs five containers instead of six, and both decisions finally carry a written reason. ## The idle cron container `moodle-cron` ran `sleep infinity` and nothing else. Ofelia's `job-exec` needs a running container to exec into, so a service was added purely to be that target — while the application container, which runs anyway, can host the job. The labels move there and the service is gone. `job-run` would also avoid the idle container, but it needs every volume, network and environment variable repeated in labels: a second copy of the service definition, free to drift from the first. `job-exec` on a container that already exists has no such duplication. ## The pre_start hook The bootstrap was written as a `pre_start` hook, which is how compose models init containers, and then reverted to a one-shot service because the compose on the CI runners rejected the key. That was the wrong repair — it changed working code to suit an outdated linter. netresearch/.github#420 makes the lint validate against current compose, so the hook returns: the step stays subordinate to the service instead of appearing as a peer in `compose ps`. ## Verified On a full stack, not from the file: five containers instead of six, `pre_start` creates database and user, the Moodle install completes, ofelia registers `moodle.moodle-cron` and runs it to completion (`failed: false, error: none`), **19 scheduled tasks show a `lastruntime`** — so Moodle actually processed the runs — and the site answers 200. yamllint and markdownlint report nothing. ## Note Neither arrangement was documented anywhere before; the README described *that* there was a dedicated cron container, never why. Both now carry their reasoning inline, including why `job-run` was not chosen. _Assisted by claude-code:claude-opus-5 — [Session](https://claude.ai/code/session_01FRHeDxbgUnv868eNhVCCsr)_
Merging this makes the compose lint validate against current Docker Compose instead of whatever version the runner image happens to ship. Files that current compose accepts stop being reported as broken.
Why
The job ran
docker compose configwith the runner's preinstalled version — v2.38 today, while compose is at v5.5.1. The gap is not cosmetic. v2.38 answerspre_startwithservices.<name> additional properties 'pre_start' not allowed, even though that is the documented way to model init containers and current compose accepts it.The consequence showed up in netresearch/moodle-docker: an init container written as a
pre_starthook failed this lint, and the fix was to remove the construct and go back to a one-shot service. The check made the code worse rather than better — it validated against an older parser than the one deployments actually run.What changed
docker/setup-compose-action(SHA-pinned) installs the version to validate against, defaulting tolatest. Acompose-versioninput lets callers pin a specific one, and it is forwarded throughdocker-image-ci.yml.Blast radius
Measured before touching this: two repositories reference
lint-compose.ymldirectly, plus the callers of thedocker-image-cimeta workflow. A newer compose accepts a superset of what v2.38 did, so files passing today keep passing; the change can only turn currently-failing valid files green.Assisted by claude-code:claude-opus-5 — Session