Add functionality to sync org members on GitHub

Signed-off-by: Rohit Dandamudi <[email protected]>

src/github/api/read.rs

@@ -13,6 +13,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<HashSet<u64>>;
+
     /// Get the app installations of an org
     fn org_app_installations(&self, org: &str) -> anyhow::Result<Vec<OrgAppInstallation>>;
 
@@ -120,6 +123,23 @@ impl GithubRead for GitHubApiRead {
         Ok(owners)
     }
 
+    fn org_members(&self, org: &str) -> anyhow::Result<HashSet<u64>> {
+        #[derive(serde::Deserialize, Eq, PartialEq, Hash)]
+        struct User {
+            id: u64,
+        }
+        let mut members = HashSet::new();
+        self.client.rest_paginated(
+            &Method::GET,
+            format!("orgs/{org}/members"),
+            |resp: Vec<User>| {
+                members.extend(resp.into_iter().map(|u| u.id));
+                Ok(())
+            },
+        )?;
+        Ok(members)
+    }
+
     fn org_app_installations(&self, org: &str) -> anyhow::Result<Vec<OrgAppInstallation>> {
         #[derive(serde::Deserialize, Debug)]
         struct InstallationPage {

src/github/api/write.rs

@@ -375,6 +375,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 = &format!("orgs/{org}/members/{user}");
+            let resp = self.client.req(method.clone(), url)?.send()?;
+            allow_not_found(resp, method, url)?;
+        }
+        Ok(())
+    }
+
     /// Remove a collaborator from a repo
     pub(crate) fn remove_collaborator_from_repo(
         &self,

src/github/mod.rs

@@ -4,8 +4,9 @@ mod tests;
 
 use self::api::{BranchProtectionOp, TeamPrivacy, TeamRole};
 use crate::github::api::{GithubRead, Login, PushAllowanceActor, RepoPermission, RepoSettings};
+use anyhow::anyhow;
 use log::debug;
-use rust_team_data::v1::{Bot, BranchProtectionMode, MergeBot};
+use rust_team_data::v1::{Bot, BranchProtectionMode, GitHubTeam, MergeBot};
 use std::collections::{HashMap, HashSet};
 use std::fmt::{Display, Formatter, Write};
 
@@ -73,6 +74,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, HashSet<u64>>,
     org_apps: HashMap<OrgName, Vec<OrgAppInstallation>>,
 }
 
@@ -103,10 +105,12 @@ impl SyncGitHub {
             .collect::<HashSet<_>>();
 
         let mut org_owners = HashMap::new();
+        let mut org_members = HashMap::new();
         let mut org_apps = 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)?);
 
             let mut installations: Vec<OrgAppInstallation> = vec![];
 
@@ -134,17 +138,21 @@ impl SyncGitHub {
             repos,
             usernames_cache,
             org_owners,
+            org_members,
             org_apps,
         })
     }
 
     pub(crate) fn diff_all(&self) -> anyhow::Result<Diff> {
         let team_diffs = self.diff_teams()?;
         let repo_diffs = self.diff_repos()?;
+        let org_team_members = self.map_teams_to_org()?;
+        let toml_github_diffs = self.diff_teams_gh_org(org_team_members)?;
 
         Ok(Diff {
             team_diffs,
             repo_diffs,
+            toml_github_diffs,
         })
     }
 
@@ -195,6 +203,66 @@ impl SyncGitHub {
         Ok(diffs)
     }
 
