Skip to content

Commit 2146114

Browse files
committed
use rulesets in the rust repo
1 parent cda8072 commit 2146114

File tree

4 files changed

+151
-27
lines changed

4 files changed

+151
-27
lines changed

config.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ enable-rulesets-repos = [
9999
"rust-lang/rust-clippy",
100100
"rust-lang/rustc-perf",
101101
"rust-lang/rustfmt",
102+
"rust-lang/rust",
102103
"rust-lang/stdarch",
103104
"rust-lang/triagebot",
104105
]

repos/rust-lang/rust.toml

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,66 +26,114 @@ rust-analyzer = "write"
2626
style = "write"
2727
types = "write"
2828
triage = "write"
29+
# needed for the branch protection
30+
rust-timer = "write"
2931

32+
# Only `bors` can push to `main`
3033
[[branch-protections]]
34+
name = "main"
3135
pattern = "main"
3236
allowed-merge-apps = ["bors"]
37+
prevent-update = true
3338

39+
# No one can force push to main
3440
[[branch-protections]]
41+
name = "main - force-push"
42+
pattern = "main"
43+
# `main` already requires users to open PRs.
44+
# This allows bors to push without opening a PR.
45+
pr-required = false
46+
47+
# Only `bors` and `promote-release` can push to `stable`
48+
[[branch-protections]]
49+
name = "stable"
3550
pattern = "stable"
36-
allowed-merge-apps = ["bors"]
51+
allowed-merge-apps = ["bors", "promote-release"]
52+
prevent-update = true
53+
54+
# Only `promote-release` can force-push to `stable`
55+
[[branch-protections]]
56+
name = "stable - force-push"
57+
pattern = "stable"
58+
allowed-merge-apps = ["promote-release"]
59+
# `stable` already requires users to open PRs.
60+
# This allows bors to push without opening a PR.
61+
pr-required = false
3762

63+
# Only `bors` and `promote-release` can push to `beta`
3864
[[branch-protections]]
65+
name = "beta"
3966
pattern = "beta"
40-
allowed-merge-apps = ["bors"]
67+
allowed-merge-apps = ["bors", "promote-release"]
68+
prevent-update = true
69+
70+
# Only `promote-release` can force-push to `beta`
71+
[[branch-protections]]
72+
name = "beta - force-push"
73+
pattern = "beta"
74+
allowed-merge-apps = ["promote-release"]
75+
# `beta` already requires users to open PRs.
76+
# This allows bors to push without opening a PR.
77+
pr-required = false
4178

4279
[[branch-protections]]
4380
pattern = "*"
81+
allowed-merge-apps = ["promote-release"]
82+
prevent-deletion = false
83+
prevent-update = true
4484

4585
[[branch-protections]]
4686
pattern = "*/**/*"
4787
pr-required = false
88+
prevent-update = true
89+
prevent-deletion = false
4890

4991
[[branch-protections]]
5092
pattern = "cargo_update"
5193
pr-required = false
94+
prevent-deletion = false
95+
prevent-force-push = false
5296

5397
# Required for running try builds created by bors.
5498
# Must support force-pushes.
5599
[[branch-protections]]
56100
pattern = "automation/bors/try"
57101
allowed-merge-apps = ["bors"]
102+
prevent-update = true
58103

59104
# Required for running try builds created by bors.
60105
# Must support force-pushes.
61106
[[branch-protections]]
62107
pattern = "automation/bors/try-merge"
63108
allowed-merge-apps = ["bors"]
109+
prevent-update = true
64110

65111
# Required for running auto builds created by bors.
66112
# Must support force-pushes.
67113
[[branch-protections]]
68114
pattern = "automation/bors/auto"
69115
allowed-merge-apps = ["bors"]
116+
prevent-update = true
70117

71118
# Required for running auto builds created by bors.
72119
# Must support force-pushes.
73120
[[branch-protections]]
74121
pattern = "automation/bors/auto-merge"
75122
allowed-merge-apps = ["bors"]
123+
prevent-update = true
76124

77125
# Required for unrolled PR builds created by perfbot.
78126
# Must support force-pushes.
79127
[[branch-protections]]
80128
pattern = "try-perf"
81-
allowed-merge-apps = ["rust-timer"]
129+
allowed-merge-teams = ["rust-timer"]
82130
pr-required = false
83131

