From 1ac30e8d0b73a152dce1c9a4951f1f0d5c8079eb Mon Sep 17 00:00:00 2001 From: jyn Date: Mon, 18 Dec 2023 16:35:08 -0500 Subject: [PATCH 1/8] don't re-run command handlers when a pr title is edited doing so leads to weird errors: > Could not assign reviewer from: jyn514. > User(s) jyn514 are either the PR author, already assigned, or on vacation, and there are no other candidates. > Use r? to specify someone else to assign. this was likely missed when transitioning from highfive to triagebot. see https://github.com/rust-lang/rust/pull/118993 for an example of a bug this fixes --- src/handlers.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/handlers.rs b/src/handlers.rs index d2bf47d87..4edd9c38e 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -178,7 +178,17 @@ macro_rules! command_handlers { errors: &mut Vec, ) { match event { - Event::Issue(e) => if !matches!(e.action, IssuesAction::Opened | IssuesAction::Edited) { + // always handle new PRs / issues + Event::Issue(IssuesEvent { action: IssuesAction::Opened, .. }) => {}, + Event::Issue(IssuesEvent { action: IssuesAction::Edited, .. }) => { + // if the issue was edited, but we don't get a `changes[body]` diff, it means only the title was edited, not the body. + // don't process the same commands twice. + if event.comment_from().is_none() { + log::debug!("skipping title-only edit event"); + return; + } + }, + Event::Issue(e) => { // no change in issue's body for these events, so skip log::debug!("skipping event, issue was {:?}", e.action); return; From 89760c057ddcae73e65539ea24a897baf6da4a2d Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 21 Jan 2024 12:46:52 -0800 Subject: [PATCH 2/8] Squelch dead-code warning. --- src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 629cf6466..b0d353b16 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -114,7 +114,10 @@ impl fmt::Display for EventName { } #[derive(Debug)] -pub struct WebhookError(anyhow::Error); +pub struct WebhookError( + #[allow(dead_code)] // Used in debug display + anyhow::Error, +); impl From for WebhookError { fn from(e: anyhow::Error) -> WebhookError { From a04a0750552fac5d723f6220a51d2d1f7cf42f81 Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Sat, 20 Jan 2024 13:58:53 -0700 Subject: [PATCH 3/8] remove labels added by `no-merges` when rebase is pushed --- src/handlers/no_merges.rs | 54 +++++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/src/handlers/no_merges.rs b/src/handlers/no_merges.rs index 9856c8719..eccdd5ea2 100644 --- a/src/handlers/no_merges.rs +++ b/src/handlers/no_merges.rs @@ -24,6 +24,9 @@ pub(super) struct NoMergesInput { struct NoMergesState { /// Hashes of merge commits that have already been mentioned by triagebot in a comment. mentioned_merge_commits: HashSet, + /// Labels that the bot added as part of the no-merges check. + #[serde(default)] + added_labels: Vec, } pub(super) async fn parse_input( @@ -72,10 +75,8 @@ pub(super) async fn parse_input( } } - if merge_commits.is_empty() { - return Ok(None); - } - + // Run the handler even if we have no merge commits, + // so we can take an action if some were removed. Ok(Some(NoMergesInput { merge_commits })) } @@ -103,6 +104,32 @@ pub(super) async fn handle_input( let mut client = ctx.db.get().await; let mut state: IssueData<'_, NoMergesState> = IssueData::load(&mut client, &event.issue, NO_MERGES_KEY).await?; + + // No merge commits. + if input.merge_commits.is_empty() { + if state.data.mentioned_merge_commits.is_empty() { + // No merge commits from before, so do nothing. + return Ok(()); + } + + // Merge commits were removed, so remove the labels we added. + for name in state.data.added_labels.iter() { + event + .issue + .remove_label(&ctx.github, name) + .await + .context("failed to remove label")?; + } + + // FIXME: Minimize prior no_merges comments. + + // Clear from state. + state.data.mentioned_merge_commits.clear(); + state.data.added_labels.clear(); + state.save().await?; + return Ok(()); + } + let first_time = state.data.mentioned_merge_commits.is_empty(); let mut message = config @@ -137,7 +164,7 @@ pub(super) async fn handle_input( if !first_time { // Check if the labels are still set. // Otherwise, they were probably removed manually. - let any_removed = config.labels.iter().any(|label| { + let any_removed = state.data.added_labels.iter().any(|label| { // No label on the issue matches. event.issue.labels().iter().all(|l| &l.name != label) }); @@ -150,13 +177,18 @@ pub(super) async fn handle_input( } } + let existing_labels = event.issue.labels(); + + let mut labels = Vec::new(); + for name in config.labels.iter() { + // Only add labels not already on the issue. + if existing_labels.iter().all(|l| &l.name != name) { + state.data.added_labels.push(name.clone()); + labels.push(Label { name: name.clone() }); + } + } + // Set labels - let labels = config - .labels - .iter() - .cloned() - .map(|name| Label { name }) - .collect(); event .issue .add_labels(&ctx.github, labels) From 9aa64e12a490aa15727ba877407c3c320d775dbe Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 6 Feb 2024 08:57:57 +0000 Subject: [PATCH 4/8] Update auto-assignment message --- src/handlers/assign.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/handlers/assign.rs b/src/handlers/assign.rs index 1c3eb6f32..be62824b0 100644 --- a/src/handlers/assign.rs +++ b/src/handlers/assign.rs @@ -39,7 +39,8 @@ mod tests { } const NEW_USER_WELCOME_MESSAGE: &str = "Thanks for the pull request, and welcome! \ -The Rust team is excited to review your changes, and you should hear from {who} soon."; +The Rust team is excited to review your changes, and you should hear from {who} \ +some time within the next two weeks."; const CONTRIBUTION_MESSAGE: &str = "Please see [the contribution \ instructions]({contributing_url}) for more information. Namely, in order to ensure the \ @@ -56,7 +57,11 @@ const WELCOME_WITHOUT_REVIEWER: &str = "@Mark-Simulacrum (NB. this repo may be m const RETURNING_USER_WELCOME_MESSAGE: &str = "r? @{assignee} -({bot} has picked a reviewer for you, use r? to override)"; +{bot} has assigned @{assignee}. +They will have a look at your PR within the next two weeks and either review your PR or +reassign to another reviewer. + +Use r? to explicitly pick a reviewer"; const RETURNING_USER_WELCOME_MESSAGE_NO_REVIEWER: &str = "@{author}: no appropriate reviewer found, use r? to override"; From 548253f0231bcb34d4e7932d90c257f4ebe4a9f0 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 8 Feb 2024 09:34:03 +0000 Subject: [PATCH 5/8] Fix newline in message --- src/handlers/assign.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/handlers/assign.rs b/src/handlers/assign.rs index be62824b0..8064300fa 100644 --- a/src/handlers/assign.rs +++ b/src/handlers/assign.rs @@ -58,7 +58,7 @@ const WELCOME_WITHOUT_REVIEWER: &str = "@Mark-Simulacrum (NB. this repo may be m const RETURNING_USER_WELCOME_MESSAGE: &str = "r? @{assignee} {bot} has assigned @{assignee}. -They will have a look at your PR within the next two weeks and either review your PR or +They will have a look at your PR within the next two weeks and either review your PR or \ reassign to another reviewer. Use r? to explicitly pick a reviewer"; From 2169397430b22f0033b6b398371e06c0dee624f2 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Tue, 13 Feb 2024 15:11:01 +0100 Subject: [PATCH 6/8] Don't use `usize` for ids and such --- Cargo.lock | 6 ++--- src/db/notifications.rs | 37 +++++++++++++++------------- src/github.rs | 18 +++++++------- src/handlers/notification.rs | 46 +++++++++++------------------------ src/handlers/rustc_commits.rs | 2 +- src/zulip.rs | 32 ++++++++++++------------ 6 files changed, 63 insertions(+), 78 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 73f1cc3cf..39fbf9c7b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -971,7 +971,6 @@ checksum = "0f647032dfaa1f8b6dc29bd3edb7bbef4861b8b8007ebb118d6db284fd59f6ee" dependencies = [ "autocfg", "hashbrown 0.11.2", - "serde", ] [[package]] @@ -982,6 +981,7 @@ checksum = "d530e1a18b1cb4c484e6e34556a0d948706958449fca0cab753d649f2bce3d1f" dependencies = [ "equivalent", "hashbrown 0.14.3", + "serde", ] [[package]] @@ -1826,9 +1826,9 @@ checksum = "afab94fb28594581f62d981211a9a4d53cc8130bbcbbb89a0440d9b8e81a7746" [[package]] name = "rust_team_data" version = "1.0.0" -source = "git+https://github.com/rust-lang/team#49683ec8af9ca6b546b3af516ea4654424e302eb" +source = "git+https://github.com/rust-lang/team#1ff0fa95e5ead9fbbb4be3975cac8ede35b9d3d5" dependencies = [ - "indexmap 1.8.1", + "indexmap 2.1.0", "serde", ] diff --git a/src/db/notifications.rs b/src/db/notifications.rs index 5b1857933..57c919943 100644 --- a/src/db/notifications.rs +++ b/src/db/notifications.rs @@ -4,7 +4,7 @@ use tokio_postgres::Client as DbClient; use tracing as log; pub struct Notification { - pub user_id: i64, + pub user_id: u64, pub origin_url: String, pub origin_html: String, pub short_description: Option, @@ -15,10 +15,10 @@ pub struct Notification { pub team_name: Option, } -pub async fn record_username(db: &DbClient, user_id: i64, username: String) -> anyhow::Result<()> { +pub async fn record_username(db: &DbClient, user_id: u64, username: String) -> anyhow::Result<()> { db.execute( "INSERT INTO users (user_id, username) VALUES ($1, $2) ON CONFLICT DO NOTHING", - &[&user_id, &username], + &[&(user_id as i64), &username], ) .await .context("inserting user id / username")?; @@ -31,7 +31,7 @@ pub async fn record_ping(db: &DbClient, notification: &Notification) -> anyhow:: $1, $2, $3, $4, $5, $6, (SELECT max(notifications.idx) + 1 from notifications where notifications.user_id = $1) )", - &[¬ification.user_id, ¬ification.origin_url, ¬ification.origin_html, ¬ification.time, ¬ification.short_description, ¬ification.team_name], + &[&(notification.user_id as i64), ¬ification.origin_url, ¬ification.origin_html, ¬ification.time, ¬ification.short_description, ¬ification.team_name], ).await.context("inserting notification")?; Ok(()) @@ -40,14 +40,14 @@ pub async fn record_ping(db: &DbClient, notification: &Notification) -> anyhow:: #[derive(Copy, Clone)] pub enum Identifier<'a> { Url(&'a str), - Index(std::num::NonZeroUsize), + Index(std::num::NonZeroU32), /// Glob identifier (`all` or `*`). All, } pub async fn delete_ping( db: &mut DbClient, - user_id: i64, + user_id: u64, identifier: Identifier<'_>, ) -> anyhow::Result> { match identifier { @@ -56,7 +56,7 @@ pub async fn delete_ping( .query( "DELETE FROM notifications WHERE user_id = $1 and origin_url = $2 RETURNING origin_html, time, short_description, metadata", - &[&user_id, &origin_url], + &[&(user_id as i64), &origin_url], ) .await .context("delete notification query")?; @@ -91,13 +91,13 @@ pub async fn delete_ping( from notifications where user_id = $1 order by idx asc nulls last;", - &[&user_id], + &[&(user_id as i64)], ) .await .context("failed to get ordering")?; let notification_id: i64 = notifications - .get(idx.get() - 1) + .get((idx.get() - 1) as usize) .ok_or_else(|| anyhow::anyhow!("No such notification with index {}", idx.get()))? .get(0); @@ -144,7 +144,7 @@ pub async fn delete_ping( .query( "DELETE FROM notifications WHERE user_id = $1 RETURNING origin_url, origin_html, time, short_description, metadata", - &[&user_id], + &[&(user_id as i64)], ) .await .context("delete all notifications query")?; @@ -180,10 +180,12 @@ pub struct NotificationData { pub async fn move_indices( db: &mut DbClient, - user_id: i64, - from: usize, - to: usize, + user_id: u64, + from: u32, + to: u32, ) -> anyhow::Result<()> { + let from = usize::try_from(from)?; + let to = usize::try_from(to)?; loop { let t = db .build_transaction() @@ -198,7 +200,7 @@ pub async fn move_indices( from notifications where user_id = $1 order by idx asc nulls last;", - &[&user_id], + &[&(user_id as i64)], ) .await .context("failed to get initial ordering")?; @@ -257,10 +259,11 @@ pub async fn move_indices( pub async fn add_metadata( db: &mut DbClient, - user_id: i64, - idx: usize, + user_id: u64, + idx: u32, metadata: Option<&str>, ) -> anyhow::Result<()> { + let idx = usize::try_from(idx)?; loop { let t = db .build_transaction() @@ -275,7 +278,7 @@ pub async fn add_metadata( from notifications where user_id = $1 order by idx asc nulls last;", - &[&user_id], + &[&(user_id as i64)], ) .await .context("failed to get initial ordering")?; diff --git a/src/github.rs b/src/github.rs index b987599b5..4a1556a22 100644 --- a/src/github.rs +++ b/src/github.rs @@ -19,12 +19,12 @@ use tracing as log; #[derive(Debug, PartialEq, Eq, serde::Deserialize)] pub struct User { pub login: String, - pub id: Option, + pub id: Option, } impl GithubClient { async fn send_req(&self, req: RequestBuilder) -> anyhow::Result<(Bytes, String)> { - const MAX_ATTEMPTS: usize = 2; + const MAX_ATTEMPTS: u32 = 2; log::debug!("send_req with {:?}", req); let req_dbg = format!("{:?}", req); let req = req @@ -80,7 +80,7 @@ impl GithubClient { &self, req: Request, sleep: Duration, - remaining_attempts: usize, + remaining_attempts: u32, ) -> BoxFuture> { #[derive(Debug, serde::Deserialize)] struct RateLimit { @@ -203,7 +203,7 @@ impl User { } // Returns the ID of the given user, if the user is in the `all` team. - pub async fn get_id<'a>(&'a self, client: &'a GithubClient) -> anyhow::Result> { + pub async fn get_id<'a>(&'a self, client: &'a GithubClient) -> anyhow::Result> { let permission = crate::team_data::teams(client).await?; let map = permission.teams; Ok(map["all"] @@ -504,7 +504,7 @@ impl Issue { self.state == IssueState::Open } - pub async fn get_comment(&self, client: &GithubClient, id: usize) -> anyhow::Result { + pub async fn get_comment(&self, client: &GithubClient, id: u64) -> anyhow::Result { let comment_url = format!("{}/issues/comments/{}", self.repository().url(client), id); let comment = client.json(client.get(&comment_url)).await?; Ok(comment) @@ -540,7 +540,7 @@ impl Issue { pub async fn edit_comment( &self, client: &GithubClient, - id: usize, + id: u64, new_body: &str, ) -> anyhow::Result<()> { let comment_url = format!("{}/issues/comments/{}", self.repository().url(client), id); @@ -1030,7 +1030,7 @@ pub fn parse_diff(diff: &str) -> Vec { #[derive(Debug, serde::Deserialize)] pub struct IssueSearchResult { - pub total_count: usize, + pub total_count: u64, pub incomplete_results: bool, pub items: Vec, } @@ -1049,7 +1049,7 @@ struct Ordering<'a> { pub sort: &'a str, pub direction: &'a str, pub per_page: &'a str, - pub page: usize, + pub page: u64, } impl Repository { @@ -1136,7 +1136,7 @@ impl Repository { .await .with_context(|| format!("failed to list issues from {}", url))?; issues.extend(result.items); - if issues.len() < result.total_count { + if (issues.len() as u64) < result.total_count { ordering.page += 1; continue; } diff --git a/src/handlers/notification.rs b/src/handlers/notification.rs index 718b2b24b..54a89dbbb 100644 --- a/src/handlers/notification.rs +++ b/src/handlers/notification.rs @@ -11,7 +11,7 @@ use crate::{ }; use anyhow::Context as _; use std::collections::HashSet; -use std::convert::{TryFrom, TryInto}; +use std::convert::TryInto; use tracing as log; pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> { @@ -58,7 +58,7 @@ pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> { let mut users_notified = HashSet::new(); if let Some(from) = event.comment_from() { for login in parser::get_mentions(from).into_iter() { - if let Some((Ok(users), _)) = id_from_user(ctx, login).await? { + if let Some((users, _)) = id_from_user(ctx, login).await? { users_notified.extend(users.into_iter().map(|user| user.id.unwrap())); } } @@ -85,14 +85,6 @@ pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> { None => continue, }; - let users = match users { - Ok(users) => users, - Err(err) => { - log::error!("getting users failed: {:?}", err); - continue; - } - }; - let client = ctx.db.get().await; for user in users { if !users_notified.insert(user.id.unwrap()) { @@ -132,7 +124,7 @@ pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> { async fn id_from_user( ctx: &Context, login: &str, -) -> anyhow::Result>, Option)>> { +) -> anyhow::Result, Option)>> { if login.contains('/') { // This is a team ping. For now, just add it to everyone's agenda on // that team, but also mark it as such (i.e., a team ping) for @@ -174,15 +166,11 @@ async fn id_from_user( Ok(Some(( team.members .into_iter() - .map(|member| { - let id = i64::try_from(member.github_id) - .with_context(|| format!("user id {} out of bounds", member.github_id))?; - Ok(github::User { - id: Some(id), - login: member.github, - }) + .map(|member| github::User { + id: Some(member.github_id), + login: member.github, }) - .collect::>>(), + .collect::>(), Some(team.name), ))) } else { @@ -194,22 +182,16 @@ async fn id_from_user( .get_id(&ctx.github) .await .with_context(|| format!("failed to get user {} ID", user.login))?; - let id = match id { - Some(id) => id, + let Some(id) = id else { // If the user was not in the team(s) then just don't record it. - None => { - log::trace!("Skipping {} because no id found", user.login); - return Ok(None); - } + log::trace!("Skipping {} because no id found", user.login); + return Ok(None); }; - let id = i64::try_from(id).with_context(|| format!("user id {} out of bounds", id)); Ok(Some(( - id.map(|id| { - vec![github::User { - login: user.login.clone(), - id: Some(id), - }] - }), + vec![github::User { + login: user.login.clone(), + id: Some(id), + }], None, ))) } diff --git a/src/handlers/rustc_commits.rs b/src/handlers/rustc_commits.rs index d5dc30a94..8af282345 100644 --- a/src/handlers/rustc_commits.rs +++ b/src/handlers/rustc_commits.rs @@ -10,7 +10,7 @@ use std::collections::VecDeque; use std::convert::TryInto; use tracing as log; -const BORS_GH_ID: i64 = 3372342; +const BORS_GH_ID: u64 = 3372342; pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> { let body = match event.comment_body() { diff --git a/src/zulip.rs b/src/zulip.rs index a158db506..66e79cf9a 100644 --- a/src/zulip.rs +++ b/src/zulip.rs @@ -71,17 +71,17 @@ struct Response { pub const BOT_EMAIL: &str = "triage-rust-lang-bot@zulipchat.com"; -pub async fn to_github_id(client: &GithubClient, zulip_id: usize) -> anyhow::Result> { +pub async fn to_github_id(client: &GithubClient, zulip_id: u64) -> anyhow::Result> { let map = crate::team_data::zulip_map(client).await?; - Ok(map.users.get(&zulip_id).map(|v| *v as i64)) + Ok(map.users.get(&zulip_id).copied()) } -pub async fn to_zulip_id(client: &GithubClient, github_id: i64) -> anyhow::Result> { +pub async fn to_zulip_id(client: &GithubClient, github_id: u64) -> anyhow::Result> { let map = crate::team_data::zulip_map(client).await?; Ok(map .users .iter() - .find(|(_, github)| **github == github_id as usize) + .find(|&(_, &github)| github == github_id) .map(|v| *v.0)) } @@ -113,7 +113,7 @@ async fn process_zulip_request(ctx: &Context, req: Request) -> anyhow::Result Ok(gh_id), Ok(None) => Err(format_err!( "Unknown Zulip user. Please add `zulip-id = {}` to your file in \ @@ -128,7 +128,7 @@ async fn process_zulip_request(ctx: &Context, req: Request) -> anyhow::Result( ctx: &'a Context, - gh_id: anyhow::Result, + gh_id: anyhow::Result, words: &'a str, message_data: &'a Message, ) -> std::pin::Pin>> + Send + 'a>> @@ -263,7 +263,7 @@ async fn execute_for_other_user( .find(|m| m.user_id == zulip_user_id) .ok_or_else(|| format_err!("Could not find Zulip user email."))?; - let output = handle_command(ctx, Ok(user_id as i64), &command, message_data) + let output = handle_command(ctx, Ok(user_id), &command, message_data) .await? .unwrap_or_default(); @@ -479,7 +479,7 @@ impl<'a> UpdateMessageApiRequest<'a> { async fn acknowledge( ctx: &Context, - gh_id: i64, + gh_id: u64, mut words: impl Iterator, ) -> anyhow::Result> { let filter = match words.next() { @@ -491,9 +491,9 @@ async fn acknowledge( } None => anyhow::bail!("not enough words"), }; - let ident = if let Ok(number) = filter.parse::() { + let ident = if let Ok(number) = filter.parse::() { Identifier::Index( - std::num::NonZeroUsize::new(number) + std::num::NonZeroU32::new(number) .ok_or_else(|| anyhow::anyhow!("index must be at least 1"))?, ) } else if filter == "all" || filter == "*" { @@ -534,7 +534,7 @@ async fn acknowledge( async fn add_notification( ctx: &Context, - gh_id: i64, + gh_id: u64, mut words: impl Iterator, ) -> anyhow::Result> { let url = match words.next() { @@ -572,7 +572,7 @@ async fn add_notification( async fn add_meta_notification( ctx: &Context, - gh_id: i64, + gh_id: u64, mut words: impl Iterator, ) -> anyhow::Result> { let idx = match words.next() { @@ -580,7 +580,7 @@ async fn add_meta_notification( None => anyhow::bail!("idx not present"), }; let idx = idx - .parse::() + .parse::() .context("index")? .checked_sub(1) .ok_or_else(|| anyhow::anyhow!("1-based indexes"))?; @@ -604,7 +604,7 @@ async fn add_meta_notification( async fn move_notification( ctx: &Context, - gh_id: i64, + gh_id: u64, mut words: impl Iterator, ) -> anyhow::Result> { let from = match words.next() { @@ -616,12 +616,12 @@ async fn move_notification( None => anyhow::bail!("from idx not present"), }; let from = from - .parse::() + .parse::() .context("from index")? .checked_sub(1) .ok_or_else(|| anyhow::anyhow!("1-based indexes"))?; let to = to - .parse::() + .parse::() .context("to index")? .checked_sub(1) .ok_or_else(|| anyhow::anyhow!("1-based indexes"))?; From 60e608e2aeb3020bed7f7433371f2e0ae0f3210a Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Tue, 13 Feb 2024 15:30:37 +0100 Subject: [PATCH 7/8] fixup tests to accomodate team config update --- src/handlers/assign/tests/tests_candidates.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/handlers/assign/tests/tests_candidates.rs b/src/handlers/assign/tests/tests_candidates.rs index 861ca7aaf..3febb1033 100644 --- a/src/handlers/assign/tests/tests_candidates.rs +++ b/src/handlers/assign/tests/tests_candidates.rs @@ -53,6 +53,7 @@ fn convert_simplified( "members": serde_json::Value::Array(members), "alumni": [], "discord": [], + "roles": [], }); } let teams = serde_json::value::from_value(teams_config).unwrap(); From cac949815ff98ff5f6886a5652701a4cd81a5b09 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Tue, 13 Feb 2024 21:14:41 -0500 Subject: [PATCH 8/8] Update the RDS root CA list The current root is expiring in a few months, so we need to migrate to a new one. We'll be copying similar code to perf, but we can start with making sure it works with triagebot. I've checked that the new CA file contains the old certificate, so this should keep working with our current database (i.e. doesn't need to be synchronized deployment wise with anything). --- Cargo.lock | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++-- Cargo.toml | 1 + src/db.rs | 33 +++++++++++---- 3 files changed, 139 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 73f1cc3cf..ac9faad06 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -25,9 +25,9 @@ checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe" [[package]] name = "ahash" -version = "0.7.7" +version = "0.7.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a824f2aa7e75a0c98c5a504fceb80649e9c35265d44525b5f94de4771a395cd" +checksum = "891477e0c6a8957309ee5c45a6368af3ae14bb510732d2684ffa19af310920f9" dependencies = [ "getrandom", "once_cell", @@ -120,6 +120,12 @@ version = "0.21.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9ba43ea6f343b788c8764558649e08df62f86c6ef251fdaeb1ffd010a9ae50a2" +[[package]] +name = "base64ct" +version = "1.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8c3c1a368f70d6cf7302d78f8f7093da241fb8e8807c05cc9e51a125895a6d5b" + [[package]] name = "bitflags" version = "1.3.2" @@ -286,6 +292,12 @@ dependencies = [ "xdg", ] +[[package]] +name = "const-oid" +version = "0.9.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2459377285ad874054d797f3ccebf984978aa39129f6eafde5cdc8315b612f8" + [[package]] name = "core-foundation" version = "0.9.3" @@ -429,6 +441,30 @@ dependencies = [ "syn 1.0.91", ] +[[package]] +name = "der" +version = "0.7.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fffa369a668c8af7dbf8b5e56c9f744fbd399949ed171606040001947de40b1c" +dependencies = [ + "const-oid", + "der_derive", + "flagset", + "pem-rfc7468", + "zeroize", +] + +[[package]] +name = "der_derive" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5fe87ce4529967e0ba1dcf8450bab64d97dfd5010a6256187ffe2e43e6f0e049" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.37", +] + [[package]] name = "deranged" version = "0.3.8" @@ -535,6 +571,12 @@ dependencies = [ "instant", ] +[[package]] +name = "flagset" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d52a7e408202050813e6f1d9addadcaafef3dca7530c7ddfb005d4081cce6779" + [[package]] name = "fnv" version = "1.0.7" @@ -1374,6 +1416,15 @@ dependencies = [ "base64 0.13.0", ] +[[package]] +name = "pem-rfc7468" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "88b39c9bfcfc231068454382784bb460aae594343fb030d46e9f50a645418412" +dependencies = [ + "base64ct", +] + [[package]] name = "percent-encoding" version = "2.3.0" @@ -2134,6 +2185,16 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6e63cff320ae2c57904679ba7cb63280a3dc4613885beafb148ee7bf9aa9042d" +[[package]] +name = "spki" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d91ed6c858b01f942cd56b37a94b3e0a1798290327d1236e4d9cf4eaca44d29d" +dependencies = [ + "base64ct", + "der", +] + [[package]] name = "static_assertions" version = "1.1.0" @@ -2303,6 +2364,27 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cda74da7e1a664f795bb1f8a87ec406fb89a02522cf6e50620d016add6dbbf5c" +[[package]] +name = "tls_codec" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b5e78c9c330f8c85b2bae7c8368f2739157db9991235123aa1b15ef9502bfb6a" +dependencies = [ + "tls_codec_derive", + "zeroize", +] + +[[package]] +name = "tls_codec_derive" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8d9ef545650e79f30233c0003bcc2504d7efac6dad25fca40744de773fe2049c" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.37", +] + [[package]] name = "tokio" version = "1.17.0" @@ -2601,6 +2683,7 @@ dependencies = [ "tracing-subscriber", "url", "uuid 0.8.2", + "x509-cert", ] [[package]] @@ -3042,6 +3125,18 @@ dependencies = [ "tap", ] +[[package]] +name = "x509-cert" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1301e935010a701ae5f8655edc0ad17c44bad3ac5ce8c39185f75453b720ae94" +dependencies = [ + "const-oid", + "der", + "spki", + "tls_codec", +] + [[package]] name = "xdg" version = "2.4.1" @@ -3053,6 +3148,20 @@ dependencies = [ [[package]] name = "zeroize" -version = "1.6.0" +version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a0956f1ba7c7909bfb66c2e9e4124ab6f6482560f6628b5aaeba39207c9aad9" +checksum = "525b4ec142c6b68a2d10f01f7bbf6755599ca3f81ea53b8431b7dd348f5fdb2d" +dependencies = [ + "zeroize_derive", +] + +[[package]] +name = "zeroize_derive" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce36e65b0d2999d2aafac989fb249189a141aee1f53c612c1f37d72631959f69" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.37", +] diff --git a/Cargo.toml b/Cargo.toml index 277360946..280e977b3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ chrono = { version = "0.4", features = ["serde"] } tokio-postgres = { version = "0.7.2", features = ["with-chrono-0_4", "with-serde_json-1", "with-uuid-0_8"] } postgres-native-tls = "0.5.0" native-tls = "0.2" +x509-cert = { version = "0.2.5", features = ["pem"] } serde_path_to_error = "0.1.2" octocrab = "0.30.1" comrak = { version = "0.8.2", default-features = false } diff --git a/src/db.rs b/src/db.rs index 272601b0b..145138921 100644 --- a/src/db.rs +++ b/src/db.rs @@ -12,10 +12,10 @@ pub mod jobs; pub mod notifications; pub mod rustc_commits; -const CERT_URL: &str = "https://s3.amazonaws.com/rds-downloads/rds-ca-2019-root.pem"; +const CERT_URL: &str = "https://truststore.pki.rds.amazonaws.com/global/global-bundle.pem"; lazy_static::lazy_static! { - static ref CERTIFICATE_PEM: Vec = { + static ref CERTIFICATE_PEMS: Vec = { let client = reqwest::blocking::Client::new(); let resp = client .get(CERT_URL) @@ -94,12 +94,11 @@ impl ClientPool { async fn make_client() -> anyhow::Result { let db_url = std::env::var("DATABASE_URL").expect("needs DATABASE_URL"); if db_url.contains("rds.amazonaws.com") { - let cert = &CERTIFICATE_PEM[..]; - let cert = Certificate::from_pem(&cert).context("made certificate")?; - let connector = TlsConnector::builder() - .add_root_certificate(cert) - .build() - .context("built TlsConnector")?; + let mut builder = TlsConnector::builder(); + for cert in make_certificates() { + builder.add_root_certificate(cert); + } + let connector = builder.build().context("built TlsConnector")?; let connector = MakeTlsConnector::new(connector); let (db_client, connection) = match tokio_postgres::connect(&db_url, connector).await { @@ -134,6 +133,24 @@ async fn make_client() -> anyhow::Result { } } +fn make_certificates() -> Vec { + use x509_cert::der::pem::LineEnding; + use x509_cert::der::EncodePem; + + let certs = x509_cert::Certificate::load_pem_chain(&CERTIFICATE_PEMS[..]).unwrap(); + certs + .into_iter() + .map(|cert| Certificate::from_pem(cert.to_pem(LineEnding::LF).unwrap().as_bytes()).unwrap()) + .collect() +} + +// Makes sure we successfully parse the RDS certificates and load them into native-tls compatible +// format. +#[test] +fn cert() { + make_certificates(); +} + pub async fn run_migrations(client: &DbClient) -> anyhow::Result<()> { client .execute(