diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index cbf70af1d..ecf0d91a1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -25,7 +25,9 @@ permissions: contents: read concurrency: - group: public-release-main + # Publication and recovery share one queue and are never cancelled. Each push run + # gets its own group, so a job waiting for approval cannot hold or overflow it. + group: ${{ github.event_name == 'push' && format('public-release-push-{0}', github.run_id) || 'public-release-main' }} queue: max cancel-in-progress: false @@ -40,11 +42,26 @@ jobs: runs-on: ubuntu-24.04 timeout-minutes: 30 environment: release-automation + # The newest push replaces an older authoring job, including one waiting for + # approval. Other events skip this job and must never cancel it: the release + # event that starts publication arrives while this job is still running. + concurrency: + group: public-release-authoring + cancel-in-progress: ${{ github.event_name == 'push' }} permissions: contents: read outputs: release_pr_head: ${{ steps.release-pr-head.outputs.sha }} steps: + - name: Refuse a run whose push is no longer the newest on main + run: | + set -euo pipefail + main_sha="$(git ls-remote "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY.git" refs/heads/main | cut -f1)" + [[ "$main_sha" =~ ^[0-9a-f]{40}$ ]] + if [ "$main_sha" != "$GITHUB_SHA" ]; then + echo "::error::$GITHUB_SHA is behind main ($main_sha); approve or rerun the newest push run." + exit 1 + fi - name: Require activated public release GitHub App credentials env: APP_ID: ${{ vars.PUBLIC_RELEASE_APP_ID }} diff --git a/RELEASE.md b/RELEASE.md index 9e22d8626..72281d1c1 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -109,13 +109,28 @@ be published with the later run's provenance. The App credential is required because its events trigger workflows; a release created with `GITHUB_TOKEN` does not provide that handoff. Publication does not depend on a tag-push event. -Release work is serialized with GitHub's `queue: max` concurrency mode and no -in-progress cancellation, so newer events do not replace pending releases. The -queue has GitHub's documented bound of 100 pending runs. The current actionlint -schema does not recognize `queue`; CI exempts only that diagnostic and separately -checks the required queue setting. See [workflow +Publication and manual recovery runs are serialized in the `public-release-main` +group with GitHub's `queue: max` concurrency mode and no in-progress cancellation, +so newer events do not replace pending releases. The queue has GitHub's documented +bound of 100 pending runs. The current actionlint schema does not recognize +`queue`; CI exempts only that diagnostic and separately checks the required queue +setting. See [workflow concurrency](https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency). +Push runs stay out of that queue, because each one needs `release-automation` +approval and would hold the queue while it waits. A push run has a group of its +own, and its `release-please` job joins the separate `public-release-authoring` +group, where a newer push cancels the older job. GitHub does not allow `queue: max` +with in-progress cancellation, so that group uses the default single queue. No +release is lost when a job is replaced: the next approved job reads the repository +state, finds any merged release PR still awaiting its release, and rebuilds the open +release PR. + +Authoring is therefore not serialized against publication. A release PR merged while +an earlier release is publishing can create the newer release before that run ends. +The newer release's run waits in the queue, and the earlier run leaves the floating +image aliases for it to move. + PR and candidate builds produce archives without publishing. They use the actual package versions in that source tree. Release builds additionally require the complete package set to match the release version. @@ -318,24 +333,37 @@ any automation repair through normal review and CI, then approve that commit's `release-automation` job. The manual entrypoint below only recovers failed publication; it does not refresh release PRs. -Before resuming a waiting authoring run, inspect its source revision and workflow: -release-please can read current `main`, but the run still executes the workflow -from its original push. An old run can therefore generate current release files -using outdated refresh steps. Cancel obsolete queued **push** runs before -cancelling the old waiting run, then retain the authoring run from the intended -current source. Do not cancel publication or recovery runs as part of this +Each push replaces the previous authoring job, so earlier push runs show as +cancelled and normally only the newest push is waiting for approval. Approve that +one. If another push lands while the approved job is running, that job is cancelled +too; approve the newer run. A job cancelled between creating a release and updating +the release PR's label can leave the next run failing once on the duplicate release. +That run corrects the label, and the run after it proceeds normally. + +release-please reads current `main`, but a run executes the workflow from its +original push, so an old run could generate current release files using outdated +refresh steps. GitHub does not guarantee that jobs enter the group in push order, +and its documentation does not say how in-progress cancellation treats a job +waiting on an environment. An older job can therefore occasionally replace a newer +one or stay `waiting` ahead of it. To keep that harmless, the job's first step fails +before it uses any credentials unless its commit is the newest on `main`. + +If an older job is waiting, cancel it rather than approving it. If the newest push +run was cancelled, rerun that exact push run while GitHub still permits the rerun; +do not replay older runs. If `main` moved without starting a run, the next push +starts a current one. Do not cancel publication or recovery runs as part of this cleanup. Keep the environment protections and approve the retained run normally. -If the intended push run was cancelled, rerun that exact push run after checking -its source and workflow, while GitHub still permits the rerun; do not replay the -obsolete backlog. Verify the generated PR's source ancestry after refresh, too: -the approved run's revision does not freeze release-please's reads of `main`. - -An authoring run left waiting for approval holds the release concurrency group, -so every later push queues behind it. Once 100 runs are pending, the queue is -full and GitHub does not admit further `Release` runs. Each one appears as a +Verify the generated PR's source ancestry after refresh, too: the approved run's +revision does not freeze release-please's reads of `main`. + +An unapproved push run does not delay publication. A publication or recovery run +waiting on an environment approval does hold `public-release-main`, including a +dispatched recovery run nobody has approved, so resolve those first if publication +appears stuck. If 100 runs were ever pending there, the queue would be full and +GitHub would not admit further publication or recovery runs. Each one appears as a failed run with zero jobs, and GitHub reports that it likely failed because of a -workflow file issue. Before debugging the workflow file, look for a `Release` -run in the `waiting` state and apply the cleanup above. +workflow file issue. Before debugging the workflow file, look for a `Release` run +in the `waiting` state. ### Artifact publication diff --git a/tools/ci/check_release_contract.py b/tools/ci/check_release_contract.py index d4f0ca645..e733a943e 100755 --- a/tools/ci/check_release_contract.py +++ b/tools/ci/check_release_contract.py @@ -527,9 +527,32 @@ def license_policy_errors() -> list[str]: ) +RELEASE_QUEUE_GROUP = ( + "${{ github.event_name == 'push' && format('public-release-push-{0}', github.run_id) || 'public-release-main' }}" +) +RELEASE_AUTHORING_CONCURRENCY = ( + " concurrency:\n" + " group: public-release-authoring\n" + " cancel-in-progress: ${{ github.event_name == 'push' }}\n" +) +RELEASE_AUTHORING_FRESHNESS_STEP = """\ + - name: Refuse a run whose push is no longer the newest on main + run: | + set -euo pipefail + main_sha="$(git ls-remote "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY.git" refs/heads/main | cut -f1)" + [[ "$main_sha" =~ ^[0-9a-f]{40}$ ]] + if [ "$main_sha" != "$GITHUB_SHA" ]; then + echo "::error::$GITHUB_SHA is behind main ($main_sha); approve or rerun the newest push run." + exit 1 + fi +""" + + def release_queue_errors(top: str, ci: str) -> list[str]: errors = [] concurrency = top.partition("concurrency:\n")[2].partition("\njobs:")[0] + if re.findall(r"^ group: (.+)$", concurrency, re.MULTILINE) != [RELEASE_QUEUE_GROUP]: + errors.append("each push run needs its own group so an approval wait cannot hold the publication queue") if re.findall(r"^ queue: (.+)$", concurrency, re.MULTILINE) != ["max"]: errors.append("release concurrency must preserve all pending runs with queue: max") if re.findall(r"^ cancel-in-progress: (.+)$", concurrency, re.MULTILINE) != ["false"]: @@ -539,6 +562,20 @@ def release_queue_errors(top: str, ci: str) -> list[str]: return errors +def release_authoring_concurrency_errors(blocks: dict[str, str]) -> list[str]: + errors = [] + authoring = blocks.get("release-please", "") + mappings = re.findall(r"^ concurrency:\n(?: .*(?:\n|\Z))*", authoring, re.MULTILINE) + if mappings != [RELEASE_AUTHORING_CONCURRENCY]: + errors.append("a newer push must replace an older release-please job through its own concurrency group") + if not authoring.partition(" steps:\n")[2].startswith(RELEASE_AUTHORING_FRESHNESS_STEP): + errors.append("release-please must refuse a push that is behind main before using any credentials") + others = [name for name, block in blocks.items() if name != "release-please" and "concurrency:" in block] + if others: + errors.append(f"job-level concurrency could cancel or displace release work: {others}") + return errors + + def artifact_retention_errors(name: str, text: str) -> list[str]: errors = [] for retention in re.findall(r"^\s*retention-days:\s*([^\n]*)$", text, re.MULTILINE): @@ -623,6 +660,7 @@ def release_workflow_errors() -> list[str]: ): errors.append("release candidate rehearsal must be read-only and unable to publish") errors.extend(release_queue_errors(top, (ROOT / ".github/workflows/ci.yml").read_text())) + errors.extend(release_authoring_concurrency_errors(blocks)) recover = blocks.get("recover", "") if "release_recovery" not in recover or "id-token:" in recover or "publish-" in recover: errors.append("manual recovery must only rerun the original jobs") diff --git a/tools/ci/tests/test_release_contract.py b/tools/ci/tests/test_release_contract.py index 72b21008c..db8d7a70c 100644 --- a/tools/ci/tests/test_release_contract.py +++ b/tools/ci/tests/test_release_contract.py @@ -461,6 +461,91 @@ def test_release_queue_cancellation_and_linter_exception_are_exact(): assert not re.fullmatch(contract.QUEUE_SCHEMA_DIAGNOSTIC, diagnostic.replace('"queue"', '"bogus"')) +@pytest.mark.parametrize( + "group", + [ + "public-release-main", + "${{ github.event_name == 'push' && 'public-release-push' || 'public-release-main' }}", + "${{ github.event_name == 'release' && format('run-{0}', github.run_id) || 'public-release-main' }}", + ], +) +def test_push_runs_cannot_hold_the_publication_queue(group): + top = (contract.ROOT / ".github/workflows/release.yml").read_text() + ci = (contract.ROOT / ".github/workflows/ci.yml").read_text() + assert contract.RELEASE_QUEUE_GROUP in top + assert contract.release_queue_errors(top.replace(contract.RELEASE_QUEUE_GROUP, group), ci) + + +@pytest.mark.parametrize( + "replacement", + [ + "", + contract.RELEASE_AUTHORING_CONCURRENCY.replace("public-release-authoring", "public-release-main"), + " concurrency:\n group: public-release-authoring\n cancel-in-progress: false\n", + " concurrency:\n group: public-release-authoring\n cancel-in-progress: true\n", + " concurrency:\n group: public-release-authoring\n queue: max\n", + contract.RELEASE_AUTHORING_CONCURRENCY + " queue: max\n", + ], +) +def test_newest_push_must_replace_the_authoring_job(replacement): + blocks = contract.workflow_job_blocks(".github/workflows/release.yml") + assert contract.release_authoring_concurrency_errors(blocks) == [] + assert contract.RELEASE_AUTHORING_CONCURRENCY in blocks["release-please"] + authoring = blocks["release-please"].replace(contract.RELEASE_AUTHORING_CONCURRENCY, replacement) + assert contract.release_authoring_concurrency_errors({**blocks, "release-please": authoring}) + + +@pytest.mark.parametrize("job", ["prepare", "python-publish", "docker", "complete", "recover"]) +def test_only_authoring_may_use_job_level_concurrency(job): + blocks = contract.workflow_job_blocks(".github/workflows/release.yml") + changed = blocks[job] + "\n concurrency:\n group: public-release-authoring\n" + assert contract.release_authoring_concurrency_errors({**blocks, job: changed}) + + +def test_authoring_must_check_freshness_before_any_other_step(): + blocks = contract.workflow_job_blocks(".github/workflows/release.yml") + authoring = blocks["release-please"] + step = contract.RELEASE_AUTHORING_FRESHNESS_STEP + assert step in authoring + removed = authoring.replace(step, "") + moved = removed + "\n" + step + assert contract.release_authoring_concurrency_errors({**blocks, "release-please": removed}) + assert contract.release_authoring_concurrency_errors({**blocks, "release-please": moved}) + + +def test_authoring_freshness_step_refuses_a_push_behind_main(tmp_path): + work = tmp_path / "work" + remote = tmp_path / "owner" / "repo.git" + git_env = {**os.environ, "GIT_CONFIG_GLOBAL": os.devnull, "GIT_CONFIG_NOSYSTEM": "1"} + + def git(*args, cwd=work): + command = ["git", *args] + result = subprocess.run(command, cwd=cwd, env=git_env, check=True, capture_output=True, text=True) # noqa: S603 + return result.stdout.strip() + + work.mkdir() + git("init", "-q", "-b", "main") + identity = ("-c", "user.name=test", "-c", "user.email=test@example.invalid") + git(*identity, "commit", "-q", "--allow-empty", "-m", "older") + older = git("rev-parse", "HEAD") + git(*identity, "commit", "-q", "--allow-empty", "-m", "newer") + newer = git("rev-parse", "HEAD") + git("clone", "-q", "--bare", str(work), str(remote)) + script = textwrap.dedent(contract.RELEASE_AUTHORING_FRESHNESS_STEP.partition("run: |\n")[2]) + + def run_step(sha): + env = {**git_env, "GITHUB_SERVER_URL": tmp_path.as_uri(), "GITHUB_REPOSITORY": "owner/repo", "GITHUB_SHA": sha} + command = ["bash", "-c", script] + return subprocess.run(command, cwd=tmp_path, env=env, check=False, capture_output=True, text=True) # noqa: S603 + + assert run_step(newer).returncode == 0 + stale = run_step(older) + assert stale.returncode == 1 + assert f"::error::{older} is behind main ({newer})" in stale.stdout + git("update-ref", "-d", "refs/heads/main", cwd=remote) + assert run_step(newer).returncode != 0 + + def test_authoring_no_longer_assumes_release_sha_is_push_sha(): jobs = contract.workflow_job_blocks(".github/workflows/release.yml") assert 'test "$RELEASE_SHA" = "$EXPECTED_SHA"' not in jobs["release-please"]