From 64ae71d7ab0b17edb451483c651cef5687107ee9 Mon Sep 17 00:00:00 2001 From: Tudor Date: Tue, 15 Sep 2026 16:01:59 +0100 Subject: [PATCH] fix(ci): make a failed release check say what it actually saw MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The staging poller swallowed every failure identically, so a run that timed out told us only that the expected release never appeared — not whether the proxy refused us, the endpoint was down, or the containers were still serving an older build. The public staging proxy also answers 403 to urllib's default user agent while the release endpoint is healthy, which looked exactly like a deployment that never arrived. Identify the poller, and report each distinct observation once: HTTP status, connection failure type, invalid JSON, or the release identities actually reported. The timeout error carries the last observation and the identity it wanted. Responses and the base URL stay out of the logs — only validated sha/build_id fields are echoed back. Co-Authored-By: Claude Opus 5 --- docs/DEPLOY.md | 9 +++++ scripts/ci/release.py | 50 +++++++++++++++++++++--- scripts/ci/tests/test_release.py | 65 ++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 6 deletions(-) diff --git a/docs/DEPLOY.md b/docs/DEPLOY.md index 63e85e0..5eac5cb 100644 --- a/docs/DEPLOY.md +++ b/docs/DEPLOY.md @@ -291,6 +291,15 @@ workflow first. The release route must be reachable through the configured `/api` proxy limitation. No new deployment secret is required. `scripts/ci/release.py` implements identity polling and digest verification. +The poller identifies itself as `SchoolCompare-Release-Check/1.0`: the public +staging proxy has returned HTTP 403 to Python's default urllib user agent even +while the release endpoint was healthy. It logs changes in HTTP/connection +failures or observed release identities, and includes the last observation in +the timeout error. If verification fails, use that observation to distinguish +proxy rejection (403), an unavailable release endpoint (503), and containers +still reporting an older SHA/build ID. Check the configured base URL from the +CI runner; a successful request from another machine does not establish runner +connectivity. Do not bypass identity verification to unblock a deployment. Its mocked tests run in PR checks alongside backend and index-publication tests. The new Playwright journeys also check deployed identity and stale pagination. Local unit checks do not validate registry credentials, Portainer behaviour, diff --git a/scripts/ci/release.py b/scripts/ci/release.py index fd41c62..b003b08 100644 --- a/scripts/ci/release.py +++ b/scripts/ci/release.py @@ -10,6 +10,7 @@ from pathlib import Path import re import subprocess import time +from urllib.error import HTTPError, URLError from urllib.request import Request, urlopen COMPONENTS = ('BACKEND', 'FRONTEND', 'PIPELINE') @@ -88,8 +89,29 @@ def promote(sha): def matches(payload, sha, build_id): - return all(payload.get(component) == {'sha': sha, 'build_id': build_id} - for component in ('frontend', 'backend')) + return isinstance(payload, dict) and all( + payload.get(component) == {'sha': sha, 'build_id': build_id} + for component in ('frontend', 'backend')) + + +def describe_identity(payload): + """Log only release fields, never arbitrary response bodies or secret URLs.""" + if not isinstance(payload, dict): + return 'Invalid release response: expected a JSON object' + identities = [] + for component in ('frontend', 'backend'): + identity = payload.get(component) + if not isinstance(identity, dict): + identities.append(f'{component}=missing or invalid') + continue + values = [] + for field, length in (('sha', 40), ('build_id', 32)): + value = identity.get(field) + valid = isinstance(value, str) and ( + value == 'development' or re.fullmatch(r'[0-9a-f]{' + str(length) + '}', value)) + values.append(f'{field}={value if valid else "missing or invalid"}') + identities.append(f'{component}: {", ".join(values)}') + return 'Release mismatch: ' + '; '.join(identities) def wait(base_url, sha, build_id, timeout): @@ -97,19 +119,35 @@ def wait(base_url, sha, build_id, timeout): if not re.fullmatch(r'[0-9a-f]{32}', build_id): raise ValueError('Missing expected build identity') deadline = time.monotonic() + timeout + last_observation = 'No response received' + print(f'Waiting for deployed release {sha} / {build_id}', flush=True) while time.monotonic() < deadline: try: req = Request(f'{base_url.rstrip("/")}/release.json?check={time.time_ns()}', - headers={'Cache-Control': 'no-cache'}) + headers={'Cache-Control': 'no-cache', + 'User-Agent': 'SchoolCompare-Release-Check/1.0', + 'Accept': 'application/json'}) with urlopen(req, timeout=min(10, max(.1, deadline - time.monotonic()))) as response: payload = json.load(response) if matches(payload, sha, build_id): print(f'Verified deployed release {sha} / {build_id}') return - except (OSError, ValueError): - pass + observation = describe_identity(payload) + except HTTPError as exc: + observation = f'Release endpoint returned HTTP {exc.code}' + exc.close() + except URLError as exc: + observation = f'Release endpoint connection failed ({type(exc.reason).__name__})' + except OSError as exc: + observation = f'Release endpoint request failed ({type(exc).__name__})' + except ValueError: + observation = 'Release endpoint returned invalid JSON or request configuration' + if observation != last_observation: + print(observation, flush=True) + last_observation = observation time.sleep(min(5, max(0, deadline - time.monotonic()))) - raise RuntimeError('Deployment did not report the expected frontend/backend release') + raise RuntimeError('Deployment did not report the expected frontend/backend release ' + f'{sha} / {build_id}. Last observation: {last_observation}') def main(): diff --git a/scripts/ci/tests/test_release.py b/scripts/ci/tests/test_release.py index c98af86..9636acb 100644 --- a/scripts/ci/tests/test_release.py +++ b/scripts/ci/tests/test_release.py @@ -1,4 +1,6 @@ import json +from io import BytesIO +from urllib.error import HTTPError, URLError from unittest.mock import Mock import pytest from scripts.ci import release @@ -63,3 +65,66 @@ def test_missing_candidate_fails_before_any_tag_is_changed(docker): docker.side_effect = RuntimeError('missing verified tag') with pytest.raises(RuntimeError): release.promote(SHA) assert not any(c.args[0] == 'create' for c in docker.call_args_list) + + +@pytest.fixture +def poll(monkeypatch): + now = [0.0] + monkeypatch.setattr(release.time, 'monotonic', lambda: now[0]) + monkeypatch.setattr(release.time, 'sleep', lambda seconds: now.__setitem__(0, now[0] + seconds)) + opener = Mock() + monkeypatch.setattr(release, 'urlopen', opener) + return opener + + +def response(payload): + return BytesIO(json.dumps(payload).encode()) + + +def test_wait_identifies_its_client_and_retries_until_both_services_match(poll, capsys): + poll.side_effect = [ + HTTPError('https://secret.example', 503, 'unavailable', {}, None), + response({'frontend': {'sha': SHA, 'build_id': BUILD}, + 'backend': {'sha': SHA, 'build_id': 'c' * 32}}), + response({component: {'sha': SHA, 'build_id': BUILD} + for component in ('frontend', 'backend')}), + ] + release.wait('https://secret.example/', SHA, BUILD, 15) + assert poll.call_count == 3 + request = poll.call_args.args[0] + assert request.get_header('User-agent') == 'SchoolCompare-Release-Check/1.0' + assert request.get_header('Cache-control') == 'no-cache' + assert request.get_header('Accept') == 'application/json' + assert '/release.json?check=' in request.full_url + output = capsys.readouterr().out + assert 'HTTP 503' in output + assert 'backend: sha=' + SHA + ', build_id=' + 'c' * 32 in output + assert 'Verified deployed release' in output + assert 'secret.example' not in output + + +@pytest.mark.parametrize('failure, expected', [ + (lambda: HTTPError('https://secret.example', 403, 'secret response', {}, None), 'HTTP 403'), + (lambda: URLError(OSError('secret address')), 'connection failed (OSError)'), + (lambda: TimeoutError('secret address'), 'request failed (TimeoutError)'), + (lambda: BytesIO(b'secret response'), 'invalid JSON'), + (lambda: response([]), 'expected a JSON object'), + (lambda: response({'frontend': {'sha': 'secret response'}}), 'missing or invalid'), +]) +def test_wait_timeout_reports_last_failure_without_leaking_response_or_url(poll, capsys, failure, expected): + poll.side_effect = lambda *args, **kwargs: result_or_raise(failure()) + with pytest.raises(RuntimeError) as error: + release.wait('https://secret.example', SHA, BUILD, 10) + assert expected in str(error.value) + assert SHA in str(error.value) + assert BUILD in str(error.value) + output = capsys.readouterr().out + assert sum(expected in line for line in output.splitlines()) == 1 + assert 'secret' not in output + str(error.value) + assert poll.call_count == 2 + + +def result_or_raise(result): + if isinstance(result, Exception): + raise result + return result -- 2.54.0