feat: implement rulesets for branch protections #2168

amustaque97 · 2025-12-10T04:53:52Z

Related to #1735

github-actions · 2025-12-10T05:08:21Z

Dry-run check results


[WARN  sync_team] sync-team is running in dry mode, no changes will be applied.
[INFO  sync_team] synchronizing crates-io
[INFO  sync_team] synchronizing github

marcoieni · 2025-12-15T21:30:11Z

unfortunately there are merge conflicts 😭

sync-team/src/github/mod.rs

marcoieni · 2025-12-16T13:49:40Z

sync-team/src/github/tests/test_utils.rs

+        _org: &str,
+        _repo: &str,
+    ) -> anyhow::Result<Vec<crate::github::api::Ruleset>> {
+        // Return empty list for tests - rulesets not used in mock tests yet


do you want to do this in another PR?

My initial idea was to submit a PR with minimum changes and improve it further based on the feedback, and write test cases. This was simple enough, so I pushed the actual test case.
Thanks for highlighting it.

marcoieni · 2025-12-17T11:44:46Z

This feature is missing:

documentation updates for the toml config file
a change to a repository (eg bors) to test that the feature works.

Do you want to do these changes in another PR?

sync-team/src/github/mod.rs

sync-team/src/github/api/mod.rs

config.toml

amustaque97 · 2025-12-18T08:37:14Z

Do you want to do these changes in another PR?

I was thinking that when we're sure the feature is stable, we can update the docs and extend the feature to have its own standalone config. Right now, it just copies the branch protection rules without an explicit config. Sure, we can accommodate both changes in a single PR. WDYT?

marcoieni · 2025-12-18T10:55:01Z

sync-team/src/github/mod.rs


+/// Convert a branch protection to a ruleset with merge queue enabled
+pub fn construct_ruleset(
+    _expected_repo: &rust_team_data::v1::Repo,


why this parameter is unused? Should we remove it?

This variable will be used during the implementation of the bypass actor sub-property.

I’m fine keeping this unused variable, since I’ll be addressing the next bypass actor–related issue right after this PR, and then the final configuration and documentation PR.

marcoieni · 2025-12-18T10:55:02Z

sync-team/src/github/mod.rs

+    let uses_merge_bot = !branch_protection.merge_bots.is_empty();
+    let branch_protection_mode = if uses_merge_bot {
+        BranchProtectionMode::PrNotRequired
+    } else {
+        branch_protection.mode.clone()
+    };


no repository uses merge_bot as far as I can see.
This option is not even documented. Why using it?

Maybe I'm missing something.

It's used by rust-lang/rust.

marcoieni · 2025-12-18T10:56:56Z

sync-team/src/github/mod.rs

+            min_entries_to_merge: 0,
+            min_entries_to_merge_wait_minutes: 0,
+        },
+    });


no all repos use merge queues.
Probably merge queues should be configured from the repository toml file.
But it can be done in another pr

marcoieni · 2025-12-18T11:22:35Z

The dry run seems to work:

    📝 Editing repo 'rust-lang/bors':
      Branch Protections:
          main
            Deleting branch protection
      Rulesets:
          Branch protection for main
            Creating ruleset with merge queue enabled

However it would be nice if the dry run would print the ruleset itself (ie its rules etc).

As an example, you can have a look at the following dry run:

    📝 Editing repo 'rust-lang/cmake-rs':
      Branch Protections:
          main
            Dismiss Stale Reviews: false
            Is admin enforced: true
            Required Approving Review Count: 0
            Requires PR: true
            Required Checks: ["success"]
            Allowances: []
          master
            Deleting branch protection

marcoieni · 2025-12-19T12:23:58Z

New dry run:

[WARN  sync_team] sync-team is running in dry mode, no changes will be applied.
[INFO  sync_team] synchronizing crates-io
[INFO  sync_team] synchronizing github
[INFO  sync_team] 💻 Repo Diffs:
    📝 Editing repo 'rust-lang/bors':
      Branch Protections:
          main
            Deleting branch protection
      Rulesets:
          Branch protection for main
            Target: Branch
            Source Type: Repository
            Enforcement: Active
            Include Branches: ["main"]
            Rule: Pull Request
              Dismiss Stale Reviews: false
              Require Code Owner Review: false
              Require Last Push Approval: false
              Required Approving Review Count: 0
              Required Review Thread Resolution: false
            Rule: Required Status Checks
              Strict Policy: false
              Checks: ["Test", "Test Docker"]
            Rule: Merge Queue
              Timeout: 360 minutes
              Grouping Strategy: Allgreen
              Merge Method: Merge
              Max Entries to Build: 5
              Max Entries to Merge: 5
              Min Entries to Merge: 0
              Min Wait Time: 0 minutes

amustaque97 · 2025-12-19T13:48:38Z

New dry run:

[WARN  sync_team] sync-team is running in dry mode, no changes will be applied.
[INFO  sync_team] synchronizing crates-io
[INFO  sync_team] synchronizing github
[INFO  sync_team] 💻 Repo Diffs:
    📝 Editing repo 'rust-lang/bors':
      Branch Protections:
          main
            Deleting branch protection
      Rulesets:
          Branch protection for main
            Target: Branch
            Source Type: Repository
            Enforcement: Active
            Include Branches: ["main"]
            Rule: Pull Request
              Dismiss Stale Reviews: false
              Require Code Owner Review: false
              Require Last Push Approval: false
              Required Approving Review Count: 0
              Required Review Thread Resolution: false
            Rule: Required Status Checks
              Strict Policy: false
              Checks: ["Test", "Test Docker"]
            Rule: Merge Queue
              Timeout: 360 minutes
              Grouping Strategy: Allgreen
              Merge Method: Merge
              Max Entries to Build: 5
              Max Entries to Merge: 5
              Min Entries to Merge: 0
              Min Wait Time: 0 minutes

