The appliance boot gate reports success when it skips the boot #14

New issue

Open

opened 2026-09-27 23:34:01 -03:00 by marcos-mendez · 2 comments

marcos-mendez commented

2026-09-27 23:34:01 -03:00

(Migrated from github.com)

Measured on Keel-Linux/keel-redis#5, run 36369705307, job 108763227333,
2026-09-28. The job appliance / build-and-boot reported success after
11 seconds. Its steps:

#	Step	Conclusion
5	Is the layer published?	success — answered no
6	Check the published parent against the caller	skipped
7	Pull the layer chain from the mirror	skipped
8	Verify the layers	skipped
9	Assemble and boot, then run the appliance's boot test	skipped
10	Summary	skipped

So the gate that is supposed to prove an appliance boots reported that it does,
having never built, pulled, verified or booted anything.

This is by design. .github/workflows/test-appliance.yml line 29 says "A layer
that has never been published is not a failure: the job says so", and steps 6
through 10 are all guarded by if: steps.layer.outputs.published == 'true'.

Why it is still a defect

The intent is reasonable — you cannot boot a layer that does not exist, and a
brand new appliance repository has to be able to open its first pull request.
The consequence is not reasonable:

A new appliance gets a permanently green boot gate until somebody
publishes its layer, and nothing reminds anyone to.
Branch protection that requires appliance / build-and-boot is satisfied by
that green, so the protection is inert during exactly the window where the
code is newest and least proven.
The green is indistinguishable, in the pull request UI and in gh pr checks,
from a green that booted a container and ran the real service. A reviewer
reading "pass" has no way to know which one they got without opening the job
and reading the step list.

keel-redis#5 is in that state right now and looks ready to merge.

What closes this

A skipped boot must not be reportable as a pass. Any of these would do, and the
choice should be argued in the pull request rather than assumed:

Report the job as neutral rather than success when the layer is
unpublished, so branch protection is not satisfied by it.
Require an explicit caller input (for example allow_unpublished: true) that
a new appliance sets deliberately and removes once its layer is published,
so the exemption is visible in the caller's own workflow file and dies
naturally.
Fail when the layer has ever been published before, so the exemption applies
only to the genuine bootstrap case and cannot regress.

Whatever is chosen, the check's name or conclusion must differ between
"booted the appliance" and "did not boot anything", because the failure here is
that the two are indistinguishable.

Scope

Every repository that calls the shared test-appliance workflow is affected.
Audit which of them currently rely on it while unpublished, and say so in the
pull request — a green that was never earned may exist on more than one repo.

Measured on Keel-Linux/keel-redis#5, run 36369705307, job 108763227333, 2026-09-28. The job `appliance / build-and-boot` reported **success** after 11 seconds. Its steps: | # | Step | Conclusion | |---|---|---| | 5 | Is the layer published? | success — answered **no** | | 6 | Check the published parent against the caller | skipped | | 7 | Pull the layer chain from the mirror | skipped | | 8 | Verify the layers | skipped | | 9 | Assemble and boot, then run the appliance's boot test | skipped | | 10 | Summary | skipped | So the gate that is supposed to prove an appliance boots reported that it does, having never built, pulled, verified or booted anything. This is by design. `.github/workflows/test-appliance.yml` line 29 says "A layer that has never been published is not a failure: the job says so", and steps 6 through 10 are all guarded by `if: steps.layer.outputs.published == 'true'`. ## Why it is still a defect The intent is reasonable — you cannot boot a layer that does not exist, and a brand new appliance repository has to be able to open its first pull request. The consequence is not reasonable: - A new appliance gets a **permanently green boot gate** until somebody publishes its layer, and nothing reminds anyone to. - Branch protection that requires `appliance / build-and-boot` is satisfied by that green, so the protection is inert during exactly the window where the code is newest and least proven. - The green is indistinguishable, in the pull request UI and in `gh pr checks`, from a green that booted a container and ran the real service. A reviewer reading "pass" has no way to know which one they got without opening the job and reading the step list. `keel-redis#5` is in that state right now and looks ready to merge. ## What closes this A skipped boot must not be reportable as a pass. Any of these would do, and the choice should be argued in the pull request rather than assumed: - Report the job as **neutral** rather than success when the layer is unpublished, so branch protection is not satisfied by it. - Require an explicit caller input (for example `allow_unpublished: true`) that a new appliance sets deliberately and removes once its layer is published, so the exemption is visible in the caller's own workflow file and dies naturally. - Fail when the layer has ever been published before, so the exemption applies only to the genuine bootstrap case and cannot regress. Whatever is chosen, the check's **name or conclusion** must differ between "booted the appliance" and "did not boot anything", because the failure here is that the two are indistinguishable. ## Scope Every repository that calls the shared `test-appliance` workflow is affected. Audit which of them currently rely on it while unpublished, and say so in the pull request — a green that was never earned may exist on more than one repo.

marcos-mendez commented

2026-09-27 23:34:54 -03:00

(Migrated from github.com)

Two corrections after auditing, one in the workflow's favour and one against.

In its favour: the skip is not silent. Step 5 writes a ::notice:: and a
job summary that says, in as many words, "Nothing was built, assembled or
booted", and tells the reader to build and publish the layer. So the workflow
does record what it did. The defect is narrower than first stated, and it is
this: the conclusion is success, and a conclusion is what branch
protection reads and what gh pr checks prints. A summary nobody opens does
not stop an unearned merge.

