diff --git a/src/github/api/read.rs b/src/github/api/read.rs index 91f29e8..b9c74c4 100644 --- a/src/github/api/read.rs +++ b/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>; + /// Get the members of an org + fn org_members(&self, org: &str) -> anyhow::Result>; + /// Get the app installations of an org fn org_app_installations(&self, org: &str) -> anyhow::Result>; @@ -120,6 +123,23 @@ impl GithubRead for GitHubApiRead { Ok(owners) } + fn org_members(&self, org: &str) -> anyhow::Result> { + #[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| { + members.extend(resp.into_iter().map(|u| u.id)); + Ok(()) + }, + )?; + Ok(members) + } + fn org_app_installations(&self, org: &str) -> anyhow::Result> { #[derive(serde::Deserialize, Debug)] struct InstallationPage { diff --git a/src/github/api/write.rs b/src/github/api/write.rs index 6f1e9e6..85cd078 100644 --- a/src/github/api/write.rs +++ b/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, diff --git a/src/github/mod.rs b/src/github/mod.rs index 481cf65..09fc282 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -73,6 +73,7 @@ struct SyncGitHub { repos: Vec, usernames_cache: HashMap, org_owners: HashMap>, + org_members: HashMap>, org_apps: HashMap>, } @@ -103,10 +104,12 @@ impl SyncGitHub { .collect::>(); 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 = vec![]; @@ -134,6 +137,7 @@ impl SyncGitHub { repos, usernames_cache, org_owners, + org_members, org_apps, }) } @@ -141,10 +145,13 @@ impl SyncGitHub { pub(crate) fn diff_all(&self) -> anyhow::Result { 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 +202,55 @@ impl SyncGitHub { Ok(diffs) } + // collect all org and respective teams members in a HashMap + fn map_teams_to_org(&self) -> anyhow::Result>> { + let mut org_team_members: HashMap> = HashMap::new(); + + for team in &self.teams { + let mut team_org; + + if let Some(gh) = &team.github { + for toml_gh_team in &gh.teams { + team_org = toml_gh_team.org.clone(); + let toml_team_mems_gh_id: HashSet = + toml_gh_team.members.iter().copied().collect(); + + org_team_members + .entry(team_org) + .or_default() + .extend(toml_team_mems_gh_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>, + ) -> anyhow::Result { + let mut org_with_members_to_be_removed: HashMap> = 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 members_to_be_removed = (&toml_members_across_teams - gh_org_members) + .into_iter() + .map(|user| self.usernames_cache[&user].clone()) + .collect::>(); + + 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 { // Ensure the team exists and is consistent let team = match self.github.team(&github_team.org, &github_team.name)? { @@ -667,6 +723,7 @@ const BOTS_TEAMS: &[&str] = &["bors", "highfive", "rfcbot", "bots"]; pub(crate) struct Diff { team_diffs: Vec, repo_diffs: Vec, + toml_github_diffs: OrgMembershipDiff, } impl Diff { @@ -679,6 +736,8 @@ impl Diff { repo_diff.apply(sync)?; } + self.toml_github_diffs.apply(sync)?; + Ok(()) } } @@ -720,6 +779,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>, +} + +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, diff --git a/src/github/tests/mod.rs b/src/github/tests/mod.rs index ab3e6bd..ddf1b23 100644 --- a/src/github/tests/mod.rs +++ b/src/github/tests/mod.rs @@ -116,6 +116,33 @@ 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(); + + model + .get_team("team-1") + .remove_gh_member("members-gh", user); + + let gh_org_diff = model.diff_toml_gh_org_teams(gh); + + 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(); diff --git a/src/github/tests/test_utils.rs b/src/github/tests/test_utils.rs index e00c5ba..feadacb 100644 --- a/src/github/tests/test_utils.rs +++ b/src/github/tests/test_utils.rs @@ -1,13 +1,13 @@ use std::collections::{HashMap, HashSet}; use derive_builder::Builder; -use rust_team_data::v1::{GitHubTeam, Person, TeamGitHub, TeamKind}; +use rust_team_data::v1::{GitHubTeam, Person, Team as TomlTeam, TeamGitHub, TeamKind}; 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,29 @@ impl DataModel { GithubMock { users, owners: Default::default(), + members: Default::default(), teams, team_memberships, team_invitations: Default::default(), } } + pub fn diff_toml_gh_org_teams( + &self, + github: GithubMock, + // members: HashSet, + ) -> OrgMembershipDiff { + let teams: Vec = 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 org_team_members = sync.map_teams_to_org().unwrap(); + + sync.diff_teams_gh_org(org_team_members) + .expect("Cannot diff toml teams") + } + pub fn diff_teams(&self, github: GithubMock) -> Vec { let teams = self.teams.iter().map(|r| r.to_data()).collect(); let repos = vec![]; @@ -184,6 +201,9 @@ pub struct GithubMock { // org name -> user ID owners: HashMap>, teams: Vec, + + // org name -> user ID (members) + members: HashMap>, // Team name -> members team_memberships: HashMap>, // Team name -> list of invited users @@ -219,6 +239,16 @@ impl GithubRead for GithubMock { .collect()) } + fn org_members(&self, org: &str) -> anyhow::Result> { + Ok(self + .members + .get(org) + .cloned() + .unwrap_or_default() + .into_iter() + .collect()) + } + fn org_app_installations(&self, _org: &str) -> anyhow::Result> { Ok(vec![]) }