add org members allow list

marcoieni · marcoieni · commit ebdec801bc3a · 2025-07-15T09:13:20.000+02:00
diff --git a/config.toml b/config.toml
@@ -27,3 +27,23 @@ permissions-bools = [
     "dev-desktop",
     "sync-team-confirmation",
 ]
+
+# GitHub accounts that are allowed to stay in the orgs,
+# even if they are not members of any team.
+allowed-org-members = [
+    # Bots.
+    "bors",
+    "craterbot",
+    "rust-lang-owner",
+
+    # Infra admins.
+    "marcoieni",
+    "pietroalbini",
+    "jdno",
+    "Mark-Simulacrum",
+
+    # Ferrous systems employees who manage the github sponsors
+    # for rust-analyzer.
+    "skade",
+    "missholmes",
+]
diff --git a/rust_team_data/src/v1.rs b/rust_team_data/src/v1.rs
@@ -1,8 +1,15 @@
+use std::collections::HashSet;
+
 use indexmap::IndexMap;
 use serde::{Deserialize, Serialize};
 
 pub static BASE_URL: &str = "https://team-api.infra.rust-lang.org/v1";
 
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Default)]
+pub struct Config {
+    pub allowed_org_members: HashSet<String>,
+}
+
 #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq)]
 #[serde(rename_all = "snake_case")]
 pub enum TeamKind {
diff --git a/src/schema.rs b/src/schema.rs
@@ -12,6 +12,7 @@ pub(crate) struct Config {
     allowed_github_orgs: HashSet<String>,
     permissions_bors_repos: HashSet<String>,
     permissions_bools: HashSet<String>,
+    allowed_org_members: HashSet<String>,
 }
 
 impl Config {
@@ -30,6 +31,10 @@ impl Config {
     pub(crate) fn permissions_bools(&self) -> &HashSet<String> {
         &self.permissions_bools
     }
+
+    pub(crate) fn allowed_org_members(&self) -> &HashSet<String> {
+        &self.allowed_org_members
+    }
 }
 
 // This is an enum to allow two kinds of values for the email field:
diff --git a/src/static_api.rs b/src/static_api.rs
@@ -33,6 +33,7 @@ impl<'a> Generator<'a> {
         self.generate_rfcbot()?;
         self.generate_zulip_map()?;
         self.generate_people()?;
+        self.generate_config()?;
         self.generate_index_html()?;
         Ok(())
     }
@@ -458,6 +459,15 @@ impl<'a> Generator<'a> {
         Ok(())
     }
 
+    fn generate_config(&self) -> Result<(), Error> {
+        let config = v1::Config {
+            allowed_org_members: self.data.config().allowed_org_members().clone(),
+        };
+
+        self.add("v1/config.json", &config)?;
+        Ok(())
+    }
+
     fn generate_index_html(&self) -> Result<(), Error> {
         const CONTENT: &[u8] = b"\
             <!DOCTYPE html>\n\
diff --git a/sync-team/src/github/mod.rs b/sync-team/src/github/mod.rs
@@ -18,8 +18,9 @@ pub(crate) fn create_diff(
     github: Box<dyn GithubRead>,
     teams: Vec<rust_team_data::v1::Team>,
     repos: Vec<rust_team_data::v1::Repo>,
+    config: rust_team_data::v1::Config,
 ) -> anyhow::Result<Diff> {
-    let github = SyncGitHub::new(github, teams, repos)?;
+    let github = SyncGitHub::new(github, teams, repos, config)?;
     github.diff_all()
 }
 
@@ -29,6 +30,7 @@ struct SyncGitHub {
     github: Box<dyn GithubRead>,
     teams: Vec<rust_team_data::v1::Team>,
     repos: Vec<rust_team_data::v1::Repo>,
+    config: rust_team_data::v1::Config,
     usernames_cache: HashMap<u64, String>,
     org_owners: HashMap<OrgName, HashSet<u64>>,
     org_members: HashMap<OrgName, HashMap<u64, String>>,
@@ -39,6 +41,7 @@ impl SyncGitHub {
         github: Box<dyn GithubRead>,
         teams: Vec<rust_team_data::v1::Team>,
         repos: Vec<rust_team_data::v1::Repo>,
+        config: rust_team_data::v1::Config,
     ) -> anyhow::Result<Self> {
         debug!("caching mapping between user ids and usernames");
         let users = teams
@@ -72,6 +75,7 @@ impl SyncGitHub {
             github,
             teams,
             repos,
+            config,
             usernames_cache,
             org_owners,
             org_members,
@@ -118,12 +122,29 @@ impl SyncGitHub {
                 panic!("GitHub organization {org} not found");
             };
 
-            // Remove all members that are in TOML teams
+            // Initialize `members_to_remove` to all GitHub members in the org.
+            // Next, we'll delete members from `members_to_remove` that don't respect certain criteria.
             let mut members_to_remove = gh_org_members.clone();
+
+            // Remove all members that are in TOML teams.
+            // We want people who belong to a team to stay in the org.
             for member in toml_members {
                 members_to_remove.remove(&member);
             }
 
+            // Remove members that are explicitly allowed by config.
+            // These are usually bots or admins that should stay in the org.
+            for allowed_member in &self.config.allowed_org_members {
+                // Look for the member by username in the members_to_remove map
+                if let Some(member_to_retain) = members_to_remove
+                    .iter()
+                    .find(|(_, username)| username == &allowed_member)
+                    .map(|(id, _)| *id)
+                {
+                    members_to_remove.remove(&member_to_retain);
+                }
+            }
+
             // The rest are members that should be removed
             if !members_to_remove.is_empty() {
                 let mut members_to_remove: Vec<String> = members_to_remove.into_values().collect();
@@ -735,7 +756,7 @@ struct OrgMembershipDiff {
 impl OrgMembershipDiff {
     fn apply(self, sync: &GitHubWrite) -> anyhow::Result<()> {
         for member in &self.members_to_remove {
-            sync.remove_gh_member_from_org(&self.org, &member)?;
+            sync.remove_gh_member_from_org(&self.org, member)?;
         }
 
         Ok(())
diff --git a/sync-team/src/github/tests/mod.rs b/sync-team/src/github/tests/mod.rs
@@ -105,6 +105,11 @@ fn remove_org_members() {
     let mut gh = model.gh_model();
     gh.add_member("rust-lang", "martin");
 
+    // Add a bot that shouldn't be removed from the org.
+    let bot = "my-bot";
+    gh.add_member("rust-lang", bot);
+    model.add_allowed_org_member(bot);
+
     let gh_org_diff = model.diff_org_membership(gh);
 
     insta::assert_debug_snapshot!(gh_org_diff, @r#"
diff --git a/sync-team/src/github/tests/test_utils.rs b/sync-team/src/github/tests/test_utils.rs
@@ -1,9 +1,9 @@
 use std::collections::{BTreeSet, HashMap, HashSet};
 
 use derive_builder::Builder;
-use rust_team_data::v1;
 use rust_team_data::v1::{
-    Bot, BranchProtectionMode, GitHubTeam, MergeBot, Person, RepoPermission, TeamGitHub, TeamKind,
+    self, Bot, BranchProtectionMode, Config, GitHubTeam, MergeBot, Person, RepoPermission,
+    TeamGitHub, TeamKind,
 };
 
 use crate::github::api::{
@@ -28,6 +28,7 @@ pub struct DataModel {
     people: Vec<Person>,
     teams: Vec<TeamData>,
     repos: Vec<RepoData>,
+    config: Config,
 }
 
 impl DataModel {
@@ -65,6 +66,10 @@ impl DataModel {
             .expect("Repo not found")
     }
 
+    pub fn add_allowed_org_member(&mut self, member: &str) {
+        self.config.allowed_org_members.insert(member.to_string());
+    }
+
     /// Creates a GitHub model from the current team data mock.
     /// Note that all users should have been created before calling this method, so that
     /// GitHub knows about the users' existence.
@@ -196,8 +201,9 @@ impl DataModel {
     fn create_sync(&self, github: GithubMock) -> SyncGitHub {
         let teams = self.teams.iter().cloned().map(|t| t.into()).collect();
         let repos = self.repos.iter().cloned().map(|r| r.into()).collect();
+        let config = self.config.clone();
 
-        SyncGitHub::new(Box::new(github), teams, repos).expect("Cannot create SyncGitHub")
+        SyncGitHub::new(Box::new(github), teams, repos, config).expect("Cannot create SyncGitHub")
     }
 }
 
diff --git a/sync-team/src/lib.rs b/sync-team/src/lib.rs
@@ -31,7 +31,8 @@ pub fn run_sync_team(
                 let gh_read = Box::new(GitHubApiRead::from_client(client.clone())?);
                 let teams = team_api.get_teams()?;
                 let repos = team_api.get_repos()?;
-                let diff = create_diff(gh_read, teams, repos)?;
+                let config = team_api.get_config()?;
+                let diff = create_diff(gh_read, teams, repos, config)?;
                 if !diff.is_empty() {
                     info!("{diff}");
                 }
diff --git a/sync-team/src/team_api.rs b/sync-team/src/team_api.rs
@@ -47,6 +47,11 @@ impl TeamApi {
         self.req::<rust_team_data::v1::ZulipStreams>("zulip-streams.json")
     }
 
+    pub(crate) fn get_config(&self) -> anyhow::Result<rust_team_data::v1::Config> {
+        debug!("loading config from the Team API");
+        self.req::<rust_team_data::v1::Config>("config.json")
+    }
+
     fn req<T: serde::de::DeserializeOwned>(&self, url: &str) -> anyhow::Result<T> {
         match self {
             TeamApi::Production => {
diff --git a/tests/static-api/_expected/v1/config.json b/tests/static-api/_expected/v1/config.json
@@ -0,0 +1,6 @@
+{
+  "allowed_org_members": [
+    "test-admin",
+    "test-bot"
+  ]
+}
diff --git a/tests/static-api/config.toml b/tests/static-api/config.toml
@@ -14,3 +14,8 @@ permissions-bors-repos = [
 permissions-bools = [
     "crater",
 ]
+
+allowed-org-members = [
+    "test-bot",
+    "test-admin",
+]

Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,7 @@ pub(crate) struct Config {`
`12`	`12`	`allowed_github_orgs: HashSet<String>,`
`13`	`13`	`permissions_bors_repos: HashSet<String>,`
`14`	`14`	`permissions_bools: HashSet<String>,`
	`15`	`+ allowed_org_members: HashSet<String>,`
`15`	`16`	`}`
`16`	`17`
`17`	`18`	`impl Config {`
`@@ -30,6 +31,10 @@ impl Config {`
`30`	`31`	`pub(crate) fn permissions_bools(&self) -> &HashSet<String> {`
`31`	`32`	`&self.permissions_bools`
`32`	`33`	`}`
	`34`	`+`
	`35`	`+ pub(crate) fn allowed_org_members(&self) -> &HashSet<String> {`
	`36`	`+ &self.allowed_org_members`
	`37`	`+ }`
`33`	`38`	`}`
`34`	`39`
`35`	`40`	`// This is an enum to allow two kinds of values for the email field:`