Skip to content

Commit 0eeb1ef

Browse files
committed
ci: let only the trusted release checks pass a release pull request's Verify
Release Please pull requests are version-only, so the Verify workflow skipped their suites and its Verify check run succeeded within seconds. That check run satisfied the required Verify before the release job pushed the lockfile refresh and before its trusted checks posted Verify on the refreshed head. On same-repository release-please-- branches the Verify workflow's aggregate job now reports as "Release pull request suites" (it still always runs; a skipped job would count as passing). The only Verify on such a head is the commit status from the release checks, so the pull request waits until they pass on its current head, and every later push waits again. The release checks also run the offline version check before dispatching anything. A check-release-gate static check, part of check-ci, models how the ruleset resolves Verify for release, feature, fork and merge group pull requests against the real workflow files.
1 parent 7698e13 commit 0eeb1ef

22 files changed

Lines changed: 1067 additions & 11 deletions

‎.github/workflows/release-please.yml‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,17 @@ jobs:
144144
exit 1
145145
}
146146
147-
# Verify is the required status, so it only succeeds after every trusted check passes,
148-
# including building the distributions that Publish reuses once the release is tagged.
149-
# Verify's test suites skip release pull requests, whose product code already passed
150-
# them on main; release performance runs on main after each merge and does not gate.
151-
# Each dispatched run carries this attempt's request id in its run name and is matched
152-
# on it exactly.
147+
# Verify is the required status. On Release Please branches this commit status is its
148+
# only source, because the Verify workflow reports its job under another name there, so
149+
# the pull request merges only after every trusted check passes on this head, which
150+
# already includes any lockfile refresh. The checks include building the distributions
151+
# that Publish reuses once the release is tagged. Verify's test suites skip release pull
152+
# requests, whose product code already passed them on main; release performance runs on
153+
# main after each merge and does not gate. Each dispatched run carries this attempt's
154+
# request id in its run name and is matched on it exactly.
155+
if ! python3 -m scripts.check_native_layering; then
156+
fail_verify "Release versions or Cargo.lock workspace versions disagree"
157+
fi
153158
workflows=(pull-request-metadata.yml version-guard.yml release-distributions.yml)
154159
contexts=("Pull Request Metadata" "Block automatic 1.0 releases" "Release distributions")
155160
for context in "${contexts[@]}"; do

‎.github/workflows/verify.yml‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,9 @@ jobs:
104104
105105
# Release Please pull requests change only release metadata on top of product code that
106106
# already passed every suite before it reached main, so the suites are skipped. Their
107-
# required Verify status is posted by the trusted release checks in release-please.yml
108-
# after the metadata, version and distribution checks pass.
107+
# required Verify status is posted only by the trusted release checks in
108+
# release-please.yml after the metadata, version and distribution checks pass; the
109+
# verify job below reports under another name on Release Please branches.
109110
if [[ "${EVENT_NAME}" == "pull_request" \
110111
&& "${HEAD_REPOSITORY}" == "${GITHUB_REPOSITORY}" \
111112
&& "${HEAD_REF}" == release-please--branches--* ]] && release_metadata_only; then
@@ -861,7 +862,17 @@ jobs:
861862
sdist: false
862863

