amtronics/platform-actions

Watch

Fork 0

fix: deploy-vps arms a monitoring profile the compose file may not declare #39

New issue

Closed

opened 2026-09-04 11:43:04 +00:00 by ahmad · 1 comment

ahmad commented

2026-09-04 11:43:04 +00:00

Owner

Context

deploy-vps decides whether to arm the monitoring profile from the credentials it was
given, not from the compose file it is deploying. When the Grafana variables are
present it sets COMPOSE_PROFILES=monitoring and runs docker compose up.

Compose does not treat an unknown profile as an error. It selects the services carrying
that profile, finds none, and starts the rest normally — exit code 0, deploy reported
healthy. The result is a deployment that looks fully monitored and emits nothing, which
surfaces later as a not-reporting alert firing forever against a healthy service. That is
the worst shape a failure can take: silent at the moment it happens, and misattributed
when it is finally noticed.

This was hit for real in ahmad/imamah (its #134): one of two compose variants declared
the collector and the other did not, and the deploy armed the profile against both. That
instance is fixed in that repository. The action still has the hole, for every repository
that uses it.

Scope

In: deploy-vps refusing to arm a profile the target compose file does not declare, and
saying so.
Out: deciding which services a repository should run under monitoring; the imamah
fix, already done.

Acceptance criteria

Given a compose file that declares no service with the monitoring profile, when the
deploy runs with the Grafana credentials present, then it fails with a message naming
the profile and the compose file — rather than starting the remaining services.
Given a compose file that does declare the profile, when the deploy runs with the
credentials present, then it arms it exactly as it does today.
Given the credentials are absent, when the deploy runs, then the profile is not armed
and the check does not run.
Given the check is added, when the action's tests run, then all three cases above are
covered.

Notes

docker compose config --profiles lists the profiles a compose file declares, which is
enough to make this a two-line guard rather than a parser.

Whether the mismatch should fail the deploy or only warn loudly is worth a moment's
thought: failing is right if a monitored deploy is a requirement, and wrong if some targets
are legitimately unmonitored while sharing credentials. The criteria above assume failing,
because the alternative is what produced the silent case; say so on this issue if that
assumption is wrong for some target.

### Context `deploy-vps` decides whether to arm the monitoring profile from the **credentials it was given**, not from the **compose file it is deploying**. When the Grafana variables are present it sets `COMPOSE_PROFILES=monitoring` and runs `docker compose up`. Compose does not treat an unknown profile as an error. It selects the services carrying that profile, finds none, and starts the rest normally — exit code 0, deploy reported healthy. The result is a deployment that looks fully monitored and emits nothing, which surfaces later as a not-reporting alert firing forever against a healthy service. That is the worst shape a failure can take: silent at the moment it happens, and misattributed when it is finally noticed. This was hit for real in `ahmad/imamah` (its #134): one of two compose variants declared the collector and the other did not, and the deploy armed the profile against both. That instance is fixed in that repository. The action still has the hole, for every repository that uses it. ### Scope - In: `deploy-vps` refusing to arm a profile the target compose file does not declare, and saying so. - Out: deciding *which* services a repository should run under `monitoring`; the imamah fix, already done. ### Acceptance criteria - [ ] Given a compose file that declares no service with the `monitoring` profile, when the deploy runs with the Grafana credentials present, then it fails with a message naming the profile and the compose file — rather than starting the remaining services. - [ ] Given a compose file that does declare the profile, when the deploy runs with the credentials present, then it arms it exactly as it does today. - [ ] Given the credentials are absent, when the deploy runs, then the profile is not armed and the check does not run. - [ ] Given the check is added, when the action's tests run, then all three cases above are covered. ### Notes `docker compose config --profiles` lists the profiles a compose file declares, which is enough to make this a two-line guard rather than a parser. Whether the mismatch should fail the deploy or only warn loudly is worth a moment's thought: failing is right if a monitored deploy is a requirement, and wrong if some targets are legitimately unmonitored while sharing credentials. The criteria above assume failing, because the alternative is what produced the silent case; say so on this issue if that assumption is wrong for some target.

ahmad added the

labels

2026-09-04 11:43:24 +00:00

ahmad self-assigned this

2026-09-05 07:33:36 +00:00

ahmad added

state/in-progress

and removed

state/ready

labels

2026-09-05 07:33:44 +00:00

ahmad commented

2026-09-05 07:35:02 +00:00

Author

Owner

Taking this. One file, deploy-vps/action.yml.

Where the check goes, and why not where the bug is. The profile is armed locally, from the credentials alone. The obvious fix is to check the compose file there — but the only reliable way to ask "which profiles does this file declare" is docker compose config --profiles, and the action's local steps cannot assume a docker CLI inside the job container. Grepping YAML for profiles: would be dependency-free and wrong in both directions; a false negative would refuse a perfectly good deployment. So the guard goes in the remote script, immediately before up, where docker is guaranteed and the real parser is available. It reads the COMPOSE_PROFILES the .env already carries and compares it against the declared set.

The failure is loud rather than silent, which is the whole point. Compose treats an unknown profile as selecting nothing and still exits 0, so today an app whose compose does not declare monitoring deploys, reports success, and collects nothing — while its absent-metrics alert fires against a healthy service. That is ahmad/imamah#134 generically.

A problem with the fourth criterion, which is mine. I wrote "when the action's tests run, then all three cases are covered" — but this repository has no behavioural test harness for actions. check-actions.py parses each action.yml and runs bash -n over the run: bodies; it cannot execute one. So I will verify the three cases by extracting the guard and driving it against a stub docker compose — the same method used on the last few changes here — and say plainly that the criterion is met by verification rather than by a committed test. Building an action-test harness is a real gap and deserves its own issue rather than being smuggled in here.

Taking this. One file, `deploy-vps/action.yml`. **Where the check goes, and why not where the bug is.** The profile is armed locally, from the credentials alone. The obvious fix is to check the compose file there — but the only reliable way to ask "which profiles does this file declare" is `docker compose config --profiles`, and the action's local steps cannot assume a docker CLI inside the job container. Grepping YAML for `profiles:` would be dependency-free and wrong in both directions; a false negative would refuse a perfectly good deployment. So the guard goes in the **remote** script, immediately before `up`, where docker is guaranteed and the real parser is available. It reads the `COMPOSE_PROFILES` the .env already carries and compares it against the declared set. **The failure is loud rather than silent, which is the whole point.** Compose treats an unknown profile as selecting nothing and still exits 0, so today an app whose compose does not declare `monitoring` deploys, reports success, and collects nothing — while its absent-metrics alert fires against a healthy service. That is `ahmad/imamah#134` generically. **A problem with the fourth criterion, which is mine.** I wrote "when the action's tests run, then all three cases are covered" — but this repository has no behavioural test harness for actions. `check-actions.py` parses each `action.yml` and runs `bash -n` over the `run:` bodies; it cannot execute one. So I will verify the three cases by extracting the guard and driving it against a stub `docker compose` — the same method used on the last few changes here — and say plainly that the criterion is met by verification rather than by a committed test. Building an action-test harness is a real gap and deserves its own issue rather than being smuggled in here.

ahmad referenced this issue from a commit

2026-09-05 07:36:11 +00:00

fix: Refuse a profile the compose file does not declare

ahmad referenced this issue from a pull request that will close it,

2026-09-05 07:36:31 +00:00

fix: Refuse a profile the compose file does not declare #47

ahmad added

state/in-review

and removed

state/in-progress

labels

2026-09-05 07:37:06 +00:00