From 0bb437d9ce0acd4bcaa1f8d6aeddbfa5144bddb7 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Mon, 5 Oct 2026 22:11:48 +0200 Subject: [PATCH] feat(images): check declared architectures against the registry, and make a pin say why Nothing held a role's architectures to reality: test_image_reachable proves a tag exists, test_arm64_images proves four named roles publish arm64, and the closure lint only proves the declarations do not intersect to nothing. An amd64-only bump became a scheduling failure on an arm64 row rather than a red test. A new external test sweeps every pinned image and judges a role by the intersection over its services, because a role runs only where all of its images run; judging per image would demand arm64 from an amd64-only role's nginx and call a narrowing stale because nginx publishes arm64 while the application does not. A role declaring nothing must cover every architecture the matrix rotates; a role that narrows must narrow to exactly its capability, which is what retires its TODO by itself. An unread image downgrades the stale verdict to a warning, since the image a probe could not read may be the one the narrowing exists for. The probe reads the mirror first and caches on disk, because the sweep is ~160 manifests and Docker Hub counts its allowance per IP, shared. A new lint requires every architectures declaration to carry a nocheck marker and a Reason. bigbluebutton and opentalk keep their pins with measured reasons; decidim, pretix and taiga lose theirs, because each builds its own image and their upstream Dockerfiles are arch-neutral. Their source builds are not converted yet, so until that lands the matrix may hand them an arm64 row that fails at FROM. Co-Authored-By: Claude Opus 5 (1M context) --- .../actions/testing/suppression.md | 1 + roles/web-app-bigbluebutton/meta/services.yml | 1 + roles/web-app-decidim/meta/services.yml | 3 - roles/web-app-opentalk/meta/services.yml | 1 + roles/web-app-pretix/meta/services.yml | 3 - roles/web-app-taiga/meta/services.yml | 3 - .../update/docker/test_image_architectures.py | 195 ++++++++++++++++++ .../docker/test_min_storage_headroom.py | 20 +- .../test_architecture_pin_is_justified.py | 110 ++++++++++ .../python/utils/annotations/test_suppress.py | 65 ++++++ .../utils/docker/test_image_architectures.py | 98 +++++++++ utils/annotations/suppress.py | 31 +++ utils/docker/image/architectures.py | 70 +++++++ utils/docker/registry.py | 79 +++++++ 14 files changed, 653 insertions(+), 27 deletions(-) create mode 100644 tests/external/update/docker/test_image_architectures.py create mode 100644 tests/lint/ansible/roles/meta/test_architecture_pin_is_justified.py create mode 100644 tests/unit/python/utils/docker/test_image_architectures.py create mode 100644 utils/docker/image/architectures.py diff --git a/docs/contributing/actions/testing/suppression.md b/docs/contributing/actions/testing/suppression.md index f02f621a13..11b392d169 100644 --- a/docs/contributing/actions/testing/suppression.md +++ b/docs/contributing/actions/testing/suppression.md @@ -114,6 +114,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. | +| `architecture-pin` | same line OR comment block above key | [test_architecture_pin_is_justified.py](../../../../tests/lint/ansible/roles/meta/test_architecture_pin_is_justified.py) | Allows a `roles/*/meta/services.yml` entity to declare `architectures`, which takes the role's rows out of part of the deploy matrix so it stops being exercised where it is narrowed away. MUST be paired with a `Reason:` naming what cannot run elsewhere. Reserved for an upstream that publishes no manifest for the dropped architecture; where the role builds its own image, the default path is to build it for both instead. [test_image_architectures.py](../../../../tests/external/update/docker/test_image_architectures.py) fails once every image the role pins publishes the narrowed-away architecture, which is when the declaration and this marker are deleted together. | | `mem-limit-budget` | anywhere inside the variant's own entry | [test_resource_budget.py](../../../../tests/integration/roles/meta/variants/test_resource_budget.py) | Lets one `meta/variants.yml` entry exceed the 64 GB `mem_limit` cap. MUST carry text after the marker naming what the round transitively bills and why the overage is acceptable; a bare marker grants nothing. `mem_reservation` stays enforced, and `test_every_mem_limit_exemption_is_still_needed` fails once the variant is back under the cap. | | `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. | diff --git a/roles/web-app-bigbluebutton/meta/services.yml b/roles/web-app-bigbluebutton/meta/services.yml index b9c56e7533..4efdd2a67e 100644 --- a/roles/web-app-bigbluebutton/meta/services.yml +++ b/roles/web-app-bigbluebutton/meta/services.yml @@ -27,6 +27,7 @@ email: # nocheck: playwright-service-flag: shared provider entity; the app itself is covered by this role's own Playwright spec bigbluebutton: # TODO: make this role arm64-capable so the declaration below can go + # nocheck: architecture-pin Reason: alangecker/bbb-docker-web answers with a single linux/amd64 manifest rather than a manifest list, and the stack is pulled rather than built here, so no arm64 node can run it architectures: - amd64 modes: diff --git a/roles/web-app-decidim/meta/services.yml b/roles/web-app-decidim/meta/services.yml index b02de206fe..c43ecbbf69 100644 --- a/roles/web-app-decidim/meta/services.yml +++ b/roles/web-app-decidim/meta/services.yml @@ -36,9 +36,6 @@ decidim: enabled: true swarm: enabled: true - # TODO: drop this once upstream publishes an arm64 manifest for decidim - architectures: - - amd64 image: ghcr.io/decidim/decidim version: 0.31.6 min_storage: 1206MB diff --git a/roles/web-app-opentalk/meta/services.yml b/roles/web-app-opentalk/meta/services.yml index bc6067a322..5662666ffd 100644 --- a/roles/web-app-opentalk/meta/services.yml +++ b/roles/web-app-opentalk/meta/services.yml @@ -53,6 +53,7 @@ redis: pids_limit: 512 opentalk: # TODO: make this role arm64-capable so the declaration below can go + # nocheck: architecture-pin Reason: opentalk/web-frontend, opentalk/controller and opentalk/recorder each answer with a single linux/amd64 manifest on registry.opencode.de, and the images are pulled rather than built here architectures: - amd64 modes: diff --git a/roles/web-app-pretix/meta/services.yml b/roles/web-app-pretix/meta/services.yml index 65b2e17eec..70bcdb073e 100644 --- a/roles/web-app-pretix/meta/services.yml +++ b/roles/web-app-pretix/meta/services.yml @@ -52,9 +52,6 @@ pretix: enabled: true swarm: enabled: true - # TODO: drop this once upstream publishes an arm64 manifest for pretix/standalone - architectures: - - amd64 image: pretix/standalone version: 2026.8.0 custom: true diff --git a/roles/web-app-taiga/meta/services.yml b/roles/web-app-taiga/meta/services.yml index b4b87558c2..7361cdd46f 100644 --- a/roles/web-app-taiga/meta/services.yml +++ b/roles/web-app-taiga/meta/services.yml @@ -35,9 +35,6 @@ postgres: enabled: true # nocheck: dynamic-flag shared: "{{ 'svc-db-postgres' in group_names }}" taiga: - # TODO: make this role arm64-capable so the declaration below can go - architectures: - - amd64 healthcheck: test: ["CMD", "python", "-c", "import socket; socket.create_connection(('127.0.0.1',8000),5).close()"] retries: 5 diff --git a/tests/external/update/docker/test_image_architectures.py b/tests/external/update/docker/test_image_architectures.py new file mode 100644 index 0000000000..835ec13b7a --- /dev/null +++ b/tests/external/update/docker/test_image_architectures.py @@ -0,0 +1,195 @@ +"""Hold every role's ``architectures`` to what its images actually publish. + +``architectures`` in ``meta/services.yml`` is what narrows a role's rows in +the deploy matrix, and a role that declares nothing takes whatever the +rotation assigns it. Nothing has ever checked either half against a registry: +``test_image_reachable.py`` proves a tag exists, ``test_arm64_images.py`` +proves four named roles publish arm64, and the closure lint only proves the +declarations do not intersect to nothing. An amd64-only bump therefore turns +into a scheduling failure on an arm64 row rather than a red test here. + +A role can only be placed where *every* image it pins can run, so its +capability is the intersection over its services, not a property of any one +of them. Checking per image would be wrong in both directions: it would +demand arm64 from a sidecar on an amd64-only role, and it would call a +narrowing stale because ``nginx`` publishes arm64 while the application +image does not. + +Two rules, one per polarity of the declaration: + +* a role that declares no ``architectures`` is offered every architecture in + ``utils.github.variant.pools.ARCHITECTURES``, so its capability has to + cover all of them; +* a role that narrows must narrow to exactly its capability. An architecture + it claims that no longer runs cannot be scheduled, and one its images have + all since gained is a pin that keeps the role testing narrower than it + could while nothing says so. The second half is what retires a + ``# TODO: drop this once upstream publishes an arm64 manifest`` by itself. + +A ``version_variants`` ref is skipped: it is a tag the mirror must carry, not +a version a role deploys. + +Indeterminate registry answers only warn, matching ``test_image_reachable.py`` +and ``test_arm64_images.py``: a rate limit must never read as a missing +architecture. A role is judged on the images that did answer, and skipped +entirely when none did, so a throttled sweep understates rather than invents. + +Opt-in external test: it hits live third-party registries and runs only under +the external suite. The sweep is ~160 images and Docker Hub counts its +anonymous allowance per IP, shared with everything else on that address, so +the probe goes through :func:`manifest_platforms_probed`, which reads the +mirror first and caches on disk. The run reports how many answers came from +each source, because a mirror that silently stops answering turns this test +into the thing that exhausts the allowance. +""" + +from __future__ import annotations + +import collections +import concurrent.futures +import unittest + +from utils.annotations.message import warning +from utils.docker.image.architectures import role_capability +from utils.docker.image.discovery import iter_role_images +from utils.docker.registry import manifest_platforms_probed +from utils.github.variant.pools import ARCHITECTURES +from utils.roles.mapping import ROLE_FILE_META_SERVICES +from utils.roles.meta_lookup import get_role_architectures +from utils.update.base import resolve_max_fetch_workers + +from . import PROJECT_ROOT + + +def _pull_image(ref) -> str: + return ref.name if ref.registry == "docker.io" else f"{ref.registry}/{ref.name}" + + +class ImageArchitectures(unittest.TestCase): + """One registry sweep, shared by both rules.""" + + probed: dict = {} + sources: collections.Counter = collections.Counter() + by_role: dict = {} + + @classmethod + def setUpClass(cls) -> None: + refs = [ref for ref in iter_role_images(PROJECT_ROOT) if not ref.derived] + cls.by_role = collections.defaultdict(list) + for ref in refs: + cls.by_role[ref.role].append(ref) + + def _probe(pair): + return pair, manifest_platforms_probed(*pair) + + pairs = {(_pull_image(ref), ref.version) for ref in refs} + with concurrent.futures.ThreadPoolExecutor( + max_workers=resolve_max_fetch_workers() + ) as pool: + for pair, (platforms, source) in pool.map(_probe, pairs): + cls.probed[pair] = platforms + cls.sources[source] += 1 + + def _capability(self, role: str): + """Probe every image of *role* and reduce it to where the role runs. + + Args: + role: role directory name. + + Returns: + What :func:`role_capability` returns for the role's images. + """ + offered = {} + for ref in self.by_role[role]: + platforms = self.probed.get((_pull_image(ref), ref.version)) + if platforms is None: + warning( + f"{ref.role}/{ref.service}: " + f"{_pull_image(ref)}:{ref.version} published architectures " + "could not be read (network / auth / rate limit)", + title="🔍 Unverified image architectures", + file=f"roles/{ref.role}/{ROLE_FILE_META_SERVICES}", + ) + offered[f"{ref.service} ({_pull_image(ref)}:{ref.version})"] = platforms + return role_capability(offered) + + @staticmethod + def _named(blame: dict, architecture: str) -> str: + return ", ".join(sorted(blame.get(architecture, []))) or "no image" + + def test_the_sweep_reaches_the_registries(self) -> None: + if self.sources["upstream"]: + warning( + f"{self.sources['upstream']} image(s) were read from their " + "upstream registry because the mirror does not carry them; " + "that is the half of the sweep which spends a rate limit", + title="🪞 Mirror miss", + ) + self.assertGreater( + len(self.probed) - self.sources["none"], + 0, + "no registry answered for any pinned image, so both rules below " + f"would pass vacuously (sources: {dict(self.sources)})", + ) + + def test_an_undeclared_role_runs_on_every_architecture(self) -> None: + offenders = [] + for role in sorted(self.by_role): + if get_role_architectures(role): + continue + capability, blame, _unread = self._capability(role) + if capability is None: + continue + for architecture in sorted(set(ARCHITECTURES) - capability): + offenders.append( + f"{role}: declares no `architectures`, so the matrix may " + f"place it on {architecture}, which " + f"{self._named(blame, architecture)} cannot run" + ) + self.assertEqual( + [], + offenders, + "role(s) the matrix may place where their images cannot run. " + "Declare `architectures` on the role, or pin images that publish " + "them:\n" + "\n".join(f" - {o}" for o in offenders), + ) + + def test_a_declared_role_narrows_to_what_its_images_publish(self) -> None: + offenders = [] + for role in sorted(self.by_role): + declared = set(get_role_architectures(role)) + if not declared: + continue + capability, blame, unread = self._capability(role) + if capability is None: + continue + for architecture in sorted(declared - capability): + offenders.append( + f"{role}: `architectures` claims {architecture}, which " + f"{self._named(blame, architecture)} cannot run" + ) + stale = sorted(capability - declared) + if stale and unread: + warning( + f"{role}: every image that answered runs on {stale}, but " + f"{len(unread)} did not answer, and an unread image may be " + "the one the narrowing exists for", + title="🔍 Narrowing not judged", + file=f"roles/{role}/{ROLE_FILE_META_SERVICES}", + ) + elif stale: + offenders.append( + f"{role}: every image now runs on {stale}, which " + f"`architectures` narrows away; drop the declaration and " + f"the TODO above it" + ) + self.assertEqual( + [], + offenders, + "role(s) whose `architectures` disagrees with their images:\n" + + "\n".join(f" - {o}" for o in offenders), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/external/update/docker/test_min_storage_headroom.py b/tests/external/update/docker/test_min_storage_headroom.py index 9159113553..622f731250 100644 --- a/tests/external/update/docker/test_min_storage_headroom.py +++ b/tests/external/update/docker/test_min_storage_headroom.py @@ -40,7 +40,7 @@ import unittest from dataclasses import dataclass from utils.annotations.message import warning -from utils.annotations.suppress import line_has_rule +from utils.annotations.suppress import has_rule_with_reason from utils.cache.files import read_text from utils.docker.image.discovery import load_yaml from utils.docker.registry import manifest_transfer_size @@ -56,7 +56,6 @@ _GIB = 1024**3 _TOP_LEVEL_KEY = re.compile(r"^([A-Za-z0-9][A-Za-z0-9_.-]*):") _MIN_STORAGE_KEY = re.compile(r"^\s+min_storage:") -_REASON = re.compile(r"#.*\breason\b\s*:\s*\S", re.IGNORECASE) @dataclass(frozen=True) @@ -108,22 +107,7 @@ def _declarations(role_name: str) -> tuple[list[_Declaration], list[str]]: def _justified(lines: list[str], line_no: int) -> bool: - """Whether the declaration at 1-based *line_no* carries marker and reason. - - Scanned on that line and across the contiguous comment lines directly above - it; the order of the two markers within that block does not matter. - """ - idx = line_no - 1 - if idx < 0 or idx >= len(lines): - return False - has_rule = line_has_rule(lines[idx], _RULE) - has_reason = bool(_REASON.search(lines[idx])) - scan = idx - 1 - while scan >= 0 and lines[scan].lstrip().startswith("#"): - has_rule = has_rule or line_has_rule(lines[scan], _RULE) - has_reason = has_reason or bool(_REASON.search(lines[scan])) - scan -= 1 - return has_rule and has_reason + return has_rule_with_reason(lines, line_no, _RULE) def _measure(pairs: set[tuple[str, str]]) -> dict[tuple[str, str], int | None]: diff --git a/tests/lint/ansible/roles/meta/test_architecture_pin_is_justified.py b/tests/lint/ansible/roles/meta/test_architecture_pin_is_justified.py new file mode 100644 index 0000000000..7e1eda9386 --- /dev/null +++ b/tests/lint/ansible/roles/meta/test_architecture_pin_is_justified.py @@ -0,0 +1,110 @@ +"""Every pinned ``architectures`` has to say why it is pinned. + +``architectures`` in ``meta/services.yml`` takes a role's rows out of part of +the deploy matrix, so the role stops being exercised where it is narrowed +away. That is sometimes correct - an upstream that publishes only amd64 +cannot be scheduled on arm64 - and it is never self-evident from the three +lines it takes to write. Without a stated reason the next reader cannot tell +a pin that upstream forces from one that a long-gone build quirk left behind, +and the external architecture test can only say the pin disagrees with the +registry, never whether it should have existed. + +The reason is unchecked prose. The gate is that a human had to write one. + +Convention +========== +On the ``architectures`` line, or on the contiguous comment lines directly +above it, name the rule and the reason: + + # nocheck: architecture-pin Reason: + architectures: + - amd64 + +Both markers are required, in either order. A role that stops needing the +pin deletes the declaration and this comment together; the external test +``test_image_architectures.py`` is what notices that moment, by failing once +every image the role pins publishes the architecture it narrows away. +""" + +from __future__ import annotations + +import re +import unittest + +from utils.annotations.suppress import has_rule_with_reason +from utils.cache.files import read_text +from utils.roles.mapping import ROLE_FILE_META_SERVICES +from utils.roles.meta_lookup import get_role_architectures + +from . import PROJECT_ROOT + +_RULE = "architecture-pin" +_ARCHITECTURES_KEY = re.compile(r"^\s+architectures:") +ROLES_DIR = PROJECT_ROOT / "roles" + + +def _declaration_lines(lines: list[str]) -> list[int]: + """Return the 1-based lines on which ``architectures:`` is declared. + + Args: + lines: the ``meta/services.yml`` lines. + """ + return [ + number + for number, line in enumerate(lines, start=1) + if _ARCHITECTURES_KEY.match(line) + ] + + +class TestArchitecturePinIsJustified(unittest.TestCase): + def test_every_narrowed_role_states_why(self) -> None: + offenders = [] + for services_file in sorted(ROLES_DIR.glob(f"*/{ROLE_FILE_META_SERVICES}")): + role = services_file.parent.parent.name + if not get_role_architectures(role): + continue + lines = read_text(str(services_file)).splitlines() + declarations = _declaration_lines(lines) + if not declarations: + continue + for line_no in declarations: + if has_rule_with_reason(lines, line_no, _RULE): + continue + offenders.append( + f"roles/{role}/{ROLE_FILE_META_SERVICES}:{line_no}: " + f"`architectures` narrows the deploy matrix without " + f"saying why" + ) + self.assertEqual( + [], + offenders, + "architecture pin(s) without a stated reason:\n" + + "\n".join(f" - {o}" for o in offenders) + + f"\n\nAdd `# nocheck: {_RULE} Reason: ` on the declaration or directly above it, or drop " + "the declaration if the pin is no longer needed.", + ) + + def test_the_scan_finds_the_roles_that_narrow(self) -> None: + narrowed = { + services_file.parent.parent.name + for services_file in ROLES_DIR.glob(f"*/{ROLE_FILE_META_SERVICES}") + if get_role_architectures(services_file.parent.parent.name) + } + unseen = sorted( + role + for role in narrowed + if not _declaration_lines( + read_text(str(ROLES_DIR / role / ROLE_FILE_META_SERVICES)).splitlines() + ) + ) + self.assertEqual( + [], + unseen, + "role(s) whose `architectures` the resolver reads but whose line " + f"the scan does not find, so the rule passes vacuously: {unseen}", + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/python/utils/annotations/test_suppress.py b/tests/unit/python/utils/annotations/test_suppress.py index c0473ca75c..94c758f20c 100644 --- a/tests/unit/python/utils/annotations/test_suppress.py +++ b/tests/unit/python/utils/annotations/test_suppress.py @@ -5,6 +5,7 @@ from __future__ import annotations import unittest from utils.annotations.suppress import ( + has_rule_with_reason, is_suppressed_anywhere, is_suppressed_at, is_suppressed_in_head, @@ -152,5 +153,69 @@ class TestSuppressedLineNumbers(unittest.TestCase): self.assertEqual(suppressed_line_numbers(lines, "shared"), {4}) +class TestHasRuleWithReason(unittest.TestCase): + """An exemption that has to say why, written above the value it exempts.""" + + def test_both_markers_on_the_value_line(self) -> None: + lines = [" min_storage: 20GB # nocheck: rule-x Reason: the volume"] + + self.assertTrue(has_rule_with_reason(lines, 1, "rule-x")) + + def test_both_markers_in_the_comment_block_above(self) -> None: + lines = [ + " # nocheck: rule-x", + " # Reason: the data volume dominates", + " min_storage: 20GB", + ] + + self.assertTrue(has_rule_with_reason(lines, 3, "rule-x")) + + def test_the_marker_order_does_not_matter(self) -> None: + lines = [ + " # Reason: the data volume dominates", + " # nocheck: rule-x", + " min_storage: 20GB", + ] + + self.assertTrue(has_rule_with_reason(lines, 3, "rule-x")) + + def test_a_marker_without_a_reason_is_refused(self) -> None: + lines = [" # nocheck: rule-x", " min_storage: 20GB"] + + self.assertFalse(has_rule_with_reason(lines, 2, "rule-x")) + + def test_a_reason_without_the_marker_is_refused(self) -> None: + lines = [" # Reason: the data volume dominates", " min_storage: 20GB"] + + self.assertFalse(has_rule_with_reason(lines, 2, "rule-x")) + + def test_an_empty_reason_is_refused(self) -> None: + lines = [" # nocheck: rule-x Reason:", " min_storage: 20GB"] + + self.assertFalse(has_rule_with_reason(lines, 2, "rule-x")) + + def test_another_rules_marker_does_not_count(self) -> None: + lines = [" # nocheck: rule-y Reason: something else", " min_storage: 20GB"] + + self.assertFalse(has_rule_with_reason(lines, 2, "rule-x")) + + def test_a_blank_line_ends_the_comment_block(self) -> None: + lines = [ + " # nocheck: rule-x Reason: the data volume dominates", + "", + " min_storage: 20GB", + ] + + self.assertFalse( + has_rule_with_reason(lines, 3, "rule-x"), + "a marker separated by a blank line belongs to the construct above it", + ) + + def test_a_line_number_outside_the_file_is_refused(self) -> None: + for line_no in (0, 5): + with self.subTest(line_no=line_no): + self.assertFalse(has_rule_with_reason(["x"], line_no, "rule-x")) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/python/utils/docker/test_image_architectures.py b/tests/unit/python/utils/docker/test_image_architectures.py new file mode 100644 index 0000000000..7130f9a472 --- /dev/null +++ b/tests/unit/python/utils/docker/test_image_architectures.py @@ -0,0 +1,98 @@ +"""The reduction from published platforms to where a role can be placed. + +The intersection is the whole point: a role pins an application image beside +generic sidecars, and judging per image gets it wrong in both directions. It +would demand arm64 from an amd64-only role's nginx, and it would call a +narrowing stale because nginx publishes arm64 while the application does not. +""" + +from __future__ import annotations + +import unittest + +from utils.docker.image.architectures import role_capability, runnable_architectures + +BOTH = {"linux/amd64", "linux/arm64"} +AMD64 = {"linux/amd64"} +ARM64 = {"linux/arm64"} + + +class TestRunnableArchitectures(unittest.TestCase): + def test_a_multi_platform_index_offers_both(self) -> None: + self.assertEqual({"amd64", "arm64"}, runnable_architectures(BOTH)) + + def test_a_foreign_os_carries_no_row(self) -> None: + self.assertEqual( + {"amd64"}, runnable_architectures({"linux/amd64", "windows/amd64"}) + ) + + def test_an_unrotated_architecture_is_dropped(self) -> None: + self.assertEqual( + {"amd64"}, + runnable_architectures({"linux/amd64", "linux/ppc64le", "linux/s390x"}), + ) + + def test_an_empty_or_missing_set_offers_nothing(self) -> None: + for platforms in (set(), None): + with self.subTest(platforms=platforms): + self.assertEqual(set(), runnable_architectures(platforms)) + + +class TestRoleCapability(unittest.TestCase): + def test_one_amd64_only_image_pins_the_whole_role(self) -> None: + capability, blame, unread = role_capability({"app": AMD64, "nginx": BOTH}) + + self.assertEqual({"amd64"}, capability) + self.assertEqual({"arm64": ["app"]}, blame) + self.assertEqual([], unread) + + def test_a_role_whose_images_all_publish_both_runs_on_both(self) -> None: + capability, blame, _unread = role_capability({"app": BOTH, "nginx": BOTH}) + + self.assertEqual({"amd64", "arm64"}, capability) + self.assertEqual({}, blame) + + def test_images_pinned_to_opposite_architectures_run_nowhere(self) -> None: + capability, blame, _unread = role_capability({"app": AMD64, "sidecar": ARM64}) + + self.assertEqual(set(), capability) + self.assertEqual({"amd64": ["sidecar"], "arm64": ["app"]}, blame) + + def test_blame_names_every_image_lacking_the_architecture(self) -> None: + _capability, blame, _unread = role_capability( + {"worker": AMD64, "app": AMD64, "nginx": BOTH} + ) + + self.assertEqual(["app", "worker"], blame["arm64"]) + + def test_an_unreadable_image_is_reported_and_not_intersected(self) -> None: + capability, blame, unread = role_capability({"app": BOTH, "unreadable": None}) + + self.assertEqual( + {"amd64", "arm64"}, + capability, + "a throttled probe must understate, not narrow the role to nothing", + ) + self.assertEqual({}, blame) + self.assertEqual( + ["unreadable"], + unread, + "without this the skipped image reads as one that runs everywhere, " + "and a narrowing it alone justifies looks unnecessary", + ) + + def test_a_role_nobody_could_read_has_no_capability(self) -> None: + capability, blame, unread = role_capability({"app": None, "nginx": None}) + + self.assertIsNone( + capability, "None is the skip signal, distinct from an empty set" + ) + self.assertEqual({}, blame) + self.assertEqual(["app", "nginx"], unread) + + def test_a_role_without_images_has_no_capability(self) -> None: + self.assertEqual((None, {}, []), role_capability({})) + + +if __name__ == "__main__": + unittest.main() diff --git a/utils/annotations/suppress.py b/utils/annotations/suppress.py index c2b717acfe..00676c696a 100644 --- a/utils/annotations/suppress.py +++ b/utils/annotations/suppress.py @@ -55,6 +55,7 @@ _KEYWORD_RE = re.compile( ) _COMMENT_LINE = re.compile(r"^\s*#") +_REASON = re.compile(r"#.*\breason\b\s*:\s*\S", re.IGNORECASE) def _rules_on_line(line: str) -> set[str]: @@ -123,6 +124,36 @@ def is_suppressed_at( return False +def has_rule_with_reason(lines: Sequence[str], line_no: int, rule: str) -> bool: + """Whether the construct at 1-based *line_no* carries *rule* and a reason. + + Args: + lines: the file's lines. + line_no: 1-based line of the construct being exempted. + rule: the rule key the marker must name. + + Both markers are looked for on the construct's own line and across every + contiguous comment line directly above it, in either order. This is the + placement for an exemption that has to say *why*: the reason rarely fits + beside the value, so it is written as a comment block above it, and + :func:`is_suppressed_at` would only see the last line of that block. + + The reason is unchecked prose by design. The gate is that a human had to + write one, not that a parser agreed with it. + """ + idx = line_no - 1 + if idx < 0 or idx >= len(lines): + return False + has_rule = line_has_rule(lines[idx], rule) + has_reason = bool(_REASON.search(lines[idx])) + scan = idx - 1 + while scan >= 0 and lines[scan].lstrip().startswith("#"): + has_rule = has_rule or line_has_rule(lines[scan], rule) + has_reason = has_reason or bool(_REASON.search(lines[scan])) + scan -= 1 + return has_rule and has_reason + + def is_suppressed_in_head( lines: Sequence[str], rule: str, diff --git a/utils/docker/image/architectures.py b/utils/docker/image/architectures.py new file mode 100644 index 0000000000..16e4fc8286 --- /dev/null +++ b/utils/docker/image/architectures.py @@ -0,0 +1,70 @@ +"""Reduce the platforms a role's images publish to where the role can run. + +A role is schedulable only where *every* image it pins can run, so its +capability is the intersection over its services rather than a property of +any one of them. Kept apart from the registry probe so the reduction can be +tested without a network. +""" + +from __future__ import annotations + +from utils.github.variant.pools import ARCHITECTURES + +LINUX = "linux" + + +def runnable_architectures(platforms) -> set[str]: + """Return the matrix architectures *platforms* can be scheduled on. + + Args: + platforms: ``os/architecture`` strings as a registry reports them. + + Returns: + The subset of :data:`ARCHITECTURES` offered for ``linux``. Anything + else a manifest lists - another operating system, an architecture the + matrix does not rotate - is dropped, because it cannot carry a row. + """ + offered = set() + for entry in platforms or (): + os_name, _, architecture = str(entry).partition("/") + if os_name == LINUX and architecture in ARCHITECTURES: + offered.add(architecture) + return offered + + +def role_capability( + offered, +) -> tuple[set[str] | None, dict[str, list[str]], list[str]]: + """Intersect what a role's images publish, and name what each one lacks. + + Args: + offered: mapping of image label to its ``os/architecture`` set, or to + ``None`` for an image whose registry answer was indeterminate. + + Returns: + ``(capability, blame, unread)``. ``capability`` is the intersection + over the images that answered, or ``None`` when none did - which a + caller must not read as "runs nowhere", since a throttled sweep + produces it. ``blame`` maps an architecture to the labels that cannot + run it, so a failure names the pin to change rather than only the + role. ``unread`` lists the labels that did not answer. + + An incomplete read can only ever overstate the capability, because the + intersection skips what it could not see. A caller may therefore trust + ``capability`` to decide an architecture is missing, but must not conclude + from it that a narrowing is unnecessary while ``unread`` is non-empty: the + image it could not read may be the one the narrowing exists for. + """ + capability: set[str] | None = None + blame: dict[str, list[str]] = {} + unread: list[str] = [] + for label in sorted(offered): + platforms = offered[label] + if platforms is None: + unread.append(label) + continue + runnable = runnable_architectures(platforms) + for architecture in sorted(set(ARCHITECTURES) - runnable): + blame.setdefault(architecture, []).append(label) + capability = runnable if capability is None else capability & runnable + return capability, blame, unread diff --git a/utils/docker/registry.py b/utils/docker/registry.py index 7827e546e0..d5e7eca744 100644 --- a/utils/docker/registry.py +++ b/utils/docker/registry.py @@ -30,6 +30,7 @@ from utils.docker.image.ref import DOCKER_HUB_REGISTRIES, split_registry_and_nam from utils.docker.mirror import mirror_image _UA = "infinito-nexus-version-updater" +_UNKNOWN_PLATFORM = "unknown" _CACHE_ROOT = Path(tempfile.gettempdir()) / "infinito-registry-probe" _LINK_NEXT_RE = re.compile(r'<([^>]+)>\s*;\s*rel="next"', re.IGNORECASE) _CHALLENGE_PARAM_RE = re.compile(r'(\w+)="([^"]*)"') @@ -389,6 +390,84 @@ def image_healthcheck_probed( return None, "none" +def _platforms_at(image: str, reference: str) -> set[str] | None: + doc = fetch_manifest(image, reference) + if doc is None: + return None + manifests = doc.get("manifests") + if isinstance(manifests, list) and manifests: + found = set() + for entry in manifests: + if not isinstance(entry, dict): + continue + platform = entry.get("platform") or {} + os_name, architecture = platform.get("os"), platform.get("architecture") + if not os_name or not architecture or os_name == _UNKNOWN_PLATFORM: + continue + found.add(f"{os_name}/{architecture}") + return found or None + config_digest = (doc.get("config") or {}).get("digest") + if not isinstance(config_digest, str) or not config_digest: + return None + blob = _fetch_blob(image, config_digest) + if blob is None: + return None + os_name, architecture = blob.get("os"), blob.get("architecture") + if not os_name or not architecture: + return None + return {f"{os_name}/{architecture}"} + + +def manifest_platforms(image: str, reference: str) -> set[str] | None: + """Return the ``os/architecture`` set ``image:reference`` publishes. + + Args: + image: image name, with or without registry host. + reference: tag or digest. + + Returns: + Every platform of a multi-platform index, or the single platform of a + one-platform manifest, read from its config blob. ``None`` when the + answer is indeterminate: an unresolvable name, a network error, an + auth wall, a rate limit, or a body that names no platform at all. A + caller must not read ``None`` as "publishes nothing". + + Attestation entries are left out: BuildKit records them in the index under + the platform ``unknown/unknown``, and counting them would report an + architecture no node can run. + """ + return manifest_platforms_probed(image, reference)[0] + + +def manifest_platforms_probed( + image: str, reference: str +) -> tuple[set[str] | None, str]: + """:func:`manifest_platforms` plus which address answered. + + Returns: + ``(platforms, source)`` where source is ``cache``, ``mirror``, + ``upstream`` or ``none``. + + The platform set is a property of the image rather than of the registry + serving it, so the mirror answers it as well as the upstream does and is + asked first. That matters more here than for a single probe: a sweep over + every pinned image is ~160 manifest reads, and Docker Hub's anonymous + allowance is counted per IP and shared by everything else on that address. + Determinate answers are cached on disk, so a rerun spends nothing on a pin + that did not move. + """ + cached = _cache_read("platforms", image, reference) + if isinstance(cached, list): + return set(cached), "cache" + mirrored = mirror_image(image) + for candidate in _probe_order(image): + found = _platforms_at(candidate, reference) + if found is not None: + _cache_write("platforms", sorted(found), image, reference) + return found, "mirror" if candidate == mirrored else "upstream" + return None, "none" + + def manifest_exists(image: str, reference: str) -> bool | None: """Whether ``image:reference`` resolves to a manifest.