mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-07 11:22:31 +00:00
fix(images): install the curl the slim base dropped under the healthcheck
10c5b35e88moved simpleicons and roulette-wheel from node:26-bookworm to node:26-slim. Both declare healthcheck flavor curl, which renders CMD curl -f <url>, and neither Dockerfile installed anything. Measured with docker create and docker cp: node:26-bookworm carries /usr/bin/curl at 280800 bytes, node:26-slim carries neither curl nor wget. The probe therefore fails from the first beat and the container never reports healthy. sys-svc-compose waits on the database container alone in compose and on every stack task in swarm, so the consequence splits by mode. Under swarm the task dies with non-zero exit (137): dockerexec: unhealthy container, is rescheduled, dies again, and the converge wait fails after 600 attempts. Five jobs across the two runs ona534d5d46died that way, and none of them on the role they are named after: ERPNext#0, Decidim#0, Hugo#0, Nextcloud#9 and Nextcloud#12 all carry web-app-dashboard in their closure, dashboard enables simpleicons, and the failure reads 'not converged: simpleicons_simpleicons'. All five are swarm and tor, which reads like the tor axis until XWiki#1 is put beside them: swarm, tor, green, and simpleicons is not in its closure. The discriminator is the service, not the axis. Of the seven roles10c5b35e88rebased, bridgy-fed and postmarks probe over tcp, and flowise and bookwyrm install curl themselves. These two were the remainder. roulette-wheel had not been deployed yet when this was written, so it is the same defect caught before it was observed. The install goes into the runtime stage of the multi-stage build; the builder stage would have left the runtime image unchanged and nothing would have gone red. The lint reads the binary out of the probe the declaration renders rather than restating a flavor-to-binary table, so a flavor that changes its command moves the check with it, and it locates the flavor line per service so one marker cannot silence a sibling. Its scope stops at slim: nginx:1.31.6-alpine was measured too and ships curl as a real 272 KB binary plus busybox wget, so the four alpine roles that declare those flavors are served by their base, and widening the rule to them needs that measurement per image rather than an assumption from the word alpine. Not built. The image with the new layer has not been produced, and whether a second fault waits behind the healthcheck is decided by the next deploy. The apt path itself is not blind: BookWyrm#1 installs curl the same way on python:3.11-slim and is green in this run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
86f7d8c4f5
commit
c31c62efaf
4 files changed
+176
No files matched your search
@@ -108,6 +108,7 @@ The placement rule is per check. The catalog "Position" column uses these labels
|
||||
| `image-healthcheck` | same or above the service key | [test_image_healthcheck_declared.py](../../../../tests/integration/roles/test_image_healthcheck_declared.py) | Allows a `roles/*/meta/services.yml` entry whose image ships no `HEALTHCHECK` to declare none either. Default path: add `healthcheck:` to the entry, because a container without a probe counts as healthy the moment its process starts — which is how a service that came back empty passes a restore drill. Reserved for containers nothing can probe: a one-shot process that exits, or an image with neither a port nor a shell. |
|
||||
| `csp-unsafe` | same line | [test_csp_unsafe_requires_justification.py](../../../../tests/lint/ansible/roles/meta/test_csp_unsafe_requires_justification.py) | Documents an `unsafe-*: true` flag in `roles/*/meta/csp.yml`. MUST be paired with a `Reason:` on the same line naming what breaks without the relaxation. `unsafe-inline` re-admits the inline execution an XSS needs and `unsafe-eval` re-admits string-to-code, so the flag has to be verified against the running app, not copied from another role. |
|
||||
| `min-storage-headroom` | same line OR comment block above key | [test_min_storage_headroom.py](../../../../tests/external/update/docker/test_min_storage_headroom.py) | Allows a `roles/*/meta/services.yml` `min_storage` to exceed the measured compressed size of its own `image:`/`version:` by more than 1 GB. MUST be paired with a `Reason:` on the same comment line naming what occupies the space. Reserved for services whose data volume genuinely dominates the footprint; the default path is to lower the value, because an over-declared budget makes a host look too small and prunes a cache the deploy did not need to lose. |
|
||||
| `healthcheck-binary` | same or above | [test_healthcheck_binary_on_slim_base.py](../../../../tests/lint/ansible/roles/meta/test_healthcheck_binary_on_slim_base.py) | Exempts a `roles/*/meta/services.yml` healthcheck flavor from the requirement that the role's `files/Dockerfile` install the binary the probe runs, when the service sits on a `-slim` image. MUST name where the binary comes from instead. A `-slim` tag carries the runtime and nothing around it, so `curl` is absent and the probe fails from the first beat; under swarm the task is then reaped with exit 137 and every deployment holding the service in its closure fails on the convergence wait, naming the service rather than the missing binary. |
|
||||
| `galaxy-git-parity` | same or above | [test_galaxy_entries_have_git_source.py](../../../../tests/lint/repository/dependencies/test_galaxy_entries_have_git_source.py) | Exempts a `requirements/requirements.galaxy.yml` collection from the requirement to also carry a `type: git` entry in `requirements/requirements.git.yml`. Reserved for collections that publish no usable git remote; without the git entry a `galaxy.ansible.com` outage fails `make install`, because the fallback in `scripts/install/ansible.sh` cannot resolve the collection either. |
|
||||
| `loop-var-shadow` | same or above | [test_loop_var_shadowing.py](../../../../tests/lint/ansible/tasks/test_loop_var_shadowing.py) | Allows a `loop_var` to take the name of a variable declared in `group_vars/**` or in a role's `vars`/`defaults`. Inside the loop that name carries the item, so every lookup, template and nested include reading it sees the item instead, and ansible only warns. Reserved for a loop whose body reads the name as the item on purpose, typically because the item IS that value: state which meaning the body relies on. |
|
||||
| `firewall-fragment-accept` | same or above | [test_firewall_fragment_verdicts.py](../../../../tests/lint/ansible/jinja/test_firewall_fragment_verdicts.py) | Allows a rule in a `roles/*/templates/nftables.conf.j2` fragment to end in an `accept` verdict. Default path: express the intent as a `drop` of what must not pass, or put the rule in the chain carrying the policy it has to overrule, because an `accept` terminates only its own base chain and every other chain at that hook still runs, docker's `drop` policy on forwarded packets included. Reserved for an accept whose only job is to skip the fragment's own later rules. |
|
||||
|
||||
@@ -1,6 +1,12 @@
|
||||
ARG ROULETTE_IMAGE
|
||||
ARG ROULETTE_VERSION
|
||||
FROM ${ROULETTE_IMAGE}:${ROULETTE_VERSION}
|
||||
|
||||
RUN rm -rf /var/lib/apt/lists/* \
|
||||
&& apt-get -o Acquire::Retries=3 update && apt-get -o Acquire::Retries=3 install -y --no-install-recommends \
|
||||
curl \
|
||||
&& rm -rf /var/lib/apt/lists/*
|
||||
|
||||
WORKDIR /app
|
||||
ARG ROULETTE_WHEEL_APP_REL
|
||||
COPY ./${ROULETTE_WHEEL_APP_REL}/package.json ./${ROULETTE_WHEEL_APP_REL}/package-lock.json ./
|
||||
|
||||
@@ -10,6 +10,11 @@ RUN npm install
|
||||
ARG container_port
|
||||
FROM ${SIMPLEICONS_IMAGE}:${SIMPLEICONS_VERSION}
|
||||
|
||||
RUN rm -rf /var/lib/apt/lists/* \
|
||||
&& apt-get -o Acquire::Retries=3 update && apt-get -o Acquire::Retries=3 install -y --no-install-recommends \
|
||||
curl \
|
||||
&& rm -rf /var/lib/apt/lists/*
|
||||
|
||||
WORKDIR /app
|
||||
COPY --from=builder /app/node_modules ./node_modules
|
||||
COPY ./config/server.js .
|
||||
|
||||
@@ -0,0 +1,164 @@
|
||||
"""A healthcheck that shells out to a binary must find it in the image it runs in.
|
||||
|
||||
A ``-slim`` tag is the upstream's stripped variant: it carries the
|
||||
runtime and nothing around it. ``node:26-bookworm`` ships ``/usr/bin/curl``,
|
||||
``node:26-slim`` does not, and a ``healthcheck: flavor: curl`` renders
|
||||
``CMD curl -f <url>`` either way. The probe then fails from the first beat, and
|
||||
the container never reports healthy.
|
||||
|
||||
The two deploy modes answer differently, and neither answer names the binary.
|
||||
``sys-svc-compose`` waits on the database container alone in compose and on
|
||||
every stack task in swarm, so a permanently unhealthy service passes unnoticed
|
||||
under compose and stops the deploy under swarm: the task dies with ``non-zero
|
||||
exit (137): dockerexec: unhealthy container``, is rescheduled, dies again, and
|
||||
the wait fails. Every deployment holding the service in its closure fails with
|
||||
it, reported against the service that was waited for.
|
||||
|
||||
Scope
|
||||
=====
|
||||
Roles whose ``meta/services.yml`` declares a healthcheck flavor on an image
|
||||
whose ``version`` names a stripped variant. The binary is read from the probe
|
||||
the declaration renders (:func:`utils.docker.healthcheck.compose.compose`), not
|
||||
restated here, so a flavor that changes its command moves this check with it. A
|
||||
flavor whose command is a shell builtin or the image's own interpreter needs no
|
||||
package and is not checked.
|
||||
|
||||
The role's ``files/Dockerfile`` must then install that binary. An image that
|
||||
already ships it is not a defence: the tag is a moving target, and the role that
|
||||
depends on the binary is the one that should say so.
|
||||
|
||||
Per-role opt-out
|
||||
================
|
||||
``# nocheck: healthcheck-binary`` on the ``flavor`` line or the one above it,
|
||||
with a reason naming where the binary comes from instead.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import unittest
|
||||
|
||||
from utils.annotations.suppress import is_suppressed_at
|
||||
from utils.cache.files import read_text
|
||||
from utils.cache.yaml import load_yaml_any
|
||||
from utils.docker.healthcheck.compose import compose
|
||||
from utils.roles.mapping import ROLE_FILE_META_SERVICES
|
||||
|
||||
from . import PROJECT_ROOT
|
||||
|
||||
RULE = "healthcheck-binary"
|
||||
|
||||
STRIPPED = ("slim",)
|
||||
"""Tag fragments that name an upstream's stripped variant.
|
||||
|
||||
``alpine`` is deliberately out: measured against ``nginx:1.31.6-alpine``, the
|
||||
image carries ``/usr/bin/curl`` as a real 272 KB binary and ``wget`` as a
|
||||
busybox applet, so the four alpine roles that declare those flavors are served
|
||||
by their base today. Widening this tuple to cover them needs that measurement
|
||||
repeated per image, not an assumption from the word "alpine".
|
||||
"""
|
||||
|
||||
SHELL_BUILTINS = frozenset({"sh", "bash", "test", "true", "exit", "CMD-SHELL"})
|
||||
"""Probe commands that need no package: the image's own shell runs them."""
|
||||
|
||||
|
||||
def _probe_binary(flavor: object) -> str | None:
|
||||
"""The executable a declared flavor asks docker to run, if it is a program.
|
||||
|
||||
Args:
|
||||
flavor: the ``healthcheck.flavor`` value, one name or a list.
|
||||
"""
|
||||
try:
|
||||
probe = compose(flavor, port=80, path="/", hostname="localhost")
|
||||
test = probe.test()
|
||||
except Exception:
|
||||
return None
|
||||
argv = [str(part) for part in test]
|
||||
if not argv or argv[0] != "CMD" or len(argv) < 2:
|
||||
return None
|
||||
binary = argv[1]
|
||||
return None if binary in SHELL_BUILTINS else binary
|
||||
|
||||
|
||||
def _is_stripped(version: object) -> bool:
|
||||
text = str(version)
|
||||
return any(fragment in text for fragment in STRIPPED)
|
||||
|
||||
|
||||
def _installs(dockerfile_body: str, binary: str) -> bool:
|
||||
"""Whether the Dockerfile names the binary as something it installs."""
|
||||
return (
|
||||
re.search(rf"(?<![\w-]){re.escape(binary)}(?![\w-])", dockerfile_body)
|
||||
is not None
|
||||
)
|
||||
|
||||
|
||||
def _flavor_lines(lines: list[str]) -> dict[str, int]:
|
||||
"""The 1-based ``flavor:`` line of each top-level service key.
|
||||
|
||||
``meta/services.yml`` maps one service per top-level key, so the flavor a
|
||||
key owns is the first one under it. Taking the file's first flavor line
|
||||
instead would let one service's marker silence another's.
|
||||
|
||||
Args:
|
||||
lines: the file's lines, as :func:`read_text` splits them.
|
||||
"""
|
||||
found: dict[str, int] = {}
|
||||
current: str | None = None
|
||||
for number, line in enumerate(lines, start=1):
|
||||
key = re.match(r"([A-Za-z0-9_.-]+):\s*(?:#.*)?$", line)
|
||||
if key:
|
||||
current = key.group(1)
|
||||
continue
|
||||
if current and current not in found and re.match(r"\s+flavor:", line):
|
||||
found[current] = number
|
||||
return found
|
||||
|
||||
|
||||
class TestHealthcheckBinaryOnSlimBase(unittest.TestCase):
|
||||
def test_a_stripped_base_installs_the_binary_its_healthcheck_runs(self):
|
||||
offenders = []
|
||||
for services in sorted(
|
||||
(PROJECT_ROOT / "roles").glob(f"*/{ROLE_FILE_META_SERVICES}")
|
||||
):
|
||||
role_dir = services.parent.parent
|
||||
data = load_yaml_any(str(services), default_if_missing={}) or {}
|
||||
lines = read_text(str(services)).splitlines()
|
||||
flavor_lines = _flavor_lines(lines)
|
||||
for key, entry in data.items():
|
||||
if not isinstance(entry, dict):
|
||||
continue
|
||||
health = entry.get("healthcheck")
|
||||
if not isinstance(health, dict) or "flavor" not in health:
|
||||
continue
|
||||
if not _is_stripped(entry.get("version", "")):
|
||||
continue
|
||||
binary = _probe_binary(health["flavor"])
|
||||
if binary is None:
|
||||
continue
|
||||
if is_suppressed_at(lines, flavor_lines.get(key, 0), RULE):
|
||||
continue
|
||||
dockerfile = role_dir / "files" / "Dockerfile"
|
||||
body = read_text(str(dockerfile)) if dockerfile.is_file() else ""
|
||||
if _installs(body, binary):
|
||||
continue
|
||||
offenders.append(
|
||||
f"{role_dir.name}: service '{key}' declares healthcheck "
|
||||
f"flavor {health['flavor']!r}, which runs {binary!r}, on the "
|
||||
f"stripped image {entry.get('image')}:{entry.get('version')}; "
|
||||
f"{'files/Dockerfile does not install it' if body else 'the role has no files/Dockerfile'}"
|
||||
)
|
||||
|
||||
self.assertEqual(
|
||||
[],
|
||||
offenders,
|
||||
"A stripped base carries the runtime and nothing around it, so a "
|
||||
"healthcheck binary has to be installed by the role that depends on "
|
||||
"it. Add it to the role's files/Dockerfile, pick a flavor the image "
|
||||
f"can already run, or mark the line `# nocheck: {RULE}` with a "
|
||||
"reason naming where the binary comes from:\n " + "\n ".join(offenders),
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user