Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 58 additions & 2 deletions .asf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -69,20 +69,76 @@ github:
rebase: false

rulesets:
- name: Merge Queue
# Rule-for-rule identical to "Merge Queue" below; split out so the bypass

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advisory:

The PR-level gates ask a CI or workflow change to link a proof run, usually on a fork. The description explains why the ruleset half cannot be proven pre-merge, and that reasoning holds. The workflow half is fork-provable, and a fork run of Required Checks is exactly what would have surfaced the startup failure above.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dispatch mechanism is the documented GITHUB_TOKEN exception ("workflow_dispatch and repository_dispatch events always create workflow runs"), and after 1864bda a surprise there degrades to a warning rather than a failed job. The .asf.yaml half only takes effect where Infra applies it — the body's post-merge verification covers that. Happy to stage a fork demo if you'd still like one.

# here stays off main. The bypass exempts actions performed as the GitHub
# Actions app — i.e. any workflow's GITHUB_TOKEN, which is what
# direct-backport-push.yml's fast path pushes with (#8377). It cannot be
# scoped to a single workflow. People and PATs still face every rule.
#
# Listed BEFORE "Merge Queue" deliberately: asfyaml applies rulesets in

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advisory:

This comment makes file order load-bearing and names the failure mode. The one check that would catch a reorder cannot see order: test_asf_rulesets.sh:77 keys the rulesets into a dict. Both the rules-equality and the new bypass-placement assertions then run on an order-free view, so an edit that alphabetises the rulesets stays green while removing the fail-safe. Asserting that "Merge Queue (release)" precedes "Merge Queue" is a two-line addition beside test_asf_rulesets.sh:93.

# file order, so this one is created before that one stops covering the
# release branches. If GitHub rejects this ruleset, the apply aborts with
# the old protections fully intact; the failure order never leaves the
# release branches uncovered.
- name: "Merge Queue (release)"
target: branch
enforcement: active
conditions:
ref_name:
exclude: []
include:
- "~DEFAULT_BRANCH"
# Merge queue rules do NOT support wildcard ref patterns, so
# release branches must be listed explicitly (not release/*).
# Add each release line here as it is cut.
- "refs/heads/release/v1.1"
- "refs/heads/release/v1.2"
- "refs/heads/release/v1.3"
bypass_actors:
# The GitHub Actions app.
- actor_id: 15368
actor_type: Integration
bypass_mode: always
rules:
Comment thread
Yicong-Huang marked this conversation as resolved.
- type: deletion
- type: non_fast_forward
- type: merge_queue
parameters:
merge_method: SQUASH
max_entries_to_build: 2
min_entries_to_merge: 2
max_entries_to_merge: 5
min_entries_to_merge_wait_minutes: 3
grouping_strategy: HEADGREEN
check_response_timeout_minutes: 45
- type: pull_request
parameters:
allowed_merge_methods:
- squash
dismiss_stale_reviews_on_push: false
require_code_owner_review: false
require_last_push_approval: false
required_approving_review_count: 1
required_review_thread_resolution: true
- type: required_linear_history
- type: required_status_checks
parameters:
strict_required_status_checks_policy: false
required_status_checks:
- context: Required Checks
- context: Check License Headers
- context: Validate PR title

- name: Merge Queue
target: branch
enforcement: active
conditions:
ref_name:
exclude: []
include:
# Release branches carry these same rules in "Merge Queue
# (release)" above — a separate ruleset because its Actions
# bypass must not extend to main.
- "~DEFAULT_BRANCH"
rules:
- type: deletion
- type: non_fast_forward
Expand Down
113 changes: 113 additions & 0 deletions .github/scripts/test_asf_rulesets.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
#!/usr/bin/env bash
# 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.

# Invariants over the CI configuration that a plain YAML parse cannot see.
#
# 1. "Merge Queue" and "Merge Queue (release)" in .asf.yaml must carry
# identical rules: they are one policy split across two rulesets only so
# the release half can hold an Actions bypass that must not reach main.
# Nothing else keeps the copies from drifting apart.
# 2. .asf.yaml and every workflow must parse with a duplicate-key-strict
# loader. PyYAML silently keeps the last duplicate, but GitHub's loader
# rejects the file, so a duplicated trigger key passes local checks and
# then stops the workflow from ever starting.

set -uo pipefail

command -v python3 >/dev/null || { echo "python3 is required to run these tests" >&2; exit 1; }
# Runners ship python3 but not necessarily PyYAML (see release_branches.py);
# CI installs it via amber/dev-requirements.txt.
python3 -c 'import yaml' 2>/dev/null || { echo "PyYAML is required (pip install pyyaml)" >&2; exit 1; }

cd "$(git rev-parse --show-toplevel)"

python3 - <<'EOF'
import glob
import sys

import yaml
Comment thread
Yicong-Huang marked this conversation as resolved.


class StrictLoader(yaml.SafeLoader):
pass


def no_duplicates(loader, node, deep=False):
seen = set()
for key_node, _ in node.value:
key = loader.construct_object(key_node, deep=deep)
if key in seen:
raise yaml.YAMLError(
f"duplicate key {key!r} at line {key_node.start_mark.line + 1}"
)
seen.add(key)
return yaml.SafeLoader.construct_mapping(loader, node, deep)