This looks good to me. I know most of the values are default/fixed. These will be changed to a full-fledged configuration in the next PR. Thoughts @marcoieni?

marcoieni · 2025-12-19T13:55:45Z

github com_rust-lang_bors_settings_branch_protection_rules_47732890

bors branch protection rule for backup

marcoieni · 2025-12-19T14:54:59Z

sync-team/src/github/mod.rs

+                }
+            }
+        };
+    }


why this needs to be a macro vs a function taking generics for example? Maybe something we can improve in another pr

marcoieni · 2025-12-19T15:31:10Z

There was an error while applying:

[ERROR rust_team] failed: Body: "{\"message\":\"Validation Failed\",\"errors\":[\"Invalid target patterns: 'main'\"],\"documentation_url\":\"https://docs.github.com/rest/repos/rules#create-a-repository-ruleset\",\"status\":\"422\"}"

Now bors is without branch protections momentarely.

marcoieni · 2025-12-19T15:34:54Z

sync-team/src/github/api/mod.rs

+    pub(crate) id: Option<i64>,
+    pub(crate) name: String,
+    pub(crate) target: RulesetTarget,
+    pub(crate) source_type: RulesetSourceType,


this field is not present in the request, only in the response, right?

The issue was enum RulesetSourceType was not using PascalCase to match GitHub's API format.

Along with this,

I also handled the optional fields.

Fixed some deserialization issues.

marcoieni · 2025-12-19T16:07:40Z

I triggered the workflow manually from the main branch to restore branch protections in bors

amustaque97 · 2025-12-19T17:33:30Z

I have made all the fixes and run it locally; it is working. Here is the recording for reference.

Screen.Recording.2025-12-19.at.10.56.21.PM.mov

marcoieni · 2025-12-19T17:44:51Z

Thanks! I don't want to merge this on friday evening because if there are issues I can't fix them, so I will take a look on Monday morning. I hope you don't mind!

Since I re-run the workflow from the main branch, the bors repo has branch protection again 👍

amustaque97 marked this pull request as ready for review December 10, 2025 05:06

amustaque97 requested review from Mark-Simulacrum, jdno, marcoieni and pietroalbini as code owners December 10, 2025 05:06

jieyouxu added S-waiting-on-review Status: waiting on review from a team/WG/PG lead, an infra-admin, and/or a team-repo-admin. T-infra Relevant to the infrastructure team. labels Dec 10, 2025

amustaque97 force-pushed the feat/1735-add-ruleset-support branch 3 times, most recently from effc88e to a9f0337 Compare December 16, 2025 07:24

marcoieni reviewed Dec 16, 2025

View reviewed changes

sync-team/src/github/mod.rs Show resolved Hide resolved

marcoieni reviewed Dec 16, 2025

View reviewed changes

sync-team/src/github/mod.rs Show resolved Hide resolved

amustaque97 mentioned this pull request Dec 16, 2025

refactor: improve environment diffing using BTreeSet #2188

Merged

amustaque97 force-pushed the feat/1735-add-ruleset-support branch from b667d31 to 2b33998 Compare December 16, 2025 12:11

marcoieni reviewed Dec 16, 2025

View reviewed changes

marcoieni reviewed Dec 17, 2025

View reviewed changes

sync-team/src/github/mod.rs Show resolved Hide resolved

marcoieni reviewed Dec 17, 2025

View reviewed changes

sync-team/src/github/api/mod.rs Show resolved Hide resolved

marcoieni reviewed Dec 17, 2025

View reviewed changes

config.toml Outdated Show resolved Hide resolved

marcoieni reviewed Dec 18, 2025

View reviewed changes

marcoieni reviewed Dec 19, 2025

View reviewed changes

amustaque97 force-pushed the feat/1735-add-ruleset-support branch from a830b7e to de5a87c Compare December 19, 2025 15:02

marcoieni previously approved these changes Dec 19, 2025

View reviewed changes

marcoieni added this pull request to the merge queue Dec 19, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 19, 2025

marcoieni reviewed Dec 19, 2025

View reviewed changes

feat: implement rulesets

7189083

amustaque97 force-pushed the feat/1735-add-ruleset-support branch from de5a87c to 7189083 Compare December 19, 2025 16:37

amustaque97 added 3 commits December 19, 2025 22:41

fix: branch name convention

0cc3f24

fix: variable casing

f49cb45

fix: handle none condition

6cd9584

amustaque97 dismissed marcoieni’s stale review via 6cd9584 December 19, 2025 17:18

fix: clippy errors

58a80d8

feat: implement rulesets for branch protections #2168

Are you sure you want to change the base?

feat: implement rulesets for branch protections #2168

Uh oh!

Conversation

amustaque97 commented Dec 10, 2025

Uh oh!

github-actions bot commented Dec 10, 2025

Uh oh!

marcoieni commented Dec 15, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcoieni commented Dec 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

amustaque97 commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcoieni commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marcoieni commented Dec 19, 2025

Uh oh!

amustaque97 commented Dec 19, 2025

Uh oh!

marcoieni commented Dec 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marcoieni commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marcoieni Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcoieni commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amustaque97 commented Dec 19, 2025

Uh oh!

marcoieni commented Dec 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

amustaque97 commented Dec 18, 2025 •

edited

Loading

marcoieni commented Dec 18, 2025 •

edited

Loading

marcoieni commented Dec 19, 2025 •

edited

Loading

marcoieni Dec 19, 2025 •

edited

Loading

marcoieni commented Dec 19, 2025 •

edited

Loading