From 87f703bef18eb66abfce1793dd55d5d4d350a4a8 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Tue, 30 Jan 2024 21:03:07 +0100 Subject: [PATCH] Don't panic when there is no zulip token --- src/db.rs | 1 + src/handlers.rs | 3 + src/handlers/major_change.rs | 141 +++++++++++++------------ src/handlers/notify_zulip.rs | 62 +++++------ src/handlers/types_planning_updates.rs | 12 ++- src/main.rs | 59 ++++++++--- src/zulip.rs | 108 +++++++++++++------ 7 files changed, 242 insertions(+), 144 deletions(-) diff --git a/src/db.rs b/src/db.rs index 272601b0..5692e041 100644 --- a/src/db.rs +++ b/src/db.rs @@ -25,6 +25,7 @@ lazy_static::lazy_static! { }; } +#[derive(Clone)] pub struct ClientPool { connections: Arc>>, permits: Arc, diff --git a/src/handlers.rs b/src/handlers.rs index 814113b3..bbc0d66a 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -1,5 +1,6 @@ use crate::config::{self, Config, ConfigurationError}; use crate::github::{Event, GithubClient, IssueCommentAction, IssuesAction, IssuesEvent}; +use crate::zulip::ZulipTokens; use octocrab::Octocrab; use parser::command::{assign::AssignCommand, Command, Input}; use std::fmt; @@ -291,9 +292,11 @@ command_handlers! { note: Note, } +#[derive(Clone)] pub struct Context { pub github: GithubClient, pub db: crate::db::ClientPool, pub username: String, pub octocrab: Octocrab, + pub zulip: Option, } diff --git a/src/handlers/major_change.rs b/src/handlers/major_change.rs index 0678ea42..14e53718 100644 --- a/src/handlers/major_change.rs +++ b/src/handlers/major_change.rs @@ -112,59 +112,63 @@ pub(super) async fn handle_input( event.issue.number, event.issue.html_url, ), Invocation::Rename { prev_issue } => { - let issue = &event.issue; + if let Some(ctx) = crate::zulip::ZulipContext::from_ctx(ctx) { + let issue = &event.issue; - let prev_topic = zulip_topic_from_issue(&prev_issue); - let partial_issue = issue.to_zulip_github_reference(); - let new_topic = zulip_topic_from_issue(&partial_issue); + let prev_topic = zulip_topic_from_issue(&prev_issue); + let partial_issue = issue.to_zulip_github_reference(); + let new_topic = zulip_topic_from_issue(&partial_issue); - let zulip_send_req = crate::zulip::MessageApiRequest { - recipient: crate::zulip::Recipient::Stream { - id: config.zulip_stream, - topic: &prev_topic, - }, - content: "The associated GitHub issue has been renamed. Renaming this Zulip topic.", - }; - let zulip_send_res = zulip_send_req - .send(&ctx.github.raw()) - .await - .context("zulip post failed")?; + let zulip_send_req = crate::zulip::MessageApiRequest { + recipient: crate::zulip::Recipient::Stream { + id: config.zulip_stream, + topic: &prev_topic, + }, + content: + "The associated GitHub issue has been renamed. Renaming this Zulip topic.", + }; + let zulip_send_res = zulip_send_req + .send(&ctx, &ctx.github.raw()) + .await + .context("zulip post failed")?; - let zulip_send_res: crate::zulip::MessageApiResponse = zulip_send_res.json().await?; + let zulip_send_res: crate::zulip::MessageApiResponse = + zulip_send_res.json().await?; - let zulip_update_req = crate::zulip::UpdateMessageApiRequest { - message_id: zulip_send_res.message_id, - topic: Some(&new_topic), - propagate_mode: Some("change_all"), - content: None, - }; - zulip_update_req - .send(&ctx.github.raw()) - .await - .context("zulip message update failed")?; + let zulip_update_req = crate::zulip::UpdateMessageApiRequest { + message_id: zulip_send_res.message_id, + topic: Some(&new_topic), + propagate_mode: Some("change_all"), + content: None, + }; + zulip_update_req + .send(&ctx, &ctx.github.raw()) + .await + .context("zulip message update failed")?; - // after renaming the zulip topic, post an additional comment under the old topic with a url to the new, renamed topic - // this is necessary due to the lack of topic permalinks, see https://github.com/zulip/zulip/issues/15290 - let new_topic_url = crate::zulip::Recipient::Stream { - id: config.zulip_stream, - topic: &new_topic, - } - .url(); - let breadcrumb_comment = format!( + // after renaming the zulip topic, post an additional comment under the old topic with a url to the new, renamed topic + // this is necessary due to the lack of topic permalinks, see https://github.com/zulip/zulip/issues/15290 + let new_topic_url = crate::zulip::Recipient::Stream { + id: config.zulip_stream, + topic: &new_topic, + } + .url(); + let breadcrumb_comment = format!( "The associated GitHub issue has been renamed. Please see the [renamed Zulip topic]({}).", new_topic_url ); - let zulip_send_breadcrumb_req = crate::zulip::MessageApiRequest { - recipient: crate::zulip::Recipient::Stream { - id: config.zulip_stream, - topic: &prev_topic, - }, - content: &breadcrumb_comment, - }; - zulip_send_breadcrumb_req - .send(&ctx.github.raw()) - .await - .context("zulip post failed")?; + let zulip_send_breadcrumb_req = crate::zulip::MessageApiRequest { + recipient: crate::zulip::Recipient::Stream { + id: config.zulip_stream, + topic: &prev_topic, + }, + content: &breadcrumb_comment, + }; + zulip_send_breadcrumb_req + .send(&ctx, &ctx.github.raw()) + .await + .context("zulip post failed")?; + } return Ok(()); } @@ -245,20 +249,21 @@ async fn handle( ) -> anyhow::Result<()> { let github_req = issue.add_labels(&ctx.github, vec![Label { name: label_to_add }]); - let partial_issue = issue.to_zulip_github_reference(); - let zulip_topic = zulip_topic_from_issue(&partial_issue); + if let Some(ctx) = crate::zulip::ZulipContext::from_ctx(ctx) { + let partial_issue = issue.to_zulip_github_reference(); + let zulip_topic = zulip_topic_from_issue(&partial_issue); - let zulip_req = crate::zulip::MessageApiRequest { - recipient: crate::zulip::Recipient::Stream { - id: config.zulip_stream, - topic: &zulip_topic, - }, - content: &zulip_msg, - }; + let zulip_req = crate::zulip::MessageApiRequest { + recipient: crate::zulip::Recipient::Stream { + id: config.zulip_stream, + topic: &zulip_topic, + }, + content: &zulip_msg, + }; - if new_proposal { - let topic_url = zulip_req.url(); - let comment = format!( + if new_proposal { + let topic_url = zulip_req.url(); + let comment = format!( "This issue is not meant to be used for technical discussion. \ There is a Zulip [stream] for that. Use this issue to leave \ procedural comments, such as volunteering to review, indicating that you \ @@ -290,17 +295,21 @@ async fn handle( config.open_extra_text.as_deref().unwrap_or_default(), topic_url ); - issue - .post_comment(&ctx.github, &comment) - .await - .context("post major change comment")?; - } + issue + .post_comment(&ctx.github, &comment) + .await + .context("post major change comment")?; + } + + let zulip_req = zulip_req.send(&ctx, &ctx.github.raw()); - let zulip_req = zulip_req.send(&ctx.github.raw()); + let (gh_res, zulip_res) = futures::join!(github_req, zulip_req); + zulip_res.context("zulip post failed")?; + gh_res.context("label setting failed")?; + } else { + github_req.await.context("label setting failed")?; + } - let (gh_res, zulip_res) = futures::join!(github_req, zulip_req); - zulip_res.context("zulip post failed")?; - gh_res.context("label setting failed")?; Ok(()) } diff --git a/src/handlers/notify_zulip.rs b/src/handlers/notify_zulip.rs index 87d0a018..4c3dc090 100644 --- a/src/handlers/notify_zulip.rs +++ b/src/handlers/notify_zulip.rs @@ -137,37 +137,39 @@ pub(super) async fn handle_input<'a>( event: &IssuesEvent, inputs: Vec, ) -> anyhow::Result<()> { - for input in inputs { - let config = &config.labels[&input.label.name]; - - let mut topic = config.topic.clone(); - topic = topic.replace("{number}", &event.issue.number.to_string()); - topic = topic.replace("{title}", &event.issue.title); - // Truncate to 60 chars (a Zulip limitation) - let mut chars = topic.char_indices().skip(59); - if let (Some((len, _)), Some(_)) = (chars.next(), chars.next()) { - topic.truncate(len); - topic.push('…'); - } - - let mut msg = match input.notification_type { - NotificationType::Labeled => config.message_on_add.as_ref().unwrap().clone(), - NotificationType::Unlabeled => config.message_on_remove.as_ref().unwrap().clone(), - NotificationType::Closed => config.message_on_close.as_ref().unwrap().clone(), - NotificationType::Reopened => config.message_on_reopen.as_ref().unwrap().clone(), - }; - - msg = msg.replace("{number}", &event.issue.number.to_string()); - msg = msg.replace("{title}", &event.issue.title); + if let Some(ctx) = crate::zulip::ZulipContext::from_ctx(ctx) { + for input in inputs { + let config = &config.labels[&input.label.name]; + + let mut topic = config.topic.clone(); + topic = topic.replace("{number}", &event.issue.number.to_string()); + topic = topic.replace("{title}", &event.issue.title); + // Truncate to 60 chars (a Zulip limitation) + let mut chars = topic.char_indices().skip(59); + if let (Some((len, _)), Some(_)) = (chars.next(), chars.next()) { + topic.truncate(len); + topic.push('…'); + } - let zulip_req = crate::zulip::MessageApiRequest { - recipient: crate::zulip::Recipient::Stream { - id: config.zulip_stream, - topic: &topic, - }, - content: &msg, - }; - zulip_req.send(&ctx.github.raw()).await?; + let mut msg = match input.notification_type { + NotificationType::Labeled => config.message_on_add.as_ref().unwrap().clone(), + NotificationType::Unlabeled => config.message_on_remove.as_ref().unwrap().clone(), + NotificationType::Closed => config.message_on_close.as_ref().unwrap().clone(), + NotificationType::Reopened => config.message_on_reopen.as_ref().unwrap().clone(), + }; + + msg = msg.replace("{number}", &event.issue.number.to_string()); + msg = msg.replace("{title}", &event.issue.title); + + let zulip_req = crate::zulip::MessageApiRequest { + recipient: crate::zulip::Recipient::Stream { + id: config.zulip_stream, + topic: &topic, + }, + content: &msg, + }; + zulip_req.send(&ctx, &ctx.github.raw()).await?; + } } Ok(()) diff --git a/src/handlers/types_planning_updates.rs b/src/handlers/types_planning_updates.rs index a6c9c0c4..997c4413 100644 --- a/src/handlers/types_planning_updates.rs +++ b/src/handlers/types_planning_updates.rs @@ -19,6 +19,10 @@ impl Job for TypesPlanningMeetingThreadOpenJob { } async fn run(&self, ctx: &super::Context, _metadata: &serde_json::Value) -> anyhow::Result<()> { + let Some(ctx) = crate::zulip::ZulipContext::from_ctx(ctx) else { + return Ok(()); + }; + // On the last week of the month, we open a thread on zulip for the next Monday let today = chrono::Utc::now().date().naive_utc(); let first_monday = today + chrono::Duration::days(7); @@ -38,7 +42,7 @@ impl Job for TypesPlanningMeetingThreadOpenJob { }, content: &message, }; - zulip_req.send(&ctx.github.raw()).await?; + zulip_req.send(&ctx, &ctx.github.raw()).await?; // Then, we want to schedule the next Thursday after this let mut thursday = today; @@ -88,6 +92,10 @@ pub async fn request_updates( ctx: &super::Context, metadata: PlanningMeetingUpdatesPingMetadata, ) -> anyhow::Result<()> { + let Some(ctx) = crate::zulip::ZulipContext::from_ctx(ctx) else { + return Ok(()); + }; + let gh = &ctx.github; let types_repo = gh.repository(TYPES_REPO).await?; @@ -164,7 +172,7 @@ pub async fn request_updates( }, content: &message, }; - zulip_req.send(&ctx.github.raw()).await?; + zulip_req.send(&ctx, &ctx.github.raw()).await?; Ok(()) } diff --git a/src/main.rs b/src/main.rs index 444ada99..d38ad760 100644 --- a/src/main.rs +++ b/src/main.rs @@ -127,6 +127,13 @@ async fn serve_req( .unwrap()); } if req.uri.path() == "/zulip-hook" { + let Some(ctx) = triagebot::zulip::ZulipContext::from_ctx(&ctx) else { + return Ok(Response::builder() + .status(StatusCode::SERVICE_UNAVAILABLE) + .body(Body::from("zulip integration is not configured")) + .unwrap()); + }; + let mut c = body_stream; let mut payload = Vec::new(); while let Some(chunk) = c.next().await { @@ -241,24 +248,52 @@ async fn serve_req( } async fn run_server(addr: SocketAddr) -> anyhow::Result<()> { - let pool = db::ClientPool::new(); - db::run_migrations(&*pool.get().await) + let db = db::ClientPool::new(); + db::run_migrations(&*db.get().await) .await .context("database migrations")?; - let gh = github::GithubClient::new_from_env(); - let oc = octocrab::OctocrabBuilder::new() + let username = std::env::var("TRIAGEBOT_USERNAME").or_else(|err| match err { + std::env::VarError::NotPresent => Ok("rustbot".to_owned()), + err => Err(err), + })?; + + let github = github::GithubClient::new_from_env(); + + let octocrab = octocrab::OctocrabBuilder::new() .personal_token(github::default_token_from_env()) .build() - .expect("Failed to build octograb."); + .expect("Failed to build octocrab."); + + let zulip = match ( + std::env::var("ZULIP_API_TOKEN"), + std::env::var("ZULIP_TOKEN"), + ) { + (Ok(api_token), Ok(auth_token)) => Some(triagebot::zulip::ZulipTokens { + api_token, + auth_token, + }), + (Err(std::env::VarError::NotPresent), Err(std::env::VarError::NotPresent) | Ok(_)) => { + tracing::warn!( + "missing `ZULIP_API_TOKEN` env variable, zulip integration won't be enabled" + ); + None + } + (Ok(_), Err(std::env::VarError::NotPresent)) => { + tracing::warn!( + "missing `ZULIP_TOKEN` env variable, zulip integration won't be enabled" + ); + None + } + (Err(e), _) | (_, Err(e)) => Err(e)?, + }; + let ctx = Arc::new(Context { - username: std::env::var("TRIAGEBOT_USERNAME").or_else(|err| match err { - std::env::VarError::NotPresent => Ok("rustbot".to_owned()), - err => Err(err), - })?, - db: pool, - github: gh, - octocrab: oc, + username, + db, + github, + octocrab, + zulip, }); if !is_scheduled_jobs_disabled() { diff --git a/src/zulip.rs b/src/zulip.rs index a158db50..028e9bce 100644 --- a/src/zulip.rs +++ b/src/zulip.rs @@ -2,13 +2,41 @@ use crate::db::notifications::add_metadata; use crate::db::notifications::{self, delete_ping, move_indices, record_ping, Identifier}; use crate::github::{self, GithubClient}; use crate::handlers::docs_update::docs_update; -use crate::handlers::Context; use anyhow::{format_err, Context as _}; use std::convert::TryInto; -use std::env; use std::fmt::Write as _; use tracing as log; +#[derive(Clone)] +pub struct ZulipTokens { + pub api_token: String, + pub auth_token: String, +} + +pub struct ZulipContext<'a> { + sup: &'a crate::handlers::Context, + api_token: &'a str, + auth_token: &'a str, +} + +impl<'a> ZulipContext<'a> { + pub fn from_ctx(ctx: &'a crate::handlers::Context) -> Option { + ctx.zulip.as_ref().map(|tokens| Self { + sup: ctx, + api_token: &tokens.api_token, + auth_token: &tokens.api_token, + }) + } +} + +impl std::ops::Deref for ZulipContext<'_> { + type Target = crate::handlers::Context; + + fn deref(&self) -> &Self::Target { + &self.sup + } +} + #[derive(Debug, serde::Deserialize)] pub struct Request { /// Markdown body of the sent message. @@ -88,7 +116,7 @@ pub async fn to_zulip_id(client: &GithubClient, github_id: i64) -> anyhow::Resul /// Top-level handler for Zulip webhooks. /// /// Returns a JSON response. -pub async fn respond(ctx: &Context, req: Request) -> String { +pub async fn respond(ctx: &ZulipContext<'_>, req: Request) -> String { let content = match process_zulip_request(ctx, req).await { Ok(None) => { return serde_json::to_string(&ResponseNotRequired { @@ -105,10 +133,11 @@ pub async fn respond(ctx: &Context, req: Request) -> String { /// Processes a Zulip webhook. /// /// Returns a string of the response, or None if no response is needed. -async fn process_zulip_request(ctx: &Context, req: Request) -> anyhow::Result> { - let expected_token = std::env::var("ZULIP_TOKEN").expect("`ZULIP_TOKEN` set for authorization"); - - if !openssl::memcmp::eq(req.token.as_bytes(), expected_token.as_bytes()) { +async fn process_zulip_request( + ctx: &ZulipContext<'_>, + req: Request, +) -> anyhow::Result> { + if !openssl::memcmp::eq(req.token.as_bytes(), ctx.auth_token.as_bytes()) { anyhow::bail!("Invalid authorization."); } @@ -127,7 +156,7 @@ async fn process_zulip_request(ctx: &Context, req: Request) -> anyhow::Result( - ctx: &'a Context, + ctx: &'a ZulipContext, gh_id: anyhow::Result, words: &'a str, message_data: &'a Message, @@ -186,7 +215,7 @@ fn handle_command<'a>( .await .map_err(|e| format_err!("Failed to await at this time: {e:?}")) } - Some("docs-update") => return trigger_docs_update(message_data), + Some("docs-update") => return trigger_docs_update(ctx, message_data), _ => {} } } @@ -204,7 +233,7 @@ fn handle_command<'a>( // * tell the user executed for that a command was run as them by the user // given. async fn execute_for_other_user( - ctx: &Context, + ctx: &ZulipContext<'_>, mut words: impl Iterator, message_data: &Message, ) -> anyhow::Result> { @@ -235,13 +264,12 @@ async fn execute_for_other_user( assert_eq!(command.pop(), Some(' ')); // pop trailing space command }; - let bot_api_token = env::var("ZULIP_API_TOKEN").expect("ZULIP_API_TOKEN"); let members = ctx .github .raw() .get("https://rust-lang.zulipchat.com/api/v1/users") - .basic_auth(BOT_EMAIL, Some(&bot_api_token)) + .basic_auth(BOT_EMAIL, Some(&ctx.api_token)) .send() .await .map_err(|e| format_err!("Failed to get list of zulip users: {e:?}."))?; @@ -278,7 +306,7 @@ async fn execute_for_other_user( }, content: &message, } - .send(ctx.github.raw()) + .send(ctx, ctx.github.raw()) .await; match res { @@ -397,9 +425,11 @@ impl<'a> MessageApiRequest<'a> { self.recipient.url() } - pub async fn send(&self, client: &reqwest::Client) -> anyhow::Result { - let bot_api_token = env::var("ZULIP_API_TOKEN").expect("ZULIP_API_TOKEN"); - + pub async fn send( + &self, + ctx: &ZulipContext<'_>, + client: &reqwest::Client, + ) -> anyhow::Result { #[derive(serde::Serialize)] struct SerializedApi<'a> { #[serde(rename = "type")] @@ -412,7 +442,7 @@ impl<'a> MessageApiRequest<'a> { Ok(client .post("https://rust-lang.zulipchat.com/api/v1/messages") - .basic_auth(BOT_EMAIL, Some(&bot_api_token)) + .basic_auth(BOT_EMAIL, Some(&ctx.api_token)) .form(&SerializedApi { type_: match self.recipient { Recipient::Stream { .. } => "stream", @@ -448,9 +478,11 @@ pub struct UpdateMessageApiRequest<'a> { } impl<'a> UpdateMessageApiRequest<'a> { - pub async fn send(&self, client: &reqwest::Client) -> anyhow::Result { - let bot_api_token = env::var("ZULIP_API_TOKEN").expect("ZULIP_API_TOKEN"); - + pub async fn send( + &self, + ctx: &ZulipContext<'_>, + client: &reqwest::Client, + ) -> anyhow::Result { #[derive(serde::Serialize)] struct SerializedApi<'a> { #[serde(skip_serializing_if = "Option::is_none")] @@ -466,7 +498,7 @@ impl<'a> UpdateMessageApiRequest<'a> { "https://rust-lang.zulipchat.com/api/v1/messages/{}", self.message_id )) - .basic_auth(BOT_EMAIL, Some(&bot_api_token)) + .basic_auth(BOT_EMAIL, Some(&ctx.api_token)) .form(&SerializedApi { topic: self.topic, propagate_mode: self.propagate_mode, @@ -478,7 +510,7 @@ impl<'a> UpdateMessageApiRequest<'a> { } async fn acknowledge( - ctx: &Context, + ctx: &ZulipContext<'_>, gh_id: i64, mut words: impl Iterator, ) -> anyhow::Result> { @@ -533,7 +565,7 @@ async fn acknowledge( } async fn add_notification( - ctx: &Context, + ctx: &ZulipContext<'_>, gh_id: i64, mut words: impl Iterator, ) -> anyhow::Result> { @@ -571,7 +603,7 @@ async fn add_notification( } async fn add_meta_notification( - ctx: &Context, + ctx: &ZulipContext<'_>, gh_id: i64, mut words: impl Iterator, ) -> anyhow::Result> { @@ -603,7 +635,7 @@ async fn add_meta_notification( } async fn move_notification( - ctx: &Context, + ctx: &ZulipContext<'_>, gh_id: i64, mut words: impl Iterator, ) -> anyhow::Result> { @@ -651,15 +683,17 @@ struct AddReaction<'a> { } impl<'a> AddReaction<'a> { - pub async fn send(self, client: &reqwest::Client) -> anyhow::Result { - let bot_api_token = env::var("ZULIP_API_TOKEN").expect("ZULIP_API_TOKEN"); - + pub async fn send( + self, + ctx: &ZulipContext<'_>, + client: &reqwest::Client, + ) -> anyhow::Result { Ok(client .post(&format!( "https://rust-lang.zulipchat.com/api/v1/messages/{}/reactions", self.message_id )) - .basic_auth(BOT_EMAIL, Some(&bot_api_token)) + .basic_auth(BOT_EMAIL, Some(&ctx.api_token)) .form(&self) .send() .await?) @@ -699,7 +733,8 @@ impl WaitingMessage<'static> { } async fn post_waiter( - ctx: &Context, + ctx: &ZulipContext<'_>, + message: &Message, waiting: WaitingMessage<'_>, ) -> anyhow::Result> { @@ -715,7 +750,7 @@ async fn post_waiter( }, content: waiting.primary, } - .send(ctx.github.raw()) + .send(ctx, ctx.github.raw()) .await?; let body = posted.text().await?; let message_id = serde_json::from_str::(&body) @@ -727,7 +762,7 @@ async fn post_waiter( message_id, emoji_name: reaction, } - .send(&ctx.github.raw()) + .send(ctx, &ctx.github.raw()) .await .context("emoji reaction failed")?; } @@ -735,11 +770,16 @@ async fn post_waiter( Ok(None) } -fn trigger_docs_update(message: &Message) -> anyhow::Result> { +fn trigger_docs_update( + ctx: &ZulipContext<'_>, + message: &Message, +) -> anyhow::Result> { + let ctx = crate::handlers::Context::clone(&ctx); let message = message.clone(); // The default Zulip timeout of 10 seconds can be too short, so process in // the background. tokio::task::spawn(async move { + let ctx = ZulipContext::from_ctx(&ctx).unwrap(); let response = match docs_update().await { Ok(None) => "No updates found.".to_string(), Ok(Some(pr)) => format!("Created docs update PR <{}>", pr.html_url), @@ -754,7 +794,7 @@ fn trigger_docs_update(message: &Message) -> anyhow::Result> { recipient, content: &response, }; - if let Err(e) = message.send(&reqwest::Client::new()).await { + if let Err(e) = message.send(&ctx, &reqwest::Client::new()).await { log::error!("failed to send Zulip response: {e:?}\nresponse was:\n{response}"); } });