Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to app #21

Merged
merged 6 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ Eventually, we’ll just have a project-wide webhook like this. For now, if you
- Content type: <samp>application/json</samp>
- Let me select individual events → **Pull Requests**
3. Add a label <kbd>benchmark</kbd> 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

Expand All @@ -52,8 +52,8 @@ All these currently assume you have a <samp>&lt;user></samp> 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 <samp>benchmark.service</samp> file to the system, enable and start the service:
Expand Down
3 changes: 2 additions & 1 deletion src/cli.rs
Original file line number Diff line number Diff line change
@@ -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;
34 changes: 34 additions & 0 deletions src/cli/octocrab_utils.rs
Original file line number Diff line number Diff line change
@@ -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<A>(auth: A) -> Result<octocrab::Octocrab>
where
A: TryInto<Auth, Error = anyhow::Error> + 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()?)
}
}
}
57 changes: 54 additions & 3 deletions src/cli/parser.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,71 @@
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)]
#[command(propagate_version = true)]
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<SecretString>,

/// GitHub personal access token used to make API requests.
#[arg(long, short = 't', env)]
pub(crate) github_token: Option<SecretString>,
github_token: Option<SecretString>,
}

impl AuthInner {
pub(crate) async fn into_octocrab(self) -> Result<octocrab::Octocrab> {
auth_to_octocrab(self).await
}
}

pub(crate) enum Auth {
AppKey(SecretString),
GitHubToken(SecretString),
}

impl TryFrom<AuthInner> 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<Self, Self::Error> {
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)]
Expand Down
5 changes: 3 additions & 2 deletions src/constants.rs
Original file line number Diff line number Diff line change
@@ -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 =
"<!-- DO NOT REMOVE: Scverse benchmark run comment marker -->";
Expand Down
19 changes: 5 additions & 14 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@

use anyhow::{anyhow, Result};
use clap::Parser;
use secrecy::ExposeSecret;
use utils::get_credential;

mod benchmark;
mod cli;
mod constants;
mod event;
#[cfg(test)]
mod fixtures;
mod octocrab_utils;
mod repo_cache;
mod server;
mod utils;
Expand All @@ -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?;
Expand Down
33 changes: 33 additions & 0 deletions src/octocrab_utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use std::pin::pin;

use futures::{future, TryStreamExt};
use octocrab::Page;
use serde::de::DeserializeOwned;

pub(super) trait PageExt<I>
where
I: DeserializeOwned + 'static,
{
async fn find<F: Fn(&I) -> bool>(
self,
github_api: &octocrab::Octocrab,
pred: F,
) -> octocrab::Result<Option<I>>;
}

impl<I> PageExt<I> for Page<I>
where
I: DeserializeOwned + 'static,
{
async fn find<F: Fn(&I) -> bool>(
self,
github_api: &octocrab::Octocrab,
pred: F,
) -> octocrab::Result<Option<I>> {
let items = pin!(self.into_stream(github_api));
items
.try_filter(|item| future::ready(pred(item)))
.try_next()
.await
}
}
29 changes: 15 additions & 14 deletions src/server/listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,29 +73,30 @@ async fn handle_enqueue(
event: Compare,
mut state: AppState,
) -> Result<String, (StatusCode, String)> {
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))
}
}

Expand Down
34 changes: 1 addition & 33 deletions src/server/octocrab_utils.rs
Original file line number Diff line number Diff line change
@@ -1,49 +1,17 @@
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};

lazy_static! {
static ref SHA1_RE: regex::Regex = regex::Regex::new(r"^[a-f0-9]{40}$").unwrap();
}

pub(super) trait PageExt<I>
where
I: DeserializeOwned + 'static,
{
async fn find<F: Fn(&I) -> bool>(
self,
github_api: &octocrab::Octocrab,
pred: F,
) -> octocrab::Result<Option<I>>;
}

impl<I> PageExt<I> for Page<I>
where
I: DeserializeOwned + 'static,
{
async fn find<F: Fn(&I) -> bool>(
self,
github_api: &octocrab::Octocrab,
pred: F,
) -> octocrab::Result<Option<I>> {
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 {
Expand Down
2 changes: 1 addition & 1 deletion src/server/runner/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down