From 93a0352abb3b6069eb04b133dc19f26a7129943e Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Tue, 19 Mar 2024 15:58:47 +0100 Subject: [PATCH 1/6] Switch to app --- Cargo.lock | 1 + Cargo.toml | 1 + README.md | 8 +++---- src/cli.rs | 2 +- src/cli/parser.rs | 48 +++++++++++++++++++++++++++++++++++++++--- src/constants.rs | 5 +++-- src/main.rs | 45 +++++++++++++++++++++++++++------------ src/server/listener.rs | 29 +++++++++++++------------ 8 files changed, 102 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 63b182b..ccf6667 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -255,6 +255,7 @@ dependencies = [ "hmac-sha256", "http", "http-body-util", + "jsonwebtoken", "lazy_static", "libsystemd", "octocrab", diff --git a/Cargo.toml b/Cargo.toml index 919731e..cae3a6b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,7 @@ clap = { version = "4.5.1", features = ["derive", "env"] } directories = "5.0.1" futures = "0.3.30" git2 = "0.18.2" +jsonwebtoken = "9.2.0" lazy_static = "1.4.0" octocrab = { version = "0.35.0", features = ["stream", "tracing"] } regex = "1.10.3" diff --git a/README.md b/README.md index 2a5bff2..374a225 100644 --- a/README.md +++ b/README.md @@ -23,11 +23,11 @@ Eventually, we’ll just have a project-wide webhook like this. For now, if you - Content type: application/json - Let me select individual events → **Pull Requests** 3. Add a label benchmark to a PR authored by a trusted user. -4. Watch [scverse-bot][] add and update a comment with the PR’s performance impact. +4. Watch [scverse-benchmarks][] add and update a comment with the PR’s performance impact. [asv config]: https://asv.readthedocs.io/en/v0.6.1/using.html [webhook settings]: https://github.com/scverse/benchmark/settings/hooks/464592128 -[scverse-bot]: https://github.com/scverse-bot +[scverse-benchmarks]: https://github.com/apps/scverse-benchmark ## MVP Setup @@ -52,8 +52,8 @@ All these currently assume you have a <user> login with sudo rig ```shell sudo systemd-creds encrypt --name=webhook_secret secret.txt - - sudo systemd-creds encrypt --name=github_token scverse-bot-pat.txt - - shred secret.txt scverse-bot-pat.txt + sudo systemd-creds encrypt --name=app_key app-key.pem - + shred secret.txt app-key.pem ``` 3. Copy the benchmark.service file to the system, enable and start the service: diff --git a/src/cli.rs b/src/cli.rs index 2988788..3329763 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,5 +1,5 @@ mod parser; mod tracing; -pub(crate) use parser::{Cli, Commands, RunBenchmark, ServeArgs}; +pub(crate) use parser::{Auth, Cli, Commands, RunBenchmark, ServeArgs}; pub(crate) use tracing::init as init_tracing; diff --git a/src/cli/parser.rs b/src/cli/parser.rs index 0a42e3c..6100fb0 100644 --- a/src/cli/parser.rs +++ b/src/cli/parser.rs @@ -4,7 +4,7 @@ use serde::Deserialize; use secrecy::SecretString; use std::fmt::Display; -use crate::constants::ORG; +use crate::{constants::ORG, utils::get_credential}; #[derive(Parser)] #[command(version, about, long_about = None)] @@ -12,9 +12,51 @@ use crate::constants::ORG; pub(crate) struct Cli { #[command(subcommand)] pub(crate) command: Commands, - /// GitHub token used to make API requests. + + #[command(flatten)] + pub(crate) auth: AuthInner, +} + +// https://github.com/clap-rs/clap/issues/2621 +#[derive(Default, Args)] +#[group(multiple = false)] +pub(crate) struct AuthInner { + /// GitHub RSA private key for an app. + #[arg(long, short = 'k', env)] + app_key: Option, + + /// GitHub personal access token used to make API requests. #[arg(long, short = 't', env)] - pub(crate) github_token: Option, + github_token: Option, +} + +pub(crate) enum Auth { + AppKey(SecretString), + GitHubToken(SecretString), +} + +impl TryFrom for Auth { + type Error = anyhow::Error; + + /// If app key or token has been passed via CLI or env, use it, otherwise try to get as a credential. + fn try_from(inner: AuthInner) -> Result { + Ok(if let Some(app_key) = inner.app_key { + tracing::info!("Using app key from CLI"); + Self::AppKey(app_key) + } else if let Some(github_token) = inner.github_token { + tracing::info!("Using GitHub token from CLI"); + Self::GitHubToken(github_token) + } else if let Ok(app_key) = get_credential("app_key") { + tracing::info!("Using app key from credential store"); + Self::AppKey(app_key) + } else if let Ok(github_token) = get_credential("github_token") { + tracing::info!("Using GitHub token from credential store"); + Self::GitHubToken(github_token) + } else { + // This doesn’t happen when parsed from CLI, only when constructed using ::default() + anyhow::bail!("No credentials found"); + }) + } } #[derive(Subcommand)] diff --git a/src/constants.rs b/src/constants.rs index d4934ae..fd8d9c1 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -1,7 +1,8 @@ -use octocrab::models::issues::Comment; +use octocrab::models::{issues::Comment, AppId}; pub(crate) const ORG: &str = "scverse"; -pub(crate) const BOT_NAME: &str = "scverse-bot"; +pub(crate) const APP_ID: AppId = AppId(858_840); +pub(crate) const BOT_NAME: &str = "scverse-benchmark"; // TODO: or "scverse-benchmark[bot]"? pub(crate) const BENCHMARK_LABEL: &str = "benchmark"; pub(crate) const PR_COMPARISON_MARKER: &str = ""; diff --git a/src/main.rs b/src/main.rs index 937121a..5c7fc28 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,8 +2,8 @@ use anyhow::{anyhow, Result}; use clap::Parser; +use futures::TryStreamExt; use secrecy::ExposeSecret; -use utils::get_credential; mod benchmark; mod cli; @@ -19,18 +19,37 @@ mod utils; async fn main() -> Result<()> { cli::init_tracing(); - let cli = cli::Cli::parse(); - // If token has been passed via CLI or env, use it, otherwise try to get as a credential. - if let Some(github_token) = cli - .github_token - .or_else(|| get_credential("github_token").ok()) - { - let crab = octocrab::Octocrab::builder() - // https://github.com/XAMPPRocky/octocrab/issues/594 - .personal_token(github_token.expose_secret().to_owned()) - .build()?; - octocrab::initialise(crab); - } + let mut cli = cli::Cli::parse(); + + // initialize octocrab + match std::mem::take(&mut cli.auth).try_into()? { + cli::Auth::AppKey(app_key) => { + let key = jsonwebtoken::EncodingKey::from_rsa_pem(app_key.expose_secret().as_bytes())?; + let base = octocrab::Octocrab::builder() + .app(constants::APP_ID, key) + .build()?; + let insts = base + .apps() + .installations() + .send() + .await? + .into_stream(&base) + .try_collect::>() + .await?; + tracing::info!("Installations: {}", serde_json5::to_string(&insts)?); + let crab = octocrab::Octocrab::installation(&base, insts[0].id); + octocrab::initialise(crab); + } + cli::Auth::GitHubToken(github_token) => { + let crab = octocrab::Octocrab::builder() + // https://github.com/XAMPPRocky/octocrab/issues/594 + .personal_token(github_token.expose_secret().to_owned()) + .build()?; + octocrab::initialise(crab); + } + }; + + // run command match cli.command { cli::Commands::Serve(args) => { server::serve(args).await?; diff --git a/src/server/listener.rs b/src/server/listener.rs index bf6896e..827d9f6 100644 --- a/src/server/listener.rs +++ b/src/server/listener.rs @@ -73,29 +73,30 @@ async fn handle_enqueue( event: Compare, mut state: AppState, ) -> Result { - if ref_exists(&state.github_client, &event.run_benchmark) + let ref_exists = ref_exists(&state.github_client, &event.run_benchmark) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? - { + .map_err(|e| { + tracing::error!("Enqueue failed: {e:?}"); + (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()) + })?; + if ref_exists { state .sender .send(event.into()) .await .map(|()| "enqueued".to_owned()) .map_err(|_| { - ( - StatusCode::INTERNAL_SERVER_ERROR, - "Error: Failed to send event".to_owned(), - ) + let msg = "Failed to send event"; + tracing::error!("Enqueue failed: {msg}"); + (StatusCode::INTERNAL_SERVER_ERROR, msg.to_owned()) }) } else { - Err(( - StatusCode::BAD_REQUEST, - format!( - "Error: {} is not a valid repo/ref combination", - event.run_benchmark - ), - )) + let msg = format!( + "{} is not a valid repo/ref combination", + event.run_benchmark + ); + tracing::info!("Enqueue failed: {msg}"); + Err((StatusCode::BAD_REQUEST, msg)) } } From e6ba0596935d864dde3d8aac8044f6e1c41ecef1 Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Tue, 19 Mar 2024 16:05:00 +0100 Subject: [PATCH 2/6] Definitely a bot --- src/constants.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/constants.rs b/src/constants.rs index fd8d9c1..5876611 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -2,7 +2,7 @@ use octocrab::models::{issues::Comment, AppId}; pub(crate) const ORG: &str = "scverse"; pub(crate) const APP_ID: AppId = AppId(858_840); -pub(crate) const BOT_NAME: &str = "scverse-benchmark"; // TODO: or "scverse-benchmark[bot]"? +pub(crate) const BOT_NAME: &str = "scverse-benchmark[bot]"; pub(crate) const BENCHMARK_LABEL: &str = "benchmark"; pub(crate) const PR_COMPARISON_MARKER: &str = ""; From c5524538343bdd9e670cf578be45804d7fe50016 Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Tue, 19 Mar 2024 16:20:40 +0100 Subject: [PATCH 3/6] Extract code --- src/cli.rs | 1 + src/cli/octocrab_utils.rs | 35 +++++++++++++++++++++++++++++++++++ src/cli/parser.rs | 9 +++++++++ src/main.rs | 31 +------------------------------ 4 files changed, 46 insertions(+), 30 deletions(-) create mode 100644 src/cli/octocrab_utils.rs diff --git a/src/cli.rs b/src/cli.rs index 3329763..e7d72f7 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,3 +1,4 @@ +mod octocrab_utils; mod parser; mod tracing; diff --git a/src/cli/octocrab_utils.rs b/src/cli/octocrab_utils.rs new file mode 100644 index 0000000..76744e7 --- /dev/null +++ b/src/cli/octocrab_utils.rs @@ -0,0 +1,35 @@ +use anyhow::Result; +use futures::TryStreamExt; +use secrecy::ExposeSecret; + +use crate::{cli, constants::APP_ID}; + +use super::Auth; + +pub(super) async fn auth_to_octocrab(auth: &mut A) -> Result +where + A: TryInto + Default, +{ + match std::mem::take(auth).try_into()? { + cli::Auth::AppKey(app_key) => { + let key = jsonwebtoken::EncodingKey::from_rsa_pem(app_key.expose_secret().as_bytes())?; + let base = octocrab::Octocrab::builder().app(APP_ID, key).build()?; + let insts = base + .apps() + .installations() + .send() + .await? + .into_stream(&base) + .try_collect::>() + .await?; + tracing::info!("Installations: {}", serde_json5::to_string(&insts)?); + Ok(octocrab::Octocrab::installation(&base, insts[0].id)) + } + cli::Auth::GitHubToken(github_token) => { + Ok(octocrab::Octocrab::builder() + // https://github.com/XAMPPRocky/octocrab/issues/594 + .personal_token(github_token.expose_secret().to_owned()) + .build()?) + } + } +} diff --git a/src/cli/parser.rs b/src/cli/parser.rs index 6100fb0..1869513 100644 --- a/src/cli/parser.rs +++ b/src/cli/parser.rs @@ -1,11 +1,14 @@ use clap::{Args, Parser, Subcommand}; use serde::Deserialize; +use anyhow::Result; use secrecy::SecretString; use std::fmt::Display; use crate::{constants::ORG, utils::get_credential}; +use super::octocrab_utils::auth_to_octocrab; + #[derive(Parser)] #[command(version, about, long_about = None)] #[command(propagate_version = true)] @@ -30,6 +33,12 @@ pub(crate) struct AuthInner { github_token: Option, } +impl AuthInner { + pub(crate) async fn into_octocrab(&mut self) -> Result { + auth_to_octocrab(self).await + } +} + pub(crate) enum Auth { AppKey(SecretString), GitHubToken(SecretString), diff --git a/src/main.rs b/src/main.rs index 5c7fc28..e2734d5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,8 +2,6 @@ use anyhow::{anyhow, Result}; use clap::Parser; -use futures::TryStreamExt; -use secrecy::ExposeSecret; mod benchmark; mod cli; @@ -21,35 +19,8 @@ async fn main() -> Result<()> { let mut cli = cli::Cli::parse(); - // initialize octocrab - match std::mem::take(&mut cli.auth).try_into()? { - cli::Auth::AppKey(app_key) => { - let key = jsonwebtoken::EncodingKey::from_rsa_pem(app_key.expose_secret().as_bytes())?; - let base = octocrab::Octocrab::builder() - .app(constants::APP_ID, key) - .build()?; - let insts = base - .apps() - .installations() - .send() - .await? - .into_stream(&base) - .try_collect::>() - .await?; - tracing::info!("Installations: {}", serde_json5::to_string(&insts)?); - let crab = octocrab::Octocrab::installation(&base, insts[0].id); - octocrab::initialise(crab); - } - cli::Auth::GitHubToken(github_token) => { - let crab = octocrab::Octocrab::builder() - // https://github.com/XAMPPRocky/octocrab/issues/594 - .personal_token(github_token.expose_secret().to_owned()) - .build()?; - octocrab::initialise(crab); - } - }; + octocrab::initialise(cli.auth.into_octocrab().await?); - // run command match cli.command { cli::Commands::Serve(args) => { server::serve(args).await?; From 17141d46d826bd4909a2e7742541c2ff545d3a88 Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Tue, 19 Mar 2024 16:22:44 +0100 Subject: [PATCH 4/6] take out elsewhere --- src/cli/octocrab_utils.rs | 4 ++-- src/cli/parser.rs | 2 +- src/main.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cli/octocrab_utils.rs b/src/cli/octocrab_utils.rs index 76744e7..81c1fdb 100644 --- a/src/cli/octocrab_utils.rs +++ b/src/cli/octocrab_utils.rs @@ -6,11 +6,11 @@ use crate::{cli, constants::APP_ID}; use super::Auth; -pub(super) async fn auth_to_octocrab(auth: &mut A) -> Result +pub(super) async fn auth_to_octocrab(auth: A) -> Result where A: TryInto + Default, { - match std::mem::take(auth).try_into()? { + match auth.try_into()? { cli::Auth::AppKey(app_key) => { let key = jsonwebtoken::EncodingKey::from_rsa_pem(app_key.expose_secret().as_bytes())?; let base = octocrab::Octocrab::builder().app(APP_ID, key).build()?; diff --git a/src/cli/parser.rs b/src/cli/parser.rs index 1869513..5a8b97a 100644 --- a/src/cli/parser.rs +++ b/src/cli/parser.rs @@ -34,7 +34,7 @@ pub(crate) struct AuthInner { } impl AuthInner { - pub(crate) async fn into_octocrab(&mut self) -> Result { + pub(crate) async fn into_octocrab(self) -> Result { auth_to_octocrab(self).await } } diff --git a/src/main.rs b/src/main.rs index e2734d5..c9550d6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -19,7 +19,7 @@ async fn main() -> Result<()> { let mut cli = cli::Cli::parse(); - octocrab::initialise(cli.auth.into_octocrab().await?); + octocrab::initialise(std::mem::take(&mut cli.auth).into_octocrab().await?); match cli.command { cli::Commands::Serve(args) => { From e35d916f5969d0730642a125add31f43a3b270fe Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Tue, 19 Mar 2024 16:26:15 +0100 Subject: [PATCH 5/6] move --- src/main.rs | 1 + src/octocrab_utils.rs | 33 +++++++++++++++++++++++++++++++++ src/server/octocrab_utils.rs | 34 +--------------------------------- src/server/runner/comment.rs | 2 +- 4 files changed, 36 insertions(+), 34 deletions(-) create mode 100644 src/octocrab_utils.rs diff --git a/src/main.rs b/src/main.rs index c9550d6..69da3d1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,6 +9,7 @@ mod constants; mod event; #[cfg(test)] mod fixtures; +mod octocrab_utils; mod repo_cache; mod server; mod utils; diff --git a/src/octocrab_utils.rs b/src/octocrab_utils.rs new file mode 100644 index 0000000..1321fc3 --- /dev/null +++ b/src/octocrab_utils.rs @@ -0,0 +1,33 @@ +use std::pin::pin; + +use futures::{future, TryStreamExt}; +use octocrab::Page; +use serde::de::DeserializeOwned; + +pub(super) trait PageExt +where + I: DeserializeOwned + 'static, +{ + async fn find bool>( + self, + github_api: &octocrab::Octocrab, + pred: F, + ) -> octocrab::Result>; +} + +impl PageExt for Page +where + I: DeserializeOwned + 'static, +{ + async fn find bool>( + self, + github_api: &octocrab::Octocrab, + pred: F, + ) -> octocrab::Result> { + let items = pin!(self.into_stream(github_api)); + items + .try_filter(|item| future::ready(pred(item))) + .try_next() + .await + } +} diff --git a/src/server/octocrab_utils.rs b/src/server/octocrab_utils.rs index d90bbd4..1e2e198 100644 --- a/src/server/octocrab_utils.rs +++ b/src/server/octocrab_utils.rs @@ -1,14 +1,10 @@ -use std::pin::pin; - use anyhow::{Context, Result}; -use futures::{future, stream, StreamExt, TryStreamExt}; +use futures::{stream, StreamExt, TryStreamExt}; use lazy_static::lazy_static; use octocrab::{ models::repos::{Ref, RepoCommit}, params::repos::Reference, - Page, }; -use serde::de::DeserializeOwned; use crate::{cli::RunBenchmark, constants::ORG}; @@ -16,34 +12,6 @@ lazy_static! { static ref SHA1_RE: regex::Regex = regex::Regex::new(r"^[a-f0-9]{40}$").unwrap(); } -pub(super) trait PageExt -where - I: DeserializeOwned + 'static, -{ - async fn find bool>( - self, - github_api: &octocrab::Octocrab, - pred: F, - ) -> octocrab::Result>; -} - -impl PageExt for Page -where - I: DeserializeOwned + 'static, -{ - async fn find bool>( - self, - github_api: &octocrab::Octocrab, - pred: F, - ) -> octocrab::Result> { - let items = pin!(self.into_stream(github_api)); - items - .try_filter(|item| future::ready(pred(item))) - .try_next() - .await - } -} - pub(super) async fn ref_exists( github_client: &octocrab::Octocrab, RunBenchmark { diff --git a/src/server/runner/comment.rs b/src/server/runner/comment.rs index bfb3f57..3eb7b89 100644 --- a/src/server/runner/comment.rs +++ b/src/server/runner/comment.rs @@ -3,7 +3,7 @@ use chrono::Utc; use crate::constants::{is_pr_comparison, ORG, PR_COMPARISON_MARKER}; use crate::event::Compare; -use crate::server::octocrab_utils::PageExt; +use crate::octocrab_utils::PageExt; pub(super) async fn update(cmp: &Compare, markdown: &str) -> Result<()> { // TODO: as above From 32e42efad90f59113f56abd4ac94553b00a2dca2 Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Tue, 19 Mar 2024 16:37:32 +0100 Subject: [PATCH 6/6] Simplify --- src/cli/octocrab_utils.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/cli/octocrab_utils.rs b/src/cli/octocrab_utils.rs index 81c1fdb..3f671f2 100644 --- a/src/cli/octocrab_utils.rs +++ b/src/cli/octocrab_utils.rs @@ -1,8 +1,11 @@ use anyhow::Result; -use futures::TryStreamExt; +use octocrab::models::Installation; use secrecy::ExposeSecret; -use crate::{cli, constants::APP_ID}; +use crate::{ + cli, + constants::{APP_ID, ORG}, +}; use super::Auth; @@ -14,16 +17,12 @@ where cli::Auth::AppKey(app_key) => { let key = jsonwebtoken::EncodingKey::from_rsa_pem(app_key.expose_secret().as_bytes())?; let base = octocrab::Octocrab::builder().app(APP_ID, key).build()?; - let insts = base - .apps() - .installations() - .send() - .await? - .into_stream(&base) - .try_collect::>() - .await?; - tracing::info!("Installations: {}", serde_json5::to_string(&insts)?); - Ok(octocrab::Octocrab::installation(&base, insts[0].id)) + let Installation { id, html_url, .. } = base.apps().get_org_installation(ORG).await?; + tracing::info!( + "Found installation: {}", + html_url.unwrap_or_else(|| id.to_string()) + ); + Ok(octocrab::Octocrab::installation(&base, id)) } cli::Auth::GitHubToken(github_token) => { Ok(octocrab::Octocrab::builder()