use rulesets in the rust repo

marcoieni · marcoieni · commit d9ec7fad6625 · 2026-04-07T19:13:58.000+02:00
diff --git a/config.toml b/config.toml
@@ -155,6 +155,7 @@ enable-rulesets-repos = [
    "rust-lang/rustc-perf",
    "rust-lang/rustc-stable-hash",
    "rust-lang/rustfmt",
+   "rust-lang/rust",
    "rust-lang/rustup",
    "rust-lang/simpleinfra",
    "rust-lang/socket2",
diff --git a/repos/rust-lang/rust.toml b/repos/rust-lang/rust.toml
@@ -26,67 +26,131 @@ rust-analyzer = "write"
 style = "write"
 types = "write"
 triage = "write"
+# needed for the branch protection
+rust-timer = "write"
 
+# Only `bors` can push to `main`
 [[branch-protections]]
+name = "main"
 pattern = "main"
 allowed-merge-apps = ["bors"]
+prevent-update = true
 
+# No one can force push to main
 [[branch-protections]]
+name = "main - force-push"
+pattern = "main"
+# `main` already requires users to open PRs.
+# This allows bors to push without opening a PR.
+pr-required = false
+
+# Only `bors` and `promote-release` can push to `stable`
+[[branch-protections]]
+name = "stable"
 pattern = "stable"
-allowed-merge-apps = ["bors"]
+allowed-merge-apps = ["bors", "promote-release"]
+prevent-update = true
+
+# Only `promote-release` can force-push to `stable`
+[[branch-protections]]
+name = "stable - force-push"
+pattern = "stable"
+allowed-merge-apps = ["promote-release"]
+# `stable` already requires users to open PRs.
+# This allows bors to push without opening a PR.
+pr-required = false
+
+# No one create or delete the stable branch.
+[[branch-protections]]
+name = "stable - misc"
+pattern = "stable"
+pr-required = false
+prevent-force-push = false
 
+# Only `bors` and `promote-release` can push to `beta`
 [[branch-protections]]
+name = "beta"
 pattern = "beta"
-allowed-merge-apps = ["bors"]
+allowed-merge-apps = ["bors", "promote-release"]
+prevent-update = true
+
+# Only `promote-release` can force-push to `beta`
+[[branch-protections]]
+name = "beta - force-push"
+pattern = "beta"
+allowed-merge-apps = ["promote-release"]
+# `beta` already requires users to open PRs.
+# This allows bors to push without opening a PR.
+pr-required = false
+
+# no one can create or delete the beta branch.
+[[branch-protections]]
+name = "beta - misc"
+pattern = "beta"
+pr-required = false
+prevent-force-push = false
 
 [[branch-protections]]
 pattern = "*"
+allowed-merge-apps = ["promote-release"]
+prevent-deletion = false
+prevent-update = true
 
 [[branch-protections]]
 pattern = "*/**/*"
 pr-required = false
+prevent-update = true
+prevent-deletion = false
 
 [[branch-protections]]
 pattern = "cargo_update"
 pr-required = false
+prevent-deletion = false
+prevent-force-push = false
 
 # Required for running try builds created by bors.
 # Must support force-pushes.
 [[branch-protections]]
 pattern = "automation/bors/try"
 allowed-merge-apps = ["bors"]
+prevent-update = true
 
 # Required for running try builds created by bors.
 # Must support force-pushes.
 [[branch-protections]]
 pattern = "automation/bors/try-merge"
 allowed-merge-apps = ["bors"]
+prevent-update = true
 
 # Required for running auto builds created by bors.
 # Must support force-pushes.
 [[branch-protections]]
 pattern = "automation/bors/auto"
 allowed-merge-apps = ["bors"]
+prevent-update = true
 
 # Required for running auto builds created by bors.
 # Must support force-pushes.
 [[branch-protections]]
 pattern = "automation/bors/auto-merge"
 allowed-merge-apps = ["bors"]
+prevent-update = true
 
 # Required for unrolled PR builds created by perfbot.
 # Must support force-pushes.
 [[branch-protections]]
 pattern = "try-perf"
-allowed-merge-apps = ["rust-timer"]
+allowed-merge-teams = ["rust-timer"]
 pr-required = false
+prevent-update = true
 
 # Required for unrolled PR builds created by perfbot.
 # Must support force-pushes.
 [[branch-protections]]
 pattern = "perf-tmp"
-allowed-merge-apps = ["rust-timer"]
+allowed-merge-teams = ["rust-timer"]
 pr-required = false
+prevent-update = true
 
 [environments.bors]
 branches = ["automation/bors/auto", "automation/bors/try", "try-perf"]
diff --git a/src/sync/github/mod.rs b/src/sync/github/mod.rs
@@ -9,6 +9,7 @@ use crate::sync::Config;
 use crate::sync::github::api::{
     GithubRead, Login, PushAllowanceActor, RepoPermission, RepoSettings, Ruleset,
 };
+use anyhow::Context as _;
 use futures_util::StreamExt;
 use log::debug;
 use rust_team_data::v1::{Bot, BranchProtectionMode, MergeBot, ProtectionTarget};
@@ -447,6 +448,78 @@ impl SyncGitHub {
         self.config.enable_rulesets_repos.contains(&repo_full_name)
     }
 
+    async fn construct_ruleset(
+        &self,
+        expected_repo: &rust_team_data::v1::Repo,
+        branch_protection: &rust_team_data::v1::BranchProtection,
+    ) -> anyhow::Result<api::Ruleset> {
+        let bypass_actors = self.bypass_actors(expected_repo, branch_protection).await?;
+
+        Ok(construct_ruleset(branch_protection, bypass_actors))
+    }
+
+    async fn bypass_actors(
+        &self,
+        expected_repo: &rust_team_data::v1::Repo,
+        branch_protection: &rust_team_data::v1::BranchProtection,
+    ) -> Result<Vec<api::RulesetBypassActor>, anyhow::Error> {
+        use api::{RulesetActorType, RulesetBypassActor, RulesetBypassMode};
+
+        let mut bypass_actors = Vec::new();
+        let allowed_teams = self
+            .allowed_merge_teams(expected_repo, branch_protection)
+            .await?;
+        bypass_actors.extend(allowed_teams);
+        let allowed_apps = branch_protection
+            .allowed_merge_apps
+            .iter()
+            .filter_map(|app| {
+                app.app_id().map(|app_id| RulesetBypassActor {
+                    actor_id: app_id,
+                    actor_type: RulesetActorType::Integration,
+                    bypass_mode: RulesetBypassMode::Always,
+                })
+            });
+        bypass_actors.extend(allowed_apps);
+        Ok(bypass_actors)
+    }
+
+    async fn allowed_merge_teams(
+        &self,
+        expected_repo: &rust_team_data::v1::Repo,
+        branch_protection: &rust_team_data::v1::BranchProtection,
+    ) -> Result<Vec<api::RulesetBypassActor>, anyhow::Error> {
+        use api::{RulesetActorType, RulesetBypassActor, RulesetBypassMode};
+
+        let mut allowed = vec![];
+
+        for team_name in &branch_protection.allowed_merge_teams {
+            let github_team = self
+                .github
+                .team(&expected_repo.org, team_name)
+                .await?
+                .with_context(|| {
+                    format!(
+                        "failed to find GitHub team '{team_name}' in org '{}' for repo '{}/{}'",
+                        expected_repo.org, expected_repo.org, expected_repo.name
+                    )
+                })?;
+            let team_id = github_team.id.with_context(|| {
+                format!(
+                    "GitHub team '{team_name}' in org '{}' is missing an ID",
+                    expected_repo.org
+                )
+            })?;
+
+            allowed.push(RulesetBypassActor {
+                actor_id: team_id as i64,
+                actor_type: RulesetActorType::Team,
+                bypass_mode: RulesetBypassMode::Always,
+            });
+        }
+        Ok(allowed)
+    }
+
     async fn diff_repo(
         &self,
         expected_repo: &rust_team_data::v1::Repo,
@@ -481,7 +554,9 @@ impl SyncGitHub {
                 let use_rulesets = self.should_use_rulesets(expected_repo);
                 if use_rulesets {
                     for branch_protection in &expected_repo.branch_protections {
-                        let ruleset = construct_ruleset(branch_protection);
+                        let ruleset = self
+                            .construct_ruleset(expected_repo, branch_protection)
+                            .await?;
                         rulesets.push(ruleset);
                     }
                 }
@@ -799,7 +874,9 @@ impl SyncGitHub {
 
         // Process each branch protection as a potential ruleset
         for branch_protection in &expected_repo.branch_protections {
-            let expected_ruleset = construct_ruleset(branch_protection);
+            let expected_ruleset = self
+                .construct_ruleset(expected_repo, branch_protection)
+                .await?;
 
             if let Some(actual_ruleset) = rulesets_by_name.remove(&expected_ruleset.name) {
                 let Ruleset {
@@ -1179,11 +1256,12 @@ fn github_int(value: u32) -> i32 {
     i32::try_from(value).unwrap_or_else(|_| panic!("Value {value} exceeds GitHub's Int range"))
 }
 
-pub fn construct_ruleset(branch_protection: &rust_team_data::v1::BranchProtection) -> api::Ruleset {
+pub fn construct_ruleset(
+    branch_protection: &rust_team_data::v1::BranchProtection,
+    bypass_actors: Vec<api::RulesetBypassActor>,
+) -> api::Ruleset {
     use api::*;
 
-    let branch_protection_mode = get_branch_protection_mode(branch_protection);
-
     // Use a BTreeSet to ensure a consistent order. This avoids unnecessary diffs when the order of rules changes,
     // since GitHub does not guarantee any specific order for rules.
     let mut rules: BTreeSet<RulesetRule> = BTreeSet::new();
@@ -1214,22 +1292,22 @@ pub fn construct_ruleset(branch_protection: &rust_team_data::v1::BranchProtectio
     // Add pull request rule if PRs are required
     if let BranchProtectionMode::PrRequired {
         required_approvals, ..
-    } = &branch_protection_mode
+    } = branch_protection.mode
     {
         rules.insert(RulesetRule::PullRequest {
             parameters: PullRequestParameters {
                 dismiss_stale_reviews_on_push: branch_protection.dismiss_stale_review,
                 require_code_owner_review: REQUIRE_CODE_OWNER_REVIEW_DEFAULT,
                 require_last_push_approval: REQUIRE_LAST_PUSH_APPROVAL_DEFAULT,
-                required_approving_review_count: github_int(*required_approvals),
+                required_approving_review_count: github_int(required_approvals),
                 required_review_thread_resolution: branch_protection
                     .require_conversation_resolution,
             },
         });
     }
 
     // Add required status checks if any
-    if let BranchProtectionMode::PrRequired { ci_checks, .. } = &branch_protection_mode
+    if let BranchProtectionMode::PrRequired { ci_checks, .. } = &branch_protection.mode
         && !ci_checks.is_empty()
     {
         let mut checks = ci_checks.clone();
@@ -1271,19 +1349,6 @@ pub fn construct_ruleset(branch_protection: &rust_team_data::v1::BranchProtectio
         rules.insert(RulesetRule::MergeQueue { parameters });
     }
 
-    // Build bypass actors from allowed merge apps
-    let bypass_actors: Vec<RulesetBypassActor> = branch_protection
-        .allowed_merge_apps
-        .iter()
-        .filter_map(|app| {
-            app.app_id().map(|app_id| RulesetBypassActor {
-                actor_id: app_id,
-                actor_type: RulesetActorType::Integration,
-                bypass_mode: RulesetBypassMode::Always,
-            })
-        })
-        .collect();
-
     api::Ruleset {
         id: None,
         name: branch_protection
diff --git a/src/sync/github/tests/mod.rs b/src/sync/github/tests/mod.rs
@@ -980,7 +980,7 @@ fn ruleset_creation_logs_non_default_disabled_flags() {
     protection.prevent_deletion = false;
     protection.prevent_force_push = false;
 
-    let ruleset = construct_ruleset(&protection);
+    let ruleset = construct_ruleset(&protection, vec![]);
     let mut rendered = String::new();
     log_ruleset(&ruleset, None, &mut rendered).unwrap();
 
@@ -992,15 +992,17 @@ fn ruleset_creation_logs_non_default_disabled_flags() {
 
 #[test]
 fn ruleset_updates_log_disabled_toggle_rules_as_false() {
-    let old =
-        construct_ruleset(&BranchProtectionBuilder::pr_required("main", &["test"], 1).build());
+    let old = construct_ruleset(
+        &BranchProtectionBuilder::pr_required("main", &["test"], 1).build(),
+        vec![],
+    );
 
     let mut new_protection = BranchProtectionBuilder::pr_required("main", &["test"], 1).build();
 
     // Change default
     new_protection.prevent_force_push = false;
 
-    let new = construct_ruleset(&new_protection);
+    let new = construct_ruleset(&new_protection, vec![]);
     let mut rendered = String::new();
     log_ruleset(&old, Some(&new), &mut rendered).unwrap();
 
diff --git a/src/validate.rs b/src/validate.rs
@@ -1150,12 +1150,22 @@ fn validate_branch_protections(data: &Data, errors: &mut Vec<String>) {
 
     wrapper(data.repos(), errors, |repo, _| {
         let bors_configured = repo.bots.iter().any(|b| matches!(b, Bot::Bors));
-        let mut patterns = HashSet::new();
+        let mut pattern_counts = HashMap::new();
 
         for protection in &repo.branch_protections {
-            if !patterns.insert((protection.target, &protection.pattern)) {
+            *pattern_counts
+                .entry((protection.target, protection.pattern.as_str()))
+                .or_insert(0usize) += 1;
+        }
+
+        for protection in &repo.branch_protections {
+            let key = (protection.target, protection.pattern.as_str());
+            let occurrences = pattern_counts
+                .get(&key)
+                .with_context(|| format!("pattern_counts should contain the key {key:?}"))?;
+            if *occurrences > 1 && protection.name.is_none() {
                 bail!(
-                    r#"repo '{}' uses multiple {:?} protections with the pattern `{}`"#,
+                    r#"repo '{}' uses multiple {:?} protections with the pattern `{}`; when multiple protections share the same target and pattern, each protection must specify `name`"#,
                     repo.name,
                     protection.target,
                     protection.pattern,
@@ -1166,17 +1176,15 @@ fn validate_branch_protections(data: &Data, errors: &mut Vec<String>) {
                 let key = (repo.org.clone(), team.clone());
                 if !github_teams.contains(&key) {
                     bail!(
-                        r#"repo '{}' uses a branch protection for {} that mentions the '{}' github team;
-but that team does not seem to exist"#,
+                        r#"repo '{}' uses a branch protection for {} that mentions the '{}' github team; but that team does not seem to exist"#,
                         repo.name,
                         protection.pattern,
                         team
                     );
                 }
                 if !repo.access.teams.contains_key(team) {
                     bail!(
-                        r#"repo '{}' uses a branch protection for {} that has an allowed merge team '{}',
-but that team is not mentioned in [access.teams]"#,
+                        r#"repo '{}' uses a branch protection for {} that has an allowed merge team '{}', but that team is not mentioned in [access.teams]"#,
                         repo.name,
                         protection.pattern,
                         team
