add org members allow list

marcoieni · marcoieni · commit 6a05230b29aa · 2025-07-15T10:57:41.000+02:00
diff --git a/config.toml b/config.toml
@@ -27,3 +27,29 @@ permissions-bools = [
     "dev-desktop",
     "sync-team-confirmation",
 ]
+
+# GitHub accounts that are allowed to stay in the orgs,
+# even if they may not be members of any team.
+allowed-org-members = [
+    # Bots.
+    "bors",
+    "craterbot",
+    "rust-embedded-bot",
+    "rust-highfive",
+    "rust-lang-core-team-agenda-bot",
+    "rust-lang-glacier-bot",
+    "rust-lang-owner",
+    "rust-timer",
+    "rustbot",
+
+    # Infra admins.
+    "jdno",
+    "marcoieni",
+    "Mark-Simulacrum",
+    "pietroalbini",
+
+    # Ferrous systems employees who manage the github sponsors
+    # for rust-analyzer.
+    "missholmes",
+    "skade",
+]
diff --git a/rust_team_data/src/v1.rs b/rust_team_data/src/v1.rs
@@ -1,8 +1,15 @@
+use std::collections::BTreeSet;
+
 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: BTreeSet<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
@@ -3,7 +3,7 @@ pub(crate) use crate::permissions::Permissions;
 use anyhow::{bail, format_err, Error};
 use serde::de::{Deserialize, Deserializer};
 use serde_untagged::UntaggedEnumVisitor;
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeSet, HashMap, HashSet};
 
 #[derive(serde_derive::Deserialize, Debug)]
 #[serde(deny_unknown_fields, rename_all = "kebab-case")]
@@ -12,6 +12,8 @@ pub(crate) struct Config {
     allowed_github_orgs: HashSet<String>,
     permissions_bors_repos: HashSet<String>,
     permissions_bools: HashSet<String>,
+    // Use a BTreeSet for consistent ordering in tests
+    allowed_org_members: BTreeSet<String>,
 }
 
 impl Config {
@@ -30,6 +32,10 @@ impl Config {
     pub(crate) fn permissions_bools(&self) -> &HashSet<String> {
         &self.permissions_bools
     }
+
+    pub(crate) fn allowed_org_members(&self) -> &BTreeSet<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,
@@ -90,7 +94,7 @@ impl SyncGitHub {
         })
     }
 
-    // Collect all org members from the respective teams
+    /// Collect all org members from the respective teams
     fn get_org_members_from_teams(&self) -> HashMap<OrgName, HashSet<u64>> {
         let mut org_team_members: HashMap<OrgName, HashSet<u64>> = HashMap::new();
 
@@ -107,7 +111,7 @@ impl SyncGitHub {
         org_team_members
     }
 
-    // Diff organization memberships between TOML teams and GitHub
+    /// Diff organization memberships between TOML teams and GitHub
     fn diff_org_memberships(&self) -> anyhow::Result<Vec<OrgMembershipDiff>> {
         let toml_org_team_members = self.get_org_members_from_teams();
 
@@ -118,11 +122,7 @@ impl SyncGitHub {
                 panic!("GitHub organization {org} not found");
             };
 
-            // Remove all members that are in TOML teams
-            let mut members_to_remove = gh_org_members.clone();
-            for member in toml_members {
-                members_to_remove.remove(&member);
-            }
+            let members_to_remove = self.members_to_remove(toml_members, gh_org_members);
 
             // The rest are members that should be removed
             if !members_to_remove.is_empty() {
@@ -142,6 +142,34 @@ impl SyncGitHub {
         Ok(org_diffs.into_values().collect())
     }
 
+    /// Return GitHub members that should be removed from the organization.
+    fn members_to_remove(
+        &self,
+        toml_members: HashSet<u64>,
+        gh_org_members: &HashMap<u64, String>,
+    ) -> HashMap<u64, String> {
+        // 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();
+
+        // People who belong to a team should stay in the org.
+        for member in toml_members {
+            members_to_remove.remove(&member);
+        }
+
+        // Members that are explicitly allowed in the `config.toml` file should stay in the org.
+        for allowed_member in &self.config.allowed_org_members {
+            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);
+            }
+        }
+        members_to_remove
+    }
+
     fn diff_teams(&self) -> anyhow::Result<Vec<TeamDiff>> {
         let mut diffs = Vec::new();
         let mut unseen_github_teams = HashMap::new();
@@ -735,7 +763,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",
+]
