diff --git a/.asf.yaml b/.asf.yaml index 497fc7f728db9..dd90b457981aa 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 @@ -105,6 +108,38 @@ github: required_signatures: false rulesets: + - name: "Require CI on main" + type: branch + branches: + includes: + - "main" + # Committers can still merge a PR whose CI is red for an unrelated reason, by bypassing the rule. + bypass_teams: + - "airflow-committers" + required_status_checks: + - name: "CI passed" + app_slug: "github-actions" + - name: "check-newsfragment-pr-number" + app_slug: "github-actions" + # 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" + type: branch + branches: + includes: + - "v*-test" + - "v*-stable" + - "providers-*/v*" + - "chart/v*-test" + - "chart/v*-stable" + - "airflow-ctl/v*-test" + - "airflow-ctl/v*-stable" + bypass_teams: + - "airflow-committers" + required_status_checks: + - name: "CI passed" + app_slug: "github-actions" - 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 3f81dbb8d0e0a..80dd07e33dc0b 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 82faa9b3cfef2..30632bffc8227 100644 --- a/.github/workflows/ci-amd.yml +++ b/.github/workflows/ci-amd.yml @@ -1251,6 +1251,64 @@ 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. + 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 + - 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 6f5eb5eb27e0e..1fa86cb31f5f1 100644 --- a/.github/workflows/ci-arm.yml +++ b/.github/workflows/ci-arm.yml @@ -1240,6 +1240,64 @@ 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. + 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 + - 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 da1fd112e583b..08b7f7cba792e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -975,6 +975,13 @@ 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 + 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 d99a81a60f7a1..d7f2037e08460 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) @@ -179,6 +180,41 @@ 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 --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. Backport the job to +the active release branches when this lands. + +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. When project CI does not run at all, as for a PR that the +fork CI gate (AIP-120) leaves to CI in the contributor's fork, 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. + ## 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 0000000000000..63e4207cc1e98 --- /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 0000000000000..77248def0abc6 --- /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"]) == [] + + +@pytest.mark.parametrize( + ("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