863864
verify:
864-
name: Verify
865+
# The required Verify of a Release Please pull request is the commit status that the trusted
866+
# release checks in release-please.yml post on the branch's current head, after the lockfile
867+
# refresh and every dispatched check. This job reports under another name there, so its quick
868+
# metadata-only pass can never satisfy Verify first; a skipped job would count as passing, so
869+
# the job always runs and only its name changes. Until those checks post Verify on the head,
870+
# the pull request waits for an expected check, and every new push to the branch waits again.
871+
name: >-
872+
${{ (github.event_name == 'pull_request'
873+
&& github.event.pull_request.head.repo.full_name == github.repository
874+
&& startsWith(github.event.pull_request.head.ref, 'release-please--'))
875+
&& 'Release pull request suites' || 'Verify' }}
865876
if: always()
866877
needs:
867878
[

‎Makefile‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
SHELL := /bin/bash
22

3-
.PHONY: verify verify-quick verify-pg coverage cli-preview rust-check check-native-layering check-python-boundary \
3+
.PHONY: verify verify-quick verify-pg coverage cli-preview rust-check check-native-layering check-python-boundary check-release-gate \
44
check-e2e-shards test-e2e-duckdb test-e2e-duckdb-build-core \
55
test-e2e-duckdb-build-incremental \
66
test-e2e-duckdb-cli-data test-e2e-duckdb-cli test-e2e-duckdb-inspection \
@@ -623,7 +623,7 @@ verify-quick:
623623
exit $$status
624624

625625

626-
check-ci: check-e2e-shards check-native-layering check-python-boundary rust-lint
626+
check-ci: check-e2e-shards check-native-layering check-python-boundary check-release-gate rust-lint
627627
uv run ruff format --check .
628628
uv run ruff check .
629629
uv run ty check src tests
@@ -639,6 +639,10 @@ check-python-boundary:
639639
uv run python -m scripts.check_python_boundary
640640

641641

642+
check-release-gate:
643+
uv run python -m scripts.check_release_gate
644+
645+
642646
check-pr-metadata:
643647
@test -n "$(PR_TITLE)" || { echo "PR_TITLE is required" >&2; exit 2; }
644648
@test -n "$(PR_BODY_FILE)" || { echo "PR_BODY_FILE is required" >&2; exit 2; }

‎scripts/check_release_gate.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
"""Direct wrapper for the release pull request Verify gate check."""
2+
3+
from __future__ import annotations
4+
5+
from scripts.release_gate.main.check import run_release_gate_check
6+
7+
8+
def main(argv: list[str] | None = None) -> int:
9+
"""Check the release pull request Verify gate."""
10+
return run_release_gate_check(argv)
11+
12+
13+
if __name__ == "__main__":
14+
raise SystemExit(main())

‎scripts/release_gate/__init__.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
"""Release pull request Verify gate checks."""
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
"""Release gate helpers."""
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
"""Evaluate the subset of GitHub Actions expressions that workflow job names use."""
2+
3+
from __future__ import annotations
4+
5+
import re
6+
from typing import Any
7+
8+
from scripts.release_gate.constants import (
9+
EXPRESSION_LITERALS,
10+
FALSY_VALUES,
11+
STARTS_WITH_ARGUMENTS,
12+
STARTS_WITH_FUNCTION,
13+
)
14+
from scripts.release_gate.exceptions import ReleaseGateError
15+
from scripts.release_gate.types import Operator, TokenKind
16+
17+
_TEMPLATE: re.Pattern[str] = re.compile(r"\$\{\{(.*?)\}\}", re.DOTALL)
18+
_TOKEN: re.Pattern[str] = re.compile(
19+
r"\s*(?:(?P<string>'(?:[^']|'')*')|(?P<operator>&&|\|\||==|!=|!|\(|\)|,)"
20+
r"|(?P<name>[A-Za-z_][A-Za-z0-9_-]*(?:\.[A-Za-z_][A-Za-z0-9_-]*)*))"
21+
)
22+
23+
24+
def evaluate_template(*, template: str, contexts: dict[str, Any]) -> str:
25+
"""Substitute every `${{ }}` expression in `template` with its value under `contexts`."""
26+
return _TEMPLATE.sub(
27+
lambda match: _to_string(evaluate(expression=match.group(1), contexts=contexts)),
28+
template,
29+
)
30+
31+
32+
def evaluate(*, expression: str, contexts: dict[str, Any]) -> Any:
33+
"""Return the value of one expression under `contexts`."""
34+
parser: _Parser = _Parser(tokens=_tokenize(expression), contexts=contexts)
35+
value: Any = parser.parse_or()
36+
if parser.peek() is not None:
37+
raise ReleaseGateError(f"Unexpected token {parser.peek()!r} in {expression!r}")
38+
return value
39+
40+
41+
def _tokenize(expression: str) -> list[tuple[TokenKind, str]]:
42+
tokens: list[tuple[TokenKind, str]] = []
43+
position: int = 0
44+
while expression[position:].strip():
45+
match: re.Match[str] | None = _TOKEN.match(expression, position)
46+
if match is None or match.lastgroup is None:
47+
raise ReleaseGateError(f"Unsupported expression syntax at {expression[position:]!r}")
48+
tokens.append((TokenKind(match.lastgroup), match.group(match.lastgroup)))
49+
position = match.end()
50+
return tokens
51+
52+
53+
class _Parser:
54+
def __init__(self, *, tokens: list[tuple[TokenKind, str]], contexts: dict[str, Any]) -> None:
55+
self._tokens: list[tuple[TokenKind, str]] = tokens
56+
self._contexts: dict[str, Any] = contexts
57+
self._index: int = 0
58+
59+
def peek(self) -> str | None:
60+
return self._tokens[self._index][1] if self._index < len(self._tokens) else None
61+
62+
def _take(self) -> tuple[TokenKind, str]:
63+
if self._index >= len(self._tokens):
64+
raise ReleaseGateError("Unexpected end of expression")
65+
token: tuple[TokenKind, str] = self._tokens[self._index]
66+
self._index += 1
67+
return token
68+
69+
def _expect(self, operator: Operator) -> None:
70+
if self._take() != (TokenKind.OPERATOR, operator):
71+
raise ReleaseGateError(f"Expected {operator.value!r}")
72+
73+
def parse_or(self) -> Any:
74+
value: Any = self._parse_and()
75+
while self.peek() == Operator.OR:
76+
self._take()
77+
right: Any = self._parse_and()
78+
value = value if _truthy(value) else right
79+
return value
80+
81+
def _parse_and(self) -> Any:
82+
value: Any = self._parse_equality()
83+
while self.peek() == Operator.AND:
84+
self._take()
85+
right: Any = self._parse_equality()
86+
value = right if _truthy(value) else value
87+
return value
88+
89+
def _parse_equality(self) -> Any:
90+
value: Any = self._parse_unary()
91+
if self.peek() in (Operator.EQUAL, Operator.NOT_EQUAL):
92+
operator: str = self._take()[1]
93+
equal: bool = _equal(left=value, right=self._parse_unary())
94+
return equal if operator == Operator.EQUAL else not equal
95+
return value
96+
97+
def _parse_unary(self) -> Any:
98+
if self.peek() == Operator.NOT:
99+
self._take()
100+
return not _truthy(self._parse_unary())
101+
return self._parse_primary()
102+
103+
def _parse_primary(self) -> Any:
104+
kind, text = self._take()
105+
if (kind, text) == (TokenKind.OPERATOR, Operator.OPEN):
106+
value: Any = self.parse_or()
107+
self._expect(Operator.CLOSE)
108+
return value
109+
if kind is TokenKind.STRING:
110+
return text[1:-1].replace("''", "'")
111+
if kind is not TokenKind.NAME:
112+
raise ReleaseGateError(f"Unexpected token {text!r}")
113+
if text in EXPRESSION_LITERALS:
114+
return EXPRESSION_LITERALS[text]
115+
if self.peek() == Operator.OPEN:
116+
return self._call(text)
117+
return self._lookup(text)
118+
119+
def _call(self, function: str) -> Any:
120+
self._expect(Operator.OPEN)
121+
arguments: list[Any] = [self.parse_or()]
122+
while self.peek() == Operator.SEPARATOR:
123+
self._take()
124+
arguments.append(self.parse_or())
125+
self._expect(Operator.CLOSE)
126+
if function.lower() == STARTS_WITH_FUNCTION and len(arguments) == STARTS_WITH_ARGUMENTS:
127+
return _to_string(arguments[0]).lower().startswith(_to_string(arguments[1]).lower())
128+
raise ReleaseGateError(f"Unsupported function {function}")
129+
130+
def _lookup(self, path: str) -> Any:
131+
value: Any = self._contexts
132+
for key in path.split("."):
133+
value = value.get(key) if isinstance(value, dict) else None
134+
return value
135+
136+
137+
def _truthy(value: Any) -> bool:
138+
return value not in FALSY_VALUES
139+
140+
141+
def _equal(*, left: Any, right: Any) -> bool:
142+
if isinstance(left, str) and isinstance(right, str):
143+
return left.lower() == right.lower()
144+
return left == right
145+
146+
147+
def _to_string(value: Any) -> str:
148+
if value is None:
149+
return ""
150+
if isinstance(value, bool):
151+
return str(value).lower()
152+
return str(value)
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
"""Check that only the trusted release checks can satisfy a release pull request's Verify."""
2+
3+
from __future__ import annotations
4+
5+
from pathlib import Path
6+
from typing import Any
7+
8+
import yaml
9+
10+
from scripts.release_gate._helpers.expressions import evaluate_template
11+
from scripts.release_gate._helpers.resolution import required_check_passes
12+
from scripts.release_gate.constants import (
13+
ALWAYS_CONDITION,
14+
GATE_SCENARIOS,
15+
PULL_REQUEST_EVENT,
16+
RELEASE_BRANCH_OUTPUT,
17+
RELEASE_CHECKS_JOB,
18+
RELEASE_PLEASE_JOB,
19+
RELEASE_VERIFY_SUCCESS,
20+
RELEASE_WORKFLOW,
21+
REQUIRED_CHECK,
22+
SCENARIO_HEADS,
23+
VERIFY_JOB,
24+
VERIFY_WORKFLOW,
25+
)
26+
from scripts.release_gate.exceptions import ReleaseGateError
27+
from scripts.release_gate.models import CheckRun, GateScenario, WorkflowEvent
28+
29+
30+
def get_release_gate_errors(root: Path) -> tuple[str, ...]:
31+
"""Return every way the workflows in `root` let something other than release checks pass."""
32+
verify: dict[str, Any] = _job(root=root, workflow=VERIFY_WORKFLOW, job=VERIFY_JOB)
33+
errors: list[str] = [
34+
_scenario_error(scenario)
35+
for scenario in GATE_SCENARIOS
36+
if _mergeable(job=verify, scenario=scenario) is not scenario.expected_mergeable
37+
]
38+
if verify.get("if") != ALWAYS_CONDITION:
39+
errors.append(
40+
f"{VERIFY_WORKFLOW} job {VERIFY_JOB} must run with `if: {ALWAYS_CONDITION}`: "
41+
"a skipped job satisfies a required check."
42+
)
43+
errors.extend(_release_checks_errors(root))
44+
return tuple(errors)
45+
46+
47+
def verify_check_name(*, job: dict[str, Any], event: WorkflowEvent) -> str:
48+
"""Return the check run name the Verify workflow's aggregate job reports for `event`."""
49+
pull_request: dict[str, Any] = {
50+
"head": {"ref": event.head_ref, "repo": {"full_name": event.head_repository}}
51+
}
52+
contexts: dict[str, Any] = {
53+
"github": {
54+
"event_name": event.event_name,
55+
"repository": event.repository,
56+
"event": {"pull_request": pull_request}
57+
if event.event_name == PULL_REQUEST_EVENT
58+
else {},
59+
}
60+
}
61+
return evaluate_template(template=str(job.get("name", VERIFY_JOB)), contexts=contexts).strip()
62+
63+
64+
def _mergeable(*, job: dict[str, Any], scenario: GateScenario) -> bool:
65+
name: str = verify_check_name(job=job, event=scenario.event)
66+
check_runs: tuple[CheckRun, ...] = tuple(
67+
CheckRun(sha=sha, name=name, conclusion=scenario.verify_conclusion)
68+
for sha in SCENARIO_HEADS
69+
)
70+
return required_check_passes(
71+
name=REQUIRED_CHECK,
72+
head_sha=scenario.head_sha,
73+
check_runs=check_runs,
74+
statuses=scenario.statuses,
75+
)
76+
77+
78+
def _scenario_error(scenario: GateScenario) -> str:
79+
outcome: str = "stays blocked" if scenario.expected_mergeable else "can merge"
80+
return f"A {scenario.description} {outcome} under {VERIFY_WORKFLOW}."
81+
82+
83+
def _release_checks_errors(root: Path) -> list[str]:
84+
job: dict[str, Any] = _job(root=root, workflow=RELEASE_WORKFLOW, job=RELEASE_CHECKS_JOB)
85+
needs: Any = job.get("needs")
86+
errors: list[str] = []
87+
if RELEASE_PLEASE_JOB not in (needs if isinstance(needs, list) else [needs]):
88+
errors.append(
89+
f"{RELEASE_WORKFLOW} job {RELEASE_CHECKS_JOB} must need {RELEASE_PLEASE_JOB}, "
90+
"so it checks the head after the lockfile refresh."
91+
)
92+
steps: list[dict[str, Any]] = job.get("steps", [])
93+
if not any(step.get("with", {}).get("ref") == RELEASE_BRANCH_OUTPUT for step in steps):
94+
errors.append(
95+
f"{RELEASE_WORKFLOW} job {RELEASE_CHECKS_JOB} must check out the release branch, "
96+
"so it posts Verify on the branch's current head."
97+
)
98+
if not any(RELEASE_VERIFY_SUCCESS in str(step.get("run", "")) for step in steps):
99+
errors.append(
100+
f"{RELEASE_WORKFLOW} job {RELEASE_CHECKS_JOB} never posts a successful "
101+
f"{REQUIRED_CHECK} status, so release pull requests could never merge."
102+
)
103+
return errors
104+
105+
106+
def _job(*, root: Path, workflow: str, job: str) -> dict[str, Any]:
107+
document: Any = yaml.safe_load((root / workflow).read_text(encoding="utf-8"))
108+
jobs: Any = document.get("jobs", {}) if isinstance(document, dict) else {}
109+
found: Any = jobs.get(job) if isinstance(jobs, dict) else None
110+
if not isinstance(found, dict):
111+
raise ReleaseGateError(f"{workflow} has no job {job}")
112+
return found
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
"""Model how a ruleset resolves one required check name on a pull request head."""
2+
3+
from __future__ import annotations
4+
5+
from scripts.release_gate.constants import PASSING_STATES
6+
from scripts.release_gate.models import CheckRun, CommitStatus
7+
from scripts.release_gate.types import CheckState
8+
9+
10+
def required_check_passes(
11+
*,
12+
name: str,
13+
head_sha: str,
14+
check_runs: tuple[CheckRun, ...],
15+
statuses: tuple[CommitStatus, ...],
16+
) -> bool:
17+
"""Return whether the required check `name` lets the pull request at `head_sha` merge."""
18+
reported: list[CheckState] = [
19+
run.conclusion for run in check_runs if run.sha == head_sha and run.name == name
20+
]
21+
reported.extend(
22+
[status.state for status in statuses if status.sha == head_sha and status.context == name][
23+
-1:
24+
]
25+
)
26+
return bool(reported) and all(state in PASSING_STATES for state in reported)

0 commit comments

Comments
 (0)