StrictLoader.add_constructor(
yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, no_duplicates
)

failures = []

files = sorted(glob.glob(".github/workflows/*.yml")) + [".asf.yaml"]
for path in files:
with open(path) as fh:
try:
yaml.load(fh, StrictLoader)
except yaml.YAMLError as exc:
failures.append(f"{path}: {exc}")

with open(".asf.yaml") as fh:
rulesets = {
r.get("name"): r
for r in yaml.safe_load(fh)["github"]["rulesets"]
if isinstance(r, dict)
}
main_rs = rulesets.get("Merge Queue")
release_rs = rulesets.get("Merge Queue (release)")
if main_rs is None or release_rs is None:
failures.append(
".asf.yaml: expected rulesets named 'Merge Queue' and 'Merge Queue (release)'"
)
elif main_rs["rules"] != release_rs["rules"]:
failures.append(
".asf.yaml: 'Merge Queue' and 'Merge Queue (release)' rules differ -- "
"these are one policy in two rulesets; change both or neither"
)
if main_rs is not None and "bypass_actors" in main_rs:
failures.append(
".asf.yaml: 'Merge Queue' must not carry bypass_actors -- "
"keeping the bypass off main is what the split exists for"
)
ACTIONS_APP = [{"actor_id": 15368, "actor_type": "Integration", "bypass_mode": "always"}]
if release_rs is not None and release_rs.get("bypass_actors") != ACTIONS_APP:
failures.append(
".asf.yaml: 'Merge Queue (release)' bypass_actors must be exactly the "
"GitHub Actions app -- widen this list and the test together, deliberately"
)

for failure in failures:
print(f"FAIL: {failure}")
if failures:
sys.exit(1)
print(
f"OK: {len(files)} files duplicate-key clean; "
"Merge Queue rules identical; bypass only on the release ruleset"
)
EOF
36 changes: 31 additions & 5 deletions .github/workflows/direct-backport-push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,15 @@ jobs:
needs: discover
if: ${{ needs.discover.outputs.has_push == 'true' }}
runs-on: ubuntu-latest
permissions:
# Everything this job's steps call, and nothing more: push the
# cherry-pick and comment on the commit (contents), dispatch Required
# Checks (actions), set the per-target commit status (statuses),
# annotate the original PR (issues).
actions: write
contents: write
issues: write
statuses: write
name: "backport #${{ matrix.pr_number }} to ${{ matrix.target }}"
strategy:
fail-fast: false
Expand All @@ -356,11 +365,12 @@ jobs:
uses: actions/checkout@v7
with:
fetch-depth: 0
# Use AUTO_MERGE_TOKEN (fine-grained PAT) so the push to the release
# branch retriggers workflows on that branch. GITHUB_TOKEN-authored
# pushes are excluded from triggering downstream workflows, which
# silences post-merge CI on backport commits.
token: ${{ secrets.AUTO_MERGE_TOKEN || secrets.GITHUB_TOKEN }}
# Push with the default GITHUB_TOKEN: the release rulesets admit the
# GitHub Actions app as a bypass actor, while a PAT-authored push is
# evaluated as that person and rejected. A GITHUB_TOKEN push starts
# no downstream workflows, so the step after the cherry-pick
# dispatches Required Checks itself — workflow_dispatch runs are the
# documented exception that GITHUB_TOKEN may create.
- name: Cherry-pick merge commit onto target branch
id: cherry_pick
env:
Expand Down Expand Up @@ -571,6 +581,22 @@ jobs:
log "new_sha=${new_sha}"
echo "new_sha=${new_sha}" >> "$GITHUB_OUTPUT"

- name: Run Required Checks on the pushed release branch
Comment thread
Yicong-Huang marked this conversation as resolved.
if: success()
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
TARGET_BRANCH: ${{ matrix.target }}
run: |
# The GITHUB_TOKEN push above starts no push-triggered workflows;
# dispatch the run the release branch would otherwise have gotten.
# Best-effort: the backport itself has landed, so a dispatch
# failure must not demote this job to failed -- the failure
# reporter below would then claim a landed backport was lost.
if ! gh workflow run required-checks.yml \
--repo "${GITHUB_REPOSITORY}" --ref "${TARGET_BRANCH}"; then
echo "::warning::Could not start Required Checks on ${TARGET_BRANCH}; start it manually from the Actions tab."
fi

- name: Annotate original PR and commit on success
if: success()
uses: actions/github-script@v9
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/required-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ on:
- labeled
- unlabeled
merge_group:
# Also dispatched by direct-backport-push.yml after its GITHUB_TOKEN push
# to a release branch, which starts no push-triggered runs.
workflow_dispatch:

permissions:
Expand Down
2 changes: 2 additions & 0 deletions amber/dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,5 @@ betterproto[compiler]==2.0.0b7

# Required by `bin/local-dev.sh -i` (the interactive Textual TUI).
textual==8.2.8
# .github/scripts/test_asf_rulesets.sh (infra job shell tests)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Polish:

Every other comment and pin group in this file is blank-line separated (:22, :27, :31, :35, :39). This entry reads as part of the textual group.

Suggested change
# .github/scripts/test_asf_rulesets.sh (infra job shell tests)
# .github/scripts/test_asf_rulesets.sh (infra job shell tests)

pyyaml==6.0.3
Loading