Sync organization members

Author: me-diru

sync-team/src/github/api/read.rs

@@ -15,6 +15,9 @@ pub(crate) trait GithubRead {
     /// Get the owners of an org
     fn org_owners(&self, org: &str) -> anyhow::Result<HashSet<u64>>;
 
+    /// Get the members of an org
+    fn org_members(&self, org: &str) -> anyhow::Result<HashMap<u64, String>>;
+
     /// Get all teams associated with a org
     ///
     /// Returns a list of tuples of team name and slug
@@ -119,6 +122,26 @@ impl GithubRead for GitHubApiRead {
         Ok(owners)
     }
 
+    fn org_members(&self, org: &str) -> anyhow::Result<HashMap<u64, String>> {
+        #[derive(serde::Deserialize, Eq, PartialEq, Hash)]
+        struct User {
+            id: u64,
+            login: String,
+        }
+        let mut members = HashMap::new();
+        self.client.rest_paginated(
+            &Method::GET,
+            &GitHubUrl::orgs(org, "members")?,
+            |resp: Vec<User>| {
+                for user in resp {
+                    members.insert(user.id, user.login);
+                }
+                Ok(())
+            },
+        )?;
+        Ok(members)
+    }
+
     fn org_teams(&self, org: &str) -> anyhow::Result<Vec<(String, String)>> {
         let mut teams = Vec::new();
 

sync-team/src/github/api/write.rs

@@ -344,6 +344,18 @@ impl GitHubWrite {
         Ok(())
     }
 
+    /// Remove a member from an org
+    pub(crate) fn remove_gh_member_from_org(&self, org: &str, user: &str) -> anyhow::Result<()> {
+        debug!("Removing user {user} from org {org}");
+        if !self.dry_run {
+            let method = Method::DELETE;
+            let url = GitHubUrl::orgs(org, &format!("members/{user}"))?;
+            let resp = self.client.req(method.clone(), &url)?.send()?;
+            allow_not_found(resp, method, url.url())?;
+        }
+        Ok(())
+    }
+
     /// Remove a collaborator from a repo
     pub(crate) fn remove_collaborator_from_repo(
         &self,

sync-team/src/github/mod.rs

@@ -6,7 +6,7 @@ use self::api::{BranchProtectionOp, TeamPrivacy, TeamRole};
 use crate::github::api::{GithubRead, Login, PushAllowanceActor, RepoPermission, RepoSettings};
 use log::debug;
 use rust_team_data::v1::{Bot, BranchProtectionMode, MergeBot};
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeMap, HashMap, HashSet};
 use std::fmt::Write;
 
 pub(crate) use self::api::{GitHubApiRead, GitHubWrite, HttpClient};
@@ -31,6 +31,7 @@ struct SyncGitHub {
     repos: Vec<rust_team_data::v1::Repo>,
     usernames_cache: HashMap<u64, String>,
     org_owners: HashMap<OrgName, HashSet<u64>>,
+    org_members: HashMap<OrgName, HashMap<u64, String>>,
 }
 
 impl SyncGitHub {
@@ -60,9 +61,11 @@ impl SyncGitHub {
             .collect::<HashSet<_>>();
 
         let mut org_owners = HashMap::new();
+        let mut org_members = HashMap::new();
 
         for org in &orgs {
             org_owners.insert((*org).to_string(), github.org_owners(org)?);
+            org_members.insert((*org).to_string(), github.org_members(org)?);
         }
 
         Ok(SyncGitHub {
@@ -71,19 +74,74 @@ impl SyncGitHub {
             repos,
             usernames_cache,
             org_owners,
+            org_members,
         })
     }
 
     pub(crate) fn diff_all(&self) -> anyhow::Result<Diff> {
         let team_diffs = self.diff_teams()?;
         let repo_diffs = self.diff_repos()?;
+        let org_membership_diffs = self.diff_org_memberships()?;
 
         Ok(Diff {
             team_diffs,
             repo_diffs,
+            org_membership_diffs,
         })
     }
 
+    // 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();
+
+        for team in &self.teams {
+            if let Some(gh) = &team.github {
+                for toml_gh_team in &gh.teams {
+                    org_team_members
+                        .entry(toml_gh_team.org.clone())
+                        .or_default()
+                        .extend(toml_gh_team.members.iter().copied());
+                }
+            }
+        }
+        org_team_members
+    }
+
+    // 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();
+
+        let mut org_diffs: BTreeMap<String, OrgMembershipDiff> = BTreeMap::new();
+
+        for (org, toml_members) in toml_org_team_members {
+            let Some(gh_org_members) = self.org_members.get(&org) else {
+                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);
+            }
+
+            // 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();
+                members_to_remove.sort();
+
+                org_diffs.insert(
+                    org.clone(),
+                    OrgMembershipDiff {
+                        org,
+                        members_to_remove,
+                    },
+                );
+            }
+        }
+
+        Ok(org_diffs.into_values().collect())
+    }
+
     fn diff_teams(&self) -> anyhow::Result<Vec<TeamDiff>> {
         let mut diffs = Vec::new();
         let mut unseen_github_teams = HashMap::new();
@@ -584,6 +642,7 @@ const BOTS_TEAMS: &[&str] = &["bors", "highfive", "rfcbot", "bots"];
 pub(crate) struct Diff {
     team_diffs: Vec<TeamDiff>,
     repo_diffs: Vec<RepoDiff>,
+    org_membership_diffs: Vec<OrgMembershipDiff>,
 }
 
 impl Diff {
@@ -595,12 +654,17 @@ impl Diff {
         for repo_diff in self.repo_diffs {
             repo_diff.apply(sync)?;
         }
+        for org_diff in self.org_membership_diffs {
+            org_diff.apply(sync)?;
+        }
 
         Ok(())
     }
 
     pub(crate) fn is_empty(&self) -> bool {
-        self.team_diffs.is_empty() && self.repo_diffs.is_empty()
+        self.team_diffs.is_empty()
+            && self.repo_diffs.is_empty()
+            && self.org_membership_diffs.is_empty()
     }
 }
 
@@ -620,6 +684,13 @@ impl std::fmt::Display for Diff {
             }
         }
 
