From ac2803f14c78d52a8d5bedadecdc04b8d3d129ae Mon Sep 17 00:00:00 2001 From: Stephen Freudenthaler Date: Tue, 11 Aug 2026 16:45:16 -0400 Subject: [PATCH 1/4] fix(evergreen-tracks): authenticate Hub tag reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docker Hub now refuses ANONYMOUS pagination past offset 1000 ("pagination offset too large for anonymous requests; sign in to page further"). dotcms/dotcms has ~7.8k tags / 79 pages, so registry.list_tags() — which read the Hub API with no auth at all — 403s on page 11 every time. This broke the daily promote cron (first failure 2026-08-11, run 31481784007; the 2026-08-10 run still walked all 79 pages), and would also have broken the `latest` promote in the release pipeline on the next GA cut, since both call the same read path. executor.hub_login() already minted a Hub JWT for deletes and every calling workflow already had DOCKER_USERNAME/DOCKER_TOKEN for docker/login-action — the read path just never used them. Reuse it, and export the secrets to the promote (plan + apply) and release latest-promote steps. The admin workflow already exported them. Also stop discarding the engine's stderr in the promote steps. `2>/dev/null` turned this 403 into a bare "Process completed with exit code 1" with no cause in the log or the Slack alert. stderr now goes to a file (it must stay off stdout, which is the plan text the apply job diffs) and is printed on failure. Closes: #37025 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PqQqV1iv96Db8AzBSWTXV4 --- .../src/evergreen_tracks/registry.py | 31 ++++++++++++++-- .../evergreen-tracks/tests/test_registry.py | 36 +++++++++++++++++++ .github/workflows/cicd_6-release.yml | 6 ++++ .../cicd_evergreen-tracks-promote.yml | 27 +++++++++++--- 4 files changed, 93 insertions(+), 7 deletions(-) diff --git a/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/registry.py b/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/registry.py index 4d0cdae65829..ca63c0838223 100644 --- a/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/registry.py +++ b/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/registry.py @@ -1,10 +1,20 @@ -"""Read side: list tags and digests from the Docker Hub API. Public repos need no auth.""" +"""Read side: list tags and digests from the Docker Hub API. + +Reads are authenticated when DOCKER_USERNAME/DOCKER_TOKEN are set. Hub refuses +ANONYMOUS pagination past offset 1000 ("pagination offset too large for anonymous +requests; sign in to page further"), and dotcms/dotcms is at ~7.8k tags / 79 pages, +so an unauthenticated walk 403s on page 11. Auth is therefore required in practice +for the real repos; it stays optional so tests and small repos need no creds. +""" from __future__ import annotations +import os from dataclasses import dataclass import requests +from .executor import hub_login + _HUB = "https://hub.docker.com/v2" _TIMEOUT = 30 @@ -24,13 +34,30 @@ def _digest_of(result: dict) -> str | None: return None +def _auth_headers() -> dict[str, str]: + """Hub JWT header, or {} when no creds are in the environment.""" + username = os.environ.get("DOCKER_USERNAME") + password = os.environ.get("DOCKER_TOKEN") + if not username or not password: + return {} + return {"Authorization": f"JWT {hub_login(username, password)}"} + + def list_tags(repo: str) -> list[Tag]: """All tags in the repo with their manifest digests, following pagination.""" namespace, name = repo.split("/", 1) url = f"{_HUB}/namespaces/{namespace}/repositories/{name}/tags?page_size=100" + # One login per walk, not per page: the JWT outlives a full 79-page listing. + headers = _auth_headers() out: list[Tag] = [] while url: - resp = requests.get(url, timeout=_TIMEOUT) + resp = requests.get(url, headers=headers, timeout=_TIMEOUT) + if resp.status_code == 403 and not headers: + raise RuntimeError( + f"Docker Hub refused anonymous pagination of {repo} at {url}. " + "Set DOCKER_USERNAME and DOCKER_TOKEN — Hub caps anonymous " + "requests at offset 1000 and this repo has more tags than that." + ) resp.raise_for_status() body = resp.json() for result in body.get("results", []): diff --git a/.github/actions/core-cicd/evergreen-tracks/tests/test_registry.py b/.github/actions/core-cicd/evergreen-tracks/tests/test_registry.py index dc43b0c05312..48aff4957ca8 100644 --- a/.github/actions/core-cicd/evergreen-tracks/tests/test_registry.py +++ b/.github/actions/core-cicd/evergreen-tracks/tests/test_registry.py @@ -1,8 +1,11 @@ import json import pathlib +import pytest import responses from evergreen_tracks.registry import list_tags +TAGS_URL = "https://hub.docker.com/v2/namespaces/dotcms/repositories/dotcms-test/tags" + FIXTURE = pathlib.Path(__file__).parent / "fixtures" / "hub_tags.json" @responses.activate @@ -39,3 +42,36 @@ def test_list_tags_paginates_and_returns_name_digest(): assert "26.03.12-01" in tag_names assert "26049-docker-build-and-publish" in tag_names assert all(isinstance(t.name, str) and t.digest.startswith("sha256:") for t in tags) + + +@responses.activate +def test_list_tags_authenticates_when_creds_present(monkeypatch): + """With creds set, every page request carries the Hub JWT — anonymous walks 403 past + offset 1000 on the real repos (#37025), so losing this header re-breaks promotion.""" + monkeypatch.setenv("DOCKER_USERNAME", "bot") + monkeypatch.setenv("DOCKER_TOKEN", "pat") + responses.add(responses.POST, "https://hub.docker.com/v2/users/login", + json={"token": "jwt-abc"}, status=200) + fixture_data = json.loads(FIXTURE.read_text()) + responses.add(responses.GET, TAGS_URL, + json={"count": 1, "next": None, "results": fixture_data["results"][:1]}, + status=200) + + list_tags("dotcms/dotcms-test") + + tag_calls = [c for c in responses.calls if c.request.url.startswith(TAGS_URL)] + assert tag_calls, "expected at least one tag-listing request" + assert all(c.request.headers.get("Authorization") == "JWT jwt-abc" for c in tag_calls) + + +@responses.activate +def test_list_tags_anonymous_403_explains_itself(monkeypatch): + """A creds-less 403 must name the cause, not surface a bare HTTPError traceback.""" + monkeypatch.delenv("DOCKER_USERNAME", raising=False) + monkeypatch.delenv("DOCKER_TOKEN", raising=False) + responses.add( + responses.GET, TAGS_URL, + json={"message": "pagination offset too large for anonymous requests"}, status=403, + ) + with pytest.raises(RuntimeError, match="DOCKER_USERNAME"): + list_tags("dotcms/dotcms-test") diff --git a/.github/workflows/cicd_6-release.yml b/.github/workflows/cicd_6-release.yml index 0ff904b8d132..d965b97e9cef 100644 --- a/.github/workflows/cicd_6-release.yml +++ b/.github/workflows/cicd_6-release.yml @@ -302,6 +302,12 @@ jobs: password: ${{ secrets.DOCKER_TOKEN }} - name: Move latest to ${{ needs.release-prepare.outputs.release_version }} working-directory: .github/actions/core-cicd/evergreen-tracks + env: + # The engine reads the Hub tag list before it moves anything, and Hub + # refuses anonymous pagination past offset 1000 (these repos are well + # past it). docker/login-action above does not cover the Hub API. + DOCKER_USERNAME: ${{ secrets.DOCKER_USERNAME }} + DOCKER_TOKEN: ${{ secrets.DOCKER_TOKEN }} run: | for repo in dotcms/dotcms dotcms/dotcms-dev; do echo "--- promoting latest on ${repo} ---" diff --git a/.github/workflows/cicd_evergreen-tracks-promote.yml b/.github/workflows/cicd_evergreen-tracks-promote.yml index f0e81d03b7df..a6b261e50a43 100644 --- a/.github/workflows/cicd_evergreen-tracks-promote.yml +++ b/.github/workflows/cicd_evergreen-tracks-promote.yml @@ -81,17 +81,27 @@ jobs: REPO: ${{ github.event.inputs.repo || 'dotcms/dotcms' }} STANDARD_DAYS: ${{ github.event.inputs.standard_days || '14' }} TRAILING_DAYS: ${{ github.event.inputs.trailing_days || '28' }} + # Hub refuses ANONYMOUS pagination past offset 1000, and dotcms/dotcms is + # ~79 pages, so even this read-only dry-run needs credentials. + DOCKER_USERNAME: ${{ secrets.DOCKER_USERNAME }} + DOCKER_TOKEN: ${{ secrets.DOCKER_TOKEN }} run: | echo "repo=$REPO (dry-run plan; trigger=${{ github.event_name }})" # --tracks standard,trailing: this workflow never moves `latest` — the # release pipeline owns it and moves it unattended (possibly mid-approval). # Scoping here keeps the drift check below immune to `latest` churn. - # 2>/dev/null: keep only the engine's stdout plan, drop uv's stderr chatter. - PLAN=$(uv run evergreen-tracks promote \ + # stderr goes to a file, not /dev/null: it must stay off stdout (that is + # the plan text, diffed in `apply`) but discarding it cost us a whole + # morning once — a Hub 403 surfaced as a bare "exit code 1". + if ! PLAN=$(uv run evergreen-tracks promote \ --repo "$REPO" \ --tracks standard,trailing \ --standard-days "$STANDARD_DAYS" \ - --trailing-days "$TRAILING_DAYS" 2>/dev/null) + --trailing-days "$TRAILING_DAYS" 2>plan.err); then + echo "::error::evergreen-tracks promote (dry-run) failed:" + cat plan.err >&2 + exit 1 + fi echo "$PLAN" { echo 'plan</dev/null) + --trailing-days "$TRAILING_DAYS" 2>current.err); then + echo "::error::evergreen-tracks promote (re-plan) failed:" + cat current.err >&2 + exit 1 + fi # A never-empty plan ("no track moves needed" at minimum) guards against # a silent pass if plan capture ever breaks (e.g. logging leaves stdout). if [ -z "$APPROVED_PLAN" ] || [ -z "$CURRENT_PLAN" ]; then From 8a1acbbc0eca600680a45d9efbfdbcb0579878a0 Mon Sep 17 00:00:00 2001 From: Stephen Freudenthaler Date: Wed, 12 Aug 2026 12:01:55 -0400 Subject: [PATCH 2/4] =?UTF-8?q?fix(evergreen-tracks):=20address=20review?= =?UTF-8?q?=20=E2=80=94=20test=20isolation=20and=20403=20message=20accurac?= =?UTF-8?q?y?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both findings from the review on #37027 were real. 1. Test isolation. `list_tags` now reads DOCKER_USERNAME/DOCKER_TOKEN, which made the pre-existing test_list_tags_paginates_and_returns_name_digest depend on the ambient environment: with creds exported it attempts a real hub_login() POST that `responses` never registered and fails with ConnectionError. Reproduced, then fixed with an autouse fixture that clears both vars for the module; the tests that want auth still set them explicitly. Verified the suite passes both with and without creds in the environment. 2. 403 message accuracy. The error hardcoded "this repo has more tags than that", which is an invented cause: measured, the cap keys off the OFFSET alone — even library/hello-world (3 tags) 403s on page 11. The message now quotes Hub's own `message` field and falls back to the status code when the body is not JSON, so a future 403 for a different reason cannot be mislabelled. The remedy hint is unchanged because credentials fix it either way. The reviewer's assumption checked out: a missing repo or namespace returns 404, not 403, so the offset cap is the only anonymous 403 reachable here today. Closes: #37025 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PqQqV1iv96Db8AzBSWTXV4 --- .../src/evergreen_tracks/registry.py | 20 +++++++++-- .../evergreen-tracks/tests/test_registry.py | 36 ++++++++++++++++--- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/registry.py b/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/registry.py index ca63c0838223..a02156ed5b55 100644 --- a/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/registry.py +++ b/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/registry.py @@ -34,6 +34,14 @@ def _digest_of(result: dict) -> str | None: return None +def _hub_message(resp: requests.Response) -> str: + """Hub's own error text, when the body carries one.""" + try: + return (resp.json() or {}).get("message") or "" + except ValueError: + return "" + + def _auth_headers() -> dict[str, str]: """Hub JWT header, or {} when no creds are in the environment.""" username = os.environ.get("DOCKER_USERNAME") @@ -53,10 +61,16 @@ def list_tags(repo: str) -> list[Tag]: while url: resp = requests.get(url, headers=headers, timeout=_TIMEOUT) if resp.status_code == 403 and not headers: + # Quote Hub's own reason rather than asserting one. The offset cap is the + # only anonymous 403 observed here (a missing repo or namespace 404s), but + # the cap keys off the OFFSET alone — even a repo with 3 tags 403s on + # page 11 — so "this repo has too many tags" would be an invented cause. + # Credentials are the remedy either way. raise RuntimeError( - f"Docker Hub refused anonymous pagination of {repo} at {url}. " - "Set DOCKER_USERNAME and DOCKER_TOKEN — Hub caps anonymous " - "requests at offset 1000 and this repo has more tags than that." + f"Docker Hub refused an anonymous read of {repo} at {url} — " + f"{_hub_message(resp) or f'HTTP {resp.status_code}'}. " + "Set DOCKER_USERNAME and DOCKER_TOKEN: anonymous requests cannot page " + "past offset 1000." ) resp.raise_for_status() body = resp.json() diff --git a/.github/actions/core-cicd/evergreen-tracks/tests/test_registry.py b/.github/actions/core-cicd/evergreen-tracks/tests/test_registry.py index 48aff4957ca8..5dbc0758b7ad 100644 --- a/.github/actions/core-cicd/evergreen-tracks/tests/test_registry.py +++ b/.github/actions/core-cicd/evergreen-tracks/tests/test_registry.py @@ -6,8 +6,23 @@ TAGS_URL = "https://hub.docker.com/v2/namespaces/dotcms/repositories/dotcms-test/tags" + +@pytest.fixture(autouse=True) +def _no_ambient_hub_creds(monkeypatch): + """Neutralise DOCKER_USERNAME / DOCKER_TOKEN inherited from the shell. + + `list_tags` reads them to decide whether to authenticate, so a developer or runner + with Docker creds exported would send the anonymous-path tests through a real + `hub_login()` POST that `responses` never registered — turning a deterministic test + into an environment-dependent one. Tests that WANT auth set the vars themselves. + """ + monkeypatch.delenv("DOCKER_USERNAME", raising=False) + monkeypatch.delenv("DOCKER_TOKEN", raising=False) + + FIXTURE = pathlib.Path(__file__).parent / "fixtures" / "hub_tags.json" + @responses.activate def test_list_tags_paginates_and_returns_name_digest(): """Two-page pagination: first response has 'next' pointing to page 2, second has next=null.""" @@ -65,13 +80,24 @@ def test_list_tags_authenticates_when_creds_present(monkeypatch): @responses.activate -def test_list_tags_anonymous_403_explains_itself(monkeypatch): - """A creds-less 403 must name the cause, not surface a bare HTTPError traceback.""" - monkeypatch.delenv("DOCKER_USERNAME", raising=False) - monkeypatch.delenv("DOCKER_TOKEN", raising=False) +def test_list_tags_anonymous_403_explains_itself(): + """A creds-less 403 must name the cause, not surface a bare HTTPError traceback. + + It must quote Hub's OWN reason: the offset cap is the only 403 seen here today, so + hardcoding that cause would silently mislabel any future one. + """ responses.add( responses.GET, TAGS_URL, json={"message": "pagination offset too large for anonymous requests"}, status=403, ) - with pytest.raises(RuntimeError, match="DOCKER_USERNAME"): + with pytest.raises(RuntimeError, match="DOCKER_USERNAME") as err: + list_tags("dotcms/dotcms-test") + assert "pagination offset too large for anonymous requests" in str(err.value) + + +@responses.activate +def test_list_tags_403_without_a_json_body_still_raises_cleanly(): + """Hub 403s with an HTML/empty body must not turn into a JSON decode error.""" + responses.add(responses.GET, TAGS_URL, body="gateway", status=403) + with pytest.raises(RuntimeError, match="HTTP 403"): list_tags("dotcms/dotcms-test") From 71cfdeca5a52e9ed81abb2a8e7a67cea63c6c02e Mon Sep 17 00:00:00 2001 From: Stephen Freudenthaler Date: Wed, 12 Aug 2026 12:04:24 -0400 Subject: [PATCH 3/4] test(evergreen-tracks): move creds isolation to conftest so it covers every module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A module-level autouse fixture only protects its own module. Verified by adding a second test file that exercised the same env-reading path: it still failed with ambient DOCKER_USERNAME/DOCKER_TOKEN because test_registry.py's fixture did not reach it. In conftest the isolation covers every module, including ones not written yet — which matters because this trap is invisible until someone's shell happens to have Docker creds exported. Closes: #37025 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PqQqV1iv96Db8AzBSWTXV4 --- .../evergreen-tracks/tests/conftest.py | 20 +++++++++++++++++++ .../evergreen-tracks/tests/test_registry.py | 13 ------------ 2 files changed, 20 insertions(+), 13 deletions(-) create mode 100644 .github/actions/core-cicd/evergreen-tracks/tests/conftest.py diff --git a/.github/actions/core-cicd/evergreen-tracks/tests/conftest.py b/.github/actions/core-cicd/evergreen-tracks/tests/conftest.py new file mode 100644 index 000000000000..9043ce44b710 --- /dev/null +++ b/.github/actions/core-cicd/evergreen-tracks/tests/conftest.py @@ -0,0 +1,20 @@ +"""Shared test isolation for the whole suite.""" +from __future__ import annotations + +import pytest + + +@pytest.fixture(autouse=True) +def _no_ambient_hub_creds(monkeypatch): + """Neutralise DOCKER_USERNAME / DOCKER_TOKEN inherited from the shell. + + `registry.list_tags` reads them to decide whether to authenticate, so a developer or + runner with Docker creds exported would send the anonymous-path tests through a real + `hub_login()` POST that `responses` never registered — turning deterministic tests + into environment-dependent ones. Tests that WANT auth set the vars themselves. + + In conftest rather than one test module so it covers every module, including ones + not written yet: the trap is invisible until someone's shell happens to have creds. + """ + monkeypatch.delenv("DOCKER_USERNAME", raising=False) + monkeypatch.delenv("DOCKER_TOKEN", raising=False) diff --git a/.github/actions/core-cicd/evergreen-tracks/tests/test_registry.py b/.github/actions/core-cicd/evergreen-tracks/tests/test_registry.py index 5dbc0758b7ad..9e7dbccc44f6 100644 --- a/.github/actions/core-cicd/evergreen-tracks/tests/test_registry.py +++ b/.github/actions/core-cicd/evergreen-tracks/tests/test_registry.py @@ -7,19 +7,6 @@ TAGS_URL = "https://hub.docker.com/v2/namespaces/dotcms/repositories/dotcms-test/tags" -@pytest.fixture(autouse=True) -def _no_ambient_hub_creds(monkeypatch): - """Neutralise DOCKER_USERNAME / DOCKER_TOKEN inherited from the shell. - - `list_tags` reads them to decide whether to authenticate, so a developer or runner - with Docker creds exported would send the anonymous-path tests through a real - `hub_login()` POST that `responses` never registered — turning a deterministic test - into an environment-dependent one. Tests that WANT auth set the vars themselves. - """ - monkeypatch.delenv("DOCKER_USERNAME", raising=False) - monkeypatch.delenv("DOCKER_TOKEN", raising=False) - - FIXTURE = pathlib.Path(__file__).parent / "fixtures" / "hub_tags.json" From cd0eb0e46be915dd2b4f429e7b9dfdb733577475 Mon Sep 17 00:00:00 2001 From: Steve Freudenthaler <31257998+sfreudenthaler@users.noreply.github.com> Date: Wed, 12 Aug 2026 13:46:24 -0400 Subject: [PATCH 4/4] perf(evergreen-tracks): plan promotions from filtered reads instead of a 79-call full tag scan (#37030) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gonna put in with the #37027 because @wezell called it out in a comment on that review > **Stacked on #37027** (the Hub auth outage fix). Review that one first — this branches off it, so the diff here is only the efficiency work. ## Problem Moving one floating tag paged the entire repo. `dotcms/dotcms` has 7,832 tags and Hub caps `page_size` at 100 (250 and 1000 are silently ignored), so every run was **79 calls / ~45s** — against a Hub budget of **180 requests / 60s per IP**. The promoter has no 429 handling, so a single rate-limited page is a hard failure and, since #37027, a Slack page. Most of that listing is noise: 1,946 `*SNAPSHOT` branch builds, 677 `*_lts`. Promotion needs a few GA CalVer tags and three marker names. ## Approach Same technique the in-cluster reconciler already uses in dotCMS/infrastructure-as-code (`reconcile.py::resolve_track`) — Hub's `name=` **substring** filter: - **one call per requested track** → the `` digest, and its `_hold` marker for free (both match the track name as a substring) - **one call per month of history** → that month's GA releases *and* their `_tainted` markers, which share the version's month prefix A month is one page (6–28 tags). `--tracks` is now resolved *before* any registry read, so the release pipeline's latest-only promote no longer pays to resolve `standard` and `trailing`. ## Measured against the live registry | path | before | after | |---|---|---| | daily `standard,trailing` | 79 calls | **4** | | `--tracks latest` (release pipeline) | 79 calls | **2** | | all three tracks | 79 calls | **5** | | CI plan job wall-clock | 29s | **2.9s** | Plans are byte-identical to the full scan's — `standard -> 26.07.28-01`, digest `c2526c56…`, matching the authenticated full-scan run [31534540024](https://github.com/dotCMS/core/actions/runs/31534540024). Verified in CI on this branch: [31544087963](https://github.com/dotCMS/core/actions/runs/31544087963) (`read registry state in 4 filtered call(s)`), cancelled at the approval gate so nothing applied. ## The correctness argument worth reviewing Everything rests on fetching months **newest-first and contiguously**. `planner.plan`'s forward-only guard only bites when a track already sits on something *newer* than the newest eligible release. Any such version lives in a month at or above the one the walk stopped on — so it has necessarily already been read. A track on something *older* resolves to "move forward", which is the same decision the full listing produces. **Nothing here can move a track backwards that the full scan would have held.** The tempting "optimization" that breaks this is starting the walk at the threshold cutoff month instead of today (why read months newer than the cutoff?). `test_promote_narrow_reads_still_refuse_to_move_a_track_backwards` fails on exactly that mutation. I originally also ported the reconciler's push-month lookup to name each track's current version. Mutation testing showed removing it changed no promotion outcome — the argument above is why it was dead weight — so it's gone, along with the `Tag.month` plumbing it needed. ## Fallback Returns to the full listing when the walk window holds nothing promotable, so a broken read path surfaces as the expensive *right* answer rather than a quiet "no moves" stall — which is precisely how tracks would silently stop advancing. `admin` deliberately keeps the full scan: rare, human-triggered, and it addresses arbitrarily old versions. ## Tests 73 pass (68 existing unchanged + 5 new). The new ones were mutation-tested — each of these breaks at least one: | mutation | caught by | |---|---| | skip months newer than the cutoff | backwards-move + walk-back tests | | never walk back a month | 3 tests | | drop the empty-window fallback | fallback test | | stop month-filtering | 3 tests | | ignore `--tracks` when reading | scoping test | `ruff` is not wired into CI for this package; I checked my changes are net-neutral against the baseline anyway (21 findings before and after, same rules, shifted line numbers). Closes: #37028 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01PqQqV1iv96Db8AzBSWTXV4 --------- Co-authored-by: Claude Opus 5 --- .../src/evergreen_tracks/cli.py | 106 +++++++++++-- .../src/evergreen_tracks/registry.py | 21 ++- .../evergreen-tracks/tests/conftest.py | 9 ++ .../evergreen-tracks/tests/test_cli.py | 145 ++++++++++++++++++ 4 files changed, 266 insertions(+), 15 deletions(-) diff --git a/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/cli.py b/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/cli.py index cfc8f47fd56e..3c2d8d78c5bd 100644 --- a/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/cli.py +++ b/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/cli.py @@ -10,7 +10,7 @@ import os import sys -from .calver import newest, parse_release +from .calver import age_days, newest, parse_release from .executor import delete_tag, hub_login, point_tag from .markers import TRACKS, held_tracks, hold_tag, tainted_versions, taint_tag from .planner import TrackState, plan @@ -18,9 +18,18 @@ log = logging.getLogger("evergreen_tracks") +# How many months the narrow read path walks back before giving up and full-scanning. +# Releases ship weekly, so promotion needs one or two months; the extra headroom is for +# a quiet stretch or a track parked on something old by a hold. +_MONTH_WALK_LIMIT = 6 + def _state(repo: str): - """Return (releases, tainted, held, name->digest) read from the registry.""" + """Return (releases, tainted, held, name->digest) read from the registry. + + The full ~79-page listing. Still the right tool for `admin` (rare, human-triggered, + and it addresses arbitrarily old versions) and the terminal fallback for promote. + """ tags = list_tags(repo) names = [t.name for t in tags] digests = {t.name: t.digest for t in tags} @@ -28,6 +37,67 @@ def _state(repo: str): return releases, tainted_versions(names), held_tracks(names), digests +def _month_str(date: dt.date) -> str: + return f"{date.year % 100:02d}.{date.month:02d}" + + +def _prev_month(ym: str) -> str: + yy, mm = int(ym[:2]), int(ym[3:5]) + return f"{yy - 1:02d}.12" if mm == 1 else f"{yy:02d}.{mm - 1:02d}" + + +def _promote_state(repo: str, wanted: list[str], max_days: int, today: dt.date): + """`_state`, assembled from a handful of filtered reads instead of the full listing. + + Moving a floating tag needs a few GA CalVer tags and three marker names, but the + full listing pages the whole repo — 7.8k tags, 79 calls, ~45s, against a Hub budget + of 180 requests/60s per IP. Hub's `name=` substring filter turns that into: + + * one call per requested track -> the `` tag's digest, plus its + `_hold` marker if one exists (both match the track name as a substring) + * one call per month of history -> that month's GA releases AND their + `_tainted` markers, which share the version's month prefix + + A month is one page (6-28 tags), so the daily standard+trailing promote is 3-4 calls. + + Months are fetched newest-first and CONTIGUOUSLY, which is what makes the narrowed + pool safe for `planner.plan`'s forward-only guard. That guard only matters when a + track already sits on something NEWER than the newest eligible release — and any + such version lives in a month at or above the one the walk stopped on, so it has + necessarily already been read. A track on something OLDER resolves to "move + forward", which is the same decision the full listing produces. Nothing here can + move a track backwards that the full scan would have held. + + Returns None if the walk window turns up nothing promotable at all, so the caller + can fall back to the full listing rather than silently reporting "no moves" — a + quiet stall is exactly how a broken read path would look. + """ + digests: dict[str, str] = {} + names: list[str] = [] + + def absorb(tags) -> None: + for tag in tags: + digests.setdefault(tag.name, tag.digest) + names.append(tag.name) + + # Track tags and their hold markers. `name=standard` also matches `standard_hold`; + # `name=latest` additionally drags in a dozen `*_latest_SNAPSHOT` build tags, which + # parse as neither releases nor markers and are simply ignored. + for track in wanted: + absorb(list_tags(repo, name_filter=track)) + + month = _month_str(today) + for walked in range(_MONTH_WALK_LIMIT): + absorb(list_tags(repo, name_filter=month)) + releases = [r for r in (parse_release(n) for n in names) if r is not None] + tainted = tainted_versions(names) + if any(age_days(r, today) >= max_days and r.version not in tainted for r in releases): + log.info("read registry state in %d filtered call(s)", len(wanted) + walked + 1) + return releases, tainted, held_tracks(names), digests + month = _prev_month(month) + return None + + def _current_version(track: str, digests: dict[str, str], releases) -> str | None: """Which GA version the floating tag currently points at, by digest match.""" track_digest = digests.get(track) @@ -40,27 +110,39 @@ def _current_version(track: str, digests: dict[str, str], releases) -> str | Non def cmd_promote(args: argparse.Namespace) -> int: - releases, tainted, held, digests = _state(args.repo) - tracks = [ - TrackState("latest", args.latest_days, _current_version("latest", digests, releases)), - TrackState("standard", args.standard_days, _current_version("standard", digests, releases)), - TrackState("trailing", args.trailing_days, _current_version("trailing", digests, releases)), - ] - # Optional subset (e.g. --tracks latest): the release pipeline invokes this # engine on-demand to move only `latest` the instant a GA ships, while an # operator manually dispatches a full promote to age standard/trailing. # One engine, two triggers. + # + # Resolved BEFORE any registry read, so an unwanted track costs no calls at all. + wanted = set(TRACKS) if args.tracks: wanted = {t.strip() for t in args.tracks.split(",") if t.strip()} unknown = wanted - set(TRACKS) if unknown: log.error("unknown track(s): %s", ", ".join(sorted(unknown))) return 2 - tracks = [t for t in tracks if t.name in wanted] - held = held & wanted + thresholds = { + "latest": args.latest_days, + "standard": args.standard_days, + "trailing": args.trailing_days, + } + ordered = [t for t in TRACKS if t in wanted] # stable, TRACKS order + today = dt.date.today() + + state = _promote_state(args.repo, ordered, max(thresholds[t] for t in ordered), today) + if state is None: + log.info("no promotable release in the filtered window; reading the full tag listing") + state = _state(args.repo) + releases, tainted, held, digests = state + held = held & wanted + + tracks = [ + TrackState(t, thresholds[t], _current_version(t, digests, releases)) for t in ordered + ] - moves = plan(releases, tainted, held, tracks, today=dt.date.today()) + moves = plan(releases, tainted, held, tracks, today=today) # Held tracks are frozen against promotion; instead reconcile the floating # tag to its _hold marker digest so a divergence self-heals. diff --git a/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/registry.py b/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/registry.py index a02156ed5b55..a6b805c50ce7 100644 --- a/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/registry.py +++ b/.github/actions/core-cicd/evergreen-tracks/src/evergreen_tracks/registry.py @@ -10,6 +10,7 @@ import os from dataclasses import dataclass +from functools import lru_cache import requests @@ -42,19 +43,33 @@ def _hub_message(resp: requests.Response) -> str: return "" +@lru_cache(maxsize=4) +def _jwt(username: str, password: str) -> str: + """Cached per credential pair: one login per process, not one per request.""" + return hub_login(username, password) + + def _auth_headers() -> dict[str, str]: """Hub JWT header, or {} when no creds are in the environment.""" username = os.environ.get("DOCKER_USERNAME") password = os.environ.get("DOCKER_TOKEN") if not username or not password: return {} - return {"Authorization": f"JWT {hub_login(username, password)}"} + return {"Authorization": f"JWT {_jwt(username, password)}"} + +def list_tags(repo: str, *, name_filter: str = "") -> list[Tag]: + """Tags in the repo with their manifest digests, following pagination. -def list_tags(repo: str) -> list[Tag]: - """All tags in the repo with their manifest digests, following pagination.""" + `name_filter` applies Hub's SUBSTRING filter (`?name=`), which is what keeps the + common path off the ~79-page full listing. It is observed behaviour rather than a + documented contract; if Hub ever drops it the filtered queries simply return the + whole repo and callers still get a correct — merely slower — answer. + """ namespace, name = repo.split("/", 1) url = f"{_HUB}/namespaces/{namespace}/repositories/{name}/tags?page_size=100" + if name_filter: + url += f"&name={name_filter}" # One login per walk, not per page: the JWT outlives a full 79-page listing. headers = _auth_headers() out: list[Tag] = [] diff --git a/.github/actions/core-cicd/evergreen-tracks/tests/conftest.py b/.github/actions/core-cicd/evergreen-tracks/tests/conftest.py index 9043ce44b710..5cf0881c81c1 100644 --- a/.github/actions/core-cicd/evergreen-tracks/tests/conftest.py +++ b/.github/actions/core-cicd/evergreen-tracks/tests/conftest.py @@ -3,6 +3,8 @@ import pytest +from evergreen_tracks.registry import _jwt + @pytest.fixture(autouse=True) def _no_ambient_hub_creds(monkeypatch): @@ -15,6 +17,13 @@ def _no_ambient_hub_creds(monkeypatch): In conftest rather than one test module so it covers every module, including ones not written yet: the trap is invisible until someone's shell happens to have creds. + + Also drops the memoised JWT. `_jwt` is deliberately cached — one Hub login per + process rather than one per request — but that cache outlives a test, so without + this a later test using the same credentials silently reuses an earlier test's + token. Verified: two tests with identical creds and different tokens, and the + second one saw the first one's JWT. """ monkeypatch.delenv("DOCKER_USERNAME", raising=False) monkeypatch.delenv("DOCKER_TOKEN", raising=False) + _jwt.cache_clear() diff --git a/.github/actions/core-cicd/evergreen-tracks/tests/test_cli.py b/.github/actions/core-cicd/evergreen-tracks/tests/test_cli.py index f6e0be381a26..122529f0eb86 100644 --- a/.github/actions/core-cicd/evergreen-tracks/tests/test_cli.py +++ b/.github/actions/core-cicd/evergreen-tracks/tests/test_cli.py @@ -601,3 +601,148 @@ def test_promote_tracks_unknown_returns_2(mock_list_tags, mock_point_tag): rc = cmd_promote(args) assert rc == 2 mock_point_tag.assert_not_called() + + +# --------------------------------------------------------------------------- +# Narrow (filtered) registry reads — see cli._promote_state +# --------------------------------------------------------------------------- + +def _fake_registry(tags): + """Stand-in for registry.list_tags that honours Hub's `name=` SUBSTRING filter. + + An empty filter returns everything, which is exactly what the real full listing + does — so `"" in calls` is the assertion for "fell back to the 79-page scan". + """ + calls: list[str] = [] + + def fake(repo, *, name_filter=""): + calls.append(name_filter) + return [t for t in tags if name_filter in t.name] + + return fake, calls + + +def _registry_tags(standard_digest="sha256:r0705", trailing_digest="sha256:r0602"): + """Four GA releases plus two floating track tags pointing into that history.""" + return [ + Tag("26.06.02-01", "sha256:r0602"), + Tag("26.07.05-01", "sha256:r0705"), + Tag("26.07.28-01", "sha256:r0728"), + Tag("26.08.10-01", "sha256:r0810"), + Tag("standard", standard_digest), + Tag("trailing", trailing_digest), + ] + + +@patch("evergreen_tracks.cli.point_tag") +@patch("evergreen_tracks.cli.list_tags") +def test_promote_uses_filtered_reads_not_the_full_listing(mock_list_tags, mock_point_tag): + """The daily promote must plan from a handful of filtered calls, never the full scan. + + This is the whole point of the narrow path: the full listing is 79 calls against a + 180-req/60s per-IP Hub budget. + """ + fake, calls = _fake_registry(_registry_tags()) + mock_list_tags.side_effect = fake + args = build_parser().parse_args( + ["promote", "--repo", "dotcms/dotcms-test", "--tracks", "standard,trailing", + "--standard-days", "14", "--trailing-days", "28"] + ) + with patch("evergreen_tracks.cli.dt") as mock_dt: + mock_dt.date.today.return_value = dt.date(2026, 8, 11) + rc = cmd_promote(args) + + assert rc == 0 + assert "" not in calls, f"fell back to the unfiltered full listing: {calls}" + assert len(calls) <= 6, f"expected a handful of filtered calls, got {calls}" + # Newest release ≥14d is 26.07.28-01; newest ≥28d is 26.07.05-01. + moved = {c.args[1]: c.args[2] for c in mock_point_tag.call_args_list} + assert moved == {"standard": "sha256:r0728", "trailing": "sha256:r0705"} + + +@patch("evergreen_tracks.cli.point_tag") +@patch("evergreen_tracks.cli.list_tags") +def test_promote_walks_back_months_for_an_eligible_release(mock_list_tags, mock_point_tag): + """The threshold cutoff can sit in an earlier month than today's, so walk 1 must + keep fetching months until a candidate actually clears the bar.""" + fake, calls = _fake_registry(_registry_tags()) + mock_list_tags.side_effect = fake + args = build_parser().parse_args( + ["promote", "--repo", "dotcms/dotcms-test", "--tracks", "trailing", + "--trailing-days", "28"] + ) + with patch("evergreen_tracks.cli.dt") as mock_dt: + mock_dt.date.today.return_value = dt.date(2026, 8, 11) + cmd_promote(args) + + # 26.08 holds nothing ≥28 days old, so it must have walked back to 26.07. + assert "26.08" in calls and "26.07" in calls + assert "" not in calls + + +@patch("evergreen_tracks.cli.point_tag") +@patch("evergreen_tracks.cli.list_tags") +def test_promote_narrow_reads_still_refuse_to_move_a_track_backwards( + mock_list_tags, mock_point_tag +): + """Forward-only must survive the narrowed pool. + + `standard` sits on 26.08.10-01, newer than anything 14 days old. If the filtered + reads failed to name the current version, `plan` would skip its forward-only guard + and demote the track to 26.07.28-01. Nothing may move here. + """ + fake, calls = _fake_registry(_registry_tags(standard_digest="sha256:r0810")) + mock_list_tags.side_effect = fake + args = build_parser().parse_args( + ["promote", "--repo", "dotcms/dotcms-test", "--tracks", "standard", + "--standard-days", "14"] + ) + with patch("evergreen_tracks.cli.dt") as mock_dt: + mock_dt.date.today.return_value = dt.date(2026, 8, 11) + rc = cmd_promote(args) + + assert rc == 0 + assert "" not in calls + mock_point_tag.assert_not_called() + + +@patch("evergreen_tracks.cli.point_tag") +@patch("evergreen_tracks.cli.list_tags") +def test_promote_falls_back_to_full_listing_when_the_window_is_empty( + mock_list_tags, mock_point_tag +): + """Nothing promotable in the walked months must fall back to the full listing. + + "No moves" is indistinguishable from a broken read path, and a quiet stall is how + tracks would silently stop advancing — so the expensive answer is worth having here. + """ + fake, calls = _fake_registry(_registry_tags()) + mock_list_tags.side_effect = fake + args = build_parser().parse_args( + ["promote", "--repo", "dotcms/dotcms-test", "--tracks", "standard", + "--standard-days", "14"] + ) + # Far enough ahead that the 6-month walk window holds no release at all. + with patch("evergreen_tracks.cli.dt") as mock_dt: + mock_dt.date.today.return_value = dt.date(2027, 6, 11) + rc = cmd_promote(args) + + assert rc == 0 + assert "" in calls, f"expected the terminal full-listing fallback, got {calls}" + + +@patch("evergreen_tracks.cli.point_tag") +@patch("evergreen_tracks.cli.list_tags") +def test_promote_scoped_to_latest_never_reads_other_tracks(mock_list_tags, mock_point_tag): + """--tracks latest (the release pipeline's path) must not spend calls on the + tracks it was told to leave alone.""" + fake, calls = _fake_registry(_registry_tags()) + mock_list_tags.side_effect = fake + args = build_parser().parse_args( + ["promote", "--repo", "dotcms/dotcms-test", "--tracks", "latest", "--latest-days", "0"] + ) + with patch("evergreen_tracks.cli.dt") as mock_dt: + mock_dt.date.today.return_value = dt.date(2026, 8, 11) + cmd_promote(args) + + assert "standard" not in calls and "trailing" not in calls, calls