Skip to content

Commit b8a431b

Browse files
marcoienimarcoieni
committed
add org members allow list
1 parent 3e935a6 commit b8a431b

File tree

11 files changed

+115
-11
lines changed

11 files changed

+115
-11
lines changed

‎config.toml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,29 @@ permissions-bools = [
2727
"dev-desktop",
2828
"sync-team-confirmation",
2929
]
30+
31+
# GitHub accounts that are allowed to stay in the orgs,
32+
# even if they are not members of any team.
33+
allowed-org-members = [
34+
# Bots.
35+
"bors",
36+
"craterbot",
37+
"rust-embedded-bot",
38+
"rust-highfive",
39+
"rust-lang-core-team-agenda-bot",
40+
"rust-lang-glacier-bot",
41+
"rust-lang-owner",
42+
"rust-timer",
43+
"rustbot",
44+
45+
# Infra admins.
46+
"jdno",
47+
"marcoieni",
48+
"Mark-Simulacrum",
49+
"pietroalbini",
50+
51+
# Ferrous systems employees who manage the github sponsors
52+
# for rust-analyzer.
53+
"missholmes",
54+
"skade",
55+
]

‎rust_team_data/src/v1.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
1+
use std::collections::HashSet;
2+
13
use indexmap::IndexMap;
24
use serde::{Deserialize, Serialize};
35

46
pub static BASE_URL: &str = "https://team-api.infra.rust-lang.org/v1";
57

