Repository navigation
ci: require approval and verify checks via a ruleset - #1413
Merged
Merged
Conversation
Make 'approval' a required status check on main, so an allow-list PR cannot merge unapproved (apache#1409 did, as the check was advisory). A maintainer can waive it with the allowlist-approval-override label, for their own allow-list PRs; it counts only if whoever last added it has write access. Generated-by: Claude Opus 5
Both verify workflows now start on every PR so the check can be required; a first step decides from the PR's file list and skips the rest when the allow list is untouched, failing closed. Generated-by: Claude Opus 5
Both verify jobs watch all three allow-list paths, so the one that runs for a PR's author covers each; file names are matched as JSON strings and counted as entries, and a list at the API cap is verified. Generated-by: Claude Opus 5
Only a ruleset can say who may bypass a required check: admins always (asfgit's direct sync pushes), write access on pull requests only. That replaces the allowlist-approval-override label, so the approval script is back to main's version. Generated-by: Claude Opus 5
potiuk
changed the title
This was referenced Oct 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
approvalandverifyonmainthrough aRequired checksruleset in.asf.yaml. Until now both were advisory, so an allow-list PR could merge unapproved, as action-allowlist-review: bump j178/prek-action from 3.0.1 to 3.0.2 in /.github/actions/for-dependabot-triggered-reviews #1409 did.update.ymlpushes its sync commit directly asasfgit, an admin), and anyone with write access on pull requests only, as a deliberate "bypass rules" merge, never a push.verifyworkflows now start on every PR (a required check that never starts blocks a PR forever). A first step reads the PR's file list and, for a PR touching none of the three allow-list files, passes with no checkout or build. The decision is inside the job, so a failed lookup failsverifyrather than skipping it; names are compared as JSON strings and a list at the API cap is verified.Unverified, for Infra to confirm: the ruleset bypasses the built-in write role (id 4). Whether that also covers the custom
committersorg role, which most committers here hold, is unknown; listing that role needs its id, which needs org-admin access.Test plan:
uv run pytest scripts/; theverifyscope step run underbash -e -o pipefailagainst an allow-list PR, an unrelated PR, a 20,000-file list, a truncated list, a 3000-file list, a rename and a newline-containing name;.asf.yamlparsed;prek runclean.🤖 Generated with Claude Code