Merge pull request #2160 from Kobzol/remove-unused-trusted-publishing

marcoieni · web-flow · commit 162e7ad6acc8 · 2025-12-10T21:18:24.000Z
Batch load trusted publishing configs and remove unused ones
diff --git 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
@@ -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
@@ -42,26 +44,50 @@ impl CratesIoApi {
         self.dry_run
     }
 
-    /// List existing trusted publishing configurations for a given crate.
-    pub(crate) fn list_trusted_publishing_github_configs(
-        &self,
-        krate: &str,
-    ) -> anyhow::Result<Vec<TrustedPublishingGitHubConfig>> {
+    /// Return the user ID based on the username.
+    pub(crate) fn get_user_id(&self, username: &str) -> anyhow::Result<UserId> {
         #[derive(serde::Deserialize)]
-        struct GetTrustedPublishing {
-            github_configs: Vec<TrustedPublishingGitHubConfig>,
+        struct User {
+            id: u32,
         }
 
-        let response: GetTrustedPublishing = self
+        #[derive(serde::Deserialize)]
+        struct UserResponse {
+            user: User,
+        }
+
+        let response: UserResponse = self
             .req::<()>(
                 reqwest::Method::GET,
-                &format!("/trusted_publishing/github_configs?crate={krate}"),
+                &format!("/users/{username}"),
+                HashMap::new(),
                 None,
             )?
             .error_for_status()?
             .json_annotated()?;
 
-        Ok(response.github_configs)
+        Ok(UserId(response.user.id))
+    }
+
+    /// List existing trusted publishing configurations for a given crate.
+    pub(crate) fn list_trusted_publishing_github_configs(
+        &self,
+        user_id: UserId,
+    ) -> anyhow::Result<Vec<TrustedPublishingGitHubConfig>> {
+        #[derive(serde::Deserialize)]
+        struct GetTrustedPublishing {
+            github_configs: Vec<TrustedPublishingGitHubConfig>,
+        }
+
+        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(configs)
     }
 
     /// Create a new trusted publishing configuration for a given crate.
@@ -110,6 +136,7 @@ impl CratesIoApi {
         self.req(
             reqwest::Method::POST,
             "/trusted_publishing/github_configs",
+            HashMap::new(),
             Some(&request),
         )?
         .error_for_status()
@@ -129,6 +156,7 @@ impl CratesIoApi {
             self.req::<()>(
                 reqwest::Method::DELETE,
                 &format!("/trusted_publishing/github_configs/{}", id.0),
+                HashMap::new(),
                 None,
             )?
             .error_for_status()
@@ -138,20 +166,24 @@ impl CratesIoApi {
         Ok(())
     }
 
-    /// Get information about a crate.
-    pub(crate) fn get_crate(&self, krate: &str) -> anyhow::Result<CratesIoCrate> {
+    /// Return all crates owned by the given user.
+    pub(crate) fn get_crates_owned_by(&self, user: UserId) -> anyhow::Result<Vec<CratesIoCrate>> {
         #[derive(serde::Deserialize)]
-        struct CrateResponse {
-            #[serde(rename = "crate")]
-            krate: CratesIoCrate,
+        struct CratesResponse {
+            crates: Vec<CratesIoCrate>,
         }
 
-        let response: CrateResponse = self
-            .req::<()>(reqwest::Method::GET, &format!("/crates/{krate}"), 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
@@ -176,6 +208,7 @@ impl CratesIoApi {
             self.req(
                 reqwest::Method::PATCH,
                 &format!("/crates/{krate}"),
+                HashMap::new(),
                 Some(&PatchCrateRequest {
                     krate: Crate {
                         trustpub_only: value,
@@ -194,20 +227,76 @@ impl CratesIoApi {
         &self,
         method: reqwest::Method,
         path: &str,
+        query: HashMap<String, String>,
         data: Option<&T>,
     ) -> anyhow::Result<reqwest::blocking::Response> {
         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<T: Serialize, R: DeserializeOwned, F>(
+        &self,
+        path: &str,
+        mut query: HashMap<String, String>,
+        data: Option<&T>,
+        mut handle_response: F,
+    ) -> anyhow::Result<()>
+    where
+        F: FnMut(R),
+    {
+        #[derive(serde::Deserialize, Debug)]
+        struct Response<R> {
+            meta: MetaResponse,
+            #[serde(flatten)]
+            data: R,
+        }
+
+        #[derive(serde::Deserialize, Debug)]
+        struct MetaResponse {
+            next_page: Option<String>,
+        }
+
+        if !query.contains_key("per_page") {
+            query.insert("per_page".to_string(), "100".to_string());
+        }
+
+        let mut query = query;
+        let mut path_extra: Option<String> = None;
+        loop {
+            let path = match path_extra {
+                Some(p) => format!("{path}{p}"),
+                None => path.to_owned(),
+            };
+
+            let response: Response<R> = 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)]
+pub struct UserId(pub u32);
+
 #[derive(serde::Deserialize, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
 pub struct TrustedPublishingId(u64);
 
@@ -217,8 +306,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,
@@ -228,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
@@ -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, CratesIoCrate, TrustedPublishingGitHubConfig, UserId};
 use anyhow::Context;
 use secrecy::SecretString;
 use std::collections::HashMap;
@@ -31,15 +31,20 @@ struct CrateConfig {
 pub(crate) struct SyncCratesIo {
     crates_io_api: CratesIoApi,
     crates: HashMap<CrateName, CrateConfig>,
+    user_id: UserId,
+    username: String,
 }
 
 impl SyncCratesIo {
     pub(crate) fn new(
         token: SecretString,
+        username: String,
         team_api: &TeamApi,
         dry_run: bool,
     ) -> anyhow::Result<Self> {
         let crates_io_api = CratesIoApi::new(token, dry_run);
+        let user_id = crates_io_api.get_user_id(&username)?;
+
         let crates: HashMap<CrateName, CrateConfig> = team_api
             .get_repos()?
             .into_iter()
@@ -69,6 +74,8 @@ impl SyncCratesIo {
         Ok(Self {
             crates_io_api,
             crates,
+            user_id,
+            username,
         })
     }
 
@@ -77,6 +84,31 @@ impl SyncCratesIo {
         let mut crate_diffs: Vec<CrateDiff> = 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<String, Vec<TrustedPublishingGitHubConfig>> = tp_configs
+                .into_iter()
+                .fold(HashMap::new(), |mut map, config| {
+                    map.entry(config.krate.clone()).or_default().push(config);
+                    map
+                });
+            tp_configs
+        };
+
+        // Batch load all crates owned by the current user
+        let crates: HashMap<String, CratesIoCrate> = 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 {
@@ -85,15 +117,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,22 +150,29 @@ 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;
-            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,
                 });
             }
         }
 
+        // 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.
diff --git 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}");
