This is an automated email from the ASF dual-hosted git repository. potiuk pushed a commit to branch backport-a5af91d-v3-4-stable in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 9919651d0d7f31687ecb81c7c140681325b19c9c Author: Jarek Potiuk <[email protected]> AuthorDate: Sat Oct 24 14:53:33 2026 +0200 [v3-4-stable] Enable auto-merge, gated on a dedicated CI passed check (#75142) (cherry picked from commit a5af91d2c52fb410a58eb6939ab83e44612afdfe) Generated-by: Claude Opus 5 --- .asf.yaml | 65 +++++++++++++++++++- .github/workflows/check-newsfragment-pr-number.yml | 2 +- .github/workflows/ci-amd.yml | 60 +++++++++++++++++++ .github/workflows/ci-arm.yml | 60 +++++++++++++++++++ .pre-commit-config.yaml | 9 +++ dev/README_AIRFLOW3_DEV.md | 40 +++++++++++++ scripts/ci/prek/check_ci_gate_needs.py | 70 ++++++++++++++++++++++ scripts/tests/ci/prek/test_check_ci_gate_needs.py | 60 +++++++++++++++++++ 8 files changed, 363 insertions(+), 3 deletions(-) diff --git a/.asf.yaml b/.asf.yaml index 497fc7f728d..1992824fb1f 100644 --- a/.asf.yaml +++ b/.asf.yaml @@ -72,8 +72,11 @@ github: rebase: false pull_requests: - # allow auto-merge - allow_auto_merge: false + # Auto-merge waits for the required "CI passed" check enforced by the "Require CI on main" and + # "Require CI on release branches" rulesets. Without a required check it would merge as soon as + # the PR is approved, before CI finishes. The setting is repository-wide, so every branch that + # PRs target needs one of those rulesets. + allow_auto_merge: true # enable updating head branches of pull requests allow_update_branch: true # auto-delete head branches after being merged @@ -104,7 +107,65 @@ github: required_conversation_resolution: true required_signatures: false + # The two CI rulesets use the raw Rulesets API payload, because the convenience syntax can only name + # teams as bypass actors and GitHub refuses secret teams such as `airflow-committers`. Instead the + # Write repository role (actor_id 4) bypasses them: on this repository only committers hold it, + # granted by ASF Infra from the `airflow` LDAP group, so it needs no update when committers change. + # Committers can still merge a PR whose CI is red for an unrelated reason, by bypassing the rule. + # + # The required checks are not pinned to the GitHub Actions app: asfyaml passes raw values to the + # API as strings except for a fixed set of keys, and a string `integration_id` is rejected. rulesets: + - name: "Require CI on main" + target: branch + enforcement: active + conditions: + ref_name: + include: + - "refs/heads/main" + exclude: [] + bypass_actors: + - actor_id: 4 + actor_type: RepositoryRole + bypass_mode: always + rules: + - type: deletion + - type: non_fast_forward + - type: required_status_checks + parameters: + strict_required_status_checks_policy: false + required_status_checks: + - context: "CI passed" + - context: "check-newsfragment-pr-number" + # The branches `ci-amd.yml` runs pull request CI for, apart from `main`. The newsfragment check + # runs only on `main`, so these require "CI passed" alone. A release branch gets the "CI passed" + # job only when it is backported there; until then its PRs need a committer to bypass the rule. + - name: "Require CI on release branches" + target: branch + enforcement: active + conditions: + ref_name: + include: + - "refs/heads/v*-test" + - "refs/heads/v*-stable" + - "refs/heads/providers-*/v*" + - "refs/heads/chart/v*-test" + - "refs/heads/chart/v*-stable" + - "refs/heads/airflow-ctl/v*-test" + - "refs/heads/airflow-ctl/v*-stable" + exclude: [] + bypass_actors: + - actor_id: 4 + actor_type: RepositoryRole + bypass_mode: always + rules: + - type: deletion + - type: non_fast_forward + - type: required_status_checks + parameters: + strict_required_status_checks_policy: false + required_status_checks: + - context: "CI passed" - name: "Branch protection" type: branch branches: diff --git a/.github/workflows/check-newsfragment-pr-number.yml b/.github/workflows/check-newsfragment-pr-number.yml index 3f81dbb8d0e..80dd07e33dc 100644 --- a/.github/workflows/check-newsfragment-pr-number.yml +++ b/.github/workflows/check-newsfragment-pr-number.yml @@ -21,7 +21,7 @@ on: # yamllint disable-line rule:truthy pull_request: branches: - main - types: [opened, reopened, synchronize] + types: [opened, reopened, synchronize, labeled, unlabeled] permissions: contents: read diff --git a/.github/workflows/ci-amd.yml b/.github/workflows/ci-amd.yml index 87c734b11e5..9cf33000791 100644 --- a/.github/workflows/ci-amd.yml +++ b/.github/workflows/ci-amd.yml @@ -1348,6 +1348,66 @@ jobs: use-uv: ${{ needs.build-info.outputs.use-uv }} debug-resources: ${{ needs.build-info.outputs.debug-resources }} + ci-passed: + # A skipped build-info means project CI did not run at all: the fork CI gate (AIP-120) left the + # pull request to CI in the contributor's fork, and every job here is skipped. Reporting + # "CI passed" then would let it merge on no CI, so the job reports under another name and + # the fork CI reconciler posts "CI passed" from the fork run instead. Until the gate lands, + # build-info has no condition and this always reports "CI passed". + name: >- + ${{ needs.build-info.result == 'skipped' && 'CI passed (project CI not run)' || 'CI passed' }} + # The one check main's ruleset requires, so auto-merge waits for CI to finish. GitHub treats a + # skipped required check as passed, so this job always runs and fails itself when any job it + # needs failed or was cancelled. Jobs skipped by selective checks count as passed. + if: always() + needs: + - additional-ci-image-checks + - additional-prod-image-tests + - basic-tests + - build-ci-images + - build-info + - build-java-sdk-docs + - build-prod-images + - build-ts-sdk-docs + - check-run-needed + - ci-image-checks + - finalize-tests + - generate-constraints + - migration-round-trip + - mypy-providers + - print-platform + - providers + - tests-agent-frameworks + - tests-airflow-ctl + - tests-go-sdk + - tests-helm + - tests-integration-system + - tests-java-sdk + - tests-kubernetes + - tests-kustomize-overlays + - tests-mysql-core + - tests-mysql-providers + - tests-non-db-core + - tests-non-db-providers + - tests-postgres-core + - tests-postgres-providers + - tests-special + - tests-sqlite-core + - tests-sqlite-providers + - tests-task-sdk + - tests-ts-sdk + - tests-with-lowest-direct-resolution-core + - tests-with-lowest-direct-resolution-providers + runs-on: ["ubuntu-slim"] + steps: + - name: "Fail if any CI job failed or was cancelled" + if: contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') + run: | + echo "At least one CI job failed or was cancelled." + exit 1 + - name: "All CI jobs passed or were skipped" + run: echo "All CI jobs passed or were skipped." + notify-slack: name: "Notify Slack" needs: diff --git a/.github/workflows/ci-arm.yml b/.github/workflows/ci-arm.yml index fb76a3bed5d..4352ce5d05b 100644 --- a/.github/workflows/ci-arm.yml +++ b/.github/workflows/ci-arm.yml @@ -1327,6 +1327,66 @@ jobs: use-uv: ${{ needs.build-info.outputs.use-uv }} debug-resources: ${{ needs.build-info.outputs.debug-resources }} + ci-passed: + # A skipped build-info means project CI did not run at all: the fork CI gate (AIP-120) left the + # pull request to CI in the contributor's fork, and every job here is skipped. Reporting + # "CI passed" then would let it merge on no CI, so the job reports under another name and + # the fork CI reconciler posts "CI passed" from the fork run instead. Until the gate lands, + # build-info has no condition and this always reports "CI passed". + name: >- + ${{ needs.build-info.result == 'skipped' && 'CI passed (project CI not run)' || 'CI passed' }} + # The one check main's ruleset requires, so auto-merge waits for CI to finish. GitHub treats a + # skipped required check as passed, so this job always runs and fails itself when any job it + # needs failed or was cancelled. Jobs skipped by selective checks count as passed. + if: always() + needs: + - additional-ci-image-checks + - additional-prod-image-tests + - basic-tests + - build-ci-images + - build-info + - build-java-sdk-docs + - build-prod-images + - build-ts-sdk-docs + - check-run-needed + - ci-image-checks + - finalize-tests + - generate-constraints + - migration-round-trip + - mypy-providers + - print-platform + - providers + - tests-agent-frameworks + - tests-airflow-ctl + - tests-go-sdk + - tests-helm + - tests-integration-system + - tests-java-sdk + - tests-kubernetes + - tests-kustomize-overlays + - tests-mysql-core + - tests-mysql-providers + - tests-non-db-core + - tests-non-db-providers + - tests-postgres-core + - tests-postgres-providers + - tests-special + - tests-sqlite-core + - tests-sqlite-providers + - tests-task-sdk + - tests-ts-sdk + - tests-with-lowest-direct-resolution-core + - tests-with-lowest-direct-resolution-providers + runs-on: ["ubuntu-slim"] + steps: + - name: "Fail if any CI job failed or was cancelled" + if: contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') + run: | + echo "At least one CI job failed or was cancelled." + exit 1 + - name: "All CI jobs passed or were skipped" + run: echo "All CI jobs passed or were skipped." + notify-slack: name: "Notify Slack" needs: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7695edb7c75..cb86dc8b4d9 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -989,6 +989,15 @@ repos: entry: "^\\s*from urllib\\.parse import ((\\|, )(urlparse\\|urlunparse))+$" pass_filenames: true files: \.py$ + - id: check-ci-gate-needs + name: Check the CI passed gate job needs every CI job + env: + PYTHONPYCACHEPREFIX: .build/pycache + description: Auto-merge waits for "CI passed"; a job missing from its needs could fail without stopping a merge + entry: ./scripts/ci/prek/check_ci_gate_needs.py + language: python + pass_filenames: false + files: ^\.github/workflows/ci-amd\.yml$|^scripts/ci/prek/check_ci_gate_needs\.py$ - id: check-auth-manager-mock-targets name: Patch SimpleAuthManager, not BaseAuthManager, for methods it overrides description: The API test client uses SimpleAuthManager, so patching an overridden BaseAuthManager method has no effect diff --git a/dev/README_AIRFLOW3_DEV.md b/dev/README_AIRFLOW3_DEV.md index 9249f063551..4ebd6069272 100644 --- a/dev/README_AIRFLOW3_DEV.md +++ b/dev/README_AIRFLOW3_DEV.md @@ -28,6 +28,7 @@ - [Developing for Airflow 3](#developing-for-airflow-3) - [Developing for Airflow 2.11.x](#developing-for-airflow-211x) - [Committers / PMCs](#committers--pmcs) + - [Auto-merge](#auto-merge) - [Merging PRs for providers and Helm chart](#merging-prs-for-providers-and-helm-chart) - [Merging PRs targeted for Airflow 3.X](#merging-prs-targeted-for-airflow-3x) - [What do we backport to `v3-2-test` branch?](#what-do-we-backport-to-v3-2-test-branch) @@ -180,6 +181,45 @@ PRs should **never** target `v2-11-stable` directly unless explicitly instructed The following sections explains the protocol for merging PRs. +## Auto-merge + +Auto-merge is enabled for PRs targeting `main`. Instead of waiting for CI to finish, approve the PR and +click **Enable auto-merge** (or run `gh pr merge <number> --squash --auto`). GitHub squash-merges the PR +as soon as it has an approval, its conversations are resolved and the required checks are green. Any +committer can cancel it with **Disable auto-merge**. A push by the PR author (or anyone else without +write access) disables auto-merge, so the author cannot change the PR after you approved it; review the +new commits and enable it again. Pushes by committers keep it enabled. + +Auto-merge is enabled for the whole repository, so every branch that PRs target requires CI. The +"Require CI on release branches" ruleset requires **CI passed** on the release, providers, chart and +`airflow-ctl` branches. A release branch gets the **CI passed** job only once it is backported there; until +then, its PRs never report the check, and a committer merges them by bypassing the rule. + +The `main` ruleset requires two checks. **CI passed** is a job in the `Tests (AMD)` workflow that always +runs and fails when any other CI job failed or was cancelled; jobs that selective checks skip count as +passed, so PRs that run only part of the tests still get a result. The `check-ci-gate-needs` prek hook +makes sure every new CI job is added to it. `Tests (ARM)`, which runs on pushes and on a schedule rather +than on PRs, has the same job, kept identical by the `check-ci-workflows-in-sync` hook. Once the fork CI +gate (AIP-120) is in place, a PR that it leaves to CI in the contributor's fork does not run project CI, +so the job reports as **CI passed (project CI not run)** instead, and the fork CI reconciler posts **CI +passed** once the fork run for the PR's latest commit is green and trusted. +**check-newsfragment-pr-number** runs on every PR too. + +Auto-merge does not wait for checks from other workflows: **CodeQL**, **Dependency review** and +**Registry tests**. The last two only run when dependency or registry files change. Look at those +checks before enabling auto-merge on a PR that triggers them. + +A PR that changes `.github/workflows/` runs its own version of the workflows, including the **CI passed** +job, so review such changes before enabling auto-merge: the check only means as much as the workflow +that produced it. + +When CI is red for a reason unrelated to the PR, for example a test that is broken on `main`, a +committer can still merge by ticking **Merge without waiting for requirements to be met (bypass +rules)**. Use it only when you are sure the failure is not caused by the PR, and say so in a comment. +The rulesets grant the bypass to the repository's Write role, which only committers hold here, so a new +committer gets it once ASF Infra adds them to the repository after they link their GitHub account in +[GitBox](https://gitbox.apache.org/setup/); nothing in `.asf.yaml` needs to change. + ## Merging PRs for providers and Helm chart Make sure PR targets `main` branch. diff --git a/scripts/ci/prek/check_ci_gate_needs.py b/scripts/ci/prek/check_ci_gate_needs.py new file mode 100755 index 00000000000..63e4207cc1e --- /dev/null +++ b/scripts/ci/prek/check_ci_gate_needs.py @@ -0,0 +1,70 @@ +#!/usr/bin/env python +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# /// script +# requires-python = ">=3.11,<3.12" +# dependencies = [ +# "pyyaml>=6.0.3", +# ] +# /// +"""Check that the ``CI passed`` gate job in ``ci-amd.yml`` needs every other job. + +``CI passed`` is the check main's ruleset requires, and auto-merge waits for it. It only fails +when a job it needs failed, so a job missing from its ``needs`` could fail and the PR would still +auto-merge. Reporting-only jobs that cannot make a PR wrong are exempt. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import yaml + +AIRFLOW_ROOT = Path(__file__).parents[3] +CI_WORKFLOW = AIRFLOW_ROOT / ".github/workflows/ci-amd.yml" +GATE_JOB = "ci-passed" +EXEMPT_JOBS = {"notify-slack", "summarize-warnings"} + + +def find_problems(jobs: dict) -> list[str]: + if GATE_JOB not in jobs: + return [f"job {GATE_JOB!r} is missing"] + needs = jobs[GATE_JOB].get("needs") or [] + needs = {needs} if isinstance(needs, str) else set(needs) + expected = set(jobs) - EXEMPT_JOBS - {GATE_JOB} + problems = [ + f"job {job!r} is not in {GATE_JOB}.needs, so its failure would not stop auto-merge" + for job in sorted(expected - needs) + ] + problems += [f"{GATE_JOB}.needs lists {job!r}, which is not a job" for job in sorted(needs - set(jobs))] + problems += [ + f"{GATE_JOB}.needs lists exempt reporting job {job!r}" for job in sorted(needs & EXEMPT_JOBS) + ] + return problems + + +def main() -> int: + jobs = yaml.safe_load(CI_WORKFLOW.read_text())["jobs"] + problems = find_problems(jobs) + for problem in problems: + print(f"{CI_WORKFLOW.relative_to(AIRFLOW_ROOT)}: {problem}") + return 1 if problems else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/ci/prek/test_check_ci_gate_needs.py b/scripts/tests/ci/prek/test_check_ci_gate_needs.py new file mode 100644 index 00000000000..77248def0ab --- /dev/null +++ b/scripts/tests/ci/prek/test_check_ci_gate_needs.py @@ -0,0 +1,60 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import pytest +import yaml +from check_ci_gate_needs import CI_WORKFLOW, find_problems + + +def test_real_workflow_gate_needs_every_job(): + assert find_problems(yaml.safe_load(CI_WORKFLOW.read_text())["jobs"]) == [] + + [email protected]( + ("jobs", "expected"), + [ + pytest.param( + {"build": {}, "test": {}, "notify-slack": {}, "ci-passed": {"needs": ["build", "test"]}}, + [], + id="complete", + ), + pytest.param({"build": {}}, ["job 'ci-passed' is missing"], id="no-gate"), + pytest.param( + {"build": {}, "test": {}, "ci-passed": {"needs": ["build"]}}, + ["job 'test' is not in ci-passed.needs, so its failure would not stop auto-merge"], + id="missing-job", + ), + pytest.param( + {"build": {}, "ci-passed": {"needs": "build"}}, + [], + id="needs-as-string", + ), + pytest.param( + {"build": {}, "ci-passed": {"needs": ["build", "gone"]}}, + ["ci-passed.needs lists 'gone', which is not a job"], + id="unknown-job", + ), + pytest.param( + {"build": {}, "summarize-warnings": {}, "ci-passed": {"needs": ["build", "summarize-warnings"]}}, + ["ci-passed.needs lists exempt reporting job 'summarize-warnings'"], + id="exempt-job-listed", + ), + ], +) +def test_find_problems(jobs, expected): + assert find_problems(jobs) == expected
