Skip to content

Commit

Permalink
Allow specifying push allowances for branch protection (#1282)
Browse files Browse the repository at this point in the history
* Allow specifying push allowances for branch protection

This enables to allow only some teams to push/merge to specific branches, to make branch protection more fine-grained.

* Validate that the option is not combined with bors

* Validate that the mentioned GitHub team exists
  • Loading branch information
Kobzol authored Jan 23, 2024
1 parent c2bb14a commit 1a4ef20
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 9 deletions.
5 changes: 5 additions & 0 deletions docs/toml-schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,4 +306,9 @@ dismiss-stale-review = false
# This option is only relevant if bors is not used.
# (optional - default `1`)
required-approvals = 1
# Which GitHub teams have access to push/merge to this branch.
# If unspecified, all teams/contributors with write or higher access
# can push/merge to the branch.
# (optional)
allowed-merge-teams = ["awesome-team"]
```
1 change: 1 addition & 0 deletions rust_team_data/src/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ pub struct BranchProtection {
pub ci_checks: Vec<String>,
pub dismiss_stale_review: bool,
pub required_approvals: u32,
pub allowed_merge_teams: Vec<String>,
}

#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
Expand Down
2 changes: 2 additions & 0 deletions src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -782,4 +782,6 @@ pub(crate) struct BranchProtection {
pub dismiss_stale_review: bool,
#[serde(default)]
pub required_approvals: Option<u32>,
#[serde(default)]
pub allowed_merge_teams: Vec<String>,
}
1 change: 1 addition & 0 deletions src/static_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ impl<'a> Generator<'a> {
ci_checks: b.ci_checks.clone(),
dismiss_stale_review: b.dismiss_stale_review,
required_approvals: b.required_approvals.unwrap_or(1),
allowed_merge_teams: b.allowed_merge_teams.clone(),
})
.collect();
let repo = v1::Repo {
Expand Down
37 changes: 31 additions & 6 deletions src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -778,16 +778,41 @@ fn validate_repos(data: &Data, errors: &mut Vec<String>) {

/// Validate that branch protections make sense in combination with used bots.
fn validate_branch_protections(data: &Data, errors: &mut Vec<String>) {
let github_teams = data.github_teams();

wrapper(data.repos(), errors, |repo, _| {
let bors_used = repo.bots.iter().any(|b| matches!(b, Bot::Bors));
for protection in &repo.branch_protections {
if bors_used && protection.required_approvals.is_some() {
bail!(
r#"repo '{}' uses bors and its branch protection for {} uses the `required-approvals` attribute;
for team in &protection.allowed_merge_teams {
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"#,
repo.name,
protection.pattern,
team
);
}
}

if bors_used {
if protection.required_approvals.is_some() {
bail!(
r#"repo '{}' uses bors and its branch protection for {} uses the `required-approvals` attribute;
please remove the attribute when using bors"#,
repo.name,
protection.pattern,
);
repo.name,
protection.pattern,
);
}
if !protection.allowed_merge_teams.is_empty() {
bail!(
r#"repo '{}' uses bors and its branch protection for {} uses the `allowed-merge-teams` attribute;
please remove the attribute when using bors"#,
repo.name,
protection.pattern,
);
}
}
}
Ok(())
Expand Down
5 changes: 4 additions & 1 deletion tests/static-api/_expected/v1/repos.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
"CI"
],
"dismiss_stale_review": false,
"required_approvals": 1
"required_approvals": 1,
"allowed_merge_teams": [
"foo"
]
}
]
}
Expand Down
5 changes: 4 additions & 1 deletion tests/static-api/_expected/v1/repos/some_repo.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
"CI"
],
"dismiss_stale_review": false,
"required_approvals": 1
"required_approvals": 1,
"allowed_merge_teams": [
"foo"
]
}
]
}
3 changes: 2 additions & 1 deletion tests/static-api/repos/test-org/some_repo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ foo = "admin"

[[branch-protections]]
pattern = "master"
ci-checks = ["CI"]
ci-checks = ["CI"]
allowed-merge-teams = ["foo"]

0 comments on commit 1a4ef20

Please sign in to comment.