84132
# Required for unrolled PR builds created by perfbot.
85133
# Must support force-pushes.
86134
[[branch-protections]]
87135
pattern = "perf-tmp"
88-
allowed-merge-apps = ["rust-timer"]
136+
allowed-merge-teams = ["rust-timer"]
89137
pr-required = false
90138

91139
[environments.bors]

src/sync/github/mod.rs

Lines changed: 83 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use crate::sync::Config;
99
use crate::sync::github::api::{
1010
GithubRead, Login, PushAllowanceActor, RepoPermission, RepoSettings, Ruleset,
1111
};
12+
use anyhow::Context as _;
1213
use futures_util::StreamExt;
1314
use log::debug;
1415
use rust_team_data::v1::{Bot, BranchProtectionMode, MergeBot, ProtectionTarget};
@@ -370,6 +371,78 @@ impl SyncGitHub {
370371
self.config.enable_rulesets_repos.contains(&repo_full_name)
371372
}
372373

374+
async fn construct_ruleset(
375+
&self,
376+
expected_repo: &rust_team_data::v1::Repo,
377+
branch_protection: &rust_team_data::v1::BranchProtection,
378+
) -> anyhow::Result<api::Ruleset> {
379+
let bypass_actors = self.bypass_actors(expected_repo, branch_protection).await?;
380+
381+
Ok(construct_ruleset(branch_protection, bypass_actors))
382+
}
383+
384+
async fn bypass_actors(
385+
&self,
386+
expected_repo: &rust_team_data::v1::Repo,
387+
branch_protection: &rust_team_data::v1::BranchProtection,
388+
) -> Result<Vec<api::RulesetBypassActor>, anyhow::Error> {
389+
use api::{RulesetActorType, RulesetBypassActor, RulesetBypassMode};
390+
391+
let mut bypass_actors = Vec::new();
392+
let allowed_teams = self
393+
.allowed_merge_teams(expected_repo, branch_protection)
394+
.await?;
395+
bypass_actors.extend(allowed_teams);
396+
let allowed_apps = branch_protection
397+
.allowed_merge_apps
398+
.iter()
399+
.filter_map(|app| {
400+
app.app_id().map(|app_id| RulesetBypassActor {
401+
actor_id: app_id,
402+
actor_type: RulesetActorType::Integration,
403+
bypass_mode: RulesetBypassMode::Always,
404+
})
405+
});
406+
bypass_actors.extend(allowed_apps);
407+
Ok(bypass_actors)
408+
}
409+
410+
async fn allowed_merge_teams(
411+
&self,
412+
expected_repo: &rust_team_data::v1::Repo,
413+
branch_protection: &rust_team_data::v1::BranchProtection,
414+
) -> Result<Vec<api::RulesetBypassActor>, anyhow::Error> {
415+
use api::{RulesetActorType, RulesetBypassActor, RulesetBypassMode};
416+
417+
let mut allowed = vec![];
418+
419+
for team_name in &branch_protection.allowed_merge_teams {
420+
let github_team = self
421+
.github
422+
.team(&expected_repo.org, team_name)
423+
.await?
424+
.with_context(|| {
425+
format!(
426+
"failed to find GitHub team '{team_name}' in org '{}' for repo '{}/{}'",
427+
expected_repo.org, expected_repo.org, expected_repo.name
428+
)
429+
})?;
430+
let team_id = github_team.id.with_context(|| {
431+
format!(
432+
"GitHub team '{team_name}' in org '{}' is missing an ID",
433+
expected_repo.org
434+
)
435+
})?;
436+
437+
allowed.push(RulesetBypassActor {
438+
actor_id: team_id as i64,
439+
actor_type: RulesetActorType::Team,
440+
bypass_mode: RulesetBypassMode::Always,
441+
});
442+
}
443+
Ok(allowed)
444+
}
445+
373446
async fn diff_repo(
374447
&self,
375448
expected_repo: &rust_team_data::v1::Repo,
@@ -404,7 +477,9 @@ impl SyncGitHub {
404477
let use_rulesets = self.should_use_rulesets(expected_repo);
405478
if use_rulesets {
406479
for branch_protection in &expected_repo.branch_protections {
407-
let ruleset = construct_ruleset(branch_protection);
480+
let ruleset = self
481+
.construct_ruleset(expected_repo, branch_protection)
482+
.await?;
408483
rulesets.push(ruleset);
409484
}
410485
}
@@ -697,7 +772,9 @@ impl SyncGitHub {
697772

698773
// Process each branch protection as a potential ruleset
699774
for branch_protection in &expected_repo.branch_protections {
700-
let expected_ruleset = construct_ruleset(branch_protection);
775+
let expected_ruleset = self
776+
.construct_ruleset(expected_repo, branch_protection)
777+
.await?;
701778

702779
if let Some(actual_ruleset) = rulesets_by_name.remove(&expected_ruleset.name) {
703780
let Ruleset {
@@ -1015,7 +1092,10 @@ fn github_int(value: u32) -> i32 {
10151092
i32::try_from(value).unwrap_or_else(|_| panic!("Value {value} exceeds GitHub's Int range"))
10161093
}
10171094

1018-
pub fn construct_ruleset(branch_protection: &rust_team_data::v1::BranchProtection) -> api::Ruleset {
1095+
pub fn construct_ruleset(
1096+
branch_protection: &rust_team_data::v1::BranchProtection,
1097+
bypass_actors: Vec<api::RulesetBypassActor>,
1098+
) -> api::Ruleset {
10191099
use api::*;
10201100

10211101
let branch_protection_mode = get_branch_protection_mode(branch_protection);
@@ -1102,19 +1182,6 @@ pub fn construct_ruleset(branch_protection: &rust_team_data::v1::BranchProtectio
11021182
rules.insert(RulesetRule::MergeQueue { parameters });
11031183
}
11041184

1105-
// Build bypass actors from allowed merge apps
1106-
let bypass_actors: Vec<RulesetBypassActor> = branch_protection
1107-
.allowed_merge_apps
1108-
.iter()
1109-
.filter_map(|app| {
1110-
app.app_id().map(|app_id| RulesetBypassActor {
1111-
actor_id: app_id,
1112-
actor_type: RulesetActorType::Integration,
1113-
bypass_mode: RulesetBypassMode::Always,
1114-
})
1115-
})
1116-
.collect();
1117-
11181185
api::Ruleset {
11191186
id: None,
11201187
name: branch_protection

src/validate.rs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,12 +1150,22 @@ fn validate_branch_protections(data: &Data, errors: &mut Vec<String>) {
11501150

11511151
wrapper(data.repos(), errors, |repo, _| {
11521152
let bors_configured = repo.bots.iter().any(|b| matches!(b, Bot::Bors));
1153-
let mut patterns = HashSet::new();
1153+
let mut pattern_counts = HashMap::new();
11541154

11551155
for protection in &repo.branch_protections {
1156-
if !patterns.insert((protection.target, &protection.pattern)) {
1156+
*pattern_counts
1157+
.entry((protection.target, protection.pattern.as_str()))
1158+
.or_insert(0usize) += 1;
1159+
}
1160+
1161+
for protection in &repo.branch_protections {
1162+
let key = (protection.target, protection.pattern.as_str());
1163+
let occurrences = pattern_counts
1164+
.get(&key)
1165+
.with_context(|| format!("pattern_counts should contain the key {key:?}"))?;
1166+
if *occurrences > 1 && protection.name.is_none() {
11571167
bail!(
1158-
r#"repo '{}' uses multiple {:?} protections with the pattern `{}`"#,
1168+
r#"repo '{}' uses multiple {:?} protections with the pattern `{}`; when multiple protections share the same target and pattern, each protection must specify `name`"#,
11591169
repo.name,
11601170
protection.target,
11611171
protection.pattern,
@@ -1166,17 +1176,15 @@ fn validate_branch_protections(data: &Data, errors: &mut Vec<String>) {
11661176
let key = (repo.org.clone(), team.clone());
11671177
if !github_teams.contains(&key) {
11681178
bail!(
1169-
r#"repo '{}' uses a branch protection for {} that mentions the '{}' github team;
1170-
but that team does not seem to exist"#,
1179+
r#"repo '{}' uses a branch protection for {} that mentions the '{}' github team; but that team does not seem to exist"#,
11711180
repo.name,
11721181
protection.pattern,
11731182
team
11741183
);
11751184
}
11761185
if !repo.access.teams.contains_key(team) {
11771186
bail!(
1178-
r#"repo '{}' uses a branch protection for {} that has an allowed merge team '{}',
1179-
but that team is not mentioned in [access.teams]"#,
1187+
r#"repo '{}' uses a branch protection for {} that has an allowed merge team '{}', but that team is not mentioned in [access.teams]"#,
11801188
repo.name,
11811189
protection.pattern,
11821190
team

0 commit comments

Comments
 (0)