diff --git a/.github/AGENTS.md b/.github/AGENTS.md new file mode 100644 index 00000000000..e7689490f68 --- /dev/null +++ b/.github/AGENTS.md @@ -0,0 +1,38 @@ + + + + +**Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)* + +- [The `.github` folder — Agent Instructions](#the-github-folder--agent-instructions) + - [Keep the CI-minutes estimates in sync with the workflows](#keep-the-ci-minutes-estimates-in-sync-with-the-workflows) + + + +# The `.github` folder — Agent Instructions + +## Keep the CI-minutes estimates in sync with the workflows + +`breeze ci approve-workflow-runs` (run by `workflows/auto-approve-workflow-runs.yml`) decides whether +to approve an external contributor's CI runs from an estimate of the job-minutes they take. The +numbers live in `dev/breeze/src/airflow_breeze/utils/ci_minutes_estimate.py` and are medians +measured on recent `Tests (AMD)` PR runs on public runners. + +After you change the CI workflows in a way that adds, removes, splits or resizes jobs (new test +groups, a changed matrix, new composite workflows, a move between runner types), refresh the +estimates once the change has run on enough PRs, and in any case about once a month: + +1. Run `uv run dev/calibrate_ci_minutes_estimates.py --since ` + from the repository root with an authenticated `gh` CLI. Pick a date that covers at least a + week of runs after the workflow change. +2. Copy the medians and fits it prints into the constants of `ci_minutes_estimate.py`, and update + the "Last calibrated" line next to them. +3. When you add a new job group to the workflows, add a matching group to both + `ci_minutes_estimate.py` (with the selective-check output that turns it on) and the + `JOB_GROUPS` patterns of the calibration script. Unmatched jobs fall into the base group. +4. Run `uv run --project dev/breeze pytest dev/breeze/tests/test_ci_minutes_estimate.py + dev/breeze/tests/test_workflow_run_approval.py -xvs`. + +The estimates cover public runners only. Self-hosted (RunsOn) runners need their own set once there +is enough data from them. diff --git a/.github/workflows/auto-approve-workflow-runs.yml b/.github/workflows/auto-approve-workflow-runs.yml new file mode 100644 index 00000000000..b89f201bb2f --- /dev/null +++ b/.github/workflows/auto-approve-workflow-runs.yml @@ -0,0 +1,68 @@ +# 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. +# +--- +# Approves the waiting workflow runs of external contributors' PRs to main when the change is safe to +# run without a maintainer's look, and labels the others "need workflow approval". A PR whose CI runs +# in the contributor's fork (AIP-120) is approved unless it changes CI definition files, and then gets +# a comment saying why its runs wait. The rules are in +# dev/breeze/src/airflow_breeze/utils/workflow_run_approval.py and documented in +# contributing-docs/05_pull_requests.rst. +# +# This workflow runs only trusted code from main. It reads the PR's metadata, changed-file list and +# git objects, and never checks out or runs the PR's code. Approving with GITHUB_TOKEN starts the +# approved runs right away, and workflow_run workflows that follow them still trigger. +name: Auto approve workflow runs +on: # yamllint disable-line rule:truthy + schedule: + - cron: '*/30 * * * *' + workflow_dispatch: + inputs: + report-only: + description: "Only report the decisions and estimates, without approving or labelling" + type: boolean + default: false +permissions: + # Approving workflow runs. + actions: write + # Adding and removing the "need workflow approval" label, and commenting on a fork CI PR whose + # CI change keeps its runs waiting. + pull-requests: write + contents: read +concurrency: + group: auto-approve-workflow-runs + cancel-in-progress: false +jobs: + approve-workflow-runs: + name: "Approve workflow runs of external contributors" + runs-on: ubuntu-latest + if: github.repository == 'apache/airflow' + timeout-minutes: 20 + steps: + - name: "Checkout main" + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + ref: main + - name: "Install Breeze" + uses: ./.github/actions/breeze + - name: "Approve workflow runs" + env: + GITHUB_TOKEN: ${{ github.token }} + GITHUB_REPOSITORY: ${{ github.repository }} + REPORT_ONLY: ${{ inputs.report-only && 'true' || 'false' }} + run: breeze ci approve-workflow-runs diff --git a/contributing-docs/05_pull_requests.rst b/contributing-docs/05_pull_requests.rst index c7fee6d2677..0b178c13322 100644 --- a/contributing-docs/05_pull_requests.rst +++ b/contributing-docs/05_pull_requests.rst @@ -242,6 +242,47 @@ is the PMC private list — see the ``private@airflow.apache.org``. That document also covers the further steps the project may take when the same behaviour persists across PRs. +CI workflow runs of external contributors +----------------------------------------- + +GitHub does not start the CI workflows of a PR from a fork until a maintainer approves them, +for the contributors that the repository's approval setting covers. To keep you from waiting +for a maintainer on every push, a scheduled workflow checks the waiting runs every 30 minutes +and approves them automatically when all of these hold: + +* The PR targets ``main``. PRs to release branches, such as ``v3-4-test``, always wait for a + maintainer. +* The PR does not change anything under ``.github/``, ``dev/breeze/`` or ``scripts/ci/``. Those + files define the CI workflows and choose the runners they use. +* The change does not need the full set of tests, does not change dependencies (``pyproject.toml``, + lock files, ``package.json``, ``go.mod``, Gradle build files) and does not add a new provider. +* The CI jobs your change selects are estimated to take at most 310 job-minutes, about a quarter of + a full CI run. Changes to documentation, to the UI, to the Helm chart, to the language SDKs, to + one area of Airflow core or to a small provider usually stay under it. Changes to large providers + such as Amazon or Google, to several providers, or to the Task SDK (which runs the tests of all + providers) usually do not. +* The PR changes at most 100 lines of source code. Tests (including end-to-end tests), + documentation, newsfragments, translations and lock files are not counted, so a fix with + thorough tests still qualifies. Larger changes are usually new features, and a maintainer looks + at them first. + +If your PR's CI runs in your fork (it carries the ``fork ci required`` label), the CI run in +``apache/airflow`` only checks that, so none of the rules above apply. Its runs are approved +automatically unless the PR changes files that define CI: anything under ``.github/``, +``dev/breeze/``, ``scripts/ci/``, ``scripts/docker/`` or ``scripts/in_container/``, a ``Dockerfile`` +or ``.pre-commit-config.yaml``. Then the runs wait for a maintainer and a comment on the PR explains +why. CI in your fork runs either way. + +Each new push is checked again, because approval applies to one commit. Only runs started in the +last 24 hours are considered, so older waiting runs stay with maintainers. If your PR does not +qualify, it gets the ``need workflow approval`` label and a maintainer approves the runs after +looking at the change. That can take a while, so please do not ping maintainers about it; it is +not a judgement of your PR. The label is removed when a later commit is approved automatically. + +Maintainers can see each decision, the CI jobs selective checks expect to run and their estimated +time in the summary of the ``Auto approve workflow runs`` workflow. The rules are described in +more detail in `CI workflows <../dev/breeze/doc/ci/05_workflows.md>`__. + Gen-AI Assisted contributions ----------------------------- diff --git a/contributing-docs/25_maintainer_pr_triage.md b/contributing-docs/25_maintainer_pr_triage.md index e5f5313c5b7..e3f6909194c 100644 --- a/contributing-docs/25_maintainer_pr_triage.md +++ b/contributing-docs/25_maintainer_pr_triage.md @@ -238,6 +238,7 @@ The triage workflow uses the following labels and states: | Label / State | Meaning | |---|---| | `ready for maintainer review` | PR has passed Stage 1 and is queued for human review. Applied only after the skill has verified that no workflow runs are awaiting approval. | +| `need workflow approval` | The PR's CI workflow runs wait for a maintainer, because the `Auto approve workflow runs` workflow did not approve them: a PR to a branch other than `main`, a change to CI definition files, a full-tests or dependency change, a change whose CI jobs are estimated at more than 310 job-minutes, or a change of more than 100 lines of source code (see [Pull Requests](05_pull_requests.rst)). For a PR whose CI runs in the contributor's fork, only a change to CI definition files keeps its runs waiting, and the workflow comments on the PR to say so. The workflow also tries to add the label when processing the PR failed; its run then fails too. Approve the runs after looking at the diff. The workflow removes the label when a later commit is approved automatically. | | `closed because of multiple quality violations` | PR was closed because the author has multiple open PRs with quality issues. | | `suspicious changes detected` | PR (and all open PRs by the same author) was closed because the diff contained suspicious patterns (secret exfiltration, malicious CI modifications, etc.). | | `closed because of open PR limit` | PR was closed in the one-time closure that introduced the [open PR limit](32_open_pull_request_limit.rst). Applied by `dev/close_prs_over_open_pr_limit.py`, never by the triage skill. If the author reopens the PR, triage it like any other. | diff --git a/dev/breeze/doc/08_ci_tasks.rst b/dev/breeze/doc/08_ci_tasks.rst index b1bf0d9f5fb..591e796eff6 100644 --- a/dev/breeze/doc/08_ci_tasks.rst +++ b/dev/breeze/doc/08_ci_tasks.rst @@ -200,6 +200,26 @@ These are all available flags of ``fork-ci-reconcile`` command: :width: 100% :alt: Breeze ci fork-ci-reconcile +Approving workflow runs of external contributors +------------------------------------------------ + +The ``auto-approve-workflow-runs`` workflow runs ``approve-workflow-runs`` every 30 minutes. It approves +the waiting workflow runs of external contributors' PRs to ``main`` whose CI jobs are estimated at no more +than 310 job-minutes, that change at most 100 lines of source code and that do not touch CI definition files, and labels the other PRs ``need workflow approval``. PRs whose CI runs in +the contributor's fork (AIP-120) only run the fork CI gate in ``apache/airflow``, so their runs are +approved whatever their size unless they change CI definition files; such a PR also gets a comment +explaining that its CI change needs a maintainer. For every PR it also +estimates the CI jobs selective checks will run and how long they take, and writes them to the job +summary. Use ``--report-only`` to see the decisions without approving runs or changing labels. The rules +are described in `CI workflows `_. + +These are all available flags of ``approve-workflow-runs`` command: + +.. image:: ./images/output_ci_approve-workflow-runs.svg + :target: https://raw.githubusercontent.com/apache/airflow/main/dev/breeze/images/output_ci_approve-workflow-runs.svg + :width: 100% + :alt: Breeze ci approve-workflow-runs + ----- Next step: Follow the `Release management tasks <09_release_management_tasks.rst>`_ guide to learn how diff --git a/dev/breeze/doc/ci/05_workflows.md b/dev/breeze/doc/ci/05_workflows.md index 7ec7871993c..c06d4f8a267 100644 --- a/dev/breeze/doc/ci/05_workflows.md +++ b/dev/breeze/doc/ci/05_workflows.md @@ -98,6 +98,68 @@ whose CI runs in the contributor's fork (AIP-120), the gate skips `build-info` a everything after it, so the run finishes in seconds; see [Fork CI for external contributors](08_fork_ci_for_external_contributors.md). +### Approval of pull request runs + +GitHub holds the runs of a fork PR in `action_required` until someone with write access approves +them, for the contributors the repository's approval setting covers. The +`auto-approve-workflow-runs.yml` workflow runs every 30 minutes on `main` and calls +`breeze ci approve-workflow-runs`, which approves the waiting runs of a commit when all of these hold: + +- the PR targets `main`; PRs to release branches (`v3-4-test`, `airflow-ctl/v0-1-test`, ...) run those + branches' own workflows and always wait for a maintainer +- the PR changes nothing under `.github/`, `dev/breeze/` or `scripts/ci/`, which define the workflows + and choose the runners +- the PR changes no dependency manifest or lock file (`pyproject.toml`, `uv.lock`, `package.json`, + `pnpm-lock.yaml`, `go.mod`, Gradle build files, ...) and adds no new provider (selective checks + run from `main`'s tree and do not know it) +- selective checks for the PR's merge commit succeed, and do not need the full set of tests, all Python + and database versions, a canary build or upgraded dependencies (the estimate does not model them) +- the jobs selective checks select are estimated at no more than 310 job-minutes on public runners + (about 23% of a full-tests run, just above the cluster of single core areas and the Helm chart) +- the PR changes at most 100 lines of source code; tests (including e2e tests), docs, newsfragments, + translations, lock files and images are not counted + +A PR to `main` whose CI runs in the contributor's fork (AIP-120: the `Fork CI gate` job would skip +project CI for it) is decided by one rule instead, because its `Tests (AMD)` run only executes the +gate: its runs are approved whatever its size, cost or dependency changes, unless it changes a file +the fork CI reconciler counts as CI-defining (`.github/`, `dev/breeze/`, `scripts/ci/`, +`scripts/docker/`, `scripts/in_container/`, `Dockerfile*`, `.pre-commit-config.yaml`). Such a PR gets +the `need workflow approval` label and one comment telling the author that the CI change needs a +maintainer; see +[Approving gated runs](08_fork_ci_for_external_contributors.md#approving-gated-runs). + +Only runs created in the last 24 hours (`--max-run-age-hours`) are considered, so the workflow never +approves a backlog of older waiting runs at once; those stay with maintainers. A commit that is the +head of several open PRs is never approved automatically, because its runs cannot be told apart by +PR, and only runs started after the PR was opened and after its base branch last changed are +approved (retargeting a PR does not start new runs). A run selects its jobs from the labels the PR +had when it started, while the decision uses the current labels, so for PRs that run project CI a +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 +`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 +of the PR is approved automatically. + +The workflow uses only `main`'s code. It reads the PR's metadata and fetches its merge commit as git +objects to list the changed files; it never checks out or runs the PR's code. The merge commit must +have been built from the head commit whose runs get approved, so a push after the check cannot slip +into an approved run. Approving with the workflow's `GITHUB_TOKEN` starts the runs right away, and +`workflow_run` workflows that follow them still trigger. + +The estimate comes from `dev/breeze/src/airflow_breeze/utils/ci_minutes_estimate.py`, which turns the +selective-check outputs into job groups with job-minutes and a wall-clock estimate, weighting +providers by the size of their test suites. The job summary lists every waiting PR with its decision, +the expected job groups and the estimated time. The numbers are medians of recent `Tests (AMD)` PR +runs on public runners; refresh them with `dev/calibrate_ci_minutes_estimates.py`, as +`.github/AGENTS.md` describes. Run `breeze ci approve-workflow-runs --report-only` (or dispatch the +workflow with `report-only`) to see the decisions without approving or labelling anything. + ## Fork push run These runs are triggered by a push to any branch of a contributor's fork in which the 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 1f398373944..5f58617cf56 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 @@ -337,6 +337,49 @@ where `*` is any characters except `/`). The `check-ci-workflows-in-sync` prek hook allowlist is extended to permit the `gate` job and the `build-info` dependency to exist only in the AMD workflow. +### Approving gated runs + +GitHub holds the `pull_request` runs of a fork pull request in `action_required` until someone with +write access approves them, for the contributors the repository's approval setting covers. Until +then the gate job does not run either. A gated pull request would stay in the maintainers' approval +queue even though its project CI run only executes the gate, and the reconciler could not delete a +gated run that never ran. + +`breeze ci approve-workflow-runs`, run every 30 minutes by `auto-approve-workflow-runs.yml` (see +[CI workflows](05_workflows.md#approval-of-pull-request-runs)), handles them. For a pull request to +`main` it calls `fork_ci_gate.decide` with the configuration of its own `main` checkout and the +pull request's author, association, creation time, head repository and current labels, the inputs +the gate job of the waiting run uses. When the reason is gated (`external`, `pilot` or +`label_fork_ci`), the rules for pull requests that run project CI (estimated job-minutes, lines of +source code, dependency files, full tests) do not apply, because none of those jobs run. One rule +applies instead: + +- The waiting runs of the head commit are approved when the pull request's merge commit changes no + file that the reconciler counts as CI-defining: `.github/`, `dev/breeze/`, `scripts/ci/`, + `scripts/docker/`, `scripts/in_container/`, `Dockerfile*` and `.pre-commit-config.yaml` (see + "Trusting a green fork run"). The approved `ci-amd.yml` run executes only the gate, and the other + `pull_request` workflows (CodeQL for the changed languages, the newsfragment check, dependency + review) run on GitHub-hosted runners. +- Otherwise the runs wait for a maintainer, the pull request is labelled `need workflow approval`, + and the workflow posts one comment explaining that the change to the listed CI files needs a + maintainer to review it and approve the runs. The comment starts with the marker + `` and is posted once per pull request; only comments by + `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. + +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 +left waiting for approval and labelled `needs project ci` once its fork run is green. The gate +tampering check still applies to every run a maintainer approves by hand. + +The approval reads only API metadata and the git objects of the merge commit; it never checks out or +runs the pull request's code. Pull requests to other branches keep the rules for project CI, which +leave them to a maintainer. + ## Fork mode CI ### Trigger @@ -1022,6 +1065,9 @@ as a failed check rather than a crash. see "Trusting a green fork run". - Job statuses carry names from the contributor's workflow under their own `Fork CI job / ` prefix, so they cannot post the reconciler's own contexts or `CI passed`; see "Job statuses". +- Workflow runs of a gated pull request are approved without a maintainer only when it changes no + CI-defining file, so a pull request that edits the gate away still waits for a maintainer before + any run starts; see "Approving gated runs". - The reconciler runs on `main` with `pull-requests: write` and never checks out or executes pull request content. Its inputs are API data; its comment templates are static. - Fork runs get the fork's `GITHUB_TOKEN` only. Nothing in project CI trusts a fork run's diff --git a/dev/breeze/doc/images/output_ci.svg b/dev/breeze/doc/images/output_ci.svg index 7158eac2e10..d175e86af7b 100644 --- a/dev/breeze/doc/images/output_ci.svg +++ b/dev/breeze/doc/images/output_ci.svg @@ -1,4 +1,4 @@ - +