+    // collect all org and respective teams members in a HashMap
+    fn map_teams_to_org(&self) -> anyhow::Result<HashMap<String, HashSet<u64>>> {
+        let mut org_team_members: HashMap<String, HashSet<u64>> = HashMap::new();
+
+        for team in &self.teams {
+            let team_org;
+            // get the team github org through the corresponding github team
+            if let Some(gh) = &team.github {
+                let github_teams = &gh.teams;
+                let github_team: &GitHubTeam = github_teams
+                    .iter()
+                    .find(|&gt| gt.name == team.name)
+                    .expect("Team Not Found");
+                team_org = github_team.org.clone();
+            } else {
+                return Err(anyhow!(
+                    "TeamGitHub object not found, got {:?}",
+                    &team.github
+                ));
+            }
+
+            let team_members_github_id: HashSet<u64> =
+                team.members.iter().map(|member| member.github_id).collect();
+
+            org_team_members
+                .entry(team_org)
+                .or_default()
+                .extend(team_members_github_id);
+        }
+        Ok(org_team_members)
+    }
+
+    // create diff against github org members against toml team members
+    fn diff_teams_gh_org(
+        &self,
+        org_team_members: HashMap<String, HashSet<u64>>,
+    ) -> anyhow::Result<OrgMembershipDiff> {
+        let mut org_with_members_to_be_removed: HashMap<String, HashSet<String>> = HashMap::new();
+
+        for (gh_org, toml_members_across_teams) in org_team_members.into_iter() {
+            let gh_org_members = self.org_members.get(&gh_org).unwrap();
+
+            let mut members_to_be_removed = HashSet::new();
+
+            for toml_member in toml_members_across_teams {
+                if !gh_org_members.contains(&toml_member.clone()) {
+                    members_to_be_removed.insert(self.usernames_cache[&toml_member].clone());
+                }
+            }
+            org_with_members_to_be_removed
+                .entry(gh_org)
+                .or_default()
+                .extend(members_to_be_removed);
+        }
+
+        Ok(OrgMembershipDiff::Delete(DeleteOrgMembershipDiff {
+            org_with_members: org_with_members_to_be_removed,
+        }))
+    }
+
     fn diff_team(&self, github_team: &rust_team_data::v1::GitHubTeam) -> anyhow::Result<TeamDiff> {
         // Ensure the team exists and is consistent
         let team = match self.github.team(&github_team.org, &github_team.name)? {
@@ -667,6 +735,7 @@ const BOTS_TEAMS: &[&str] = &["bors", "highfive", "rfcbot", "bots"];
 pub(crate) struct Diff {
     team_diffs: Vec<TeamDiff>,
     repo_diffs: Vec<RepoDiff>,
+    toml_github_diffs: OrgMembershipDiff,
 }
 
 impl Diff {
@@ -679,6 +748,8 @@ impl Diff {
             repo_diff.apply(sync)?;
         }
 
+        self.toml_github_diffs.apply(sync)?;
+
         Ok(())
     }
 }
@@ -720,6 +791,55 @@ impl std::fmt::Display for RepoDiff {
     }
 }
 