8+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Default)]
9+
pub struct Config {
10+
pub allowed_org_members: HashSet<String>,
11+
}
12+
613
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq)]
714
#[serde(rename_all = "snake_case")]
815
pub enum TeamKind {

‎src/schema.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ pub(crate) struct Config {
1212
allowed_github_orgs: HashSet<String>,
1313
permissions_bors_repos: HashSet<String>,
1414
permissions_bools: HashSet<String>,
15+
allowed_org_members: HashSet<String>,
1516
}
1617

1718
impl Config {
@@ -30,6 +31,10 @@ impl Config {
3031
pub(crate) fn permissions_bools(&self) -> &HashSet<String> {
3132
&self.permissions_bools
3233
}
34+
35+
pub(crate) fn allowed_org_members(&self) -> &HashSet<String> {
36+
&self.allowed_org_members
37+
}
3338
}
3439

3540
// This is an enum to allow two kinds of values for the email field:

‎src/static_api.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ impl<'a> Generator<'a> {
3333
self.generate_rfcbot()?;
3434
self.generate_zulip_map()?;
3535
self.generate_people()?;
36+
self.generate_config()?;
3637
self.generate_index_html()?;
3738
Ok(())
3839
}
@@ -458,6 +459,15 @@ impl<'a> Generator<'a> {
458459
Ok(())
459460
}
460461

462+
fn generate_config(&self) -> Result<(), Error> {
463+
let config = v1::Config {
464+
allowed_org_members: self.data.config().allowed_org_members().clone(),
465+
};
466+
467+
self.add("v1/config.json", &config)?;
468+
Ok(())
469+
}
470+
461471
fn generate_index_html(&self) -> Result<(), Error> {
462472
const CONTENT: &[u8] = b"\
463473
<!DOCTYPE html>\n\

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

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ pub(crate) fn create_diff(
1818
github: Box<dyn GithubRead>,
1919
teams: Vec<rust_team_data::v1::Team>,
2020
repos: Vec<rust_team_data::v1::Repo>,
21+
config: rust_team_data::v1::Config,
2122
) -> anyhow::Result<Diff> {
22-
let github = SyncGitHub::new(github, teams, repos)?;
23+
let github = SyncGitHub::new(github, teams, repos, config)?;
2324
github.diff_all()
2425
}
2526

@@ -29,6 +30,7 @@ struct SyncGitHub {
2930
github: Box<dyn GithubRead>,
3031
teams: Vec<rust_team_data::v1::Team>,
3132
repos: Vec<rust_team_data::v1::Repo>,
33+
config: rust_team_data::v1::Config,
3234
usernames_cache: HashMap<u64, String>,
3335
org_owners: HashMap<OrgName, HashSet<u64>>,
3436
org_members: HashMap<OrgName, HashMap<u64, String>>,
@@ -39,6 +41,7 @@ impl SyncGitHub {
3941
github: Box<dyn GithubRead>,
4042
teams: Vec<rust_team_data::v1::Team>,
4143
repos: Vec<rust_team_data::v1::Repo>,
44+
config: rust_team_data::v1::Config,
4245
) -> anyhow::Result<Self> {
4346
debug!("caching mapping between user ids and usernames");
4447
let users = teams
@@ -72,6 +75,7 @@ impl SyncGitHub {
7275
github,
7376
teams,
7477
repos,
78+
config,
7579
usernames_cache,
7680
org_owners,
7781
org_members,
@@ -118,11 +122,7 @@ impl SyncGitHub {
118122
panic!("GitHub organization {org} not found");
119123
};
120124

121-
// Remove all members that are in TOML teams
122-
let mut members_to_remove = gh_org_members.clone();
123-
for member in toml_members {
124-
members_to_remove.remove(&member);
125-
}
125+
let members_to_remove = self.members_to_remove(toml_members, gh_org_members);
126126

127127
// The rest are members that should be removed
128128
if !members_to_remove.is_empty() {
@@ -142,6 +142,34 @@ impl SyncGitHub {
142142
Ok(org_diffs.into_values().collect())
143143
}
144144

145+
/// Return GitHub members that should be removed from the organization.
146+
fn members_to_remove(
147+
&self,
148+
toml_members: HashSet<u64>,
149+
gh_org_members: &HashMap<u64, String>,
150+
) -> HashMap<u64, String> {
151+
// Initialize `members_to_remove` to all GitHub members in the org.
152+
// Next, we'll delete members from `members_to_remove` that don't respect certain criteria.
153+
let mut members_to_remove = gh_org_members.clone();
154+
155+
// People who belong to a team should stay in the org.
156+
for member in toml_members {
157+
members_to_remove.remove(&member);
158+
}
159+
160+
// Members that are explicitly allowed in the `config.toml` file should stay in the org.
161+
for allowed_member in &self.config.allowed_org_members {
162+
if let Some(member_to_retain) = members_to_remove
163+
.iter()
164+
.find(|(_, username)| username == &allowed_member)
165+
.map(|(id, _)| *id)
166+
{
167+
members_to_remove.remove(&member_to_retain);
168+
}
169+
}
170+
members_to_remove
171+
}
172+
145173
fn diff_teams(&self) -> anyhow::Result<Vec<TeamDiff>> {
146174
let mut diffs = Vec::new();
147175
let mut unseen_github_teams = HashMap::new();
@@ -735,7 +763,7 @@ struct OrgMembershipDiff {
735763
impl OrgMembershipDiff {
736764
fn apply(self, sync: &GitHubWrite) -> anyhow::Result<()> {
737765
for member in &self.members_to_remove {
738-
sync.remove_gh_member_from_org(&self.org, &member)?;
766+
sync.remove_gh_member_from_org(&self.org, member)?;
739767
}
740768

741769
Ok(())

‎sync-team/src/github/tests/mod.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ fn remove_org_members() {
105105
let mut gh = model.gh_model();
106106
gh.add_member("rust-lang", "martin");
107107

108+
// Add a bot that shouldn't be removed from the org.
109+
let bot = "my-bot";
110+
gh.add_member("rust-lang", bot);
111+
model.add_allowed_org_member(bot);
112+
108113
let gh_org_diff = model.diff_org_membership(gh);
109114

110115
insta::assert_debug_snapshot!(gh_org_diff, @r#"

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
use std::collections::{BTreeSet, HashMap, HashSet};
22

33
use derive_builder::Builder;
4-
use rust_team_data::v1;
54
use rust_team_data::v1::{
6-
Bot, BranchProtectionMode, GitHubTeam, MergeBot, Person, RepoPermission, TeamGitHub, TeamKind,
5+
self, Bot, BranchProtectionMode, Config, GitHubTeam, MergeBot, Person, RepoPermission,
6+
TeamGitHub, TeamKind,
77
};
88

99
use crate::github::api::{
@@ -28,6 +28,7 @@ pub struct DataModel {
2828
people: Vec<Person>,
2929
teams: Vec<TeamData>,
3030
repos: Vec<RepoData>,
31+
config: Config,
3132
}
3233

3334
impl DataModel {
@@ -65,6 +66,10 @@ impl DataModel {
6566
.expect("Repo not found")
6667
}
6768

69+
pub fn add_allowed_org_member(&mut self, member: &str) {
70+
self.config.allowed_org_members.insert(member.to_string());
71+
}
72+
6873
/// Creates a GitHub model from the current team data mock.
6974
/// Note that all users should have been created before calling this method, so that
7075
/// GitHub knows about the users' existence.
@@ -196,8 +201,9 @@ impl DataModel {
196201
fn create_sync(&self, github: GithubMock) -> SyncGitHub {
197202
let teams = self.teams.iter().cloned().map(|t| t.into()).collect();
198203
let repos = self.repos.iter().cloned().map(|r| r.into()).collect();
204+
let config = self.config.clone();
199205

200-
SyncGitHub::new(Box::new(github), teams, repos).expect("Cannot create SyncGitHub")
206+
SyncGitHub::new(Box::new(github), teams, repos, config).expect("Cannot create SyncGitHub")
201207
}
202208
}
203209

‎sync-team/src/lib.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ pub fn run_sync_team(
3131
let gh_read = Box::new(GitHubApiRead::from_client(client.clone())?);
3232
let teams = team_api.get_teams()?;
3333
let repos = team_api.get_repos()?;
34-
let diff = create_diff(gh_read, teams, repos)?;
34+
let config = team_api.get_config()?;
35+
let diff = create_diff(gh_read, teams, repos, config)?;
3536
if !diff.is_empty() {
3637
info!("{diff}");
3738
}

‎sync-team/src/team_api.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ impl TeamApi {
4747
self.req::<rust_team_data::v1::ZulipStreams>("zulip-streams.json")
4848
}
4949

50+
pub(crate) fn get_config(&self) -> anyhow::Result<rust_team_data::v1::Config> {
51+
debug!("loading config from the Team API");
52+
self.req::<rust_team_data::v1::Config>("config.json")
53+
}
54+
5055
fn req<T: serde::de::DeserializeOwned>(&self, url: &str) -> anyhow::Result<T> {
5156
match self {
5257
TeamApi::Production => {

‎tests/static-api/_expected/v1/config.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"allowed_org_members": [
3+
"test-admin",
4+
"test-bot"
5+
]
6+
}

0 commit comments

Comments
 (0)