+        if !&self.org_membership_diffs.is_empty() {
+            writeln!(f, "💻 Org membership Diffs:")?;
+            for org_diff in &self.org_membership_diffs {
+                write!(f, "{org_diff}")?;
+            }
+        }
+
         Ok(())
     }
 }
@@ -655,6 +726,34 @@ impl std::fmt::Display for RepoDiff {
     }
 }
 
+#[derive(Debug)]
+struct OrgMembershipDiff {
+    org: OrgName,
+    members_to_remove: Vec<String>,
+}
+
+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)?;
+        }
+
+        Ok(())
+    }
+}
+
+impl std::fmt::Display for OrgMembershipDiff {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        if !self.members_to_remove.is_empty() {
+            writeln!(f, "❌ Removing the following members from `{}`:", self.org)?;
+            for member in &self.members_to_remove {
+                writeln!(f, "  - {member}",)?;
+            }
+        }
+        Ok(())
+    }
+}
+
 #[derive(Debug)]
 struct CreateRepoDiff {
     org: String,

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

@@ -97,6 +97,28 @@ fn team_dont_add_member_if_invitation_is_pending() {
     insta::assert_debug_snapshot!(team_diff, @"[]");
 }
 
+#[test]
+fn remove_org_members() {
+    let mut model = DataModel::default();
+    let user = model.create_user("sakura");
+    model.create_team(TeamData::new("team-1").gh_team("rust-lang", "members-gh", &[user]));
+    let mut gh = model.gh_model();
+    gh.add_member("rust-lang", "martin");
+
+    let gh_org_diff = model.diff_org_membership(gh);
+
+    insta::assert_debug_snapshot!(gh_org_diff, @r#"
+    [
+        OrgMembershipDiff {
+            org: "rust-lang",
+            members_to_remove: [
+                "martin",
+            ],
+        },
+    ]
+    "#);
+}
+
 #[test]
 fn team_remove_member() {
     let mut model = DataModel::default();

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

@@ -10,7 +10,8 @@ use crate::github::api::{
     BranchProtection, GithubRead, Repo, RepoTeam, RepoUser, Team, TeamMember, TeamPrivacy, TeamRole,
 };
 use crate::github::{
-    RepoDiff, SyncGitHub, TeamDiff, api, construct_branch_protection, convert_permission,
+    OrgMembershipDiff, RepoDiff, SyncGitHub, TeamDiff, api, construct_branch_protection,
+    convert_permission,
 };
 
 pub const DEFAULT_ORG: &str = "rust-lang";
@@ -105,7 +106,13 @@ impl DataModel {
                     slug: gh_team.name.clone(),
                 });
 
-                org.members.extend(gh_team.members.iter().copied());
+                org.members.extend(
+                    gh_team
+                        .members
+                        .iter()
+                        .copied()
+                        .map(|user_id| (user_id, users[&user_id].clone())),
+                );
             }
         }
 
@@ -168,6 +175,12 @@ impl DataModel {
         GithubMock { users, orgs }
     }
 
+    pub fn diff_org_membership(&self, github: GithubMock) -> Vec<OrgMembershipDiff> {
+        self.create_sync(github)
+            .diff_org_memberships()
+            .expect("Cannot diff org membership")
+    }
+
     pub fn diff_teams(&self, github: GithubMock) -> Vec<TeamDiff> {
         self.create_sync(github)
             .diff_teams()
@@ -418,6 +431,16 @@ pub struct GithubMock {
 }
 
 impl GithubMock {
+    pub fn add_member(&mut self, org: &str, username: &str) {
+        let user_id = self.users.len() as UserId;
+        self.users.insert(user_id, username.to_string());
+        self.orgs
+            .get_mut(org)
+            .unwrap()
+            .members
+            .insert((user_id, username.to_string()));
+    }
+
     pub fn add_invitation(&mut self, org: &str, repo: &str, user: &str) {
         self.get_org_mut(org)
             .team_invitations
@@ -457,6 +480,10 @@ impl GithubRead for GithubMock {
         Ok(self.get_org(org).owners.iter().copied().collect())
     }
 
+    fn org_members(&self, org: &str) -> anyhow::Result<HashMap<u64, String>> {
+        Ok(self.get_org(org).members.iter().cloned().collect())
+    }
+
     fn org_teams(&self, org: &str) -> anyhow::Result<Vec<(String, String)>> {
         Ok(self
             .get_org(org)
@@ -549,7 +576,7 @@ impl GithubRead for GithubMock {
 
 #[derive(Default)]
 struct GithubOrg {
-    members: BTreeSet<UserId>,
+    members: BTreeSet<(UserId, String)>,
     owners: BTreeSet<UserId>,
     teams: Vec<Team>,
     // Team name -> list of invited users