diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ac86d8858..d3ce4751b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -391,11 +391,17 @@ jobs: name: Playwright E2E (production) runs-on: ubuntu-latest # A backstop above the test step's own cap, not the primary bound: a job - # timeout is a cancellation and would skip the rollback below, whereas the - # step cap marks the step failed and keeps it reachable. + # timeout cancels the test step, and a cancelled step is not a verdict the + # rollback job acts on, whereas the step cap marks it failed. timeout-minutes: 30 needs: [production-smoke-tests] if: github.ref == 'refs/heads/main' && github.event_name == 'push' + # The test step's own outcome, for rollback-production-on-e2e-failure. Not + # the job's result: a failed checkout, npm ci or Chrome report fails the job + # too, and none of them says anything about whether production is healthy. + # Only tests that ran and failed roll production back. + outputs: + tests: ${{ steps.e2e.outcome }} # The browser is the Google Chrome the runner image ships # (e2e/playwright.config.ts) rather than one installed here: `npx # playwright install --with-deps` puts an apt update and Playwright's CDN in @@ -418,9 +424,9 @@ jobs: - name: Report the runner's Chrome run: node e2e/report-chrome.js # Bounded at the step rather than relying on the job cap: a step timeout - # marks the step failed, which still trips the rollback below, whereas a - # job timeout is a cancellation and would skip it. A production wedged - # badly enough to hang Playwright is exactly when the rollback is wanted. + # marks the step failed, which still trips the rollback job, whereas a + # job timeout cancels it and would not. A production wedged badly enough + # to hang Playwright is exactly when the rollback is wanted. - name: Run E2E tests (production) id: e2e # See the staging job: the step cap has to clear nodes.spec.ts's retried @@ -430,13 +436,29 @@ jobs: env: CI: "true" RADAR_API_KEY: ${{ secrets.RADAR_API_KEY }} - # Scoped to the test step's own outcome, as the production smoke job is. - # A bare failure() also fires when checkout, npm ci or the Chrome report - # fails, and anything that breaks before the tests run is a statement - # about the runner, not about production. Those leave the job red for a - # human to read; only tests that ran and failed roll production back. - - name: Rollback production on E2E failure - if: failure() && steps.e2e.outcome == 'failure' + + # A job of its own, so that production's root key is never handed to a job + # that has just run npm's install scripts. test_deploy_workflows.py holds + # every workflow to that. + rollback-production-on-e2e-failure: + name: Roll back production if its E2E tests failed + runs-on: ubuntu-latest + timeout-minutes: 20 + needs: [e2e-prod] + # See rollback-production-on-deploy-failure: a following push's deploy must + # not be inside the deploy directory while rollback.sh is. + concurrency: + group: production-deploy + cancel-in-progress: false + # !cancelled() rather than always(): e2e-prod failing is when this has to + # run, but an operator cancelling the run, say over a known flake, must + # still be able to stop it. The main-push gate is repeated so no other event + # can open production SSH. + if: >- + !cancelled() && github.ref == 'refs/heads/main' && github.event_name == 'push' && + needs.e2e-prod.outputs.tests == 'failure' + steps: + - name: Roll back production uses: appleboy/ssh-action@0ff4204d59e8e51228ff73bce53f80d53301dee2 # v1.2.5 with: host: ${{ secrets.SERVER_HOST }} @@ -934,8 +956,8 @@ jobs: # Reached only once the app answered /api/health. The box is no # longer mid-deploy, so drop the marker; any failure after this - # point is a smoke/E2E failure, which the rollback steps on those - # jobs already cover. + # point is a smoke/E2E failure, which the smoke job's rollback step + # and rollback-production-on-e2e-failure already cover. rm -f ${{ env.APP_DIR }}/.deploy-in-progress rollback-production-on-deploy-failure: @@ -959,9 +981,9 @@ jobs: # would then SSH into production to consider rolling back a deploy that # never ran. Keyed to the deploy job's own result, it cannot. # - # This is the gap the two existing rollbacks do not cover. Both hang off - # jobs DOWNSTREAM of the deploy (production-smoke-tests, e2e-prod), and a - # failed deploy skips those jobs and their rollback steps with them. + # This is the gap the two other rollbacks do not cover. Both hang off jobs + # DOWNSTREAM of the deploy (production-smoke-tests, e2e-prod), and a failed + # deploy skips those jobs, and so the rollbacks keyed to them. # 'cancelled' as well as 'failure'. Cancelling mid-deploy leaves production # exactly as half-deployed as a crash does, and deploy-production sets # cancel-in-progress: false, so this can only be a human pressing cancel — @@ -1368,7 +1390,7 @@ jobs: echo "" smoke_summary [ "$FAIL" -eq 0 ] || exit 1 - # Auto-rollback on smoke failure, mirroring the prod E2E job. A failed + # Auto-rollback on smoke failure, as on an E2E failure. A failed # smoke test means a bad build is live serving prod; revert to # last-known-good rather than leaving it up for a human to catch. The # condition is scoped to the smoke step's own outcome, so a failure in the @@ -1383,7 +1405,7 @@ jobs: key: ${{ secrets.SERVER_SSH_KEY }} envs: APP_DIR script: | - # See the identical assertion in e2e-prod's rollback step. + # See the identical assertion in rollback-production-on-e2e-failure. [ "$(hostname)" = "retina-prod" ] || { echo "::error::Refusing to roll back on '$(hostname)' — expected retina-prod."; exit 1; } echo "Production smoke tests failed — rolling back production..." cd ${{ env.APP_DIR }} && bash deploy/rollback.sh diff --git a/backend/tests/test_deploy_workflows.py b/backend/tests/test_deploy_workflows.py index bae525009..2ea43cfd2 100644 --- a/backend/tests/test_deploy_workflows.py +++ b/backend/tests/test_deploy_workflows.py @@ -4,6 +4,7 @@ and pin what must hold in every copy. """ +import functools import re from pathlib import Path @@ -12,6 +13,12 @@ WORKFLOWS = Path(__file__).resolve().parents[2] / ".github" / "workflows" + +@functools.cache +def _workflow(workflow: str) -> dict: + return yaml.safe_load((WORKFLOWS / workflow).read_text()) + + DEPLOYS = [ pytest.param("ci.yml", "deploy-production", id="production"), pytest.param("staging-deploy-verify.yml", "deploy", id="staging"), @@ -26,7 +33,7 @@ def _job(workflow: str, job: str) -> dict: - jobs = yaml.safe_load((WORKFLOWS / workflow).read_text())["jobs"] + jobs = _workflow(workflow)["jobs"] assert job in jobs, f"{workflow} has no job {job!r}" return jobs[job] @@ -205,22 +212,165 @@ def test_rollback_job_reads_back_what_the_deploy_wrote(workflow, deploy, rollbac assert "always()" in condition +def _allowed(workflow: str, job: str, values: dict[str, str], *, cancelled: bool = False) -> bool: + """A job's `if:`, evaluated with each context field replaced by a value.""" + condition = " ".join(_job(workflow, job)["if"].split()) + for field, value in values.items(): + # Whole fields only: github.ref must not also rewrite github.ref_name. + condition = re.sub(rf"(? list[Path]: + return sorted([*WORKFLOWS.glob("*.yml"), *WORKFLOWS.glob("*.yaml")]) + + +def _workflow_jobs() -> list: + return [ + pytest.param(path.name, name, job, id=f"{path.name}:{name}") + for path in _workflow_files() + for name, job in _workflow(path.name)["jobs"].items() + ] + + +def _runs_third_party_code(job: dict) -> bool: + steps = job.get("steps", []) + return ( + "container" in job + or "services" in job + # Another repository's reusable workflow. + or ("uses" in job and not str(job["uses"]).startswith("./")) + or any("uses" in step and not str(step["uses"]).startswith(KEY_JOB_ACTIONS) for step in steps) + or any(THIRD_PARTY_CODE.search(step.get("run", "")) for step in steps) + ) + + +def test_the_guard_recognises_what_the_jobs_run(): + # Guards the guard: patterns that matched nothing would pass every job. + assert _runs_third_party_code(_job("ci.yml", "web-build")) + assert _runs_third_party_code(_job("ci.yml", "backend-tests")) + assert _runs_third_party_code(_job("ci.yml", "lint")) + assert _runs_third_party_code(_job("ci.yml", "e2e-prod")) + assert _runs_third_party_code({"services": {"db": {"image": "postgres"}}, "steps": []}) + assert _runs_third_party_code({"steps": [{"uses": "docker://alpine:3"}]}) + assert _runs_third_party_code({"steps": [{"uses": "./.github/actions/setup-uv"}]}) + assert _runs_third_party_code({"steps": [{"run": "uv run python scripts/check.py"}]}) + assert _runs_third_party_code({"steps": [{"run": "docker run --rm some/image"}]}) + assert _runs_third_party_code({"steps": [{"run": "npm --prefix frontend ci"}]}) + assert _runs_third_party_code({"steps": [{"run": "sudo apt-get -y install jq"}]}) + assert _runs_third_party_code({"uses": "someorg/repo/.github/workflows/x.yml@main"}) + assert not _runs_third_party_code(_job("ci.yml", "staging")) + assert not _runs_third_party_code(_job("ci.yml", "deploy-production")) + assert not _runs_third_party_code(_job("ci.yml", "production-smoke-tests")) + assert SSH_KEY.search(yaml.safe_dump(_job("ci.yml", "deploy-production"))) + assert SSH_KEY.search("key: ${{ secrets['STAGING_SSH_KEY'] }}") + assert SSH_KEY.search("key: ${{ secrets[format('{0}_SSH_KEY', inputs.env)] }}") + assert SSH_KEY.search("ALL: ${{ toJson(secrets) }}") + assert SSH_KEY.search("key: ${{ secrets.server_ssh_key }}") + assert SSH_KEY.search("key: ${{ secrets.DEPLOY_SSH_PRIVATE_KEY }}") + assert SSH_KEY.search(yaml.safe_dump({"uses": "someorg/repo/.github/workflows/x.yml@main", "secrets": "inherit"})) + assert not SSH_KEY.search("RADAR_API_KEY: ${{ secrets.RADAR_API_KEY }}") + + +@pytest.mark.parametrize(("workflow", "name", "job"), _workflow_jobs()) +def test_no_job_that_runs_third_party_code_can_read_a_droplet_key(workflow, name, job): + if _runs_third_party_code(job): + assert not SSH_KEY.search(yaml.safe_dump(job)), f"{workflow}:{name} runs third-party code" + + +@pytest.mark.parametrize("path", _workflow_files(), ids=lambda path: path.name) +def test_no_droplet_key_is_handed_to_every_job(path): + # Workflow-level env reaches every job, the npm ones included. + assert not SSH_KEY.search(yaml.safe_dump(_workflow(path.name).get("env", {}))) + + +# ── Production's E2E rollback ──────────────────────────────────────────────── + + +def test_the_e2e_rollback_acts_on_the_test_step_alone(): + # A failed pull, checkout or npm ci fails e2e-prod as well, and says + # nothing about production. Only the test step's own verdict may count. + e2e = _job("ci.yml", "e2e-prod") + assert [step for step in e2e["steps"] if step.get("id") == "e2e" and "test:e2e:prod" in step["run"]] + assert e2e["outputs"]["tests"] == "${{ steps.e2e.outcome }}" + + +def test_the_e2e_rollback_holds_the_production_lock_on_the_runner(): + rollback = _job("ci.yml", "rollback-production-on-e2e-failure") + assert "container" not in rollback + assert rollback["needs"] == ["e2e-prod"] + assert rollback["concurrency"] == {"group": "production-deploy", "cancel-in-progress": False} + script = _script("ci.yml", "rollback-production-on-e2e-failure") + identity = _index(script, re.escape('[ "$(hostname)" = "retina-prod" ]')) + assert identity < _index(script, r"\bbash deploy/rollback\.sh$") + + +@pytest.mark.parametrize("ref", ["refs/heads/main", "refs/heads/topic", "refs/pull/417/merge"]) +@pytest.mark.parametrize("event", ["push", "pull_request", "workflow_dispatch"]) +@pytest.mark.parametrize("tests", ["failure", "cancelled", "success", "skipped", ""]) +@pytest.mark.parametrize("cancelled", [False, True]) +def test_the_e2e_rollback_requires_failed_tests_on_a_main_push(ref, event, tests, cancelled): + # "" is what the output reads when e2e-prod never reached the test step, + # or was skipped outright. Cancelling the run stops it: an operator cancels + # to keep a known flake from reverting a healthy production. + allowed = _allowed( + "ci.yml", + "rollback-production-on-e2e-failure", + {"github.ref": ref, "github.event_name": event, "needs.e2e-prod.outputs.tests": tests}, + cancelled=cancelled, + ) + assert allowed is (ref == "refs/heads/main" and event == "push" and tests == "failure" and not cancelled)