From d21720ae782f1d24962963c02095c0ae0bee9530 Mon Sep 17 00:00:00 2001 From: Mario Apra Date: Fri, 18 Sep 2026 10:30:10 +0100 Subject: [PATCH] feat: add deploy gate silent-fail watchdog A build-and-push workflow is a deploy gate: when it fails, no image lands, the deployer has nothing new to sync, and prod just keeps serving whatever image last made it through. That failure is silent, the branch still looks green, and the only trace is a grey cross nobody opens. It happened on angkor-platform-frontend: Trivy started failing before the push step, main went unshipped for days, and prod kept serving an image with known auth-bypass CVEs. It only surfaced because someone's unrelated fix never showed up in prod. Add a reusable workflow_call workflow that checks, on the caller's own schedule, whether a branch's head is covered by a successful run of a named gating workflow. Quiet inside a grace window (default 90 minutes), then it opens one deduplicated labelled issue and exits 1, closing that issue again once the branch ships. It's a watchdog on purpose, not an on-failure hook: a hook only fires when the gate ran and lost, never when it didn't run at all (disabled, path-filtered away, runner outage, schedule stopped), which is exactly what stayed hidden the longest here. Runs on the built-in GITHUB_TOKEN, no Slack webhook or org secret, since the repo that needed this has neither and no admin to add one. Every input is checked against an allowlist before any API call, with distinct exit codes (0 shipped/in grace, 1 stale, 2 bad args). Tests in tests/deploy-gate-watchdog/ extract the watchdog's shell body straight out of the workflow YAML and run it against a stubbed gh, so there's one copy of the logic and the suite is pinned to what actually ships. Logic stays inline instead of in a script file because a reusable workflow runs in the caller's checkout, where a script from this repo isn't on disk. Added repo self-CI (ci.yaml) running the same commands locally and in CI, plus tests/lint_workflows.sh running actionlint with a shrink-only exemption list for five workflows that already had findings. actionlint and PyYAML are pinned as tool directives with matching Dependabot entries so the pins get bumped instead of rotting. Replaying the original incident through the watchdog's own query shows one run, concluded failure, zero successes, so it would've been reported within the grace window instead of days later. Callers need to grant actions: read and issues: write; the reusable workflow's permissions block can only cap what's granted, never add to it. ANG-2720 --- .github/dependabot.yml | 27 ++ .../actions-watchdog-deploy-gate.yaml | 207 ++++++++++++++ .github/workflows/ci.yaml | 50 ++++ README.md | 16 ++ examples/watchdog/README.md | 77 ++++++ examples/watchdog/deploy-gate-watchdog.yml | 23 ++ tests/actionlint-legacy.txt | 15 ++ tests/deploy-gate-watchdog/extract_step.py | 50 ++++ tests/deploy-gate-watchdog/run_tests.sh | 255 ++++++++++++++++++ tests/deploy-gate-watchdog/stub-gh | 58 ++++ tests/lint_workflows.sh | 84 ++++++ tests/requirements.txt | 4 + tools/actionlint/go.mod | 20 ++ tools/actionlint/go.sum | 27 ++ 14 files changed, 913 insertions(+) create mode 100644 .github/workflows/actions-watchdog-deploy-gate.yaml create mode 100644 .github/workflows/ci.yaml create mode 100644 examples/watchdog/README.md create mode 100644 examples/watchdog/deploy-gate-watchdog.yml create mode 100644 tests/actionlint-legacy.txt create mode 100755 tests/deploy-gate-watchdog/extract_step.py create mode 100755 tests/deploy-gate-watchdog/run_tests.sh create mode 100755 tests/deploy-gate-watchdog/stub-gh create mode 100755 tests/lint_workflows.sh create mode 100644 tests/requirements.txt create mode 100644 tools/actionlint/go.mod create mode 100644 tools/actionlint/go.sum diff --git a/.github/dependabot.yml b/.github/dependabot.yml index d94dce0..737d7e8 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -11,3 +11,30 @@ updates: all-github-actions: patterns: - "*" + + # actionlint is pinned as a tool directive in tools/actionlint/go.mod precisely so this + # entry can bump it. A version pinned where Dependabot cannot see it is a version that + # stays where it was put. + - package-ecosystem: gomod + directory: /tools/actionlint + schedule: + interval: weekly + commit-message: + prefix: "fix" + include: "scope" + groups: + all-go-tools: + patterns: + - "*" + + - package-ecosystem: pip + directory: /tests + schedule: + interval: weekly + commit-message: + prefix: "fix" + include: "scope" + groups: + all-test-deps: + patterns: + - "*" diff --git a/.github/workflows/actions-watchdog-deploy-gate.yaml b/.github/workflows/actions-watchdog-deploy-gate.yaml new file mode 100644 index 0000000..41ab155 --- /dev/null +++ b/.github/workflows/actions-watchdog-deploy-gate.yaml @@ -0,0 +1,207 @@ +name: Deploy gate watchdog + +# A "deploy gate" is the workflow whose success is what actually publishes an artifact: +# docker-build-push-jfrog and friends. When that gate goes red on the release branch, +# nothing is pushed, the deployer has nothing new to sync, and production quietly keeps +# serving the last image that made it through. Nothing in GitHub makes that loud — a red +# run on the default branch is a grey cross in a commit list nobody is watching — so the +# branch can sit unshipped for days while looking merged and done. +# +# This workflow asks the only question that matters, on a schedule: is the head of the +# release branch represented by a *successful* run of the gate? If not, it files one +# deduplicated issue and fails, and it closes that issue again when the branch ships. +# +# It is deliberately a watchdog rather than an on-failure hook. A hook is a strictly +# weaker signal: it can only fire when the gate ran and lost. It cannot fire when the +# gate never ran at all — workflow disabled, trigger deleted, runner outage, a push that +# matched no path filter — and those are the failures that stay hidden longest. + +on: + workflow_call: + inputs: + workflow_file: + description: "File name of the gating workflow, e.g. build-images.yaml. The file name rather than the display name, so renaming the workflow's `name:` cannot silently detach the watchdog." + type: string + required: true + branch: + description: "Branch whose head must be shipped." + type: string + required: false + default: "main" + grace_minutes: + description: "How long a commit may go unshipped before it counts as stale. Must cover a normal end-to-end run of the gate, or every push trips the watchdog while it is still building." + type: number + required: false + default: 90 + issue_label: + description: "Label used to find and deduplicate the watchdog's issue. One open issue per label, reused across consecutive failures." + type: string + required: false + default: "deploy-gate-stale" + issue_assignees: + description: "Comma-separated GitHub usernames to assign the issue to. Empty leaves it unassigned." + type: string + required: false + default: "" + +permissions: + contents: read + # Reading workflow run history is the whole check; writing issues is the whole alert. + actions: read + issues: write + +jobs: + watchdog: + name: Check deploy gate + runs-on: ubuntu-latest + steps: + - name: Check gate freshness + id: check + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + INPUT_WORKFLOW_FILE: ${{ inputs.workflow_file }} + INPUT_BRANCH: ${{ inputs.branch }} + INPUT_GRACE_MINUTES: ${{ inputs.grace_minutes }} + INPUT_ISSUE_LABEL: ${{ inputs.issue_label }} + INPUT_ISSUE_ASSIGNEES: ${{ inputs.issue_assignees }} + run: | + set -euo pipefail + + # Exit codes are distinct so the tests can assert *which* thing happened rather + # than only "non-zero": 0 shipped or still inside the grace window, 1 stale, + # 2 called with arguments this cannot act on. + die() { echo "::error::$*"; exit 2; } + summary() { echo "$*" >> "${GITHUB_STEP_SUMMARY:-/dev/null}"; } + + # Validate before touching the API. Allowlists, not blocklists: every one of + # these values is interpolated into a REST path, a query string or a gh + # argument, and "reject what looks wrong" is how a traversal or an injected + # query parameter gets through. + [[ "${INPUT_WORKFLOW_FILE}" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*\.(yml|yaml)$ ]] || + die "workflow_file must be a workflow file name such as build-images.yaml, got: ${INPUT_WORKFLOW_FILE}" + [[ "${INPUT_BRANCH}" =~ ^[A-Za-z0-9][A-Za-z0-9._/-]*$ ]] || + die "branch must be a plain branch name, got: ${INPUT_BRANCH}" + [[ "${INPUT_GRACE_MINUTES}" =~ ^[0-9]+$ ]] || + die "grace_minutes must be a non-negative integer, got: ${INPUT_GRACE_MINUTES}" + [[ "${INPUT_ISSUE_LABEL}" =~ ^[A-Za-z0-9][A-Za-z0-9._:-]{0,48}$ ]] || + die "issue_label must be 1-49 characters of [A-Za-z0-9._:-], got: ${INPUT_ISSUE_LABEL}" + [[ "${INPUT_ISSUE_ASSIGNEES}" =~ ^([A-Za-z0-9-]+(,[A-Za-z0-9-]+)*)?$ ]] || + die "issue_assignees must be comma-separated GitHub usernames, got: ${INPUT_ISSUE_ASSIGNEES}" + + work="$(mktemp -d)" + + # --- observe ------------------------------------------------------------- + + gh api "repos/${REPO}/commits/${INPUT_BRANCH}" > "${work}/head.json" + head_sha="$(jq -r '.sha' "${work}/head.json")" + head_date="$(jq -r '.commit.committer.date' "${work}/head.json")" + + # `status` on this endpoint takes conclusions as well as statuses, so + # status=success means "concluded successfully". Worth knowing: an unrecognised + # value there returns an empty list rather than a 4xx, so if that enum ever + # changes under us this reports a stale branch instead of silently reporting a + # healthy one. Wrong in the direction that gets looked at. + gh api "repos/${REPO}/actions/workflows/${INPUT_WORKFLOW_FILE}/runs?branch=${INPUT_BRANCH}&status=success&per_page=1" \ + > "${work}/last_ok.json" + last_ok_sha="$(jq -r '.workflow_runs[0].head_sha // ""' "${work}/last_ok.json")" + last_ok_url="$(jq -r '.workflow_runs[0].html_url // ""' "${work}/last_ok.json")" + + echo "branch head: ${head_sha} (${head_date})" + echo "last shipped head: ${last_ok_sha:-}" + + # `gh issue list --label` errors on a label that does not exist yet rather than + # returning an empty list, so the label is created first. Already-exists is the + # normal case and is not a failure. + gh label create "${INPUT_ISSUE_LABEL}" --repo "${REPO}" --color B60205 \ + --description "Release branch is not represented by a successful deploy gate run" \ + >/dev/null 2>&1 || true + + issue="$(gh issue list --repo "${REPO}" --label "${INPUT_ISSUE_LABEL}" \ + --state open --limit 1 --json number --jq '.[0].number // empty')" + + # --- shipped: close anything the watchdog opened earlier ------------------- + + if [[ -n "${last_ok_sha}" && "${last_ok_sha}" == "${head_sha}" ]]; then + echo "gate is current" + summary "### Deploy gate current" + summary "\`${INPUT_BRANCH}\` head \`${head_sha}\` shipped by [${INPUT_WORKFLOW_FILE}](${last_ok_url})." + + if [[ -n "${issue}" ]]; then + # shellcheck disable=SC2016 # backticks here are markdown code spans + { + printf 'Recovered. `%s` head `%s` was shipped by [%s](%s).\n\n' \ + "${INPUT_BRANCH}" "${head_sha}" "${INPUT_WORKFLOW_FILE}" "${last_ok_url}" + printf 'Closed by the [deploy gate watchdog](%s).\n' "${RUN_URL}" + } > "${work}/recovered.md" + gh issue comment "${issue}" --repo "${REPO}" --body-file "${work}/recovered.md" + gh issue close "${issue}" --repo "${REPO}" --reason completed + echo "closed issue #${issue}" + fi + exit 0 + fi + + # --- unshipped, but possibly still building ------------------------------- + + # python3 rather than `date -d`: the same parse then works on a developer's + # macOS while running the tests, so the tested code and the shipped code are + # not two different implementations. + head_epoch="$(python3 -c 'import datetime, sys; print(int(datetime.datetime.strptime(sys.argv[1], "%Y-%m-%dT%H:%M:%SZ").replace(tzinfo=datetime.timezone.utc).timestamp()))' "${head_date}")" + age_minutes=$(( ( $(date -u +%s) - head_epoch ) / 60 )) + + if (( age_minutes < INPUT_GRACE_MINUTES )); then + echo "head is ${age_minutes}m old, inside the ${INPUT_GRACE_MINUTES}m grace window" + summary "### Deploy gate pending" + summary "\`${head_sha}\` is ${age_minutes}m old, inside the ${INPUT_GRACE_MINUTES}m grace window." + exit 0 + fi + + # --- stale --------------------------------------------------------------- + + gh api "repos/${REPO}/actions/workflows/${INPUT_WORKFLOW_FILE}/runs?head_sha=${head_sha}&per_page=1" \ + > "${work}/head_run.json" + run_status="$(jq -r '.workflow_runs[0].status // "never ran"' "${work}/head_run.json")" + run_conclusion="$(jq -r '.workflow_runs[0].conclusion // "none"' "${work}/head_run.json")" + gate_run_url="$(jq -r '.workflow_runs[0].html_url // ""' "${work}/head_run.json")" + + # shellcheck disable=SC2016 # backticks here are markdown code spans + { + printf '`%s` is at `%s`, committed %s minutes ago, and no successful run of `%s` has that commit as its head. Whatever consumes this gate is still serving the previous artifact.\n\n' \ + "${INPUT_BRANCH}" "${head_sha}" "${age_minutes}" "${INPUT_WORKFLOW_FILE}" + printf '| | |\n|---|---|\n' + printf '| Branch head | `%s` |\n' "${head_sha}" + printf '| Head commit age | %s minutes |\n' "${age_minutes}" + printf '| Gate run for that head | %s / %s %s |\n' "${run_status}" "${run_conclusion}" "${gate_run_url}" + printf '| Last head that shipped | %s %s |\n\n' "${last_ok_sha:-none on record}" "${last_ok_url}" + printf 'Filed by the [deploy gate watchdog](%s). It closes this issue by itself once `%s` ships again.\n' \ + "${RUN_URL}" "${INPUT_BRANCH}" + } > "${work}/stale.md" + + if [[ -z "${issue}" ]]; then + assignees=() + if [[ -n "${INPUT_ISSUE_ASSIGNEES}" ]]; then + assignees=(--assignee "${INPUT_ISSUE_ASSIGNEES}") + fi + gh issue create --repo "${REPO}" \ + --title "Deploy gate stale: ${INPUT_WORKFLOW_FILE} has not succeeded for ${INPUT_BRANCH}" \ + --label "${INPUT_ISSUE_LABEL}" ${assignees[@]+"${assignees[@]}"} \ + --body-file "${work}/stale.md" + echo "opened an issue" + else + # Already reported for this exact head? The watchdog runs hourly, and an + # hourly comment about one unchanged commit trains people to mute the issue, + # which rebuilds the silence this exists to remove. + gh issue view "${issue}" --repo "${REPO}" --json body,comments \ + --jq '.body, (.comments[].body)' > "${work}/reported.txt" + if grep -qF "${head_sha}" "${work}/reported.txt"; then + echo "issue #${issue} already reports ${head_sha}" + else + gh issue comment "${issue}" --repo "${REPO}" --body-file "${work}/stale.md" + echo "commented on issue #${issue}" + fi + fi + + summary "### Deploy gate stale" + summary "\`${INPUT_BRANCH}\` head \`${head_sha}\` is ${age_minutes}m old and unshipped. Gate run: ${run_status} / ${run_conclusion}." + exit 1 diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml new file mode 100644 index 0000000..908613b --- /dev/null +++ b/.github/workflows/ci.yaml @@ -0,0 +1,50 @@ +name: CI + +# This repository ships workflows that other repositories run, so a mistake here lands in +# everyone's pipeline at once and shows up as their build breaking, not ours. Both jobs +# below are the same commands a developer runs locally — tests/lint_workflows.sh and +# tests/deploy-gate-watchdog/run_tests.sh — so there is nothing CI checks that cannot be +# reproduced from a terminal, and nothing a terminal checks that CI skips. + +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + lint: + name: Lint workflows + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + # actionlint's version is a tool directive in tools/actionlint/go.mod rather than a + # string in this file, because a version written here is invisible to Dependabot and + # rots in place. shellcheck comes from the runner image; actionlint finds it and runs + # it over every run: block. + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: tools/actionlint/go.mod + + - run: tests/lint_workflows.sh + + watchdog-tests: + name: Deploy gate watchdog + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.13" + + - run: pip install -r tests/requirements.txt + + - run: tests/deploy-gate-watchdog/run_tests.sh diff --git a/README.md b/README.md index 348188f..d2fd00e 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,22 @@ Read about it in the [examples/docker/README.md](examples/docker/README.md) file Read about it in the [examples/ai-review/README.md](examples/ai-review/README.md) file. +## Deploy Gate Watchdog + +Catches the case where a build-and-push workflow has been failing on the release branch +and nobody noticed, so production is still serving an old image. Read about it in the +[examples/watchdog/README.md](examples/watchdog/README.md) file. + +## Working on this repository + +```shell +tests/lint_workflows.sh # actionlint over the workflows +tests/deploy-gate-watchdog/run_tests.sh # watchdog behaviour +``` + +Both are exactly what CI runs. `tests/lint_workflows.sh` needs Go; the watchdog tests +need python3 with the pin in [tests/requirements.txt](tests/requirements.txt). + ## License diff --git a/examples/watchdog/README.md b/examples/watchdog/README.md new file mode 100644 index 0000000..a323378 --- /dev/null +++ b/examples/watchdog/README.md @@ -0,0 +1,77 @@ +# Deploy Gate Watchdog + +**Implementation:** [`.github/workflows/actions-watchdog-deploy-gate.yaml`](../../.github/workflows/actions-watchdog-deploy-gate.yaml) + +**Example:** [`examples/watchdog/deploy-gate-watchdog.yml`](./deploy-gate-watchdog.yml) + +## The failure this exists for + +A build-and-push workflow is a deploy gate: when it succeeds an image lands in the +registry and the deployer picks it up, and when it fails nothing lands and the deployer +has nothing new to sync. That second case is silent. Production keeps serving the last +image that made it through, the branch still looks merged and green in the PR list, and +the only signal is a grey cross on a commit page nobody opens. + +It happened on `angkor-platform-frontend`: the Trivy stage of the build started failing +before the push step, main went unshipped for days, and production stayed on an image +with known auth-bypass CVEs. It surfaced only because someone shipped an unrelated fix +and noticed it never appeared. + +This workflow asks, on a schedule, the question that was never being asked: **is the head +of the release branch represented by a successful run of the gate?** If not, it opens one +issue and fails. When the branch ships again it closes the issue by itself. + +It is deliberately a watchdog and not an on-failure notification. A failure hook is a +strictly weaker signal — it can only fire when the gate ran and lost. It cannot fire when +the gate never ran at all: workflow disabled, trigger deleted or path-filtered away, +runner outage, a schedule that stopped. Those are the failures that stay hidden longest, +and they are the ones a watchdog catches for free. + +## Usage + +Copy [`deploy-gate-watchdog.yml`](./deploy-gate-watchdog.yml) into `.github/workflows/` +and set `workflow_file` to the workflow that publishes your artifact. + +The caller must grant `actions: read` (to read run history) and `issues: write` (to file +the alert). No secrets, no webhooks: it runs on the built-in `GITHUB_TOKEN`. + +## Inputs + +| Input | Default | Notes | +|---|---|---| +| `workflow_file` | *required* | File name of the gating workflow, e.g. `build-images.yaml`. The file name and not the display name, so renaming the workflow's `name:` cannot silently detach the watchdog. | +| `branch` | `main` | Branch whose head must be shipped. | +| `grace_minutes` | `90` | How long a commit may go unshipped before it counts as stale. Must comfortably exceed an end-to-end run of the gate, or every push trips the watchdog while it is still building. | +| `issue_label` | `deploy-gate-stale` | Used to find and deduplicate the issue. One open issue per label. | +| `issue_assignees` | *(none)* | Comma-separated usernames to assign the issue to. | + +Every input is validated against an allowlist before any API call, and a rejected input +fails the run rather than being cleaned up and used. + +## Behaviour + +| Situation | Result | +|---|---| +| Branch head has a successful gate run | Succeeds. Closes the watchdog's issue if one is open. | +| Branch head unshipped, younger than `grace_minutes` | Succeeds quietly. The build is presumed to still be running. | +| Branch head unshipped and older than `grace_minutes` | Fails, and opens an issue naming the head, its age, and what the gate did for that commit. | +| Same head still stale on the next run | Fails, comments nothing. An hourly comment about one unchanged commit is how an issue gets muted. | +| A newer head is also stale | Fails, and comments on the existing issue with the new head. | + +Exit codes are distinct on purpose: `0` shipped or within grace, `1` stale, `2` called +with arguments it cannot act on. + +## Choosing `grace_minutes` + +Set it above the p95 wall-clock time of the gate, measured rather than guessed, and leave +room for queueing. Too low and the watchdog files an issue against a build that was going +to succeed five minutes later, which is the fastest way to teach people to ignore it. Too +high and a genuinely broken main stays quiet for that long. The 90 minute default suits a +multi-arch image build with a scan stage. + +## Tests + +`tests/deploy-gate-watchdog/run_tests.sh`, runnable locally with nothing but bash and +python3. The suite runs the step body extracted from the workflow file itself against a +stubbed `gh`, so there is one copy of the logic and the tests are pinned to the text that +actually ships. diff --git a/examples/watchdog/deploy-gate-watchdog.yml b/examples/watchdog/deploy-gate-watchdog.yml new file mode 100644 index 0000000..949965e --- /dev/null +++ b/examples/watchdog/deploy-gate-watchdog.yml @@ -0,0 +1,23 @@ +name: Deploy gate watchdog + +on: + schedule: + # Hourly, at a minute nobody else picked. Scheduled workflows run on the default + # branch only, which is the branch this is watching anyway. + - cron: "17 * * * *" + # Handy for checking the wiring without waiting an hour. It can create, comment on and + # close this repository's issues, and nothing else, so the worst a repository writer can + # do with it is file an issue they could have filed by hand. + workflow_dispatch: + +permissions: + contents: read + actions: read + issues: write + +jobs: + watchdog: + uses: NethermindEth/github-workflows/.github/workflows/actions-watchdog-deploy-gate.yaml@stable + with: + # The file name of the workflow that publishes the image, not its display name. + workflow_file: build-images.yaml diff --git a/tests/actionlint-legacy.txt b/tests/actionlint-legacy.txt new file mode 100644 index 0000000..8fd7376 --- /dev/null +++ b/tests/actionlint-legacy.txt @@ -0,0 +1,15 @@ +# Workflows that already had actionlint findings when linting was introduced, so that +# the gate could be turned on for everything else without a mass edit of files this +# change has no business touching. +# +# This list may only shrink. tests/lint_workflows.sh fails if a file named here now +# lints clean, which forces the entry out in the same commit that fixes the file, and it +# fails if a name here no longer exists, so a rename cannot quietly widen the exemption. +# +# Everything outstanding is shellcheck info/style (SC2086 word splitting, SC2004, SC2129) +# inside steps that interpolate no untrusted input. +.github/workflows/compute-terraform-module-name.yaml +.github/workflows/docker-build-push-jfrog.yaml +.github/workflows/docker-promote-dockerhub.yaml +.github/workflows/docker-promote-jfrog.yaml +.github/workflows/publish-terraform-module.yaml diff --git a/tests/deploy-gate-watchdog/extract_step.py b/tests/deploy-gate-watchdog/extract_step.py new file mode 100755 index 0000000..b396922 --- /dev/null +++ b/tests/deploy-gate-watchdog/extract_step.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python3 +"""Print the shell body of one step of a workflow, so the tests exercise the shipped code. + +The watchdog's logic lives inline in the workflow rather than in a script file next to +it, because a reusable workflow runs in the *caller's* checkout: a script file in this +repository is simply not on disk when the workflow runs somewhere else, and the ways +around that (checking this repository out again at a ref the called workflow cannot +reliably know) add a failure mode to a thing whose entire job is to still be working. + +Inline code is normally the end of testing, which is how a watchdog quietly stops +watching. So the tests read the step back out of the YAML and run that. One copy, and +the suite is pinned to the text that actually ships. +""" + +import sys + +import yaml + + +def main() -> int: + if len(sys.argv) != 4: + print(f"usage: {sys.argv[0]} WORKFLOW JOB_ID STEP_ID", file=sys.stderr) + return 2 + + workflow_path, job_id, step_id = sys.argv[1:4] + + with open(workflow_path, encoding="utf-8") as handle: + workflow = yaml.safe_load(handle) + + job = workflow.get("jobs", {}).get(job_id) + if job is None: + print(f"no job {job_id!r} in {workflow_path}", file=sys.stderr) + return 1 + + for step in job.get("steps", []): + if step.get("id") != step_id: + continue + run = step.get("run") + if run is None: + print(f"step {step_id!r} has no run: block", file=sys.stderr) + return 1 + sys.stdout.write(run) + return 0 + + print(f"no step {step_id!r} in job {job_id!r}", file=sys.stderr) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/deploy-gate-watchdog/run_tests.sh b/tests/deploy-gate-watchdog/run_tests.sh new file mode 100755 index 0000000..d46fb56 --- /dev/null +++ b/tests/deploy-gate-watchdog/run_tests.sh @@ -0,0 +1,255 @@ +#!/usr/bin/env bash +# +# Tests for the deploy gate watchdog. Runs the step body extracted from the workflow +# itself (see extract_step.py) against a stubbed gh CLI. +# +# Exit codes are the contract under test: 0 shipped or inside the grace window, 1 stale, +# 2 bad arguments. Asserting only "non-zero" would let a validation bug pass as a +# successful detection, which is the one confusion a watchdog cannot afford. +set -uo pipefail + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ROOT="$(cd "${HERE}/../.." && pwd)" +WORKFLOW="${ROOT}/.github/workflows/actions-watchdog-deploy-gate.yaml" + +STEP_SCRIPT="$(mktemp)" +if ! python3 "${HERE}/extract_step.py" "${WORKFLOW}" watchdog check > "${STEP_SCRIPT}"; then + echo "could not extract the watchdog step from ${WORKFLOW}" >&2 + exit 1 +fi +if [[ ! -s "${STEP_SCRIPT}" ]]; then + echo "extracted an empty step body from ${WORKFLOW}" >&2 + exit 1 +fi + +failures=0 +current_case="" + +minutes_ago() { + python3 -c 'import datetime, sys; print((datetime.datetime.now(datetime.timezone.utc) - datetime.timedelta(minutes=int(sys.argv[1]))).strftime("%Y-%m-%dT%H:%M:%SZ"))' "$1" +} + +# head_at AGE_MINUTES SHA -> the commit payload the watchdog reads for the branch head. +head_at() { + printf '{"sha":"%s","commit":{"committer":{"date":"%s"}}}\n' "$2" "$(minutes_ago "$1")" +} + +runs_with() { + if [[ -z "${1}" ]]; then + printf '{"workflow_runs":[]}\n' + else + printf '{"workflow_runs":[{"head_sha":"%s","status":"completed","conclusion":"%s","html_url":"https://example.invalid/run/1"}]}\n' "$1" "${2:-success}" + fi +} + +begin_case() { + current_case="$1" + STUB="$(mktemp -d)" + export STUB + mkdir -p "${STUB}/bin" + cp "${HERE}/stub-gh" "${STUB}/bin/gh" + chmod +x "${STUB}/bin/gh" + : > "${STUB}/calls.log" + : > "${STUB}/bodies.log" + : > "${STUB}/reported.txt" + : > "${STUB}/issue_number.txt" + runs_with "" > "${STUB}/head_run.json" +} + +# run_watchdog [VAR=VALUE ...] -> exit status in $status, stdout+stderr in $output. +run_watchdog() { + local out + out="$( + PATH="${STUB}/bin:${PATH}" \ + GH_TOKEN=stub-token \ + REPO="acme/widget" \ + RUN_URL="https://example.invalid/watchdog" \ + GITHUB_STEP_SUMMARY="${STUB}/summary.md" \ + INPUT_WORKFLOW_FILE="${INPUT_WORKFLOW_FILE-build-images.yaml}" \ + INPUT_BRANCH="${INPUT_BRANCH-main}" \ + INPUT_GRACE_MINUTES="${INPUT_GRACE_MINUTES-90}" \ + INPUT_ISSUE_LABEL="${INPUT_ISSUE_LABEL-deploy-gate-stale}" \ + INPUT_ISSUE_ASSIGNEES="${INPUT_ISSUE_ASSIGNEES-}" \ + bash "${STEP_SCRIPT}" 2>&1 + )" + status=$? + output="${out}" +} + +fail() { + echo " FAIL ${current_case}: $1" + [[ -n "${2:-}" ]] && echo " ${2}" + failures=$((failures + 1)) +} + +pass() { echo " ok ${current_case}"; } + +expect_status() { + if [[ "${status}" -ne "$1" ]]; then + fail "expected exit ${1}, got ${status}" "${output}" + return 1 + fi + return 0 +} + +expect_calls() { + if ! grep -qF -- "$1" "${STUB}/calls.log"; then + fail "expected gh to be called with: $1" "$(cat "${STUB}/calls.log")" + return 1 + fi + return 0 +} + +expect_no_calls() { + if grep -qF -- "$1" "${STUB}/calls.log"; then + fail "expected gh NOT to be called with: $1" "$(cat "${STUB}/calls.log")" + return 1 + fi + return 0 +} + +expect_body() { + if ! grep -qF -- "$1" "${STUB}/bodies.log"; then + fail "expected the published body to contain: $1" "$(cat "${STUB}/bodies.log")" + return 1 + fi + return 0 +} + +expect_output() { + if ! grep -qF -- "$1" <<< "${output}"; then + fail "expected output to contain: $1" "${output}" + return 1 + fi + return 0 +} + +readonly HEAD_SHA=1111111111111111111111111111111111111111 +readonly OLD_SHA=2222222222222222222222222222222222222222 + +# --- shipped ---------------------------------------------------------------------- + +begin_case "head is shipped: succeeds and touches no issue" +head_at 5 "${HEAD_SHA}" > "${STUB}/head.json" +runs_with "${HEAD_SHA}" > "${STUB}/last_ok.json" +run_watchdog +expect_status 0 && expect_no_calls "[issue][create]" && expect_no_calls "[issue][comment]" && pass + +begin_case "head is shipped with an issue open: comments and closes it" +head_at 5 "${HEAD_SHA}" > "${STUB}/head.json" +runs_with "${HEAD_SHA}" > "${STUB}/last_ok.json" +echo "42" > "${STUB}/issue_number.txt" +run_watchdog +expect_status 0 && expect_calls "[issue][comment][42]" && expect_calls "[issue][close][42]" && + expect_body "Recovered." && pass + +# --- unshipped but young ---------------------------------------------------------- + +begin_case "head is younger than the grace window: succeeds quietly" +head_at 10 "${HEAD_SHA}" > "${STUB}/head.json" +runs_with "${OLD_SHA}" > "${STUB}/last_ok.json" +run_watchdog +expect_status 0 && expect_no_calls "[issue][create]" && expect_no_calls "[issue][comment]" && + expect_output "inside the 90m grace window" && pass + +begin_case "grace window of zero reports a brand new commit immediately" +head_at 0 "${HEAD_SHA}" > "${STUB}/head.json" +runs_with "${OLD_SHA}" > "${STUB}/last_ok.json" +INPUT_GRACE_MINUTES=0 run_watchdog +expect_status 1 && expect_calls "[issue][create]" && pass + +# --- stale ------------------------------------------------------------------------ + +begin_case "head is stale: fails and opens one issue" +head_at 500 "${HEAD_SHA}" > "${STUB}/head.json" +runs_with "${OLD_SHA}" > "${STUB}/last_ok.json" +runs_with "${HEAD_SHA}" failure > "${STUB}/head_run.json" +run_watchdog +expect_status 1 && expect_calls "[issue][create]" && expect_body "${HEAD_SHA}" && + expect_body "completed / failure" && pass + +begin_case "gate never ran for the head: still stale, and says so" +head_at 500 "${HEAD_SHA}" > "${STUB}/head.json" +runs_with "" > "${STUB}/last_ok.json" +runs_with "" > "${STUB}/head_run.json" +run_watchdog +expect_status 1 && expect_calls "[issue][create]" && expect_body "never ran / none" && + expect_body "none on record" && pass + +begin_case "stale with an issue that predates this commit: comments" +head_at 500 "${HEAD_SHA}" > "${STUB}/head.json" +runs_with "${OLD_SHA}" > "${STUB}/last_ok.json" +echo "7" > "${STUB}/issue_number.txt" +echo "an older report about ${OLD_SHA}" > "${STUB}/reported.txt" +run_watchdog +expect_status 1 && expect_calls "[issue][comment][7]" && expect_no_calls "[issue][create]" && pass + +begin_case "stale with this commit already reported: stays quiet but still fails" +head_at 500 "${HEAD_SHA}" > "${STUB}/head.json" +runs_with "${OLD_SHA}" > "${STUB}/last_ok.json" +echo "7" > "${STUB}/issue_number.txt" +echo "already reported ${HEAD_SHA} here" > "${STUB}/reported.txt" +run_watchdog +expect_status 1 && expect_no_calls "[issue][comment]" && expect_no_calls "[issue][create]" && + expect_output "already reports" && pass + +begin_case "assignees are passed through when set" +head_at 500 "${HEAD_SHA}" > "${STUB}/head.json" +runs_with "${OLD_SHA}" > "${STUB}/last_ok.json" +INPUT_ISSUE_ASSIGNEES="octocat,hubot" run_watchdog +expect_status 1 && expect_calls "[--assignee][octocat,hubot]" && pass + +# --- rejected input --------------------------------------------------------------- +# +# Each of these asserts the specific rejection, not just exit 2: a message naming the +# wrong input is the difference between a useful failure and a mystery. + +begin_case "workflow_file with a path traversal is rejected" +INPUT_WORKFLOW_FILE="../../../etc/passwd.yaml" run_watchdog +expect_status 2 && expect_output "workflow_file must be a workflow file name" && + expect_no_calls "[api]" && pass + +begin_case "workflow_file with a query separator is rejected" +INPUT_WORKFLOW_FILE="build.yaml?status=success" run_watchdog +expect_status 2 && expect_output "workflow_file must be a workflow file name" && pass + +begin_case "workflow_file without a yaml extension is rejected" +INPUT_WORKFLOW_FILE="build-images" run_watchdog +expect_status 2 && expect_output "workflow_file must be a workflow file name" && pass + +begin_case "branch with a shell metacharacter is rejected" +INPUT_BRANCH='main;id' run_watchdog +expect_status 2 && expect_output "branch must be a plain branch name" && pass + +begin_case "branch with a query separator is rejected" +INPUT_BRANCH='main&per_page=100' run_watchdog +expect_status 2 && expect_output "branch must be a plain branch name" && pass + +begin_case "non-numeric grace_minutes is rejected" +INPUT_GRACE_MINUTES="soon" run_watchdog +expect_status 2 && expect_output "grace_minutes must be a non-negative integer" && pass + +begin_case "negative grace_minutes is rejected" +INPUT_GRACE_MINUTES="-5" run_watchdog +expect_status 2 && expect_output "grace_minutes must be a non-negative integer" && pass + +begin_case "issue_label with an illegal character is rejected" +INPUT_ISSUE_LABEL='bad!label' run_watchdog +expect_status 2 && expect_output "issue_label must be 1-49 characters" && pass + +begin_case "empty issue_label is rejected" +INPUT_ISSUE_LABEL='' run_watchdog +expect_status 2 && expect_output "issue_label must be 1-49 characters" && pass + +begin_case "issue_assignees with a shell metacharacter is rejected" +INPUT_ISSUE_ASSIGNEES='octocat;id' run_watchdog +expect_status 2 && expect_output "issue_assignees must be comma-separated" && pass + +# ---------------------------------------------------------------------------------- + +echo +if (( failures > 0 )); then + echo "${failures} failing case(s)" + exit 1 +fi +echo "all cases passed" diff --git a/tests/deploy-gate-watchdog/stub-gh b/tests/deploy-gate-watchdog/stub-gh new file mode 100755 index 0000000..c5c666c --- /dev/null +++ b/tests/deploy-gate-watchdog/stub-gh @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +# +# Stands in for the gh CLI. Every invocation is appended to $STUB/calls.log and every +# issue body the watchdog would have published is appended to $STUB/bodies.log, so a +# test can assert on what was *written*, not only on the exit code. +# +# Anything the watchdog asks for that this does not recognise is a hard failure with a +# distinct exit code rather than an empty response. A stub that silently answers "" for +# a call it does not know turns a broken watchdog into a passing test. +set -euo pipefail + +# Each argument is bracketed rather than joined with spaces. `$*` would render +# `--assignee octocat,hubot` identically whether gh received it as two arguments or as +# one fused argument, so an assertion over that log could not tell a working array +# expansion from a broken one. +for stub_arg in "$@"; do printf '[%s]' "${stub_arg}"; done >> "${STUB}/calls.log" +printf '\n' >> "${STUB}/calls.log" + +record_body() { + local previous="" arg + for arg in "$@"; do + if [[ "${previous}" == "--body-file" ]]; then + cat "${arg}" >> "${STUB}/bodies.log" + return 0 + fi + previous="${arg}" + done +} + +command_name="${1:-}" +subcommand="${2:-}" + +case "${command_name}" in + api) + case "${subcommand}" in + */commits/*) cat "${STUB}/head.json" ;; + *status=success*) cat "${STUB}/last_ok.json" ;; + *head_sha=*) cat "${STUB}/head_run.json" ;; + *) echo "stub gh: unexpected api path: ${subcommand}" >&2; exit 90 ;; + esac + ;; + label) + [[ "${subcommand}" == "create" ]] || { echo "stub gh: unexpected label subcommand: ${subcommand}" >&2; exit 91; } + ;; + issue) + case "${subcommand}" in + list) cat "${STUB}/issue_number.txt" ;; + view) cat "${STUB}/reported.txt" ;; + create|comment) record_body "$@" ;; + close) ;; + *) echo "stub gh: unexpected issue subcommand: ${subcommand}" >&2; exit 92 ;; + esac + ;; + *) + echo "stub gh: unexpected command: ${command_name}" >&2 + exit 93 + ;; +esac diff --git a/tests/lint_workflows.sh b/tests/lint_workflows.sh new file mode 100755 index 0000000..0c0e65c --- /dev/null +++ b/tests/lint_workflows.sh @@ -0,0 +1,84 @@ +#!/usr/bin/env bash +# +# actionlint over the workflows in this repository. +# +# actionlint's value here is not style. It resolves ${{ }} expressions against the real +# context schema, so a typo in an input name is an error rather than an empty string at +# 03:00, and it runs shellcheck over every run: block, which is where a reusable workflow +# actually executes other people's data. +# +# The linter arrived after the workflows did, and five of them already had findings. +# Gating on a clean repository would have meant either leaving the linter off or editing +# five unrelated workflows in a change about something else. Instead they are exempted by +# name in tests/actionlint-legacy.txt, and that list can only shrink: a file listed there +# that now lints clean fails this script, so the exemption is removed by the commit that +# earns it rather than outliving the problem. +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +LEGACY_LIST="${ROOT}/tests/actionlint-legacy.txt" + +# Built once rather than invoked through `go run` per file: six `go run` calls is six +# link steps, and `go -C` would also leave every reported path relative to the tool's own +# module directory, which makes the findings harder to click than they need to be. +ACTIONLINT="$(mktemp -d)/actionlint" +go -C "${ROOT}/tools/actionlint" build -o "${ACTIONLINT}" github.com/rhysd/actionlint/cmd/actionlint + +actionlint() { + ( cd "${ROOT}" && "${ACTIONLINT}" -no-color -oneline "$@" ) +} + +is_legacy() { + local candidate="$1" entry + while IFS= read -r entry; do + [[ "${entry}" == "${candidate}" ]] && return 0 + done < <(legacy_entries) + return 1 +} + +legacy_entries() { + sed -e 's/#.*//' -e 's/[[:space:]]*$//' "${LEGACY_LIST}" | grep -v '^$' || true +} + +status=0 + +# Every entry must still name a real file, or a rename silently widens the exemption. +while IFS= read -r entry; do + if [[ ! -f "${ROOT}/${entry}" ]]; then + echo "::error::${LEGACY_LIST} lists ${entry}, which does not exist. Remove it." + status=1 + fi +done < <(legacy_entries) + +gated=() +while IFS= read -r workflow; do + relative="${workflow#"${ROOT}/"}" + if is_legacy "${relative}"; then + continue + fi + gated+=("${relative}") +done < <(find "${ROOT}/.github/workflows" -maxdepth 1 -type f \( -name '*.yaml' -o -name '*.yml' \) | sort) + +if (( ${#gated[@]} == 0 )); then + echo "::error::no workflows left to lint, which means the exemption list swallowed all of them" + exit 1 +fi + +echo "linting ${#gated[@]} workflow(s)" +if ! actionlint "${gated[@]}"; then + status=1 +fi + +# The ratchet. A legacy file that now passes must leave the list. +while IFS= read -r entry; do + [[ -f "${ROOT}/${entry}" ]] || continue + if actionlint "${entry}" >/dev/null 2>&1; then + echo "::error::${entry} now lints clean. Remove it from ${LEGACY_LIST}." + status=1 + fi +done < <(legacy_entries) + +if (( status == 0 )); then + echo "workflows lint clean" +fi +exit "${status}" diff --git a/tests/requirements.txt b/tests/requirements.txt new file mode 100644 index 0000000..bcf230e --- /dev/null +++ b/tests/requirements.txt @@ -0,0 +1,4 @@ +# Used by tests/deploy-gate-watchdog/extract_step.py to read the watchdog's shell body +# back out of the workflow YAML. Pinned rather than relying on whatever the runner image +# happens to ship, and pinned in a file so Dependabot can bump it (see .github/dependabot.yml). +PyYAML==6.0.3 diff --git a/tools/actionlint/go.mod b/tools/actionlint/go.mod new file mode 100644 index 0000000..87f691f --- /dev/null +++ b/tools/actionlint/go.mod @@ -0,0 +1,20 @@ +module github.com/NethermindEth/github-workflows/tools/actionlint + +go 1.27.1 + +tool github.com/rhysd/actionlint/cmd/actionlint + +require ( + github.com/bmatcuk/doublestar/v4 v4.10.0 // indirect + github.com/clipperhouse/uax29/v2 v2.7.0 // indirect + github.com/fatih/color v1.19.0 // indirect + github.com/mattn/go-colorable v0.1.14 // indirect + github.com/mattn/go-isatty v0.0.20 // indirect + github.com/mattn/go-runewidth v0.0.21 // indirect + github.com/mattn/go-shellwords v1.0.12 // indirect + github.com/rhysd/actionlint v1.7.12 // indirect + github.com/robfig/cron/v3 v3.0.1 // indirect + go.yaml.in/yaml/v4 v4.0.0-rc.3 // indirect + golang.org/x/sync v0.20.0 // indirect + golang.org/x/sys v0.42.0 // indirect +) diff --git a/tools/actionlint/go.sum b/tools/actionlint/go.sum new file mode 100644 index 0000000..ddd65b3 --- /dev/null +++ b/tools/actionlint/go.sum @@ -0,0 +1,27 @@ +github.com/bmatcuk/doublestar/v4 v4.10.0 h1:zU9WiOla1YA122oLM6i4EXvGW62DvKZVxIe6TYWexEs= +github.com/bmatcuk/doublestar/v4 v4.10.0/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc= +github.com/clipperhouse/uax29/v2 v2.7.0 h1:+gs4oBZ2gPfVrKPthwbMzWZDaAFPGYK72F0NJv2v7Vk= +github.com/clipperhouse/uax29/v2 v2.7.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM= +github.com/fatih/color v1.19.0 h1:Zp3PiM21/9Ld6FzSKyL5c/BULoe/ONr9KlbYVOfG8+w= +github.com/fatih/color v1.19.0/go.mod h1:zNk67I0ZUT1bEGsSGyCZYZNrHuTkJJB+r6Q9VuMi0LE= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= +github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= +github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= +github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/mattn/go-runewidth v0.0.21 h1:jJKAZiQH+2mIinzCJIaIG9Be1+0NR+5sz/lYEEjdM8w= +github.com/mattn/go-runewidth v0.0.21/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= +github.com/mattn/go-shellwords v1.0.12 h1:M2zGm7EW6UQJvDeQxo4T51eKPurbeFbe8WtebGE2xrk= +github.com/mattn/go-shellwords v1.0.12/go.mod h1:EZzvwXDESEeg03EKmM+RmDnNOPKG4lLtQsUlTZDWQ8Y= +github.com/rhysd/actionlint v1.7.12 h1:vQ4GeJN86C0QH+gTUQcs8McmK62OLT3kmakPMtEWYnY= +github.com/rhysd/actionlint v1.7.12/go.mod h1:krOUhujIsJusovkaYzQ/VNH8PFexjNKqU0q5XI/4w+g= +github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= +github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro= +go.yaml.in/yaml/v4 v4.0.0-rc.3 h1:3h1fjsh1CTAPjW7q/EMe+C8shx5d8ctzZTrLcs/j8Go= +go.yaml.in/yaml/v4 v4.0.0-rc.3/go.mod h1:aZqd9kCMsGL7AuUv/m/PvWLdg5sjJsZ4oHDEnfPPfY0= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= +golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=