Against it: the audit. Asking the mirror for each appliance's manifest at
https://mirror.keellinux.org/layers/<name>.manifest, 2026-09-28:

Layer	Manifest
core, apache-php, nodejs-nginx, mariadb, postgresql, nodebb, wordpress, lamp, lamp-client, lapp, lapp-client	200
redis	404

So exactly one repository is in the unearned-green state today, keel-redis,
and the other eleven gates are real. That bounds the damage, and it also shows
the exemption behaves exactly as designed — which is the point: the design is
what needs changing, not an accident to be patched.

The practical unblock for keel-redis#5 is to publish the redis layer and
re-run the gate, which is already in progress. That fixes the instance. This
issue stays open for the design.

Two corrections after auditing, one in the workflow's favour and one against. **In its favour: the skip is not silent.** Step 5 writes a `::notice::` and a job summary that says, in as many words, "Nothing was built, assembled or booted", and tells the reader to build and publish the layer. So the workflow does record what it did. The defect is narrower than first stated, and it is this: the **conclusion** is `success`, and a conclusion is what branch protection reads and what `gh pr checks` prints. A summary nobody opens does not stop an unearned merge. **Against it: the audit.** Asking the mirror for each appliance's manifest at `https://mirror.keellinux.org/layers/<name>.manifest`, 2026-09-28: | Layer | Manifest | |---|---| | core, apache-php, nodejs-nginx, mariadb, postgresql, nodebb, wordpress, lamp, lamp-client, lapp, lapp-client | 200 | | redis | **404** | So exactly one repository is in the unearned-green state today, `keel-redis`, and the other eleven gates are real. That bounds the damage, and it also shows the exemption behaves exactly as designed — which is the point: the design is what needs changing, not an accident to be patched. The practical unblock for `keel-redis#5` is to publish the redis layer and re-run the gate, which is already in progress. That fixes the instance. This issue stays open for the design.

marcos-mendez commented

2026-09-27 23:55:18 -03:00

(Migrated from github.com)

A code review of keel-mariadb#11 and keel-postgresql#7 found a second, wider
face of this defect, and it is worse than the one this issue was opened for.

Even when the layer is published, the gate does not test the pull
request. appliance / build-and-boot fetches the published layer from the
mirror and boots that. On a pull request that changes the recipe, the published
layer is the one built from main — i.e. the code before the change. The job
boots it, the service answers, the check goes green, and nothing in the pull
request was exercised at all.

So the failure mode this issue records ("skipped the boot, still green") is the
special case. The general case is: the boot gate is green on the strength of
code that is already merged, whatever the branch contains.

That makes the required check appliance / build-and-boot unable to do the job
branch protection is asking of it on any recipe change. It still has real value
— it catches a published layer that does not boot, and it caught the parent
mismatch it was written for — but it is not evidence about a branch.

Both of those pull requests state this honestly in their own bodies, with the
box left unticked, so this was known to their author and is not a
misrepresentation. It was not known to me when I applied branch protection
requiring that check on keel-lamp, keel-lapp and keel-apache-php, nor when I
was about to merge on the strength of three green ticks.

What this adds to the fix

Whatever is decided for the unpublished case, the published case needs an
answer too:

The honest version is that the gate builds the layer from the branch and
boots that. That is expensive and needs the build host, which is serialised
behind a lock, so it may not be a per-pull-request check at all.
The cheap version is to keep the current job but rename it to what it
actually does — it verifies the published layer, not the change — so nobody
reads it as proof of a branch again, and stop requiring it for merge.
Either way, a recipe change needs some gate that exercises the recipe.
Today the only evidence for such a change is a hand-run build that is not
reproducible (see the review's finding on the measurement harness).

Naming a check for what it proves is the whole of this issue. It applies to
both faces.

A code review of keel-mariadb#11 and keel-postgresql#7 found a second, wider face of this defect, and it is worse than the one this issue was opened for. **Even when the layer *is* published, the gate does not test the pull request.** `appliance / build-and-boot` fetches the *published* layer from the mirror and boots that. On a pull request that changes the recipe, the published layer is the one built from `main` — i.e. the code *before* the change. The job boots it, the service answers, the check goes green, and nothing in the pull request was exercised at all. So the failure mode this issue records ("skipped the boot, still green") is the special case. The general case is: **the boot gate is green on the strength of code that is already merged, whatever the branch contains.** That makes the required check `appliance / build-and-boot` unable to do the job branch protection is asking of it on any recipe change. It still has real value — it catches a published layer that does not boot, and it caught the parent mismatch it was written for — but it is not evidence about a branch. Both of those pull requests state this honestly in their own bodies, with the box left unticked, so this was known to their author and is not a misrepresentation. It was not known to me when I applied branch protection requiring that check on keel-lamp, keel-lapp and keel-apache-php, nor when I was about to merge on the strength of three green ticks. ## What this adds to the fix Whatever is decided for the unpublished case, the published case needs an answer too: - The honest version is that the gate builds the layer *from the branch* and boots that. That is expensive and needs the build host, which is serialised behind a lock, so it may not be a per-pull-request check at all. - The cheap version is to keep the current job but rename it to what it actually does — it verifies the published layer, not the change — so nobody reads it as proof of a branch again, and stop requiring it for merge. - Either way, a recipe change needs *some* gate that exercises the recipe. Today the only evidence for such a change is a hand-run build that is not reproducible (see the review's finding on the measurement harness). Naming a check for what it proves is the whole of this issue. It applies to both faces.