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..e7d72f7 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,5 +1,6 @@ +mod octocrab_utils; 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/octocrab_utils.rs b/src/cli/octocrab_utils.rs new file mode 100644 index 0000000..3f671f2 --- /dev/null +++ b/src/cli/octocrab_utils.rs @@ -0,0 +1,34 @@ +use anyhow::Result; +use octocrab::models::Installation; +use secrecy::ExposeSecret; + +use crate::{ + cli, + constants::{APP_ID, ORG}, +}; + +use super::Auth; + +pub(super) async fn auth_to_octocrab(auth: A) -> Result +where + A: TryInto + Default, +{ + 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()?; + 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() + // 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 0a42e3c..5a8b97a 100644 --- a/src/cli/parser.rs +++ b/src/cli/parser.rs @@ -1,10 +1,13 @@ use clap::{Args, Parser, Subcommand}; use serde::Deserialize; +use anyhow::Result; use secrecy::SecretString; use std::fmt::Display; -use crate::constants::ORG; +use crate::{constants::ORG, utils::get_credential}; + +use super::octocrab_utils::auth_to_octocrab; #[derive(Parser)] #[command(version, about, long_about = None)] @@ -12,9 +15,57 @@ 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, +} + +impl AuthInner { + pub(crate) async fn into_octocrab(self) -> Result { + auth_to_octocrab(self).await + } +} + +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..5876611 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[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..69da3d1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,8 +2,6 @@ use anyhow::{anyhow, Result}; use clap::Parser; -use secrecy::ExposeSecret; -use utils::get_credential; mod benchmark; mod cli; @@ -11,6 +9,7 @@ mod constants; mod event; #[cfg(test)] mod fixtures; +mod octocrab_utils; mod repo_cache; mod server; mod utils; @@ -19,18 +18,10 @@ 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(); + + octocrab::initialise(std::mem::take(&mut cli.auth).into_octocrab().await?); + match cli.command { cli::Commands::Serve(args) => { server::serve(args).await?; 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/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)) } } 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