Add protected branches configuration for main branch

[dave2wave]

As @potiuk suggested in #638

[potiuk]

Would love others to agree.

[raboof]

LGTM but would be good to have an infra confirmation as well

[potiuk]

i think we need to add some options, otherwise default doesnt enforce anything

Agree -> required_approving_review_count = 1 likely.

[dave2wave]

Pulsar has some interesting notes about CI workflows these go with their .asf.yaml

[kevinjqliu]

not sure if we want to set require_code_owner_reviews, everything else looks good to me

[potiuk]

We do not have CODEOWNERS here ? Do we ? So I guess require_code_owner_reviews: true is not a good idea?

[raboof]

I'm +1 on the concepts and on trying this out, though we should be ready to revert if it turns out it prevents the asfgit automated commits on this repo.

[kevinjqliu]

LGTM!

Lets track to see if asfgit can still push to main after this is merged

[potiuk]

LGTM!

Lets track to see if asfgit can still push to main after this is merged

Ah... It won't be able to - when branches are protected, bots cannot push to - it - that is a major blocker.

[potiuk]

The only solution for that (and possibly a good one) would be that the fix creates a PR that we have to merge manually

[kevinjqliu]

Looks like it depends on what asfgit's role/permissions are in this repo.

By default, the restrictions of a branch protection rule don't apply to people with admin permissions to the repository or custom roles with the "bypass branch protections" permission. You can optionally apply the restrictions to administrators and roles with the "bypass branch protections" permission, too. For more information, see Managing custom repository roles for an organization.

https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches#about-branch-protection-rules

Could you run these to see if asfgit is an admin? I dont have permissions to run it:

gh api repos/apache/infrastructure-actions/collaborators/asfgit/permission
gh api orgs/apache/memberships/asfgit

Another possible solution is to allowlist asfgit explicitly with bypass_pull_request_allowances:
https://docs.github.com/en/rest/branches/branch-protection?apiVersion=2026-03-10#update-pull-request-review-protection

protected_branches:
  main:
    required_status_checks:
      strict: false
    required_pull_request_reviews:
      dismiss_stale_reviews: false
      required_approving_review_count: 1
      bypass_pull_request_allowances:
        users:
          - asfgit

[raboof]

The only solution for that (and possibly a good one) would be that the fix creates a PR that we have to merge manually

I think that would create too much work tbh

gh api repos/apache/infrastructure-actions/collaborators/asfgit/permission

I see this token apparently is indeed admin. perhaps we should consider switching to a non-admin token.

bypass_pull_request_allowances

That seems like a much nicer solution

[potiuk]

That seems like a much nicer solution

Indeed - if it will work. I remember a change in GitHub that blanket-disabled bots from being able to bypass the "pull request approval" limitation of protected branches. but maybe this is a way to bypass it.

[dave2wave]

I'll add this. Let's wait until Monday to merge and test,

      bypass_pull_request_allowances:
        users:
          - asfgit

[dave2wave]

Another possible solution is to allowlist asfgit explicitly with bypass_pull_request_allowances:
https://docs.github.com/en/rest/branches/branch-protection?apiVersion=2026-03-10#update-pull-request-review-protection

This fails because this feature is in the new apiVersion=2026-03-10 and apache/infrastructure-asfyaml is programmed for 2022 apiVersion.

[raboof]

Another possible solution is to allowlist asfgit explicitly with bypass_pull_request_allowances:
https://docs.github.com/en/rest/branches/branch-protection?apiVersion=2026-03-10#update-pull-request-review-protection

This fails because this feature is in the new apiVersion=2026-03-10 and apache/infrastructure-asfyaml is programmed for 2022 apiVersion.

filed apache/infrastructure-asfyaml#92

... but it's not a blocker for this PR since asfgit is currently still an admin token, if I understand correctly, right?

[dave2wave]

... but it's not a blocker for this PR since asfgit is currently still an admin token, if I understand correctly, right?

That's correct.

[kevinjqliu]

This fails because this feature is in the new apiVersion=2026-03-10 and apache/infrastructure-asfyaml is programmed for 2022 apiVersion.

That's weird, I see the bypass_pull_request_allowances option in the 2022-11-28 version too