+#[derive(Debug)]
+
+enum OrgMembershipDiff {
+    Delete(DeleteOrgMembershipDiff),
+}
+
+impl OrgMembershipDiff {
+    fn apply(self, sync: &GitHubWrite) -> anyhow::Result<()> {
+        match self {
+            OrgMembershipDiff::Delete(d) => d.apply(sync)?,
+        }
+
+        Ok(())
+    }
+}
+
+impl std::fmt::Display for OrgMembershipDiff {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            OrgMembershipDiff::Delete(d) => write!(f, "{d}"),
+        }
+    }
+}
+
+#[derive(Debug)]
+
+struct DeleteOrgMembershipDiff {
+    org_with_members: HashMap<String, HashSet<String>>,
+}
+
+impl DeleteOrgMembershipDiff {
+    fn apply(self, sync: &GitHubWrite) -> anyhow::Result<()> {
+        for (gh_org, members) in self.org_with_members.iter() {
+            for member in members {
+                sync.remove_gh_member_from_org(gh_org, member)?;
+            }
+        }
+
+        Ok(())
+    }
+}
+
+impl std::fmt::Display for DeleteOrgMembershipDiff {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        writeln!(f, "❌ Deleting members '{:?}'", self.org_with_members)?;
+        Ok(())
+    }
+}
+
 struct CreateRepoDiff {
     org: String,
     name: String,

src/github/tests/mod.rs

@@ -1,3 +1,5 @@
+use std::collections::HashSet;
+
 use crate::github::tests::test_utils::{DataModel, TeamData};
 
 mod test_utils;
@@ -116,6 +118,38 @@ fn team_dont_add_member_if_invitation_is_pending() {
     "###);
 }
 
+#[test]
+fn org_member_not_sync() {
+    let mut model = DataModel::default();
+    let user = model.create_user("sakura");
+    let user2 = model.create_user("hitori");
+    model.create_team(TeamData::new("team-1").gh_team("members-gh", &[user, user2]));
+    let gh = model.gh_model();
+
+    // let gh_members = gh.org_members("rust-lang").unwrap();
+
+    let gh_members = HashSet::from([user2]);
+
+    println!("User data {:?}", gh_members);
+    model
+        .get_team("team-1")
+        .remove_gh_member("members-gh", user);
+
+    let gh_org_diff = model.diff_teams_gh_org(gh, gh_members);
+
+    insta::assert_debug_snapshot!(gh_org_diff, @r###"
+    Delete(
+        DeleteOrgMembershipDiff {
+            org_with_members: {
+                "rust-lang": {
+                    "hitori",
+                },
+            },
+        },
+    )
+    "###);
+}
+
 #[test]
 fn team_remove_member() {
     let mut model = DataModel::default();

src/github/tests/test_utils.rs

@@ -7,7 +7,7 @@ use crate::github::api::{
     BranchProtection, GithubRead, OrgAppInstallation, Repo, RepoAppInstallation, RepoTeam,
     RepoUser, Team, TeamMember, TeamPrivacy, TeamRole,
 };
-use crate::github::{api, SyncGitHub, TeamDiff};
+use crate::github::{api, OrgMembershipDiff, SyncGitHub, TeamDiff};
 
 const DEFAULT_ORG: &str = "rust-lang";
 
@@ -92,12 +92,30 @@ impl DataModel {
         GithubMock {
             users,
             owners: Default::default(),
+            members: Default::default(),
             teams,
             team_memberships,
             team_invitations: Default::default(),
         }
     }
 
+    pub fn diff_teams_gh_org(
+        &self,
+        github: GithubMock,
+        members: HashSet<u64>,
+    ) -> OrgMembershipDiff {
+        let teams = self.teams.iter().map(|r| r.to_data()).collect();
+        let repos = vec![];
+
+        let read = Box::new(github);
+        let sync = SyncGitHub::new(read, teams, repos).expect("Cannot create SyncGitHub");
+        let mut org_team_members = HashMap::new();
+        org_team_members.insert("rust-lang".to_string(), members);
+
+        sync.diff_teams_gh_org(org_team_members)
+            .expect("Cannot diff toml teams")
+    }
+
     pub fn diff_teams(&self, github: GithubMock) -> Vec<TeamDiff> {
         let teams = self.teams.iter().map(|r| r.to_data()).collect();
         let repos = vec![];
@@ -184,6 +202,9 @@ pub struct GithubMock {
     // org name -> user ID
     owners: HashMap<String, Vec<UserId>>,
     teams: Vec<Team>,
+
+    // org name -> user ID (members)
+    members: HashMap<String, Vec<UserId>>,
     // Team name -> members
     team_memberships: HashMap<String, HashMap<UserId, TeamMember>>,
     // Team name -> list of invited users
@@ -219,6 +240,16 @@ impl GithubRead for GithubMock {
             .collect())
     }
 
+    fn org_members(&self, org: &str) -> anyhow::Result<HashSet<u64>> {
+        Ok(self
+            .members
+            .get(org)
+            .cloned()
+            .unwrap_or_default()
+            .into_iter()
+            .collect())
+    }
+
     fn org_app_installations(&self, _org: &str) -> anyhow::Result<Vec<OrgAppInstallation>> {
         Ok(vec![])
     }