This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 437daf6eddc Approve fork CI runs that started before "use fork ci" was
added (#75654)
437daf6eddc is described below
commit 437daf6eddcb0e692cf874600c821fee71db1af8
Author: Jarek Potiuk <[email protected]>
AuthorDate: Mon Oct 26 16:14:27 2026 +0100
Approve fork CI runs that started before "use fork ci" was added (#75654)
When a committer adds "use fork ci" right after a PR opens, the PR's first
runs were created without it, and the workflow-run approver left them for a
maintainer. It had to: the fork CI gate fell back to the labels of the run's
event when it could not read the current ones, and those labels could still
send the run to full project CI on code nobody had reviewed. Anyone able to
exhaust the shared token quota could force that fallback.
The gate now fails when it cannot read the labels, so a run only ever acts
on
the PR's current labels and a re-run decides again. With that, runs started
before the label changed are as safe to approve as later ones.
Generated-by: Claude Opus 5
---
.github/workflows/ci-amd.yml | 1 -
dev/breeze/doc/ci/05_workflows.md | 6 +--
.../doc/ci/08_fork_ci_for_external_contributors.md | 15 +++++---
.../src/airflow_breeze/utils/fork_ci_gate.py | 20 +++++++---
.../airflow_breeze/utils/workflow_run_approval.py | 13 ++-----
dev/breeze/tests/test_fork_ci_gate.py | 45 ++++++++++------------
dev/breeze/tests/test_workflow_run_approval.py | 13 +++----
scripts/ci/prek/check_ci_workflows_in_sync.py | 1 -
8 files changed, 56 insertions(+), 58 deletions(-)
diff --git a/.github/workflows/ci-amd.yml b/.github/workflows/ci-amd.yml
index e99af802e95..49456595214 100644
--- a/.github/workflows/ci-amd.yml
+++ b/.github/workflows/ci-amd.yml
@@ -233,7 +233,6 @@ jobs:
PR_AUTHOR: ${{ github.event.pull_request.user.login }}
PR_AUTHOR_ASSOCIATION: ${{
github.event.pull_request.author_association }}
PR_CREATED_AT: ${{ github.event.pull_request.created_at }}
- PR_EVENT_LABELS: ${{ toJSON(github.event.pull_request.labels.*.name)
}}
# A base branch with neither gate file predates the gate (the pull
request adding it,
# or a release branch cut before it) and keeps running project CI.
With only one of
# them present, the missing file fails the job instead of opening the
gate.
diff --git a/dev/breeze/doc/ci/05_workflows.md
b/dev/breeze/doc/ci/05_workflows.md
index c06d4f8a267..782c8dd3c80 100644
--- a/dev/breeze/doc/ci/05_workflows.md
+++ b/dev/breeze/doc/ci/05_workflows.md
@@ -138,9 +138,9 @@ run started before a label that changes which tests run was
added or removed
(`full tests needed`, `all versions`, `default versions only`, `latest
versions only`, `canary`,
`upgrade to newer dependencies`, `area:e2e-tests`, `area:kubernetes-tests`) is
left for a
maintainer, since it still runs the selection it started with; the next push
is decided afresh.
-For a PR whose CI runs in the fork the selection does not matter, but the gate
falls back to the
-labels a run started with when it cannot read the current ones, so a run
started before
-`use fork ci` or `use project ci` was added or removed is left for a
maintainer instead. Other PRs get the
+For a PR whose CI runs in the fork neither the selection nor the labels a run
started with matter:
+the gate reads the current labels when the run starts and fails when it
cannot, so a run started
+before `use fork ci` or `use project ci` was added or removed is approved too.
Other PRs get the
`need workflow approval` label and wait for a maintainer. When processing a PR
fails (for example on
a GitHub API error), the workflow tries to add the label to it and the
workflow run fails, so the
failure stays visible even if the label could not be added. The label is
removed when a later commit
diff --git a/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md
b/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md
index 5f58617cf56..85eb012b520 100644
--- a/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md
+++ b/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md
@@ -310,9 +310,11 @@ directly with the runner's `python3` without installing
breeze. The reconciler i
full project CI as `base_without_gate`.
2. For pull requests, one read-only API call for the current labels
(`gh api repos/apache/airflow/pulls/<n> --jq '.labels[].name'`), so
override labels added
- after the event are seen on a re-run. If that call fails, the gate uses the
labels of the
- event payload instead. It never falls back to running project CI: anyone
able to exhaust
- the shared token quota would otherwise switch the gate off for every pull
request.
+ after the event are seen on a re-run. If that call fails, the gate job
fails and nothing
+ after it runs; re-running the job decides again. It falls back neither to
running project
+ CI, since anyone able to exhaust the shared token quota would otherwise
switch the gate off
+ for every pull request, nor to the labels of the event payload, since runs
started before
+ `use fork ci` was added are approved automatically (see "Approving gated
runs").
3. `python3 dev/breeze/src/airflow_breeze/utils/fork_ci_gate.py` with the
event payload
fields and the labels passed as environment variables. It writes
`project-ci=true|false`
and `reason=...` to `GITHUB_OUTPUT`. A base branch with neither file skips
the script and
@@ -367,9 +369,10 @@ applies instead:
`github-actions[bot]` with the marker count. A change to `.github/` can
remove the gate or add a
workflow that does run, so such a run must not start without a review.
-A run started before `use fork ci` or `use project ci` was last added or
removed is not approved
-automatically: the gate falls back to the labels of the run's event when it
cannot read the current
-ones, and those can still send the run to project CI. The next push is decided
afresh.
+A run started before `use fork ci` or `use project ci` was last added or
removed is approved too,
+so a committer can add `use fork ci` after the PR's first runs were created
and before they are
+approved. The gate decides from the labels the PR has when the run starts,
never from those of the
+run's event, and fails when it cannot read them.
Both rules use the same list as "Trusting a green fork run", so a gated pull
request whose runs are
approved automatically is also one whose green fork run is trusted, and one
that changes CI is both
diff --git a/dev/breeze/src/airflow_breeze/utils/fork_ci_gate.py
b/dev/breeze/src/airflow_breeze/utils/fork_ci_gate.py
index dc05a3b67a5..4816e81bfd3 100644
--- a/dev/breeze/src/airflow_breeze/utils/fork_ci_gate.py
+++ b/dev/breeze/src/airflow_breeze/utils/fork_ci_gate.py
@@ -214,6 +214,10 @@ class Decision:
reason: Reason
+class LabelsUnavailableError(RuntimeError):
+ """The current labels of a pull request could not be read."""
+
+
def _is_canonical_branch(ref: str) -> bool:
branch = ref.removeprefix("refs/heads/")
return any(pattern.match(branch) for pattern in
CANONICAL_PUSH_BRANCH_PATTERNS)
@@ -300,10 +304,12 @@ def evaluate_from_env(env: Mapping[str, str]) -> Decision:
if event_name == "pull_request":
fetched = fetch_pull_request_labels(repository, env.get("PR_NUMBER",
""))
if fetched is None:
- # Never fail open: anyone able to exhaust the shared token quota
would otherwise switch
- # the gate off for every pull request. The event's labels miss
only labels added since.
- print("Falling back to the labels of the triggering event",
file=sys.stderr)
- fetched = tuple(json.loads(env.get("PR_EVENT_LABELS") or "[]"))
+ # Neither project CI nor the event's labels: the workflow-run
approver approves runs
+ # started before `use fork ci` was added, which the event's labels
would send to project
+ # CI, and anyone able to exhaust the shared token quota could
force that path.
+ raise LabelsUnavailableError(
+ f"Could not read the labels of pull request
#{env.get('PR_NUMBER', '')}; re-run the job"
+ )
labels = fetched
event = GateEvent(
event_name=event_name,
@@ -321,7 +327,11 @@ def evaluate_from_env(env: Mapping[str, str]) -> Decision:
def main(env: Mapping[str, str] | None = None) -> int:
env = os.environ if env is None else env
- decision = evaluate_from_env(env)
+ try:
+ decision = evaluate_from_env(env)
+ except LabelsUnavailableError as e:
+ print(e, file=sys.stderr)
+ return 1
project_ci = "true" if decision.run else "false"
print(f"project-ci={project_ci} reason={decision.reason.value}")
_write_line("GITHUB_OUTPUT", f"project-ci={project_ci}", env)
diff --git a/dev/breeze/src/airflow_breeze/utils/workflow_run_approval.py
b/dev/breeze/src/airflow_breeze/utils/workflow_run_approval.py
index 2f7fc1fe34b..5b0289d2e19 100644
--- a/dev/breeze/src/airflow_breeze/utils/workflow_run_approval.py
+++ b/dev/breeze/src/airflow_breeze/utils/workflow_run_approval.py
@@ -49,8 +49,6 @@ from airflow_breeze.utils.ci_minutes_estimate import
CiMinutesEstimate, estimate
from airflow_breeze.utils.console import console_print
from airflow_breeze.utils.fork_ci_gate import (
GATED_REASONS,
- LABEL_USE_FORK_CI,
- LABEL_USE_PROJECT_CI,
GateConfig,
GateEvent,
decide,
@@ -132,11 +130,6 @@ TEST_SELECTION_LABELS = frozenset(
}
)
-# Labels that move a PR between project CI and fork CI. The gate reads them
when it runs, but falls
-# back to the labels of the run's event when it cannot, so a run started
before one changed could
-# still run project CI.
-GATE_OVERRIDE_LABELS = frozenset({LABEL_USE_FORK_CI, LABEL_USE_PROJECT_CI})
-
# Read from the trusted main checkout, which is also the base branch whose
gate job decides the
# PRs that are approved automatically.
FORK_CI_GATE_CONFIG_PATH = AIRFLOW_ROOT_PATH / ".github" / "fork-ci-gate.toml"
@@ -225,7 +218,9 @@ def decide_gated_approval(*, changed_files:
list[ChangedFile]) -> ApprovalDecisi
return ApprovalDecision(
approve=True,
reasons=["its CI runs in the contributor's fork, so its project CI run
only executes the gate"],
- decision_labels=GATE_OVERRIDE_LABELS,
+ # The gate reads `use fork ci` and `use project ci` when the run
starts and fails when it
+ # cannot, so runs started before one of them changed are approved too.
+ decision_labels=frozenset(),
)
@@ -655,7 +650,7 @@ def find_runs_started_for_current_pr(
:param decision_labels: the labels the decision depends on. It is made
from the current labels,
while a run can act on those of its event: with ``full tests needed``
removed, or
``default versions only`` added to a run of all versions, the estimate
is smaller than the
- run, and without ``use fork ci`` in its event a run can fall back to
project CI.
+ run.
"""
started_after = parse_github_time(pr["created_at"])
changes = [
diff --git a/dev/breeze/tests/test_fork_ci_gate.py
b/dev/breeze/tests/test_fork_ci_gate.py
index e641e45ca82..ca4194b99d2 100644
--- a/dev/breeze/tests/test_fork_ci_gate.py
+++ b/dev/breeze/tests/test_fork_ci_gate.py
@@ -391,28 +391,14 @@ def _outputs(env: dict[str, str]) -> list[str]:
@mock.patch("airflow_breeze.utils.fork_ci_gate.subprocess.run", autospec=True)
@pytest.mark.parametrize(
- ("returncode", "stdout", "event_labels", "expected"),
+ ("stdout", "expected"),
[
- pytest.param(0, "[]\n", "[]", ["project-ci=false", "reason=external"],
id="external"),
- pytest.param(
- 0, '["use project ci"]\n', "[]", ["project-ci=true",
"reason=label_project_ci"], id="labelled"
- ),
- pytest.param(1, "", "[]", ["project-ci=false", "reason=external"],
id="gh-fails-stays-gated"),
- pytest.param(
- 0, "not json", "[]", ["project-ci=false", "reason=external"],
id="bad-output-stays-gated"
- ),
- pytest.param(
- 1,
- "",
- '["use project ci"]',
- ["project-ci=true", "reason=label_project_ci"],
- id="gh-fails-uses-event-labels",
- ),
+ pytest.param("[]\n", ["project-ci=false", "reason=external"],
id="external"),
+ pytest.param('["use project ci"]\n', ["project-ci=true",
"reason=label_project_ci"], id="labelled"),
],
)
-def test_main_pull_request(mock_run, gate_env, returncode, stdout,
event_labels, expected):
- mock_run.return_value = subprocess.CompletedProcess([], returncode,
stdout=stdout, stderr="boom")
- gate_env["PR_EVENT_LABELS"] = event_labels
+def test_main_pull_request(mock_run, gate_env, stdout, expected):
+ mock_run.return_value = subprocess.CompletedProcess([], 0, stdout=stdout,
stderr="")
assert main(gate_env) == 0
@@ -420,15 +406,24 @@ def test_main_pull_request(mock_run, gate_env,
returncode, stdout, event_labels,
assert mock_run.call_args_list[0].args[0][:3] == ["gh", "api",
"repos/apache/airflow/pulls/1"]
[email protected](
- "airflow_breeze.utils.fork_ci_gate.subprocess.run", autospec=True,
side_effect=FileNotFoundError("gh")
[email protected]("airflow_breeze.utils.fork_ci_gate.subprocess.run", autospec=True)
[email protected](
+ ("returncode", "stdout", "side_effect"),
+ [
+ pytest.param(1, "", None, id="gh-fails"),
+ pytest.param(0, "not json", None, id="bad-output"),
+ pytest.param(0, "", FileNotFoundError("gh"), id="no-gh"),
+ ],
)
-def test_main_without_gh_uses_the_event_labels(mock_run, gate_env):
- gate_env["PR_EVENT_LABELS"] = '["use project ci"]'
+def test_main_pull_request_fails_when_labels_cannot_be_read(
+ mock_run, gate_env, returncode, stdout, side_effect
+):
+ mock_run.return_value = subprocess.CompletedProcess([], returncode,
stdout=stdout, stderr="boom")
+ mock_run.side_effect = side_effect
- main(gate_env)
+ assert main(gate_env) == 1
- assert _outputs(gate_env) == ["project-ci=true", "reason=label_project_ci"]
+ assert not Path(gate_env["GITHUB_OUTPUT"]).exists()
@mock.patch("airflow_breeze.utils.fork_ci_gate.subprocess.run", autospec=True)
diff --git a/dev/breeze/tests/test_workflow_run_approval.py
b/dev/breeze/tests/test_workflow_run_approval.py
index 0877efa90d3..87e68302911 100644
--- a/dev/breeze/tests/test_workflow_run_approval.py
+++ b/dev/breeze/tests/test_workflow_run_approval.py
@@ -31,7 +31,6 @@ from airflow_breeze.utils.fork_ci_messages import
CI_CHANGE_MARKER
from airflow_breeze.utils.selective_checks import SelectiveChecks
from airflow_breeze.utils.shared_options import set_dry_run
from airflow_breeze.utils.workflow_run_approval import (
- GATE_OVERRIDE_LABELS,
MAX_AUTO_APPROVED_JOB_MINUTES,
MAX_AUTO_APPROVED_SOURCE_LINES,
MAX_ERROR_TEXT_LENGTH,
@@ -995,13 +994,11 @@ def
test_get_last_label_change_ignores_other_labels_and_reads_the_timeline_once(
@mock.patch(f"{MODULE}.load_config", autospec=True, return_value=GATE_ENABLED)
@mock.patch(f"{MODULE}.get_changed_files", autospec=True,
return_value=CORE_CHANGE)
@mock.patch(f"{MODULE}.fetch_merge_commit", autospec=True,
return_value="merge")
-def test_gated_approval_depends_on_the_gate_override_labels(
- mock_fetch, mock_get_changed_files, mock_load_config
-):
+def test_gated_approval_depends_on_no_label(mock_fetch,
mock_get_changed_files, mock_load_config):
api = mock.MagicMock(spec=GitHubApi)
api.github_repository = "apache/airflow"
- assert decide_for_pull_request(api=api, pr=_gated_pr()).decision_labels ==
GATE_OVERRIDE_LABELS
+ assert decide_for_pull_request(api=api, pr=_gated_pr()).decision_labels ==
frozenset()
@pytest.mark.parametrize("event", ["labeled", "unlabeled"])
@@ -1016,14 +1013,14 @@ def
test_get_last_label_change_counts_every_test_selection_label(label, event):
@mock.patch(f"{MODULE}.decide_for_pull_request", autospec=True)
-def
test_process_waiting_runs_leaves_gated_runs_started_before_use_fork_ci_was_added(mock_decide):
+def
test_process_waiting_runs_approves_gated_runs_started_before_use_fork_ci_was_added(mock_decide):
api = GitHubApi(github_repository="apache/airflow", github_token="token",
report_only=False)
timeline = [
{"event": "labeled", "created_at": "2026-10-22T10:30:00Z", "label":
{"name": "use fork ci"}},
{"event": "unlabeled", "created_at": "2026-10-22T10:40:00Z", "label":
{"name": "full tests needed"}},
]
mock_decide.return_value = ApprovalDecision(
- approve=True, reasons=["fork CI"], decision_labels=GATE_OVERRIDE_LABELS
+ approve=True, reasons=["fork CI"], decision_labels=frozenset()
)
with (
mock.patch.object(
@@ -1041,4 +1038,4 @@ def
test_process_waiting_runs_leaves_gated_runs_started_before_use_fork_ci_was_a
):
process_waiting_runs(api, created_since=CREATED_SINCE)
- assert approve_run.call_args_list == [mock.call(3)]
+ assert approve_run.call_args_list == [mock.call(2), mock.call(3)]
diff --git a/scripts/ci/prek/check_ci_workflows_in_sync.py
b/scripts/ci/prek/check_ci_workflows_in_sync.py
index 321ca94194c..edee96e3a9b 100755
--- a/scripts/ci/prek/check_ci_workflows_in_sync.py
+++ b/scripts/ci/prek/check_ci_workflows_in_sync.py
@@ -229,7 +229,6 @@ AMD_ONLY_GATE_BLOCK = """ gate:
PR_AUTHOR: ${{ github.event.pull_request.user.login }}
PR_AUTHOR_ASSOCIATION: ${{
github.event.pull_request.author_association }}
PR_CREATED_AT: ${{ github.event.pull_request.created_at }}
- PR_EVENT_LABELS: ${{ toJSON(github.event.pull_request.labels.*.name)
}}
# A base branch with neither gate file predates the gate (the pull
request adding it,
# or a release branch cut before it) and keeps running project CI.
With only one of
# them present, the missing file fails the job instead of opening the
gate.
