diff --git a/src/github/api/write.rs b/src/github/api/write.rs index 03e9090..1fdd5e9 100644 --- a/src/github/api/write.rs +++ b/src/github/api/write.rs @@ -279,14 +279,33 @@ impl GitHubWrite { installation_id: u64, repository_id: u64, ) -> anyhow::Result<()> { - debug!("Adding installation {installation_id} to repository {repository_id}"); + debug!("Adding repository {repository_id} to installation {installation_id}"); if !self.dry_run { self.client .req( Method::PUT, &format!("user/installations/{installation_id}/repositories/{repository_id}"), )? - .send()?; + .send()? + .custom_error_for_status()?; + } + Ok(()) + } + + pub(crate) fn remove_repo_from_app_installation( + &self, + installation_id: u64, + repository_id: u64, + ) -> anyhow::Result<()> { + debug!("Removing repository {repository_id} from installation {installation_id}"); + if !self.dry_run { + self.client + .req( + Method::DELETE, + &format!("user/installations/{installation_id}/repositories/{repository_id}"), + )? + .send()? + .custom_error_for_status()?; } Ok(()) } diff --git a/src/github/mod.rs b/src/github/mod.rs index 547a496..2ced5f5 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -3,9 +3,7 @@ mod api; mod tests; use self::api::{BranchProtectionOp, TeamPrivacy, TeamRole}; -use crate::github::api::{ - GithubRead, Login, PushAllowanceActor, Repo, RepoPermission, RepoSettings, -}; +use crate::github::api::{GithubRead, Login, PushAllowanceActor, RepoPermission, RepoSettings}; use log::debug; use rust_team_data::v1::Bot; use std::collections::{HashMap, HashSet}; @@ -28,7 +26,7 @@ pub(crate) fn create_diff( type OrgName = String; type RepoName = String; -#[derive(Clone, Debug, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq)] enum GithubApp { RenovateBot, } @@ -91,7 +89,7 @@ impl SyncGitHub { let usernames_cache = github.usernames(&users)?; debug!("caching organization owners"); - let mut orgs = teams + let orgs = teams .iter() .filter_map(|t| t.github.as_ref()) .flat_map(|gh| &gh.teams) @@ -334,13 +332,39 @@ impl SyncGitHub { archived: expected_repo.archived, auto_merge_enabled: expected_repo.auto_merge_enabled, }; + + let existing_installations = self + .org_apps + .get(&expected_repo.org) + .map(|installations| { + installations + .iter() + .filter_map(|installation| { + // Only load installations from apps that we know about, to avoid removing + // unknown installations. + if installation.repositories.contains(&actual_repo.name) { + Some(AppInstallation { + app: installation.app, + installation_id: installation.installation_id, + }) + } else { + None + } + }) + .collect::>() + }) + .unwrap_or_default(); + let app_installation_diffs = + self.diff_app_installations(expected_repo, &existing_installations)?; Ok(RepoDiff::Update(UpdateRepoDiff { org: expected_repo.org.clone(), name: actual_repo.name, repo_node_id: actual_repo.node_id, + repo_id: actual_repo.repo_id, settings_diff: (old_settings, new_settings), permission_diffs, branch_protection_diffs, + app_installation_diffs, })) } @@ -703,7 +727,7 @@ impl CreateRepoDiff { } for installation in &self.app_installations { - installation.apply(sync, &repo)?; + installation.apply(sync, repo.repo_id)?; } Ok(()) @@ -746,10 +770,12 @@ struct UpdateRepoDiff { org: String, name: String, repo_node_id: String, + repo_id: u64, // old, new settings_diff: (RepoSettings, RepoSettings), permission_diffs: Vec, branch_protection_diffs: Vec, + app_installation_diffs: Vec, } impl UpdateRepoDiff { @@ -761,6 +787,7 @@ impl UpdateRepoDiff { self.settings_diff.0 == self.settings_diff.1 && self.permission_diffs.is_empty() && self.branch_protection_diffs.is_empty() + && self.app_installation_diffs.is_empty() } fn can_be_modified(&self) -> bool { @@ -788,6 +815,10 @@ impl UpdateRepoDiff { for branch_protection in &self.branch_protection_diffs { branch_protection.apply(sync, &self.org, &self.name, &self.repo_node_id)?; } + + for app_installation in &self.app_installation_diffs { + app_installation.apply(sync, self.repo_id)?; + } Ok(()) } } @@ -841,6 +872,10 @@ impl std::fmt::Display for UpdateRepoDiff { for branch_protection_diff in &self.branch_protection_diffs { write!(f, "{branch_protection_diff}")?; } + writeln!(f, " App installation changes:")?; + for diff in &self.app_installation_diffs { + write!(f, "{diff}")?; + } Ok(()) } @@ -1010,13 +1045,13 @@ enum AppInstallationDiff { } impl AppInstallationDiff { - fn apply(&self, sync: &GitHubWrite, repo: &Repo) -> anyhow::Result<()> { + fn apply(&self, sync: &GitHubWrite, repo_id: u64) -> anyhow::Result<()> { match self { AppInstallationDiff::Add(app) => { - sync.add_repo_to_app_installation(app.installation_id, repo.repo_id)?; + sync.add_repo_to_app_installation(app.installation_id, repo_id)?; } - AppInstallationDiff::Remove(_) => { - todo!() + AppInstallationDiff::Remove(app) => { + sync.remove_repo_from_app_installation(app.installation_id, repo_id)?; } } Ok(())