https://docs.github.com/en/rest/branches/branch-protection?apiVersion=2022-11-28#update-pull-request-review-protection

[dave2wave]

The first step is actually to add the settings in asfyaml

[dave2wave]

That's weird, I see the bypass_pull_request_allowances option in the 2022-11-28 version too

I was incorrect. Each feature desired has to be programed. There are several different PRs waiting about branch protection. This will need to get sorted out over there soon.

[adoroszlai]

The workflow that updates actions.yml and approved_patterns.yml after merging dependabot PRs seems to be broken after this change.

Example:

[main 67e662b] Update actions.yml and approved_patterns.yml based on .github/workflows/dummy.yml
 2 files changed, 24 insertions(+)
remote: error: GH006: Protected branch update failed for refs/heads/main.        
remote: 
remote: - Changes must be made through a pull request.        
To https://github.com/apache/infrastructure-actions
 ! [remote rejected] main -> main (protected branch hook declined)

https://github.com/apache/infrastructure-actions/actions/runs/24166144405/job/70528047649

[potiuk]

Yeah. I think it's exactly what I thought will happen: #642 (comment)

@raboof @dave2wave -> WDYT? should we revert the protection or rather make the action create a PR that we will have to merge manually ? There is a small issue with the second solution - it will not run workflows by default (when PR is created by a workflow, it cannot start another workflow), so what we do in Airflow, we create Draft PR, and someone has to undraft it -> and this triggers the workflows, then you have to merge it. That adds more of manual operations (but simple ones).

I would be for that more safer solution.

[potiuk]

A fix to merge the missed updates #689 until we decide.

[potiuk]

Thanks @adoroszlai for notifying us!

[raboof]

The workflow that updates actions.yml and approved_patterns.yml after merging dependabot PRs seems to be broken after this change.

Example:

[main 67e662b] Update actions.yml and approved_patterns.yml based on .github/workflows/dummy.yml
 2 files changed, 24 insertions(+)
remote: error: GH006: Protected branch update failed for refs/heads/main.        
remote: 
remote: - Changes must be made through a pull request.        
To https://github.com/apache/infrastructure-actions
 ! [remote rejected] main -> main (protected branch hook declined)

https://github.com/apache/infrastructure-actions/actions/runs/24166144405/job/70528047649

Yeah, this was one we saw coming ;) - thanks for highlighting though!

should we revert the protection or rather make the action create a PR that we will have to merge manually ? There is a small issue with the second solution - it will not run workflows by default (when PR is created by a workflow, it cannot start another workflow), so what we do in Airflow, we create Draft PR, and someone has to undraft it -> and this triggers the workflows, then you have to merge it. That adds more of manual operations (but simple ones).

I would be for that more safer solution.

I'm all in favor of 'safer' options, but am not fully convinced creating a PR for every action approval or expiry will end up 'safer':

• someone will have to undraft, review and merge all those updates
• it will create delays in adding or removing elements
• it will create notification noise to the point that no-one will watch notifications for this repo
• it provides an opportunity for 'sneaking in' changes though all those noisy PRs
• it will make this repo less pleasant to work with, losing volunteers

I'm reluctantly OK with giving it a try - but not sure how we'd measure success or failure. I'm pretty sure I'd prefer the bypass_pull_request_allowances approach, though it might take a little time to find the right way to do that.

[raboof]

I'm reluctantly OK with giving it a try - but not sure how we'd measure success or failure. I'm pretty sure I'd prefer the bypass_pull_request_allowances approach, though it might take a little time to find the right way to do that.

I guess we'd want to ask Infra for a 'fine-grained PAT' that only gives write access to this repo, and then list the account associated with that PAT in bypass_pull_request_allowances?

[dave2wave]

You would also need to ask Infra to setup bypass_pull_request_allowances since that is missing in infrastructure-asfyaml.

[raboof]

You would also need to ask Infra to setup bypass_pull_request_allowances since that is missing in infrastructure-asfyaml.

Well, we can PR that ourselves, but indeed they'd need to review ;) . (linking apache/infrastructure-asfyaml#92 for reference)

[raboof]

Realistically I think we should revert this change until we've either done the refactor to have asfgit create PRs, or implemented the bypass_pull_request_allowances approach.

[dave2wave]

See #692