From 82d26941ea35713e5b48f3fa8cff62f621a1718e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 8 Jan 2024 23:13:17 +0100 Subject: [PATCH 01/10] Turn `github/api` into a module --- src/github/{api.rs => api/mod.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/github/{api.rs => api/mod.rs} (100%) diff --git a/src/github/api.rs b/src/github/api/mod.rs similarity index 100% rename from src/github/api.rs rename to src/github/api/mod.rs From 573e12c40d1ae362f60ecfedceec77355bf02dc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 8 Jan 2024 23:37:43 +0100 Subject: [PATCH 02/10] Extract `HttpClient` implementation --- src/github/api/mod.rs | 292 ++++++++++++++++++++++-------------------- src/github/mod.rs | 2 +- 2 files changed, 155 insertions(+), 139 deletions(-) diff --git a/src/github/api/mod.rs b/src/github/api/mod.rs index af5d7c9..aa51505 100644 --- a/src/github/api/mod.rs +++ b/src/github/api/mod.rs @@ -2,6 +2,7 @@ use crate::utils::ResponseExt; use anyhow::{bail, Context}; use hyper_old_types::header::{Link, RelationType}; use log::{debug, trace}; +use reqwest::header::HeaderMap; use reqwest::{ blocking::{Client, RequestBuilder, Response}, header::{self, HeaderValue}, @@ -12,19 +13,137 @@ use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::fmt; +struct HttpClient { + client: Client, +} + +impl HttpClient { + fn from_token(token: String) -> anyhow::Result { + let builder = reqwest::blocking::ClientBuilder::default(); + let mut map = HeaderMap::default(); + let mut auth = HeaderValue::from_str(&format!("token {}", token))?; + auth.set_sensitive(true); + + map.insert(header::AUTHORIZATION, auth); + map.insert( + header::USER_AGENT, + HeaderValue::from_static(crate::USER_AGENT), + ); + + Ok(Self { + client: builder.build()?, + }) + } + + fn req(&self, method: Method, url: &str) -> anyhow::Result { + let url = if url.starts_with("https://") { + Cow::Borrowed(url) + } else { + Cow::Owned(format!("https://api.github.com/{url}")) + }; + trace!("http request: {} {}", method, url); + Ok(self.client.request(method, url.as_ref())) + } + + fn send( + &self, + method: Method, + url: &str, + body: &T, + ) -> Result { + let resp = self.req(method, url)?.json(body).send()?; + resp.custom_error_for_status() + } + + fn send_option( + &self, + method: Method, + url: &str, + ) -> Result, anyhow::Error> { + let resp = self.req(method.clone(), url)?.send()?; + match resp.status() { + StatusCode::OK => Ok(Some(resp.json_annotated().with_context(|| { + format!("Failed to decode response body on {method} request to '{url}'") + })?)), + StatusCode::NOT_FOUND => Ok(None), + _ => Err(resp.custom_error_for_status().unwrap_err()), + } + } + + fn graphql(&self, query: &str, variables: V) -> anyhow::Result + where + R: serde::de::DeserializeOwned, + V: serde::Serialize, + { + #[derive(serde::Serialize)] + struct Request<'a, V> { + query: &'a str, + variables: V, + } + let resp = self + .req(Method::POST, "graphql")? + .json(&Request { query, variables }) + .send()? + .custom_error_for_status()?; + + let res: GraphResult = resp.json_annotated().with_context(|| { + format!("Failed to decode response body on graphql request with query '{query}'") + })?; + if let Some(error) = res.errors.first() { + bail!("graphql error: {}", error.message); + } else if let Some(data) = res.data { + Ok(data) + } else { + bail!("missing graphql data"); + } + } + + fn rest_paginated(&self, method: &Method, url: String, mut f: F) -> anyhow::Result<()> + where + F: FnMut(Vec) -> anyhow::Result<()>, + T: DeserializeOwned, + { + let mut next = Some(url); + while let Some(next_url) = next.take() { + let resp = self + .req(method.clone(), &next_url)? + .send()? + .custom_error_for_status()?; + + // Extract the next page + if let Some(links) = resp.headers().get(header::LINK) { + let links: Link = links.to_str()?.parse()?; + for link in links.values() { + if link + .rel() + .map(|r| r.iter().any(|r| *r == RelationType::Next)) + .unwrap_or(false) + { + next = Some(link.link().to_string()); + break; + } + } + } + + f(resp.json().with_context(|| { + format!("Failed to deserialize response body for {method} request to '{next_url}'") + })?)?; + } + Ok(()) + } +} + pub(crate) struct GitHub { - token: String, dry_run: bool, - client: Client, + client: HttpClient, } impl GitHub { - pub(crate) fn new(token: String, dry_run: bool) -> Self { - GitHub { - token, + pub(crate) fn new(token: String, dry_run: bool) -> anyhow::Result { + Ok(Self { dry_run, - client: Client::new(), - } + client: HttpClient::from_token(token)?, + }) } /// Get user names by user ids @@ -52,7 +171,7 @@ impl GitHub { let mut result = HashMap::new(); for chunk in ids.chunks(100) { - let res: GraphNodes = self.graphql( + let res: GraphNodes = self.client.graphql( QUERY, Params { ids: chunk.iter().map(|id| user_node_id(*id)).collect(), @@ -72,7 +191,7 @@ impl GitHub { id: usize, } let mut owners = HashSet::new(); - self.rest_paginated( + self.client.rest_paginated( &Method::GET, format!("orgs/{org}/members?role=admin"), |resp: Vec| { @@ -89,7 +208,7 @@ impl GitHub { pub(crate) fn org_teams(&self, org: &str) -> anyhow::Result> { let mut teams = Vec::new(); - self.rest_paginated( + self.client.rest_paginated( &Method::GET, format!("orgs/{org}/teams"), |resp: Vec| { @@ -103,7 +222,8 @@ impl GitHub { /// Get the team by name and org pub(crate) fn team(&self, org: &str, team: &str) -> anyhow::Result> { - self.send_option(Method::GET, &format!("orgs/{org}/teams/{team}")) + self.client + .send_option(Method::GET, &format!("orgs/{org}/teams/{team}")) } /// Create a team in a org @@ -138,6 +258,7 @@ impl GitHub { privacy, }; Ok(self + .client .send(Method::POST, &format!("orgs/{org}/teams"), body)? .json_annotated()?) } @@ -171,7 +292,8 @@ impl GitHub { serde_json::to_string(&req).unwrap_or_else(|_| "INVALID_REQUEST".to_string()) ); if !self.dry_run { - self.send(Method::PATCH, &format!("orgs/{org}/teams/{name}"), &req)?; + self.client + .send(Method::PATCH, &format!("orgs/{org}/teams/{name}"), &req)?; } Ok(()) @@ -183,7 +305,7 @@ impl GitHub { if !self.dry_run { let method = Method::DELETE; let url = &format!("orgs/{org}/teams/{slug}"); - let resp = self.req(method.clone(), url)?.send()?; + let resp = self.client.req(method.clone(), url)?.send()?; allow_not_found(resp, method, url)?; } Ok(()) @@ -246,7 +368,7 @@ impl GitHub { if let Some(id) = team.id { let mut page_info = GraphPageInfo::start(); while page_info.has_next_page { - let res: GraphNode = self.graphql( + let res: GraphNode = self.client.graphql( QUERY, Params { team: team_node_id(id), @@ -279,7 +401,7 @@ impl GitHub { ) -> anyhow::Result> { let mut invites = HashSet::new(); - self.rest_paginated( + self.client.rest_paginated( &Method::GET, format!("orgs/{org}/teams/{team}/invitations"), |resp: Vec| { @@ -305,7 +427,7 @@ impl GitHub { role: TeamRole, } if !self.dry_run { - self.send( + self.client.send( Method::PUT, &format!("orgs/{org}/teams/{team}/memberships/{user}"), &Req { role }, @@ -326,7 +448,7 @@ impl GitHub { if !self.dry_run { let url = &format!("orgs/{org}/teams/{team}/memberships/{user}"); let method = Method::DELETE; - let resp = self.req(method.clone(), url)?.send()?; + let resp = self.client.req(method.clone(), url)?.send()?; allow_not_found(resp, method, url)?; } @@ -335,7 +457,8 @@ impl GitHub { /// Get a repo by org and name pub(crate) fn repo(&self, org: &str, repo: &str) -> anyhow::Result> { - self.send_option(Method::GET, &format!("repos/{org}/{repo}")) + self.client + .send_option(Method::GET, &format!("repos/{org}/{repo}")) } /// Create a repo @@ -366,6 +489,7 @@ impl GitHub { }) } else { Ok(self + .client .send(Method::POST, &format!("orgs/{org}/repos"), req)? .json_annotated()?) } @@ -384,7 +508,8 @@ impl GitHub { let req = Req { description }; debug!("Editing repo {}/{} with {:?}", org, repo_name, req); if !self.dry_run { - self.send(Method::PATCH, &format!("repos/{org}/{repo_name}"), &req)?; + self.client + .send(Method::PATCH, &format!("repos/{org}/{repo_name}"), &req)?; } Ok(()) } @@ -393,7 +518,7 @@ impl GitHub { pub(crate) fn repo_teams(&self, org: &str, repo: &str) -> anyhow::Result> { let mut teams = Vec::new(); - self.rest_paginated( + self.client.rest_paginated( &Method::GET, format!("repos/{org}/{repo}/teams"), |resp: Vec| { @@ -415,7 +540,7 @@ impl GitHub { ) -> anyhow::Result> { let mut users = Vec::new(); - self.rest_paginated( + self.client.rest_paginated( &Method::GET, format!("repos/{org}/{repo}/collaborators?affiliation=direct"), |resp: Vec| { @@ -441,7 +566,7 @@ impl GitHub { } debug!("Updating permission for team {team} on {org}/{repo} to {permission:?}"); if !self.dry_run { - self.send( + self.client.send( Method::PUT, &format!("orgs/{org}/teams/{team}/repos/{org}/{repo}"), &Req { permission }, @@ -465,7 +590,7 @@ impl GitHub { } debug!("Updating permission for user {user} on {org}/{repo} to {permission:?}"); if !self.dry_run { - self.send( + self.client.send( Method::PUT, &format!("repos/{org}/{repo}/collaborators/{user}"), &Req { permission }, @@ -485,7 +610,7 @@ impl GitHub { if !self.dry_run { let method = Method::DELETE; let url = &format!("orgs/{org}/teams/{team}/repos/{org}/{repo}"); - let resp = self.req(method.clone(), url)?.send()?; + let resp = self.client.req(method.clone(), url)?.send()?; allow_not_found(resp, method, url)?; } @@ -503,7 +628,7 @@ impl GitHub { if !self.dry_run { let method = Method::DELETE; let url = &format!("repos/{org}/{repo}/collaborators/{collaborator}"); - let resp = self.req(method.clone(), url)?.send()?; + let resp = self.client.req(method.clone(), url)?.send()?; allow_not_found(resp, method, url)?; } Ok(()) @@ -570,7 +695,7 @@ impl GitHub { } let mut result = HashMap::new(); - let res: Wrapper = self.graphql(QUERY, Params { org, repo })?; + let res: Wrapper = self.client.graphql(QUERY, Params { org, repo })?; for node in res .repository .branch_protection_rules @@ -652,7 +777,7 @@ impl GitHub { } if !self.dry_run { - let _: serde_json::Value = self.graphql( + let _: serde_json::Value = self.client.graphql( &query, Params { id, @@ -692,7 +817,7 @@ impl GitHub { id: String, } - let data: Data = self.graphql(query, Params { name })?; + let data: Data = self.client.graphql(query, Params { name })?; Ok(data.user.id) } @@ -718,116 +843,7 @@ impl GitHub { } } "; - let _: serde_json::Value = self.graphql(query, Params { id })?; - } - Ok(()) - } - - fn req(&self, method: Method, url: &str) -> anyhow::Result { - let url = if url.starts_with("https://") { - Cow::Borrowed(url) - } else { - Cow::Owned(format!("https://api.github.com/{url}")) - }; - trace!("http request: {} {}", method, url); - if self.dry_run && method != Method::GET && !url.contains("graphql") { - panic!("Called a non-GET request in dry run mode: {method}"); - } - let mut auth = HeaderValue::from_str(&format!("token {}", self.token))?; - auth.set_sensitive(true); - Ok(self - .client - .request(method, url.as_ref()) - .header(header::AUTHORIZATION, auth) - .header( - header::USER_AGENT, - HeaderValue::from_static(crate::USER_AGENT), - )) - } - - fn send( - &self, - method: Method, - url: &str, - body: &T, - ) -> Result { - let resp = self.req(method, url)?.json(body).send()?; - resp.custom_error_for_status() - } - - fn send_option( - &self, - method: Method, - url: &str, - ) -> Result, anyhow::Error> { - let resp = self.req(method.clone(), url)?.send()?; - match resp.status() { - StatusCode::OK => Ok(Some(resp.json_annotated().with_context(|| { - format!("Failed to decode response body on {method} request to '{url}'") - })?)), - StatusCode::NOT_FOUND => Ok(None), - _ => Err(resp.custom_error_for_status().unwrap_err()), - } - } - - fn graphql(&self, query: &str, variables: V) -> anyhow::Result - where - R: serde::de::DeserializeOwned, - V: serde::Serialize, - { - #[derive(serde::Serialize)] - struct Request<'a, V> { - query: &'a str, - variables: V, - } - let resp = self - .req(Method::POST, "graphql")? - .json(&Request { query, variables }) - .send()? - .custom_error_for_status()?; - - let res: GraphResult = resp.json_annotated().with_context(|| { - format!("Failed to decode response body on graphql request with query '{query}'") - })?; - if let Some(error) = res.errors.first() { - bail!("graphql error: {}", error.message); - } else if let Some(data) = res.data { - Ok(data) - } else { - bail!("missing graphql data"); - } - } - - fn rest_paginated(&self, method: &Method, url: String, mut f: F) -> anyhow::Result<()> - where - F: FnMut(Vec) -> anyhow::Result<()>, - T: DeserializeOwned, - { - let mut next = Some(url); - while let Some(next_url) = next.take() { - let resp = self - .req(method.clone(), &next_url)? - .send()? - .custom_error_for_status()?; - - // Extract the next page - if let Some(links) = resp.headers().get(header::LINK) { - let links: Link = links.to_str()?.parse()?; - for link in links.values() { - if link - .rel() - .map(|r| r.iter().any(|r| *r == RelationType::Next)) - .unwrap_or(false) - { - next = Some(link.link().to_string()); - break; - } - } - } - - f(resp.json().with_context(|| { - format!("Failed to deserialize response body for {method} request to '{next_url}'") - })?)?; + let _: serde_json::Value = self.client.graphql(query, Params { id })?; } Ok(()) } diff --git a/src/github/mod.rs b/src/github/mod.rs index a0e5bbd..29f81ec 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -20,7 +20,7 @@ pub(crate) struct SyncGitHub { impl SyncGitHub { pub(crate) fn new(token: String, team_api: &TeamApi, dry_run: bool) -> anyhow::Result { - let github = GitHub::new(token, dry_run); + let github = GitHub::new(token, dry_run)?; let teams = team_api.get_teams()?; let repos = team_api.get_repos()?; From b6aef0a420335cd012c9044e65fadc2b5ae6f536 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 8 Jan 2024 23:39:33 +0100 Subject: [PATCH 03/10] Pass `GitHub` instance to `SyncGitHub` --- src/github/api/mod.rs | 2 +- src/github/mod.rs | 7 ++++--- src/main.rs | 5 +++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/github/api/mod.rs b/src/github/api/mod.rs index aa51505..deb011b 100644 --- a/src/github/api/mod.rs +++ b/src/github/api/mod.rs @@ -133,7 +133,7 @@ impl HttpClient { } } -pub(crate) struct GitHub { +pub struct GitHub { dry_run: bool, client: HttpClient, } diff --git a/src/github/mod.rs b/src/github/mod.rs index 29f81ec..c6a16bf 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -1,12 +1,14 @@ mod api; -use self::api::{BranchProtectionOp, GitHub, TeamPrivacy, TeamRole}; +use self::api::{BranchProtectionOp, TeamPrivacy, TeamRole}; use crate::{github::api::RepoPermission, TeamApi}; use log::debug; use rust_team_data::v1::Bot; use std::collections::{HashMap, HashSet}; use std::fmt::Write; +pub use self::api::GitHub; + static DEFAULT_DESCRIPTION: &str = "Managed by the rust-lang/team repository."; static DEFAULT_PRIVACY: TeamPrivacy = TeamPrivacy::Closed; @@ -19,8 +21,7 @@ pub(crate) struct SyncGitHub { } impl SyncGitHub { - pub(crate) fn new(token: String, team_api: &TeamApi, dry_run: bool) -> anyhow::Result { - let github = GitHub::new(token, dry_run)?; + pub(crate) fn new(github: GitHub, team_api: &TeamApi) -> anyhow::Result { let teams = team_api.get_teams()?; let repos = team_api.get_repos()?; diff --git a/src/main.rs b/src/main.rs index 93a8d0e..0242bdb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,7 +4,7 @@ mod team_api; mod utils; mod zulip; -use crate::github::SyncGitHub; +use crate::github::{GitHub, SyncGitHub}; use crate::team_api::TeamApi; use anyhow::Context; use log::{error, info, warn}; @@ -81,7 +81,8 @@ fn app() -> anyhow::Result<()> { match service.as_str() { "github" => { let token = get_env("GITHUB_TOKEN")?; - let sync = SyncGitHub::new(token, &team_api, dry_run)?; + let github = GitHub::new(token, dry_run)?; + let sync = SyncGitHub::new(github, &team_api)?; let diff = sync.diff_all()?; info!("{}", diff); if !only_print_plan { From 6ac20e7d42551bec68000b24413f67d44bf9f221 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 9 Jan 2024 16:40:45 +0100 Subject: [PATCH 04/10] Remove GH API reads from diffs --- src/github/mod.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/github/mod.rs b/src/github/mod.rs index c6a16bf..87fff38 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -248,6 +248,7 @@ impl SyncGitHub { Ok(RepoDiff::Update(UpdateRepoDiff { org: expected_repo.org.clone(), name: actual_repo.name, + repo_id: actual_repo.id, description_diff, permission_diffs, branch_protection_diffs, @@ -537,7 +538,7 @@ struct CreateRepoDiff { impl CreateRepoDiff { fn apply(&self, sync: &SyncGitHub) -> anyhow::Result<()> { - let _ = sync + let repo = sync .github .create_repo(&self.org, &self.name, &self.description)?; @@ -550,7 +551,7 @@ impl CreateRepoDiff { pattern: branch.clone(), operation: BranchProtectionDiffOperation::Create(protection.clone()), } - .apply(sync, &self.org, &self.name)?; + .apply(sync, &self.org, &self.name, &repo.id)?; } Ok(()) } @@ -578,6 +579,7 @@ impl std::fmt::Display for CreateRepoDiff { struct UpdateRepoDiff { org: String, name: String, + repo_id: String, description_diff: Option<(Option, String)>, permission_diffs: Vec, branch_protection_diffs: Vec, @@ -598,7 +600,7 @@ impl UpdateRepoDiff { permission.apply(sync, &self.org, &self.name)?; } for branch_protection in &self.branch_protection_diffs { - branch_protection.apply(sync, &self.org, &self.name)?; + branch_protection.apply(sync, &self.org, &self.name, &self.repo_id)?; } Ok(()) } @@ -701,12 +703,17 @@ struct BranchProtectionDiff { } impl BranchProtectionDiff { - fn apply(&self, sync: &SyncGitHub, org: &str, repo_name: &str) -> anyhow::Result<()> { - let repo_id = sync.github.repo(org, repo_name)?.unwrap().id; + fn apply( + &self, + sync: &SyncGitHub, + org: &str, + repo_name: &str, + repo_id: &str, + ) -> anyhow::Result<()> { match &self.operation { BranchProtectionDiffOperation::Create(bp) => { sync.github.upsert_branch_protection( - BranchProtectionOp::CreateForRepo(repo_id), + BranchProtectionOp::CreateForRepo(repo_id.to_string()), &self.pattern, bp, )?; From 7c52361006381c32ba9467430c2c7a372d64469e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 9 Jan 2024 16:43:17 +0100 Subject: [PATCH 05/10] Split out the write part of the GitHub API client This makes it clear that diffs only perform writes and no reads. It should also make mocking of writes easier (to be added later). --- src/github/api/mod.rs | 410 +------------------------------------- src/github/api/write.rs | 430 ++++++++++++++++++++++++++++++++++++++++ src/github/mod.rs | 72 ++++--- src/main.rs | 7 +- 4 files changed, 474 insertions(+), 445 deletions(-) create mode 100644 src/github/api/write.rs diff --git a/src/github/api/mod.rs b/src/github/api/mod.rs index deb011b..e7dcb50 100644 --- a/src/github/api/mod.rs +++ b/src/github/api/mod.rs @@ -1,3 +1,5 @@ +mod write; + use crate::utils::ResponseExt; use anyhow::{bail, Context}; use hyper_old_types::header::{Link, RelationType}; @@ -13,6 +15,8 @@ use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::fmt; +pub(crate) use write::GitHubWrite; + struct HttpClient { client: Client, } @@ -134,14 +138,12 @@ impl HttpClient { } pub struct GitHub { - dry_run: bool, client: HttpClient, } impl GitHub { - pub(crate) fn new(token: String, dry_run: bool) -> anyhow::Result { + pub(crate) fn new(token: String) -> anyhow::Result { Ok(Self { - dry_run, client: HttpClient::from_token(token)?, }) } @@ -226,91 +228,6 @@ impl GitHub { .send_option(Method::GET, &format!("orgs/{org}/teams/{team}")) } - /// Create a team in a org - pub(crate) fn create_team( - &self, - org: &str, - name: &str, - description: &str, - privacy: TeamPrivacy, - ) -> anyhow::Result { - #[derive(serde::Serialize, Debug)] - struct Req<'a> { - name: &'a str, - description: &'a str, - privacy: TeamPrivacy, - } - debug!("Creating team '{name}' in '{org}'"); - if self.dry_run { - Ok(Team { - // The `None` marks that the team is "created" by the dry run and - // doesn't actually exist on GitHub - id: None, - name: name.to_string(), - description: Some(description.to_string()), - privacy, - slug: name.to_string(), - }) - } else { - let body = &Req { - name, - description, - privacy, - }; - Ok(self - .client - .send(Method::POST, &format!("orgs/{org}/teams"), body)? - .json_annotated()?) - } - } - - /// Edit a team - pub(crate) fn edit_team( - &self, - org: &str, - name: &str, - new_name: Option<&str>, - new_description: Option<&str>, - new_privacy: Option, - ) -> anyhow::Result<()> { - #[derive(serde::Serialize, Debug)] - struct Req<'a> { - #[serde(skip_serializing_if = "Option::is_none")] - name: Option<&'a str>, - #[serde(skip_serializing_if = "Option::is_none")] - description: Option<&'a str>, - #[serde(skip_serializing_if = "Option::is_none")] - privacy: Option, - } - let req = Req { - name: new_name, - description: new_description, - privacy: new_privacy, - }; - debug!( - "Editing team '{name}' in '{org}' with request: {}", - serde_json::to_string(&req).unwrap_or_else(|_| "INVALID_REQUEST".to_string()) - ); - if !self.dry_run { - self.client - .send(Method::PATCH, &format!("orgs/{org}/teams/{name}"), &req)?; - } - - Ok(()) - } - - /// Delete a team by name and org - pub(crate) fn delete_team(&self, org: &str, slug: &str) -> anyhow::Result<()> { - debug!("Deleting team with slug '{slug}' in '{org}'"); - if !self.dry_run { - let method = Method::DELETE; - let url = &format!("orgs/{org}/teams/{slug}"); - let resp = self.client.req(method.clone(), url)?.send()?; - allow_not_found(resp, method, url)?; - } - Ok(()) - } - pub(crate) fn team_memberships( &self, team: &Team, @@ -413,107 +330,12 @@ impl GitHub { Ok(invites) } - /// Set a user's membership in a team to a role - pub(crate) fn set_team_membership( - &self, - org: &str, - team: &str, - user: &str, - role: TeamRole, - ) -> anyhow::Result<()> { - debug!("Setting membership of '{user}' in team '{team}' in org '{org}' to role '{role}'"); - #[derive(serde::Serialize, Debug)] - struct Req { - role: TeamRole, - } - if !self.dry_run { - self.client.send( - Method::PUT, - &format!("orgs/{org}/teams/{team}/memberships/{user}"), - &Req { role }, - )?; - } - - Ok(()) - } - - /// Remove a user from a team - pub(crate) fn remove_team_membership( - &self, - org: &str, - team: &str, - user: &str, - ) -> anyhow::Result<()> { - debug!("Removing membership of '{user}' from team '{team}' in org '{org}'"); - if !self.dry_run { - let url = &format!("orgs/{org}/teams/{team}/memberships/{user}"); - let method = Method::DELETE; - let resp = self.client.req(method.clone(), url)?.send()?; - allow_not_found(resp, method, url)?; - } - - Ok(()) - } - /// Get a repo by org and name pub(crate) fn repo(&self, org: &str, repo: &str) -> anyhow::Result> { self.client .send_option(Method::GET, &format!("repos/{org}/{repo}")) } - /// Create a repo - pub(crate) fn create_repo( - &self, - org: &str, - name: &str, - description: &str, - ) -> anyhow::Result { - #[derive(serde::Serialize, Debug)] - struct Req<'a> { - name: &'a str, - description: &'a str, - auto_init: bool, - } - let req = &Req { - name, - description, - auto_init: true, - }; - debug!("Creating the repo {org}/{name} with {req:?}"); - if self.dry_run { - Ok(Repo { - id: String::from("ID"), - name: name.to_string(), - org: org.to_string(), - description: Some(description.to_string()), - }) - } else { - Ok(self - .client - .send(Method::POST, &format!("orgs/{org}/repos"), req)? - .json_annotated()?) - } - } - - pub(crate) fn edit_repo( - &self, - org: &str, - repo_name: &str, - description: &str, - ) -> anyhow::Result<()> { - #[derive(serde::Serialize, Debug)] - struct Req<'a> { - description: &'a str, - } - let req = Req { description }; - debug!("Editing repo {}/{} with {:?}", org, repo_name, req); - if !self.dry_run { - self.client - .send(Method::PATCH, &format!("repos/{org}/{repo_name}"), &req)?; - } - Ok(()) - } - /// Get teams in a repo pub(crate) fn repo_teams(&self, org: &str, repo: &str) -> anyhow::Result> { let mut teams = Vec::new(); @@ -552,88 +374,6 @@ impl GitHub { Ok(users) } - /// Update a team's permissions to a repo - pub(crate) fn update_team_repo_permissions( - &self, - org: &str, - repo: &str, - team: &str, - permission: &RepoPermission, - ) -> anyhow::Result<()> { - #[derive(serde::Serialize, Debug)] - struct Req<'a> { - permission: &'a RepoPermission, - } - debug!("Updating permission for team {team} on {org}/{repo} to {permission:?}"); - if !self.dry_run { - self.client.send( - Method::PUT, - &format!("orgs/{org}/teams/{team}/repos/{org}/{repo}"), - &Req { permission }, - )?; - } - - Ok(()) - } - - /// Update a user's permissions to a repo - pub(crate) fn update_user_repo_permissions( - &self, - org: &str, - repo: &str, - user: &str, - permission: &RepoPermission, - ) -> anyhow::Result<()> { - #[derive(serde::Serialize, Debug)] - struct Req<'a> { - permission: &'a RepoPermission, - } - debug!("Updating permission for user {user} on {org}/{repo} to {permission:?}"); - if !self.dry_run { - self.client.send( - Method::PUT, - &format!("repos/{org}/{repo}/collaborators/{user}"), - &Req { permission }, - )?; - } - Ok(()) - } - - /// Remove a team from a repo - pub(crate) fn remove_team_from_repo( - &self, - org: &str, - repo: &str, - team: &str, - ) -> anyhow::Result<()> { - debug!("Removing team {team} from repo {org}/{repo}"); - if !self.dry_run { - let method = Method::DELETE; - let url = &format!("orgs/{org}/teams/{team}/repos/{org}/{repo}"); - 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, - org: &str, - repo: &str, - collaborator: &str, - ) -> anyhow::Result<()> { - debug!("Removing collaborator {collaborator} from repo {org}/{repo}"); - if !self.dry_run { - let method = Method::DELETE; - let url = &format!("repos/{org}/{repo}/collaborators/{collaborator}"); - let resp = self.client.req(method.clone(), url)?.send()?; - allow_not_found(resp, method, url)?; - } - Ok(()) - } - /// Get branch_protections pub(crate) fn branch_protections( &self, @@ -707,146 +447,6 @@ impl GitHub { } Ok(result) } - - /// Create or update a branch protection. - pub(crate) fn upsert_branch_protection( - &self, - op: BranchProtectionOp, - pattern: &str, - branch_protection: &BranchProtection, - ) -> anyhow::Result<()> { - debug!("Updating '{}' branch protection", pattern); - #[derive(Debug, serde::Serialize)] - #[serde(rename_all = "camelCase")] - struct Params<'a> { - id: &'a str, - pattern: &'a str, - contexts: &'a [String], - dismiss_stale: bool, - review_count: u8, - restricts_pushes: bool, - push_actor_ids: &'a [String], - } - let mutation_name = match op { - BranchProtectionOp::CreateForRepo(_) => "createBranchProtectionRule", - BranchProtectionOp::UpdateBranchProtection(_) => "updateBranchProtectionRule", - }; - let id_field = match op { - BranchProtectionOp::CreateForRepo(_) => "repositoryId", - BranchProtectionOp::UpdateBranchProtection(_) => "branchProtectionRuleId", - }; - let id = &match op { - BranchProtectionOp::CreateForRepo(id) => id, - BranchProtectionOp::UpdateBranchProtection(id) => id, - }; - let query = format!(" - mutation($id: ID!, $pattern:String!, $contexts: [String!], $dismissStale: Boolean, $reviewCount: Int, $pushActorIds: [ID!], $restrictsPushes: Boolean) {{ - {mutation_name}(input: {{ - {id_field}: $id, - pattern: $pattern, - requiresStatusChecks: true, - requiredStatusCheckContexts: $contexts, - isAdminEnforced: true, - requiredApprovingReviewCount: $reviewCount, - dismissesStaleReviews: $dismissStale, - requiresApprovingReviews:true, - restrictsPushes: $restrictsPushes, - pushActorIds: $pushActorIds - }}) {{ - branchProtectionRule {{ - id - }} - }} - }} - "); - let mut push_actor_ids = vec![]; - for actor in &branch_protection.push_allowances { - match actor { - PushAllowanceActor::User(UserPushAllowanceActor { login: name }) => { - push_actor_ids.push(self.user_id(name)?); - } - PushAllowanceActor::Team(TeamPushAllowanceActor { - organization: Login { login: org }, - name, - }) => push_actor_ids.push( - self.team(org, name)? - .with_context(|| format!("could not find team: {org}/{name}"))? - .name, - ), - } - } - - if !self.dry_run { - let _: serde_json::Value = self.client.graphql( - &query, - Params { - id, - pattern, - contexts: &branch_protection.required_status_check_contexts, - dismiss_stale: branch_protection.dismisses_stale_reviews, - review_count: branch_protection.required_approving_review_count, - // We restrict merges, if we have explicitly set some actors to be - // able to merge (i.e., we allow allow those with write permissions - // to merge *or* we only allow those in `push_actor_ids`) - restricts_pushes: !push_actor_ids.is_empty(), - push_actor_ids: &push_actor_ids, - }, - )?; - } - Ok(()) - } - - fn user_id(&self, name: &str) -> anyhow::Result { - #[derive(serde::Serialize)] - struct Params<'a> { - name: &'a str, - } - let query = " - query($name: String!) { - user(login: $name) { - id - } - } - "; - #[derive(serde::Deserialize)] - struct Data { - user: User, - } - #[derive(serde::Deserialize)] - struct User { - id: String, - } - - let data: Data = self.client.graphql(query, Params { name })?; - Ok(data.user.id) - } - - /// Delete a branch protection - pub(crate) fn delete_branch_protection( - &self, - org: &str, - repo_name: &str, - id: &str, - ) -> anyhow::Result<()> { - debug!("Removing protection in {}/{}", org, repo_name); - println!("Remove protection {id}"); - if !self.dry_run { - #[derive(serde::Serialize)] - #[serde(rename_all = "camelCase")] - struct Params<'a> { - id: &'a str, - } - let query = " - mutation($id: ID!) { - deleteBranchProtectionRule(input: { branchProtectionRuleId: $id }) { - clientMutationId - } - } - "; - let _: serde_json::Value = self.client.graphql(query, Params { id })?; - } - Ok(()) - } } fn allow_not_found(resp: Response, method: Method, url: &str) -> Result<(), anyhow::Error> { diff --git a/src/github/api/write.rs b/src/github/api/write.rs new file mode 100644 index 0000000..1a22074 --- /dev/null +++ b/src/github/api/write.rs @@ -0,0 +1,430 @@ +use anyhow::Context; +use log::debug; +use reqwest::Method; + +use crate::github::api::{ + allow_not_found, BranchProtection, BranchProtectionOp, HttpClient, Login, PushAllowanceActor, + Repo, RepoPermission, Team, TeamPrivacy, TeamPushAllowanceActor, TeamRole, + UserPushAllowanceActor, +}; +use crate::github::GitHub; +use crate::utils::ResponseExt; + +pub(crate) struct GitHubWrite { + client: HttpClient, + dry_run: bool, + read: GitHub, +} + +impl GitHubWrite { + pub(crate) fn new(token: String, dry_run: bool) -> anyhow::Result { + Ok(Self { + client: HttpClient::from_token(token.clone())?, + dry_run, + read: GitHub::new(token)?, + }) + } + + fn user_id(&self, name: &str) -> anyhow::Result { + #[derive(serde::Serialize)] + struct Params<'a> { + name: &'a str, + } + let query = " + query($name: String!) { + user(login: $name) { + id + } + } + "; + #[derive(serde::Deserialize)] + struct Data { + user: User, + } + #[derive(serde::Deserialize)] + struct User { + id: String, + } + + let data: Data = self.client.graphql(query, Params { name })?; + Ok(data.user.id) + } + + /// Create a team in a org + pub(crate) fn create_team( + &self, + org: &str, + name: &str, + description: &str, + privacy: TeamPrivacy, + ) -> anyhow::Result { + #[derive(serde::Serialize, Debug)] + struct Req<'a> { + name: &'a str, + description: &'a str, + privacy: TeamPrivacy, + } + debug!("Creating team '{name}' in '{org}'"); + if self.dry_run { + Ok(Team { + // The `None` marks that the team is "created" by the dry run and + // doesn't actually exist on GitHub + id: None, + name: name.to_string(), + description: Some(description.to_string()), + privacy, + slug: name.to_string(), + }) + } else { + let body = &Req { + name, + description, + privacy, + }; + Ok(self + .client + .send(Method::POST, &format!("orgs/{org}/teams"), body)? + .json_annotated()?) + } + } + + /// Edit a team + pub(crate) fn edit_team( + &self, + org: &str, + name: &str, + new_name: Option<&str>, + new_description: Option<&str>, + new_privacy: Option, + ) -> anyhow::Result<()> { + #[derive(serde::Serialize, Debug)] + struct Req<'a> { + #[serde(skip_serializing_if = "Option::is_none")] + name: Option<&'a str>, + #[serde(skip_serializing_if = "Option::is_none")] + description: Option<&'a str>, + #[serde(skip_serializing_if = "Option::is_none")] + privacy: Option, + } + let req = Req { + name: new_name, + description: new_description, + privacy: new_privacy, + }; + debug!( + "Editing team '{name}' in '{org}' with request: {}", + serde_json::to_string(&req).unwrap_or_else(|_| "INVALID_REQUEST".to_string()) + ); + if !self.dry_run { + self.client + .send(Method::PATCH, &format!("orgs/{org}/teams/{name}"), &req)?; + } + + Ok(()) + } + + /// Delete a team by name and org + pub(crate) fn delete_team(&self, org: &str, slug: &str) -> anyhow::Result<()> { + debug!("Deleting team with slug '{slug}' in '{org}'"); + if !self.dry_run { + let method = Method::DELETE; + let url = &format!("orgs/{org}/teams/{slug}"); + let resp = self.client.req(method.clone(), url)?.send()?; + allow_not_found(resp, method, url)?; + } + Ok(()) + } + + /// Set a user's membership in a team to a role + pub(crate) fn set_team_membership( + &self, + org: &str, + team: &str, + user: &str, + role: TeamRole, + ) -> anyhow::Result<()> { + debug!("Setting membership of '{user}' in team '{team}' in org '{org}' to role '{role}'"); + #[derive(serde::Serialize, Debug)] + struct Req { + role: TeamRole, + } + if !self.dry_run { + self.client.send( + Method::PUT, + &format!("orgs/{org}/teams/{team}/memberships/{user}"), + &Req { role }, + )?; + } + + Ok(()) + } + + /// Remove a user from a team + pub(crate) fn remove_team_membership( + &self, + org: &str, + team: &str, + user: &str, + ) -> anyhow::Result<()> { + debug!("Removing membership of '{user}' from team '{team}' in org '{org}'"); + if !self.dry_run { + let url = &format!("orgs/{org}/teams/{team}/memberships/{user}"); + let method = Method::DELETE; + let resp = self.client.req(method.clone(), url)?.send()?; + allow_not_found(resp, method, url)?; + } + + Ok(()) + } + + /// Create a repo + pub(crate) fn create_repo( + &self, + org: &str, + name: &str, + description: &str, + ) -> anyhow::Result { + #[derive(serde::Serialize, Debug)] + struct Req<'a> { + name: &'a str, + description: &'a str, + auto_init: bool, + } + let req = &Req { + name, + description, + auto_init: true, + }; + debug!("Creating the repo {org}/{name} with {req:?}"); + if self.dry_run { + Ok(Repo { + id: String::from("ID"), + name: name.to_string(), + org: org.to_string(), + description: Some(description.to_string()), + }) + } else { + Ok(self + .client + .send(Method::POST, &format!("orgs/{org}/repos"), req)? + .json_annotated()?) + } + } + + pub(crate) fn edit_repo( + &self, + org: &str, + repo_name: &str, + description: &str, + ) -> anyhow::Result<()> { + #[derive(serde::Serialize, Debug)] + struct Req<'a> { + description: &'a str, + } + let req = Req { description }; + debug!("Editing repo {}/{} with {:?}", org, repo_name, req); + if !self.dry_run { + self.client + .send(Method::PATCH, &format!("repos/{org}/{repo_name}"), &req)?; + } + Ok(()) + } + + /// Update a team's permissions to a repo + pub(crate) fn update_team_repo_permissions( + &self, + org: &str, + repo: &str, + team: &str, + permission: &RepoPermission, + ) -> anyhow::Result<()> { + #[derive(serde::Serialize, Debug)] + struct Req<'a> { + permission: &'a RepoPermission, + } + debug!("Updating permission for team {team} on {org}/{repo} to {permission:?}"); + if !self.dry_run { + self.client.send( + Method::PUT, + &format!("orgs/{org}/teams/{team}/repos/{org}/{repo}"), + &Req { permission }, + )?; + } + + Ok(()) + } + + /// Update a user's permissions to a repo + pub(crate) fn update_user_repo_permissions( + &self, + org: &str, + repo: &str, + user: &str, + permission: &RepoPermission, + ) -> anyhow::Result<()> { + #[derive(serde::Serialize, Debug)] + struct Req<'a> { + permission: &'a RepoPermission, + } + debug!("Updating permission for user {user} on {org}/{repo} to {permission:?}"); + if !self.dry_run { + self.client.send( + Method::PUT, + &format!("repos/{org}/{repo}/collaborators/{user}"), + &Req { permission }, + )?; + } + Ok(()) + } + + /// Remove a team from a repo + pub(crate) fn remove_team_from_repo( + &self, + org: &str, + repo: &str, + team: &str, + ) -> anyhow::Result<()> { + debug!("Removing team {team} from repo {org}/{repo}"); + if !self.dry_run { + let method = Method::DELETE; + let url = &format!("orgs/{org}/teams/{team}/repos/{org}/{repo}"); + 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, + org: &str, + repo: &str, + collaborator: &str, + ) -> anyhow::Result<()> { + debug!("Removing collaborator {collaborator} from repo {org}/{repo}"); + if !self.dry_run { + let method = Method::DELETE; + let url = &format!("repos/{org}/{repo}/collaborators/{collaborator}"); + let resp = self.client.req(method.clone(), url)?.send()?; + allow_not_found(resp, method, url)?; + } + Ok(()) + } + + /// Create or update a branch protection. + pub(crate) fn upsert_branch_protection( + &self, + op: BranchProtectionOp, + pattern: &str, + branch_protection: &BranchProtection, + ) -> anyhow::Result<()> { + debug!("Updating '{}' branch protection", pattern); + #[derive(Debug, serde::Serialize)] + #[serde(rename_all = "camelCase")] + struct Params<'a> { + id: &'a str, + pattern: &'a str, + contexts: &'a [String], + dismiss_stale: bool, + review_count: u8, + restricts_pushes: bool, + push_actor_ids: &'a [String], + } + let mutation_name = match op { + BranchProtectionOp::CreateForRepo(_) => "createBranchProtectionRule", + BranchProtectionOp::UpdateBranchProtection(_) => "updateBranchProtectionRule", + }; + let id_field = match op { + BranchProtectionOp::CreateForRepo(_) => "repositoryId", + BranchProtectionOp::UpdateBranchProtection(_) => "branchProtectionRuleId", + }; + let id = &match op { + BranchProtectionOp::CreateForRepo(id) => id, + BranchProtectionOp::UpdateBranchProtection(id) => id, + }; + let query = format!(" + mutation($id: ID!, $pattern:String!, $contexts: [String!], $dismissStale: Boolean, $reviewCount: Int, $pushActorIds: [ID!], $restrictsPushes: Boolean) {{ + {mutation_name}(input: {{ + {id_field}: $id, + pattern: $pattern, + requiresStatusChecks: true, + requiredStatusCheckContexts: $contexts, + isAdminEnforced: true, + requiredApprovingReviewCount: $reviewCount, + dismissesStaleReviews: $dismissStale, + requiresApprovingReviews:true, + restrictsPushes: $restrictsPushes, + pushActorIds: $pushActorIds + }}) {{ + branchProtectionRule {{ + id + }} + }} + }} + "); + let mut push_actor_ids = vec![]; + for actor in &branch_protection.push_allowances { + match actor { + PushAllowanceActor::User(UserPushAllowanceActor { login: name }) => { + push_actor_ids.push(self.user_id(name)?); + } + PushAllowanceActor::Team(TeamPushAllowanceActor { + organization: Login { login: org }, + name, + }) => push_actor_ids.push( + self.read + .team(org, name)? + .with_context(|| format!("could not find team: {org}/{name}"))? + .name, + ), + } + } + + if !self.dry_run { + let _: serde_json::Value = self.client.graphql( + &query, + Params { + id, + pattern, + contexts: &branch_protection.required_status_check_contexts, + dismiss_stale: branch_protection.dismisses_stale_reviews, + review_count: branch_protection.required_approving_review_count, + // We restrict merges, if we have explicitly set some actors to be + // able to merge (i.e., we allow allow those with write permissions + // to merge *or* we only allow those in `push_actor_ids`) + restricts_pushes: !push_actor_ids.is_empty(), + push_actor_ids: &push_actor_ids, + }, + )?; + } + Ok(()) + } + + /// Delete a branch protection + pub(crate) fn delete_branch_protection( + &self, + org: &str, + repo_name: &str, + id: &str, + ) -> anyhow::Result<()> { + debug!("Removing protection in {}/{}", org, repo_name); + println!("Remove protection {id}"); + if !self.dry_run { + #[derive(serde::Serialize)] + #[serde(rename_all = "camelCase")] + struct Params<'a> { + id: &'a str, + } + let query = " + mutation($id: ID!) { + deleteBranchProtectionRule(input: { branchProtectionRuleId: $id }) { + clientMutationId + } + } + "; + let _: serde_json::Value = self.client.graphql(query, Params { id })?; + } + Ok(()) + } +} diff --git a/src/github/mod.rs b/src/github/mod.rs index 87fff38..2c27bb9 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -7,7 +7,7 @@ use rust_team_data::v1::Bot; use std::collections::{HashMap, HashSet}; use std::fmt::Write; -pub use self::api::GitHub; +pub(crate) use self::api::{GitHub, GitHubWrite}; static DEFAULT_DESCRIPTION: &str = "Managed by the rust-lang/team repository."; static DEFAULT_PRIVACY: TeamPrivacy = TeamPrivacy::Closed; @@ -479,7 +479,7 @@ pub(crate) struct Diff { impl Diff { /// Apply the diff to GitHub - pub(crate) fn apply(self, sync: &SyncGitHub) -> anyhow::Result<()> { + pub(crate) fn apply(self, sync: &GitHubWrite) -> anyhow::Result<()> { for team_diff in self.team_diffs { team_diff.apply(sync)?; } @@ -511,7 +511,7 @@ enum RepoDiff { } impl RepoDiff { - fn apply(&self, sync: &SyncGitHub) -> anyhow::Result<()> { + fn apply(&self, sync: &GitHubWrite) -> anyhow::Result<()> { match self { RepoDiff::Create(c) => c.apply(sync), RepoDiff::Update(u) => u.apply(sync), @@ -537,10 +537,8 @@ struct CreateRepoDiff { } impl CreateRepoDiff { - fn apply(&self, sync: &SyncGitHub) -> anyhow::Result<()> { - let repo = sync - .github - .create_repo(&self.org, &self.name, &self.description)?; + fn apply(&self, sync: &GitHubWrite) -> anyhow::Result<()> { + let repo = sync.create_repo(&self.org, &self.name, &self.description)?; for permission in &self.permissions { permission.apply(sync, &self.org, &self.name)?; @@ -592,13 +590,14 @@ impl UpdateRepoDiff { && self.branch_protection_diffs.is_empty() } - fn apply(&self, sync: &SyncGitHub) -> anyhow::Result<()> { + fn apply(&self, sync: &GitHubWrite) -> anyhow::Result<()> { if let Some((_, description)) = &self.description_diff { - sync.github.edit_repo(&self.org, &self.name, description)?; + sync.edit_repo(&self.org, &self.name, description)?; } for permission in &self.permission_diffs { permission.apply(sync, &self.org, &self.name)?; } + for branch_protection in &self.branch_protection_diffs { branch_protection.apply(sync, &self.org, &self.name, &self.repo_id)?; } @@ -640,25 +639,25 @@ struct RepoPermissionAssignmentDiff { } impl RepoPermissionAssignmentDiff { - fn apply(&self, sync: &SyncGitHub, org: &str, repo_name: &str) -> anyhow::Result<()> { + fn apply(&self, sync: &GitHubWrite, org: &str, repo_name: &str) -> anyhow::Result<()> { match &self.diff { RepoPermissionDiff::Create(p) | RepoPermissionDiff::Update(_, p) => { match &self.collaborator { - RepoCollaborator::Team(team_name) => sync - .github - .update_team_repo_permissions(org, repo_name, team_name, p)?, - RepoCollaborator::User(user_name) => sync - .github - .update_user_repo_permissions(org, repo_name, user_name, p)?, + RepoCollaborator::Team(team_name) => { + sync.update_team_repo_permissions(org, repo_name, team_name, p)? + } + RepoCollaborator::User(user_name) => { + sync.update_user_repo_permissions(org, repo_name, user_name, p)? + } } } RepoPermissionDiff::Delete(_) => match &self.collaborator { - RepoCollaborator::Team(team_name) => sync - .github - .remove_team_from_repo(org, repo_name, team_name)?, - RepoCollaborator::User(user_name) => sync - .github - .remove_collaborator_from_repo(org, repo_name, user_name)?, + RepoCollaborator::Team(team_name) => { + sync.remove_team_from_repo(org, repo_name, team_name)? + } + RepoCollaborator::User(user_name) => { + sync.remove_collaborator_from_repo(org, repo_name, user_name)? + } }, } Ok(()) @@ -705,21 +704,21 @@ struct BranchProtectionDiff { impl BranchProtectionDiff { fn apply( &self, - sync: &SyncGitHub, + sync: &GitHubWrite, org: &str, repo_name: &str, repo_id: &str, ) -> anyhow::Result<()> { match &self.operation { BranchProtectionDiffOperation::Create(bp) => { - sync.github.upsert_branch_protection( + sync.upsert_branch_protection( BranchProtectionOp::CreateForRepo(repo_id.to_string()), &self.pattern, bp, )?; } BranchProtectionDiffOperation::Update(id, _, bp) => { - sync.github.upsert_branch_protection( + sync.upsert_branch_protection( BranchProtectionOp::UpdateBranchProtection(id.clone()), &self.pattern, bp, @@ -731,7 +730,7 @@ impl BranchProtectionDiff { the protection is not in the team repo", self.pattern, org, repo_name ); - sync.github.delete_branch_protection(org, repo_name, id)?; + sync.delete_branch_protection(org, repo_name, id)?; } } @@ -799,7 +798,7 @@ enum TeamDiff { } impl TeamDiff { - fn apply(self, sync: &SyncGitHub) -> anyhow::Result<()> { + fn apply(self, sync: &GitHubWrite) -> anyhow::Result<()> { match self { TeamDiff::Create(c) => c.apply(sync)?, TeamDiff::Edit(e) => e.apply(sync)?, @@ -829,9 +828,8 @@ struct CreateTeamDiff { } impl CreateTeamDiff { - fn apply(self, sync: &SyncGitHub) -> anyhow::Result<()> { - sync.github - .create_team(&self.org, &self.name, &self.description, self.privacy)?; + fn apply(self, sync: &GitHubWrite) -> anyhow::Result<()> { + sync.create_team(&self.org, &self.name, &self.description, self.privacy)?; for (member_name, role) in self.members { MemberDiff::Create(role).apply(&self.org, &self.name, &member_name, sync)?; } @@ -872,12 +870,12 @@ struct EditTeamDiff { } impl EditTeamDiff { - fn apply(self, sync: &SyncGitHub) -> anyhow::Result<()> { + fn apply(self, sync: &GitHubWrite) -> anyhow::Result<()> { if self.name_diff.is_some() || self.description_diff.is_some() || self.privacy_diff.is_some() { - sync.github.edit_team( + sync.edit_team( &self.org, &self.name, self.name_diff.as_deref(), @@ -946,12 +944,12 @@ enum MemberDiff { } impl MemberDiff { - fn apply(self, org: &str, team: &str, member: &str, sync: &SyncGitHub) -> anyhow::Result<()> { + fn apply(self, org: &str, team: &str, member: &str, sync: &GitHubWrite) -> anyhow::Result<()> { match self { MemberDiff::Create(role) | MemberDiff::ChangeRole((_, role)) => { - sync.github.set_team_membership(org, team, member, role)?; + sync.set_team_membership(org, team, member, role)?; } - MemberDiff::Delete => sync.github.remove_team_membership(org, team, member)?, + MemberDiff::Delete => sync.remove_team_membership(org, team, member)?, MemberDiff::Noop => {} } @@ -970,8 +968,8 @@ struct DeleteTeamDiff { } impl DeleteTeamDiff { - fn apply(self, sync: &SyncGitHub) -> anyhow::Result<()> { - sync.github.delete_team(&self.org, &self.slug)?; + fn apply(self, sync: &GitHubWrite) -> anyhow::Result<()> { + sync.delete_team(&self.org, &self.slug)?; Ok(()) } } diff --git a/src/main.rs b/src/main.rs index 0242bdb..21723c7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,7 +4,7 @@ mod team_api; mod utils; mod zulip; -use crate::github::{GitHub, SyncGitHub}; +use crate::github::{GitHub, GitHubWrite, SyncGitHub}; use crate::team_api::TeamApi; use anyhow::Context; use log::{error, info, warn}; @@ -81,12 +81,13 @@ fn app() -> anyhow::Result<()> { match service.as_str() { "github" => { let token = get_env("GITHUB_TOKEN")?; - let github = GitHub::new(token, dry_run)?; + let github = GitHub::new(token.clone())?; let sync = SyncGitHub::new(github, &team_api)?; let diff = sync.diff_all()?; info!("{}", diff); if !only_print_plan { - diff.apply(&sync)?; + let github_write = GitHubWrite::new(token, dry_run)?; + diff.apply(&github_write)?; } } "mailgun" => { From 0857aed5fa1cf11ab2ce2f926c9211cfddd46ba4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 9 Jan 2024 16:46:00 +0100 Subject: [PATCH 06/10] Rename `GitHub` to `GitHubRead` --- src/github/api/mod.rs | 315 +-------------------------------------- src/github/api/read.rs | 318 ++++++++++++++++++++++++++++++++++++++++ src/github/api/write.rs | 6 +- src/github/mod.rs | 6 +- src/main.rs | 10 +- 5 files changed, 331 insertions(+), 324 deletions(-) create mode 100644 src/github/api/read.rs diff --git a/src/github/api/mod.rs b/src/github/api/mod.rs index e7dcb50..09932d8 100644 --- a/src/github/api/mod.rs +++ b/src/github/api/mod.rs @@ -1,3 +1,4 @@ +mod read; mod write; use crate::utils::ResponseExt; @@ -12,9 +13,9 @@ use reqwest::{ }; use serde::{de::DeserializeOwned, Deserialize}; use std::borrow::Cow; -use std::collections::{HashMap, HashSet}; use std::fmt; +pub(crate) use read::GitHubRead; pub(crate) use write::GitHubWrite; struct HttpClient { @@ -137,318 +138,6 @@ impl HttpClient { } } -pub struct GitHub { - client: HttpClient, -} - -impl GitHub { - pub(crate) fn new(token: String) -> anyhow::Result { - Ok(Self { - client: HttpClient::from_token(token)?, - }) - } - - /// Get user names by user ids - pub(crate) fn usernames(&self, ids: &[usize]) -> anyhow::Result> { - #[derive(serde::Deserialize)] - #[serde(rename_all = "camelCase")] - struct Usernames { - database_id: usize, - login: String, - } - #[derive(serde::Serialize)] - struct Params { - ids: Vec, - } - static QUERY: &str = " - query($ids: [ID!]!) { - nodes(ids: $ids) { - ... on User { - databaseId - login - } - } - } - "; - - let mut result = HashMap::new(); - for chunk in ids.chunks(100) { - let res: GraphNodes = self.client.graphql( - QUERY, - Params { - ids: chunk.iter().map(|id| user_node_id(*id)).collect(), - }, - )?; - for node in res.nodes.into_iter().flatten() { - result.insert(node.database_id, node.login); - } - } - Ok(result) - } - - /// Get the owners of an org - pub(crate) fn org_owners(&self, org: &str) -> anyhow::Result> { - #[derive(serde::Deserialize, Eq, PartialEq, Hash)] - struct User { - id: usize, - } - let mut owners = HashSet::new(); - self.client.rest_paginated( - &Method::GET, - format!("orgs/{org}/members?role=admin"), - |resp: Vec| { - owners.extend(resp.into_iter().map(|u| u.id)); - Ok(()) - }, - )?; - Ok(owners) - } - - /// Get all teams associated with a org - /// - /// Returns a list of tuples of team name and slug - pub(crate) fn org_teams(&self, org: &str) -> anyhow::Result> { - let mut teams = Vec::new(); - - self.client.rest_paginated( - &Method::GET, - format!("orgs/{org}/teams"), - |resp: Vec| { - teams.extend(resp.into_iter().map(|t| (t.name, t.slug))); - Ok(()) - }, - )?; - - Ok(teams) - } - - /// Get the team by name and org - pub(crate) fn team(&self, org: &str, team: &str) -> anyhow::Result> { - self.client - .send_option(Method::GET, &format!("orgs/{org}/teams/{team}")) - } - - pub(crate) fn team_memberships( - &self, - team: &Team, - ) -> anyhow::Result> { - #[derive(serde::Deserialize)] - struct RespTeam { - members: RespMembers, - } - #[derive(serde::Deserialize)] - #[serde(rename_all = "camelCase")] - struct RespMembers { - page_info: GraphPageInfo, - edges: Vec, - } - #[derive(serde::Deserialize)] - struct RespEdge { - role: TeamRole, - node: RespNode, - } - #[derive(serde::Deserialize)] - #[serde(rename_all = "camelCase")] - struct RespNode { - database_id: usize, - login: String, - } - #[derive(serde::Serialize)] - struct Params<'a> { - team: String, - cursor: Option<&'a str>, - } - static QUERY: &str = " - query($team: ID!, $cursor: String) { - node(id: $team) { - ... on Team { - members(after: $cursor) { - pageInfo { - endCursor - hasNextPage - } - edges { - role - node { - databaseId - login - } - } - } - } - } - } - "; - - let mut memberships = HashMap::new(); - // Return the empty HashMap on new teams from dry runs - if let Some(id) = team.id { - let mut page_info = GraphPageInfo::start(); - while page_info.has_next_page { - let res: GraphNode = self.client.graphql( - QUERY, - Params { - team: team_node_id(id), - cursor: page_info.end_cursor.as_deref(), - }, - )?; - if let Some(team) = res.node { - page_info = team.members.page_info; - for edge in team.members.edges.into_iter() { - memberships.insert( - edge.node.database_id, - TeamMember { - username: edge.node.login, - role: edge.role, - }, - ); - } - } - } - } - - Ok(memberships) - } - - /// The GitHub names of users invited to the given team - pub(crate) fn team_membership_invitations( - &self, - org: &str, - team: &str, - ) -> anyhow::Result> { - let mut invites = HashSet::new(); - - self.client.rest_paginated( - &Method::GET, - format!("orgs/{org}/teams/{team}/invitations"), - |resp: Vec| { - invites.extend(resp.into_iter().map(|l| l.login)); - Ok(()) - }, - )?; - - Ok(invites) - } - - /// Get a repo by org and name - pub(crate) fn repo(&self, org: &str, repo: &str) -> anyhow::Result> { - self.client - .send_option(Method::GET, &format!("repos/{org}/{repo}")) - } - - /// Get teams in a repo - pub(crate) fn repo_teams(&self, org: &str, repo: &str) -> anyhow::Result> { - let mut teams = Vec::new(); - - self.client.rest_paginated( - &Method::GET, - format!("repos/{org}/{repo}/teams"), - |resp: Vec| { - teams.extend(resp); - Ok(()) - }, - )?; - - Ok(teams) - } - - /// Get collaborators in a repo - /// - /// Only fetches those who are direct collaborators (i.e., not a collaborator through a repo team) - pub(crate) fn repo_collaborators( - &self, - org: &str, - repo: &str, - ) -> anyhow::Result> { - let mut users = Vec::new(); - - self.client.rest_paginated( - &Method::GET, - format!("repos/{org}/{repo}/collaborators?affiliation=direct"), - |resp: Vec| { - users.extend(resp); - Ok(()) - }, - )?; - - Ok(users) - } - - /// Get branch_protections - pub(crate) fn branch_protections( - &self, - org: &str, - repo: &str, - ) -> anyhow::Result> { - #[derive(serde::Serialize)] - struct Params<'a> { - org: &'a str, - repo: &'a str, - } - static QUERY: &str = " - query($org:String!,$repo:String!) { - repository(owner:$org, name:$repo) { - branchProtectionRules(first:100) { - nodes { - id, - pattern, - isAdminEnforced, - dismissesStaleReviews, - requiredStatusCheckContexts, - requiredApprovingReviewCount - pushAllowances(first: 100) { - nodes { - actor { - ... on Actor { - login - } - ... on Team { - organization { - login - }, - name - } - } - } - } - } - } - } - } - "; - - #[derive(serde::Deserialize)] - struct Wrapper { - repository: Respository, - } - #[derive(serde::Deserialize)] - #[serde(rename_all = "camelCase")] - struct Respository { - branch_protection_rules: GraphNodes, - } - #[derive(serde::Deserialize)] - #[serde(rename_all = "camelCase")] - struct BranchProtectionWrapper { - id: String, - #[serde(flatten)] - protection: BranchProtection, - } - - let mut result = HashMap::new(); - let res: Wrapper = self.client.graphql(QUERY, Params { org, repo })?; - for node in res - .repository - .branch_protection_rules - .nodes - .into_iter() - .flatten() - { - result.insert(node.protection.pattern.clone(), (node.id, node.protection)); - } - Ok(result) - } -} - fn allow_not_found(resp: Response, method: Method, url: &str) -> Result<(), anyhow::Error> { match resp.status() { StatusCode::NOT_FOUND => { diff --git a/src/github/api/read.rs b/src/github/api/read.rs new file mode 100644 index 0000000..30c42ec --- /dev/null +++ b/src/github/api/read.rs @@ -0,0 +1,318 @@ +use crate::github::api::{ + team_node_id, user_node_id, BranchProtection, GraphNode, GraphNodes, GraphPageInfo, HttpClient, + Login, Repo, RepoTeam, RepoUser, Team, TeamMember, TeamRole, +}; +use reqwest::Method; +use std::collections::{HashMap, HashSet}; + +pub(crate) struct GitHubRead { + client: HttpClient, +} + +impl GitHubRead { + pub(crate) fn new(token: String) -> anyhow::Result { + Ok(Self { + client: HttpClient::from_token(token)?, + }) + } + + /// Get user names by user ids + pub(crate) fn usernames(&self, ids: &[usize]) -> anyhow::Result> { + #[derive(serde::Deserialize)] + #[serde(rename_all = "camelCase")] + struct Usernames { + database_id: usize, + login: String, + } + #[derive(serde::Serialize)] + struct Params { + ids: Vec, + } + static QUERY: &str = " + query($ids: [ID!]!) { + nodes(ids: $ids) { + ... on User { + databaseId + login + } + } + } + "; + + let mut result = HashMap::new(); + for chunk in ids.chunks(100) { + let res: GraphNodes = self.client.graphql( + QUERY, + Params { + ids: chunk.iter().map(|id| user_node_id(*id)).collect(), + }, + )?; + for node in res.nodes.into_iter().flatten() { + result.insert(node.database_id, node.login); + } + } + Ok(result) + } + + /// Get the owners of an org + pub(crate) fn org_owners(&self, org: &str) -> anyhow::Result> { + #[derive(serde::Deserialize, Eq, PartialEq, Hash)] + struct User { + id: usize, + } + let mut owners = HashSet::new(); + self.client.rest_paginated( + &Method::GET, + format!("orgs/{org}/members?role=admin"), + |resp: Vec| { + owners.extend(resp.into_iter().map(|u| u.id)); + Ok(()) + }, + )?; + Ok(owners) + } + + /// Get all teams associated with a org + /// + /// Returns a list of tuples of team name and slug + pub(crate) fn org_teams(&self, org: &str) -> anyhow::Result> { + let mut teams = Vec::new(); + + self.client.rest_paginated( + &Method::GET, + format!("orgs/{org}/teams"), + |resp: Vec| { + teams.extend(resp.into_iter().map(|t| (t.name, t.slug))); + Ok(()) + }, + )?; + + Ok(teams) + } + + /// Get the team by name and org + pub(crate) fn team(&self, org: &str, team: &str) -> anyhow::Result> { + self.client + .send_option(Method::GET, &format!("orgs/{org}/teams/{team}")) + } + + pub(crate) fn team_memberships( + &self, + team: &Team, + ) -> anyhow::Result> { + #[derive(serde::Deserialize)] + struct RespTeam { + members: RespMembers, + } + #[derive(serde::Deserialize)] + #[serde(rename_all = "camelCase")] + struct RespMembers { + page_info: GraphPageInfo, + edges: Vec, + } + #[derive(serde::Deserialize)] + struct RespEdge { + role: TeamRole, + node: RespNode, + } + #[derive(serde::Deserialize)] + #[serde(rename_all = "camelCase")] + struct RespNode { + database_id: usize, + login: String, + } + #[derive(serde::Serialize)] + struct Params<'a> { + team: String, + cursor: Option<&'a str>, + } + static QUERY: &str = " + query($team: ID!, $cursor: String) { + node(id: $team) { + ... on Team { + members(after: $cursor) { + pageInfo { + endCursor + hasNextPage + } + edges { + role + node { + databaseId + login + } + } + } + } + } + } + "; + + let mut memberships = HashMap::new(); + // Return the empty HashMap on new teams from dry runs + if let Some(id) = team.id { + let mut page_info = GraphPageInfo::start(); + while page_info.has_next_page { + let res: GraphNode = self.client.graphql( + QUERY, + Params { + team: team_node_id(id), + cursor: page_info.end_cursor.as_deref(), + }, + )?; + if let Some(team) = res.node { + page_info = team.members.page_info; + for edge in team.members.edges.into_iter() { + memberships.insert( + edge.node.database_id, + TeamMember { + username: edge.node.login, + role: edge.role, + }, + ); + } + } + } + } + + Ok(memberships) + } + + /// The GitHub names of users invited to the given team + pub(crate) fn team_membership_invitations( + &self, + org: &str, + team: &str, + ) -> anyhow::Result> { + let mut invites = HashSet::new(); + + self.client.rest_paginated( + &Method::GET, + format!("orgs/{org}/teams/{team}/invitations"), + |resp: Vec| { + invites.extend(resp.into_iter().map(|l| l.login)); + Ok(()) + }, + )?; + + Ok(invites) + } + + /// Get a repo by org and name + pub(crate) fn repo(&self, org: &str, repo: &str) -> anyhow::Result> { + self.client + .send_option(Method::GET, &format!("repos/{org}/{repo}")) + } + + /// Get teams in a repo + pub(crate) fn repo_teams(&self, org: &str, repo: &str) -> anyhow::Result> { + let mut teams = Vec::new(); + + self.client.rest_paginated( + &Method::GET, + format!("repos/{org}/{repo}/teams"), + |resp: Vec| { + teams.extend(resp); + Ok(()) + }, + )?; + + Ok(teams) + } + + /// Get collaborators in a repo + /// + /// Only fetches those who are direct collaborators (i.e., not a collaborator through a repo team) + pub(crate) fn repo_collaborators( + &self, + org: &str, + repo: &str, + ) -> anyhow::Result> { + let mut users = Vec::new(); + + self.client.rest_paginated( + &Method::GET, + format!("repos/{org}/{repo}/collaborators?affiliation=direct"), + |resp: Vec| { + users.extend(resp); + Ok(()) + }, + )?; + + Ok(users) + } + + /// Get branch_protections + pub(crate) fn branch_protections( + &self, + org: &str, + repo: &str, + ) -> anyhow::Result> { + #[derive(serde::Serialize)] + struct Params<'a> { + org: &'a str, + repo: &'a str, + } + static QUERY: &str = " + query($org:String!,$repo:String!) { + repository(owner:$org, name:$repo) { + branchProtectionRules(first:100) { + nodes { + id, + pattern, + isAdminEnforced, + dismissesStaleReviews, + requiredStatusCheckContexts, + requiredApprovingReviewCount + pushAllowances(first: 100) { + nodes { + actor { + ... on Actor { + login + } + ... on Team { + organization { + login + }, + name + } + } + } + } + } + } + } + } + "; + + #[derive(serde::Deserialize)] + struct Wrapper { + repository: Respository, + } + #[derive(serde::Deserialize)] + #[serde(rename_all = "camelCase")] + struct Respository { + branch_protection_rules: GraphNodes, + } + #[derive(serde::Deserialize)] + #[serde(rename_all = "camelCase")] + struct BranchProtectionWrapper { + id: String, + #[serde(flatten)] + protection: BranchProtection, + } + + let mut result = HashMap::new(); + let res: Wrapper = self.client.graphql(QUERY, Params { org, repo })?; + for node in res + .repository + .branch_protection_rules + .nodes + .into_iter() + .flatten() + { + result.insert(node.protection.pattern.clone(), (node.id, node.protection)); + } + Ok(result) + } +} diff --git a/src/github/api/write.rs b/src/github/api/write.rs index 1a22074..5a130b9 100644 --- a/src/github/api/write.rs +++ b/src/github/api/write.rs @@ -2,18 +2,18 @@ use anyhow::Context; use log::debug; use reqwest::Method; +use crate::github::api::read::GitHubRead; use crate::github::api::{ allow_not_found, BranchProtection, BranchProtectionOp, HttpClient, Login, PushAllowanceActor, Repo, RepoPermission, Team, TeamPrivacy, TeamPushAllowanceActor, TeamRole, UserPushAllowanceActor, }; -use crate::github::GitHub; use crate::utils::ResponseExt; pub(crate) struct GitHubWrite { client: HttpClient, dry_run: bool, - read: GitHub, + read: GitHubRead, } impl GitHubWrite { @@ -21,7 +21,7 @@ impl GitHubWrite { Ok(Self { client: HttpClient::from_token(token.clone())?, dry_run, - read: GitHub::new(token)?, + read: GitHubRead::new(token)?, }) } diff --git a/src/github/mod.rs b/src/github/mod.rs index 2c27bb9..6aacca9 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -7,13 +7,13 @@ use rust_team_data::v1::Bot; use std::collections::{HashMap, HashSet}; use std::fmt::Write; -pub(crate) use self::api::{GitHub, GitHubWrite}; +pub(crate) use self::api::{GitHubRead, GitHubWrite}; static DEFAULT_DESCRIPTION: &str = "Managed by the rust-lang/team repository."; static DEFAULT_PRIVACY: TeamPrivacy = TeamPrivacy::Closed; pub(crate) struct SyncGitHub { - github: GitHub, + github: GitHubRead, teams: Vec, repos: Vec, usernames_cache: HashMap, @@ -21,7 +21,7 @@ pub(crate) struct SyncGitHub { } impl SyncGitHub { - pub(crate) fn new(github: GitHub, team_api: &TeamApi) -> anyhow::Result { + pub(crate) fn new(github: GitHubRead, team_api: &TeamApi) -> anyhow::Result { let teams = team_api.get_teams()?; let repos = team_api.get_repos()?; diff --git a/src/main.rs b/src/main.rs index 21723c7..b214600 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,7 +4,7 @@ mod team_api; mod utils; mod zulip; -use crate::github::{GitHub, GitHubWrite, SyncGitHub}; +use crate::github::{GitHubRead, GitHubWrite, SyncGitHub}; use crate::team_api::TeamApi; use anyhow::Context; use log::{error, info, warn}; @@ -81,13 +81,13 @@ fn app() -> anyhow::Result<()> { match service.as_str() { "github" => { let token = get_env("GITHUB_TOKEN")?; - let github = GitHub::new(token.clone())?; - let sync = SyncGitHub::new(github, &team_api)?; + let gh_read = GitHubRead::new(token.clone())?; + let sync = SyncGitHub::new(gh_read, &team_api)?; let diff = sync.diff_all()?; info!("{}", diff); if !only_print_plan { - let github_write = GitHubWrite::new(token, dry_run)?; - diff.apply(&github_write)?; + let gh_write = GitHubWrite::new(token, dry_run)?; + diff.apply(&gh_write)?; } } "mailgun" => { From 49479e36dabd792f57a548c50068f4bc3df20da4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 9 Jan 2024 16:50:33 +0100 Subject: [PATCH 07/10] Simplify external API of diffing Also pass teams and repos to `create_diff` explicitly, which will allow easier mocking. --- src/github/mod.rs | 22 ++++++++++++++++------ src/main.rs | 7 ++++--- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/github/mod.rs b/src/github/mod.rs index 6aacca9..0451662 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -1,7 +1,7 @@ mod api; use self::api::{BranchProtectionOp, TeamPrivacy, TeamRole}; -use crate::{github::api::RepoPermission, TeamApi}; +use crate::github::api::RepoPermission; use log::debug; use rust_team_data::v1::Bot; use std::collections::{HashMap, HashSet}; @@ -12,7 +12,16 @@ pub(crate) use self::api::{GitHubRead, GitHubWrite}; static DEFAULT_DESCRIPTION: &str = "Managed by the rust-lang/team repository."; static DEFAULT_PRIVACY: TeamPrivacy = TeamPrivacy::Closed; -pub(crate) struct SyncGitHub { +pub(crate) fn create_diff( + github: GitHubRead, + teams: Vec, + repos: Vec, +) -> anyhow::Result { + let github = SyncGitHub::new(github, teams, repos)?; + github.diff_all() +} + +struct SyncGitHub { github: GitHubRead, teams: Vec, repos: Vec, @@ -21,10 +30,11 @@ pub(crate) struct SyncGitHub { } impl SyncGitHub { - pub(crate) fn new(github: GitHubRead, team_api: &TeamApi) -> anyhow::Result { - let teams = team_api.get_teams()?; - let repos = team_api.get_repos()?; - + pub(crate) fn new( + github: GitHubRead, + teams: Vec, + repos: Vec, + ) -> anyhow::Result { debug!("caching mapping between user ids and usernames"); let users = teams .iter() diff --git a/src/main.rs b/src/main.rs index b214600..a2e04e0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,7 +4,7 @@ mod team_api; mod utils; mod zulip; -use crate::github::{GitHubRead, GitHubWrite, SyncGitHub}; +use crate::github::{create_diff, GitHubRead, GitHubWrite}; use crate::team_api::TeamApi; use anyhow::Context; use log::{error, info, warn}; @@ -82,8 +82,9 @@ fn app() -> anyhow::Result<()> { "github" => { let token = get_env("GITHUB_TOKEN")?; let gh_read = GitHubRead::new(token.clone())?; - let sync = SyncGitHub::new(gh_read, &team_api)?; - let diff = sync.diff_all()?; + let teams = team_api.get_teams()?; + let repos = team_api.get_repos()?; + let diff = create_diff(gh_read, teams, repos)?; info!("{}", diff); if !only_print_plan { let gh_write = GitHubWrite::new(token, dry_run)?; From 45d59503be1f1cc9de91c9a2f4cde40899b09d16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 9 Jan 2024 17:17:16 +0100 Subject: [PATCH 08/10] Pass `HttpClient` to `GitHubRead` and `GitHubWrite` --- src/github/api/mod.rs | 13 ++++++++++--- src/github/api/read.rs | 6 ++---- src/github/api/write.rs | 6 +++--- src/github/mod.rs | 2 +- src/main.rs | 8 +++++--- 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/github/api/mod.rs b/src/github/api/mod.rs index 09932d8..7955028 100644 --- a/src/github/api/mod.rs +++ b/src/github/api/mod.rs @@ -18,12 +18,14 @@ use std::fmt; pub(crate) use read::GitHubRead; pub(crate) use write::GitHubWrite; -struct HttpClient { +#[derive(Clone)] +pub(crate) struct HttpClient { client: Client, + base_url: String, } impl HttpClient { - fn from_token(token: String) -> anyhow::Result { + pub(crate) fn from_url_and_token(mut base_url: String, token: String) -> anyhow::Result { let builder = reqwest::blocking::ClientBuilder::default(); let mut map = HeaderMap::default(); let mut auth = HeaderValue::from_str(&format!("token {}", token))?; @@ -35,8 +37,13 @@ impl HttpClient { HeaderValue::from_static(crate::USER_AGENT), ); + if !base_url.ends_with("/") { + base_url.push('/'); + } + Ok(Self { client: builder.build()?, + base_url, }) } @@ -44,7 +51,7 @@ impl HttpClient { let url = if url.starts_with("https://") { Cow::Borrowed(url) } else { - Cow::Owned(format!("https://api.github.com/{url}")) + Cow::Owned(format!("{}{url}", self.base_url)) }; trace!("http request: {} {}", method, url); Ok(self.client.request(method, url.as_ref())) diff --git a/src/github/api/read.rs b/src/github/api/read.rs index 30c42ec..e7edd7d 100644 --- a/src/github/api/read.rs +++ b/src/github/api/read.rs @@ -10,10 +10,8 @@ pub(crate) struct GitHubRead { } impl GitHubRead { - pub(crate) fn new(token: String) -> anyhow::Result { - Ok(Self { - client: HttpClient::from_token(token)?, - }) + pub(crate) fn from_client(client: HttpClient) -> anyhow::Result { + Ok(Self { client }) } /// Get user names by user ids diff --git a/src/github/api/write.rs b/src/github/api/write.rs index 5a130b9..f0adcb4 100644 --- a/src/github/api/write.rs +++ b/src/github/api/write.rs @@ -17,11 +17,11 @@ pub(crate) struct GitHubWrite { } impl GitHubWrite { - pub(crate) fn new(token: String, dry_run: bool) -> anyhow::Result { + pub(crate) fn new(client: HttpClient, dry_run: bool) -> anyhow::Result { Ok(Self { - client: HttpClient::from_token(token.clone())?, + client: client.clone(), dry_run, - read: GitHubRead::new(token)?, + read: GitHubRead::from_client(client)?, }) } diff --git a/src/github/mod.rs b/src/github/mod.rs index 0451662..7a88680 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -7,7 +7,7 @@ use rust_team_data::v1::Bot; use std::collections::{HashMap, HashSet}; use std::fmt::Write; -pub(crate) use self::api::{GitHubRead, GitHubWrite}; +pub(crate) use self::api::{GitHubRead, GitHubWrite, HttpClient}; static DEFAULT_DESCRIPTION: &str = "Managed by the rust-lang/team repository."; static DEFAULT_PRIVACY: TeamPrivacy = TeamPrivacy::Closed; diff --git a/src/main.rs b/src/main.rs index a2e04e0..646f07b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,7 +4,7 @@ mod team_api; mod utils; mod zulip; -use crate::github::{create_diff, GitHubRead, GitHubWrite}; +use crate::github::{create_diff, GitHubRead, GitHubWrite, HttpClient}; use crate::team_api::TeamApi; use anyhow::Context; use log::{error, info, warn}; @@ -81,13 +81,15 @@ fn app() -> anyhow::Result<()> { match service.as_str() { "github" => { let token = get_env("GITHUB_TOKEN")?; - let gh_read = GitHubRead::new(token.clone())?; + let client = + HttpClient::from_url_and_token("https://api.github.com/".to_string(), token)?; + let gh_read = GitHubRead::from_client(client.clone())?; let teams = team_api.get_teams()?; let repos = team_api.get_repos()?; let diff = create_diff(gh_read, teams, repos)?; info!("{}", diff); if !only_print_plan { - let gh_write = GitHubWrite::new(token, dry_run)?; + let gh_write = GitHubWrite::new(client, dry_run)?; diff.apply(&gh_write)?; } } From 357f2c7ab1e9edfd057d71b582fa4dc5159b681b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 15 Jan 2024 14:10:08 +0100 Subject: [PATCH 09/10] Add read-only flag to `HttpClient` --- src/github/api/mod.rs | 15 ++++++++++++++- src/main.rs | 14 +++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/github/api/mod.rs b/src/github/api/mod.rs index 7955028..149f023 100644 --- a/src/github/api/mod.rs +++ b/src/github/api/mod.rs @@ -22,10 +22,16 @@ pub(crate) use write::GitHubWrite; pub(crate) struct HttpClient { client: Client, base_url: String, + /// If true, only read-only HTTP methods (GET, HEAD, etc.) are allowed. + read_only: bool, } impl HttpClient { - pub(crate) fn from_url_and_token(mut base_url: String, token: String) -> anyhow::Result { + pub(crate) fn from_url_and_token( + mut base_url: String, + token: String, + read_only: bool, + ) -> anyhow::Result { let builder = reqwest::blocking::ClientBuilder::default(); let mut map = HeaderMap::default(); let mut auth = HeaderValue::from_str(&format!("token {}", token))?; @@ -44,10 +50,17 @@ impl HttpClient { Ok(Self { client: builder.build()?, base_url, + read_only, }) } fn req(&self, method: Method, url: &str) -> anyhow::Result { + if self.read_only && !method.is_safe() { + return Err(anyhow::anyhow!( + "This HTTP client can only be used to perform read-only requests. Method {method} is not allowed." + )); + } + let url = if url.starts_with("https://") { Cow::Borrowed(url) } else { diff --git a/src/main.rs b/src/main.rs index 646f07b..2b351af 100644 --- a/src/main.rs +++ b/src/main.rs @@ -81,14 +81,22 @@ fn app() -> anyhow::Result<()> { match service.as_str() { "github" => { let token = get_env("GITHUB_TOKEN")?; - let client = - HttpClient::from_url_and_token("https://api.github.com/".to_string(), token)?; - let gh_read = GitHubRead::from_client(client.clone())?; + let client_readonly = HttpClient::from_url_and_token( + "https://api.github.com/".to_string(), + token.clone(), + true, + )?; + let gh_read = GitHubRead::from_client(client_readonly)?; let teams = team_api.get_teams()?; let repos = team_api.get_repos()?; let diff = create_diff(gh_read, teams, repos)?; info!("{}", diff); if !only_print_plan { + let client = HttpClient::from_url_and_token( + "https://api.github.com/".to_string(), + token, + false, + )?; let gh_write = GitHubWrite::new(client, dry_run)?; diff.apply(&gh_write)?; } From fa5a70d73ac519de8df5d003b22b99339aff9a06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 15 Jan 2024 14:11:38 +0100 Subject: [PATCH 10/10] Fix clippy --- src/github/api/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/github/api/mod.rs b/src/github/api/mod.rs index 149f023..f81f1af 100644 --- a/src/github/api/mod.rs +++ b/src/github/api/mod.rs @@ -43,7 +43,7 @@ impl HttpClient { HeaderValue::from_static(crate::USER_AGENT), ); - if !base_url.ends_with("/") { + if !base_url.ends_with('/') { base_url.push('/'); }