From c9f93a86396300389d610074beebaf6e5a67df8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Fri, 5 Dec 2025 21:02:18 +0100 Subject: [PATCH 1/4] Load all trusted publishing configs at once To avoid making an API request per crate. --- .github/workflows/dry-run.yml | 1 + .github/workflows/main.yml | 1 + sync-team/src/crates_io/api.rs | 31 ++++++++++++++++++++++++++--- sync-team/src/crates_io/mod.rs | 36 +++++++++++++++++++++++++++------- sync-team/src/lib.rs | 3 ++- 5 files changed, 61 insertions(+), 11 deletions(-) diff --git a/.github/workflows/dry-run.yml b/.github/workflows/dry-run.yml index 965d983d2..12e168d4e 100644 --- a/.github/workflows/dry-run.yml +++ b/.github/workflows/dry-run.yml @@ -69,6 +69,7 @@ jobs: # However, even without a token, we can actually read most of the crates.io state that we # need to print a diff. CRATES_IO_TOKEN: "" + CRATES_IO_USERNAME: "rust-lang-owner" # This applies pipefail, so that the tee pipeline below fails when sync-team fails. shell: bash run: | diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f39780e88..8d217eb4e 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -104,6 +104,7 @@ jobs: ZULIP_API_TOKEN: ${{ secrets.ZULIP_API_TOKEN }} ZULIP_USERNAME: ${{ secrets.ZULIP_USERNAME }} CRATES_IO_TOKEN: ${{ secrets.CRATES_IO_TOKEN }} + CRATES_IO_USERNAME: "rust-lang-owner" run: | cargo run sync apply --src build diff --git a/sync-team/src/crates_io/api.rs b/sync-team/src/crates_io/api.rs index f01820ebf..b8c17efb9 100644 --- a/sync-team/src/crates_io/api.rs +++ b/sync-team/src/crates_io/api.rs @@ -42,10 +42,30 @@ impl CratesIoApi { self.dry_run } + /// Return the user ID based on the username. + pub(crate) fn get_user_id(&self, username: &str) -> anyhow::Result { + #[derive(serde::Deserialize)] + struct User { + id: u32, + } + + #[derive(serde::Deserialize)] + struct UserResponse { + user: User, + } + + let response: UserResponse = self + .req::<()>(reqwest::Method::GET, &format!("/users/{username}"), None)? + .error_for_status()? + .json_annotated()?; + + Ok(UserId(response.user.id)) + } + /// List existing trusted publishing configurations for a given crate. pub(crate) fn list_trusted_publishing_github_configs( &self, - krate: &str, + user_id: UserId, ) -> anyhow::Result> { #[derive(serde::Deserialize)] struct GetTrustedPublishing { @@ -55,7 +75,7 @@ impl CratesIoApi { let response: GetTrustedPublishing = self .req::<()>( reqwest::Method::GET, - &format!("/trusted_publishing/github_configs?crate={krate}"), + &format!("/trusted_publishing/github_configs?user_id={}", user_id.0), None, )? .error_for_status()? @@ -208,6 +228,9 @@ impl CratesIoApi { } } +#[derive(Copy, Clone, Debug)] +pub struct UserId(pub u32); + #[derive(serde::Deserialize, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct TrustedPublishingId(u64); @@ -217,8 +240,10 @@ impl Display for TrustedPublishingId { } } -#[derive(serde::Deserialize, Debug)] +#[derive(serde::Deserialize, Debug, Clone)] pub(crate) struct TrustedPublishingGitHubConfig { + #[serde(rename = "crate")] + pub(crate) krate: String, pub(crate) id: TrustedPublishingId, pub(crate) repository_owner: String, pub(crate) repository_name: String, diff --git a/sync-team/src/crates_io/mod.rs b/sync-team/src/crates_io/mod.rs index 7b827acd5..e6a81a836 100644 --- a/sync-team/src/crates_io/mod.rs +++ b/sync-team/src/crates_io/mod.rs @@ -3,7 +3,7 @@ mod api; use crate::team_api::TeamApi; use std::cmp::Ordering; -use crate::crates_io::api::{CratesIoApi, TrustedPublishingGitHubConfig}; +use crate::crates_io::api::{CratesIoApi, TrustedPublishingGitHubConfig, UserId}; use anyhow::Context; use secrecy::SecretString; use std::collections::HashMap; @@ -31,15 +31,19 @@ struct CrateConfig { pub(crate) struct SyncCratesIo { crates_io_api: CratesIoApi, crates: HashMap, + user_id: UserId, } impl SyncCratesIo { pub(crate) fn new( token: SecretString, + username: String, team_api: &TeamApi, dry_run: bool, ) -> anyhow::Result { let crates_io_api = CratesIoApi::new(token, dry_run); + let user_id = crates_io_api.get_user_id(&username)?; + let crates: HashMap = team_api .get_repos()? .into_iter() @@ -69,6 +73,7 @@ impl SyncCratesIo { Ok(Self { crates_io_api, crates, + user_id, }) } @@ -77,6 +82,23 @@ impl SyncCratesIo { let mut crate_diffs: Vec = vec![]; let is_ci_dry_run = std::env::var("CI").is_ok() && self.crates_io_api.is_dry_run(); + let mut tp_configs = if is_ci_dry_run { + HashMap::new() + } else { + let tp_configs = self + .crates_io_api + .list_trusted_publishing_github_configs(self.user_id) + .with_context(|| { + format!("Failed to list configs for user_id `{:?}`", self.user_id) + })?; + let tp_configs: HashMap> = tp_configs + .into_iter() + .fold(HashMap::new(), |mut map, config| { + map.entry(config.krate.clone()).or_default().push(config); + map + }); + tp_configs + }; // Note: we currently only support one trusted publishing configuration per crate for (krate, desired) in &self.crates { @@ -85,15 +107,15 @@ impl SyncCratesIo { // to enable doing a crates.io dry-run without a privileged token. // Because crates.io does not currently support read-only token if !is_ci_dry_run { - let mut configs = self - .crates_io_api - .list_trusted_publishing_github_configs(&krate.0) - .with_context(|| format!("Failed to list configs for crate '{}'", krate.0))?; + let mut empty_vec = vec![]; + let configs = tp_configs.get_mut(&krate.0).unwrap_or(&mut empty_vec); - // Find if there are config(s) that match what we need + // Find if there are config(s) that match what we need and remove them from the list + // of found configs. let matching_configs = configs .extract_if(.., |config| { let TrustedPublishingGitHubConfig { + krate: _, id: _, repository_owner, repository_name, @@ -118,7 +140,7 @@ impl SyncCratesIo { } // Non-matching configs should be deleted - config_diffs.extend(configs.into_iter().map(ConfigDiff::Delete)); + config_diffs.extend(configs.iter_mut().map(|c| ConfigDiff::Delete(c.clone()))); } let trusted_publish_only_expected = desired.trusted_publishing_only; diff --git a/sync-team/src/lib.rs b/sync-team/src/lib.rs index a413982e6..e327f0686 100644 --- a/sync-team/src/lib.rs +++ b/sync-team/src/lib.rs @@ -70,7 +70,8 @@ pub fn run_sync_team( } "crates-io" => { let token = SecretString::from(get_env("CRATES_IO_TOKEN")?); - let sync = SyncCratesIo::new(token, &team_api, dry_run)?; + let username = get_env("CRATES_IO_USERNAME")?; + let sync = SyncCratesIo::new(token, username, &team_api, dry_run)?; let diff = sync.diff_all()?; if !diff.is_empty() { info!("{diff}"); From db8f77f7ce6ee10fb7e100cb2bf449383cfd635d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Fri, 5 Dec 2025 21:02:28 +0100 Subject: [PATCH 2/4] Remove trusted publishing configs that are not configured in `team` --- sync-team/src/crates_io/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sync-team/src/crates_io/mod.rs b/sync-team/src/crates_io/mod.rs index e6a81a836..cd4b2f4e7 100644 --- a/sync-team/src/crates_io/mod.rs +++ b/sync-team/src/crates_io/mod.rs @@ -156,6 +156,12 @@ impl SyncCratesIo { } } + // If any trusted publishing configs remained in the hashmap, they are leftover and should + // be removed. + for config in tp_configs.into_values().flatten() { + config_diffs.push(ConfigDiff::Delete(config)); + } + // We want to apply deletions first, and only then create new configs, to ensure that we // don't try to create a duplicate config where e.g. only the environment differs, which // would be an error in crates.io. From fba2b86dc37c2dc99fb4aafdcf22bd6d7160e34e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 6 Dec 2025 19:20:52 +0100 Subject: [PATCH 3/4] Implement paged crates.io API requests --- sync-team/src/crates_io/api.rs | 91 +++++++++++++++++++++++++++++----- 1 file changed, 79 insertions(+), 12 deletions(-) diff --git a/sync-team/src/crates_io/api.rs b/sync-team/src/crates_io/api.rs index b8c17efb9..b0cd4c3c7 100644 --- a/sync-team/src/crates_io/api.rs +++ b/sync-team/src/crates_io/api.rs @@ -7,6 +7,8 @@ use reqwest::header; use reqwest::header::{HeaderMap, HeaderValue}; use secrecy::{ExposeSecret, SecretString}; use serde::Serialize; +use serde::de::DeserializeOwned; +use std::collections::HashMap; use std::fmt::{Display, Formatter}; // OpenAPI spec: https://crates.io/api/openapi.json @@ -55,7 +57,12 @@ impl CratesIoApi { } let response: UserResponse = self - .req::<()>(reqwest::Method::GET, &format!("/users/{username}"), None)? + .req::<()>( + reqwest::Method::GET, + &format!("/users/{username}"), + HashMap::new(), + None, + )? .error_for_status()? .json_annotated()?; @@ -72,16 +79,15 @@ impl CratesIoApi { github_configs: Vec, } - let response: GetTrustedPublishing = self - .req::<()>( - reqwest::Method::GET, - &format!("/trusted_publishing/github_configs?user_id={}", user_id.0), - None, - )? - .error_for_status()? - .json_annotated()?; + let mut configs = vec![]; + self.req_paged::<(), GetTrustedPublishing, _>( + "/trusted_publishing/github_configs", + HashMap::from([("user_id".to_string(), user_id.0.to_string())]), + None, + |resp| configs.extend(resp.github_configs), + )?; - Ok(response.github_configs) + Ok(configs) } /// Create a new trusted publishing configuration for a given crate. @@ -130,6 +136,7 @@ impl CratesIoApi { self.req( reqwest::Method::POST, "/trusted_publishing/github_configs", + HashMap::new(), Some(&request), )? .error_for_status() @@ -149,6 +156,7 @@ impl CratesIoApi { self.req::<()>( reqwest::Method::DELETE, &format!("/trusted_publishing/github_configs/{}", id.0), + HashMap::new(), None, )? .error_for_status() @@ -167,7 +175,12 @@ impl CratesIoApi { } let response: CrateResponse = self - .req::<()>(reqwest::Method::GET, &format!("/crates/{krate}"), None)? + .req::<()>( + reqwest::Method::GET, + &format!("/crates/{krate}"), + HashMap::new(), + None, + )? .error_for_status()? .json_annotated()?; @@ -196,6 +209,7 @@ impl CratesIoApi { self.req( reqwest::Method::PATCH, &format!("/crates/{krate}"), + HashMap::new(), Some(&PatchCrateRequest { krate: Crate { trustpub_only: value, @@ -214,18 +228,71 @@ impl CratesIoApi { &self, method: reqwest::Method, path: &str, + query: HashMap, data: Option<&T>, ) -> anyhow::Result { let mut req = self .client .request(method, format!("{CRATES_IO_BASE_URL}{path}")) - .bearer_auth(self.token.expose_secret()); + .bearer_auth(self.token.expose_secret()) + .query(&query); if let Some(data) = data { req = req.json(data); } Ok(req.send()?) } + + /// Fetch a resource that is paged. + fn req_paged( + &self, + path: &str, + mut query: HashMap, + data: Option<&T>, + mut handle_response: F, + ) -> anyhow::Result<()> + where + F: FnMut(R), + { + #[derive(serde::Deserialize, Debug)] + struct Response { + meta: MetaResponse, + #[serde(flatten)] + data: R, + } + + #[derive(serde::Deserialize, Debug)] + struct MetaResponse { + next_page: Option, + } + + if !query.contains_key("per_page") { + query.insert("per_page".to_string(), "100".to_string()); + } + + let mut query = query; + let mut path_extra: Option = None; + loop { + let path = match path_extra { + Some(p) => format!("{path}{p}"), + None => path.to_owned(), + }; + + let response: Response = self + .req(reqwest::Method::GET, &path, query, data)? + .error_for_status()? + .json_annotated()?; + handle_response(response.data); + match response.meta.next_page { + Some(next) => { + path_extra = Some(next); + query = HashMap::new(); + } + None => break, + } + } + Ok(()) + } } #[derive(Copy, Clone, Debug)] From fccd667a2454bb7dc5e3ef2cd919895b8b065d5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 7 Dec 2025 15:29:10 +0100 Subject: [PATCH 4/4] Batch load crates owned by the crates.io user To avoid N+1 queries to fetch crate information. --- sync-team/src/crates_io/api.rs | 30 +++++++++++++++--------------- sync-team/src/crates_io/mod.rs | 27 +++++++++++++++++++-------- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/sync-team/src/crates_io/api.rs b/sync-team/src/crates_io/api.rs index b0cd4c3c7..9b249b4e5 100644 --- a/sync-team/src/crates_io/api.rs +++ b/sync-team/src/crates_io/api.rs @@ -166,25 +166,24 @@ impl CratesIoApi { Ok(()) } - /// Get information about a crate. - pub(crate) fn get_crate(&self, krate: &str) -> anyhow::Result { + /// Return all crates owned by the given user. + pub(crate) fn get_crates_owned_by(&self, user: UserId) -> anyhow::Result> { #[derive(serde::Deserialize)] - struct CrateResponse { - #[serde(rename = "crate")] - krate: CratesIoCrate, + struct CratesResponse { + crates: Vec, } - let response: CrateResponse = self - .req::<()>( - reqwest::Method::GET, - &format!("/crates/{krate}"), - HashMap::new(), - None, - )? - .error_for_status()? - .json_annotated()?; + let mut crates = vec![]; + self.req_paged::<(), CratesResponse, _>( + "/crates", + HashMap::from([("user_id".to_string(), user.0.to_string())]), + None, + |res| { + crates.extend(res.crates); + }, + )?; - Ok(response.krate) + Ok(crates) } /// Enable or disable the `trustpub_only` crate option, which specifies whether a crate @@ -320,6 +319,7 @@ pub(crate) struct TrustedPublishingGitHubConfig { #[derive(serde::Deserialize, Debug)] pub(crate) struct CratesIoCrate { + pub(crate) name: String, #[serde(rename = "trustpub_only")] pub(crate) trusted_publishing_only: bool, } diff --git a/sync-team/src/crates_io/mod.rs b/sync-team/src/crates_io/mod.rs index cd4b2f4e7..4cf438489 100644 --- a/sync-team/src/crates_io/mod.rs +++ b/sync-team/src/crates_io/mod.rs @@ -3,7 +3,7 @@ mod api; use crate::team_api::TeamApi; use std::cmp::Ordering; -use crate::crates_io::api::{CratesIoApi, TrustedPublishingGitHubConfig, UserId}; +use crate::crates_io::api::{CratesIoApi, CratesIoCrate, TrustedPublishingGitHubConfig, UserId}; use anyhow::Context; use secrecy::SecretString; use std::collections::HashMap; @@ -32,6 +32,7 @@ pub(crate) struct SyncCratesIo { crates_io_api: CratesIoApi, crates: HashMap, user_id: UserId, + username: String, } impl SyncCratesIo { @@ -74,6 +75,7 @@ impl SyncCratesIo { crates_io_api, crates, user_id, + username, }) } @@ -100,6 +102,14 @@ impl SyncCratesIo { tp_configs }; + // Batch load all crates owned by the current user + let crates: HashMap = self + .crates_io_api + .get_crates_owned_by(self.user_id)? + .into_iter() + .map(|krate| (krate.name.clone(), krate)) + .collect(); + // Note: we currently only support one trusted publishing configuration per crate for (krate, desired) in &self.crates { // Reading trusted publishing configs requires an authenticated token @@ -143,15 +153,16 @@ impl SyncCratesIo { config_diffs.extend(configs.iter_mut().map(|c| ConfigDiff::Delete(c.clone()))); } - let trusted_publish_only_expected = desired.trusted_publishing_only; - let crates_io_crate = self - .crates_io_api - .get_crate(&krate.0) - .with_context(|| anyhow::anyhow!("Cannot load crate {krate}"))?; - if crates_io_crate.trusted_publishing_only != trusted_publish_only_expected { + let Some(crates_io_crate) = crates.get(&krate.0) else { + return Err(anyhow::anyhow!( + "Crate `{krate}` is not owned by user `{0}`. Please invite `{0}` to be its owner.", + self.username + )); + }; + if crates_io_crate.trusted_publishing_only != desired.trusted_publishing_only { crate_diffs.push(CrateDiff::SetTrustedPublishingOnly { krate: krate.to_string(), - value: trusted_publish_only_expected, + value: desired.trusted_publishing_only, }); } }