From cb7b983fd5a713ab2a578e8758b9d407bc3b7eef Mon Sep 17 00:00:00 2001 From: Everett Pompeii Date: Wed, 13 Sep 2023 10:08:24 -0400 Subject: [PATCH] github_event --- .../cli/src/bencher/sub/project/run/ci.rs | 180 +++++++++++------- services/cli/src/parser/project/run.rs | 3 - .../src/content/explanation/bencher-run.mdx | 16 +- .../src/content/how_to/github-actions.mdx | 151 ++++++++++++--- 4 files changed, 238 insertions(+), 112 deletions(-) diff --git a/services/cli/src/bencher/sub/project/run/ci.rs b/services/cli/src/bencher/sub/project/run/ci.rs index 5a6cee510..7b727fa16 100644 --- a/services/cli/src/bencher/sub/project/run/ci.rs +++ b/services/cli/src/bencher/sub/project/run/ci.rs @@ -4,7 +4,7 @@ use octocrab::{models::CommentId, Octocrab}; use crate::parser::project::run::CliRunCi; use super::urls::ReportUrls; -use crate::{cli_eprintln, cli_println}; +use crate::cli_println; #[derive(Debug)] pub enum Ci { @@ -13,22 +13,34 @@ pub enum Ci { #[derive(thiserror::Error, Debug)] pub enum CiError { - #[error("GitHub Action repository is not valid: {0}")] - GitHubRepository(String), - #[error("GitHub Action repository not found for pull request")] - NoGithubRepository, - #[error("GitHub Action ref is not for a pull request: {0}")] - GitHubRef(String), - #[error("GitHub Action ref not found for pull request")] - NoGitHubRef, + #[error("{0}")] + GitHub(#[from] GitHubError), +} + +#[derive(thiserror::Error, Debug)] +pub enum GitHubError { + #[error("Failed to get GitHub Action event path")] + NoEventPath, + #[error("Failed to read GitHub Action event path ({0}): {1}")] + BadEventPath(String, std::io::Error), + #[error("Failed to parse GitHub Action event ({0}): {1}")] + BadEvent(String, serde_json::Error), + #[error("GitHub Action running on event ({0}) but event type is {1:?}")] + BadEventType(String, octocrab::models::events::EventType), + #[error("GitHub Action event ({0}) missing payload")] + NoEventPayload(String), + #[error("GitHub Action event ({0}) has the wrong payload")] + BadEventPayload(String), + #[error("GitHub repository is not in the form `owner/repo` ({0})")] + Repository(String), #[error("Failed to authenticate as GitHub Action: {0}")] - GitHubAuth(octocrab::Error), + Auth(octocrab::Error), #[error("Failed to list GitHub PR comments: {0}")] - GitHubComments(octocrab::Error), + Comments(octocrab::Error), #[error("Failed to create GitHub PR comment: {0}")] - GitHubCreateComment(octocrab::Error), + CreateComment(octocrab::Error), #[error("Failed to update GitHub PR comment: {0}")] - GitHubUpdateComment(octocrab::Error), + UpdateComment(octocrab::Error), } impl TryFrom for Option { @@ -39,7 +51,6 @@ impl TryFrom for Option { ci_only_thresholds, ci_only_on_alert, ci_id, - ci_number, github_actions, } = ci; Ok(github_actions.map(|github_actions| { @@ -47,7 +58,6 @@ impl TryFrom for Option { ci_only_thresholds, ci_only_on_alert, ci_id, - ci_number, github_actions, )) })) @@ -57,7 +67,9 @@ impl TryFrom for Option { impl Ci { pub async fn run(&self, report_urls: &ReportUrls) -> Result<(), CiError> { match self { - Self::GitHubActions(github_actions) => github_actions.run(report_urls).await, + Self::GitHubActions(github_actions) => { + github_actions.run(report_urls).await.map_err(Into::into) + }, } } } @@ -67,7 +79,6 @@ pub struct GitHubActions { ci_only_thresholds: bool, ci_only_on_alert: bool, ci_id: Option, - ci_number: Option, token: String, } @@ -76,18 +87,17 @@ impl GitHubActions { ci_only_thresholds: bool, ci_only_on_alert: bool, ci_id: Option, - ci_number: Option, token: String, ) -> Self { Self { ci_only_thresholds, ci_only_on_alert, ci_id, - ci_number, token, } } - pub async fn run(&self, report_urls: &ReportUrls) -> Result<(), CiError> { + + pub async fn run(&self, report_urls: &ReportUrls) -> Result<(), GitHubError> { // Only post to CI if there are thresholds set if self.ci_only_thresholds && !report_urls.has_threshold() { cli_println!("No thresholds set. Skipping CI integration."); @@ -104,68 +114,96 @@ impl GitHubActions { }, } - // The name of the event that triggered the workflow. For example, `workflow_dispatch`. - let as_other_branch = match std::env::var("GITHUB_EVENT_NAME").ok().as_deref() { - Some("pull_request") => false, - Some("pull_request_target" | "workflow_run") => true, - _ => { - cli_println!( - "Not running as a GitHub Action pull request or as another branch. Skipping CI integration." - ); - return Ok(()); - }, + // The path to the file on the runner that contains the full event webhook payload. For example, /github/workflow/event.json. + let github_event_path = match std::env::var("GITHUB_EVENT_PATH").ok() { + Some(github_event_path) => github_event_path, + _ => return Err(GitHubError::NoEventPath), }; + let event_str = std::fs::read_to_string(&github_event_path) + .map_err(|e| GitHubError::BadEventPath(github_event_path, e))?; + let event: octocrab::models::events::Event = serde_json::from_str(&event_str) + .map_err(|e| GitHubError::BadEvent(event_str.clone(), e))?; - // The owner and repository name. For example, octocat/Hello-World. - let (owner, repo) = match std::env::var("GITHUB_REPOSITORY").ok() { - Some(repository) => { - if let Some((owner, repo)) = repository.split_once('/') { - (owner.to_owned(), repo.to_owned()) + // The name of the event that triggered the workflow. For example, `workflow_dispatch`. + let issue_number = match std::env::var("GITHUB_EVENT_NAME").ok().as_deref() { + // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request + // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target + Some(event_name @ ("pull_request" | "pull_request_target")) => { + if event.r#type != octocrab::models::events::EventType::PullRequestEvent { + return Err(GitHubError::BadEventType(event_name.into(), event.r#type)); + } + + let payload = event + .payload + .ok_or_else(|| GitHubError::NoEventPayload(event_name.into()))? + .specific + .ok_or_else(|| GitHubError::NoEventPayload(event_name.into()))?; + + if let octocrab::models::events::payload::EventPayload::PullRequestEvent(payload) = + payload + { + payload.number } else { - cli_eprintln!("Repository is not in the form `owner/repo` ({repository}). Skipping CI integration."); - return Err(CiError::GitHubRepository(repository)); + return Err(GitHubError::BadEventPayload(event_name.into())); } }, + // https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run + Some(event_name @ "workflow_run") => { + if event.r#type != octocrab::models::events::EventType::WorkflowRunEvent { + return Err(GitHubError::BadEventType(event_name.into(), event.r#type)); + } + + // TODO upstream a fix for: https://github.com/XAMPPRocky/octocrab/pull/162 + let event: serde_json::Value = serde_json::from_str(&event_str) + .map_err(|e| GitHubError::BadEvent(event_str, e))?; + + // https://docs.github.com/en/webhooks/webhook-events-and-payloads#workflow_run + let pull_requests = "pull_requests"; + let index = 0; + let number = "number"; + event + .get(event_name) + .ok_or_else(|| GitHubError::NoEventPayload(event_name.into()))? + .get(pull_requests) + .ok_or_else(|| { + GitHubError::NoEventPayload(format!("{event_name}/{pull_requests}")) + })? + .get(index) + .ok_or_else(|| { + GitHubError::NoEventPayload(format!("{event_name}/{pull_requests}/{index}")) + })? + .get(number) + .ok_or_else(|| { + GitHubError::NoEventPayload(format!( + "{event_name}/{pull_requests}/{index}/{number}" + )) + })? + .as_u64() + .ok_or_else(|| { + GitHubError::BadEventPayload(format!( + "{event_name}/{pull_requests}/{index}/{number}" + )) + })? + }, _ => { - cli_eprintln!("Failed to get repository. Skipping CI integration."); - return Err(CiError::NoGithubRepository); + cli_println!( + "Not running as an expected GitHub Action event (`pull_request`, `pull_request_target`, or `workflow_run`). Skipping CI integration." + ); + return Ok(()); }, }; - let issue_number = if let Some(number) = self.ci_number { - number + // The owner and repository name. For example, octocat/Hello-World. + let (owner, repo) = if let Some((owner, repo)) = event.repo.name.split_once('/') { + (owner.to_owned(), repo.to_owned()) } else { - if as_other_branch { - cli_println!("GitHub Action running on a non-pull request branch but no issue number was provided. Skipping CI integration."); - return Ok(()); - } - - // For workflows triggered by `pull_request`, this is the pull request merge branch. - // for pull requests it is `refs/pull//merge` - match std::env::var("GITHUB_REF").ok() { - Some(github_ref) => { - if let Some(issue_number) = github_ref - .strip_prefix("refs/pull/") - .and_then(|r| r.strip_suffix("/merge")) - .and_then(|r| r.parse::().ok()) - { - issue_number - } else { - cli_eprintln!("GitHub Action running on a pull request but ref is not a pull request ref ({github_ref}). Skipping CI integration."); - return Err(CiError::GitHubRef(github_ref)); - } - }, - None => { - cli_eprintln!("GitHub Action running on a pull request but failed to get ref. Skipping CI integration."); - return Err(CiError::NoGitHubRef); - }, - } + return Err(GitHubError::Repository(event.repo.name)); }; let github_client = Octocrab::builder() .user_access_token(self.token.clone()) .build() - .map_err(CiError::GitHubAuth)?; + .map_err(GitHubError::Auth)?; // Get the comment ID if it exists let comment_id = get_comment( @@ -184,7 +222,7 @@ impl GitHubActions { issue_handler .update_comment(comment_id, body) .await - .map_err(CiError::GitHubUpdateComment)? + .map_err(GitHubError::UpdateComment)? } else { if self.ci_only_on_alert && !report_urls.has_alert() { cli_println!("No alerts found. Skipping CI integration."); @@ -193,7 +231,7 @@ impl GitHubActions { issue_handler .create_comment(issue_number, body) .await - .map_err(CiError::GitHubCreateComment)? + .map_err(GitHubError::CreateComment)? }; Ok(()) @@ -206,7 +244,7 @@ pub async fn get_comment( repo: &str, issue_number: u64, bencher_tag: &str, -) -> Result, CiError> { +) -> Result, GitHubError> { const PER_PAGE: u8 = 100; let mut page: u32 = 1; @@ -218,7 +256,7 @@ pub async fn get_comment( .page(page) .send() .await - .map_err(CiError::GitHubComments)?; + .map_err(GitHubError::Comments)?; let comments_len = comments.items.len(); if comments_len == 0 { diff --git a/services/cli/src/parser/project/run.rs b/services/cli/src/parser/project/run.rs index 82cbb804f..3ed8ed5ef 100644 --- a/services/cli/src/parser/project/run.rs +++ b/services/cli/src/parser/project/run.rs @@ -210,9 +210,6 @@ pub struct CliRunCi { /// Custom ID for posting results to CI (requires: `--github-actions`) #[clap(long, requires = "ci_cd")] pub ci_id: Option, - /// Issue number for posting results to CI (requires: `--github-actions`) - #[clap(long, requires = "ci_cd")] - pub ci_number: Option, /// GitHub API authentication token for GitHub Actions to comment on PRs (ie `--github-actions ${{ secrets.GITHUB_TOKEN }}`) #[clap(long)] pub github_actions: Option, diff --git a/services/console/src/content/explanation/bencher-run.mdx b/services/console/src/content/explanation/bencher-run.mdx index 1f83a45b9..eea69bb68 100644 --- a/services/console/src/content/explanation/bencher-run.mdx +++ b/services/console/src/content/explanation/bencher-run.mdx @@ -179,17 +179,6 @@ Requires: `--github-actions`
-### `--ci-number` - -
- -Optional: Issue number for posting results to CI. -Bencher will try its best to detect the CI issue number needed to post results. -However, this isn't always available in complex setups, like using `pull_request_target` or `workflow_run` in GitHub Actions. -Requires: `--github-actions` - -
- ### `--github-actions`
@@ -199,11 +188,10 @@ When this option is set and `bencher run` is used in GitHub Actions as a part of then the results will be added to the pull request as a comment. The most convenient way to do this is the [GitHub Actions `GITHUB_TOKEN` environment variable](https://docs.github.com/en/actions/security-guides/automatic-token-authentication). -> 🐰 If you are running inside of a Docker container within GitHub Action, you will need to pass in the following environment variables: +> 🐰 If you are running inside of a Docker container within GitHub Action, you will need to pass in the following environment variables and mount the path specified by `GITHUB_EVENT_PATH`: > - `GITHUB_ACTIONS` > - `GITHUB_EVENT_NAME` -> - `GITHUB_REPOSITORY` -> - `GITHUB_REF` +> - `GITHUB_EVENT_PATH`
diff --git a/services/console/src/content/how_to/github-actions.mdx b/services/console/src/content/how_to/github-actions.mdx index 189cadd49..11ab48f46 100644 --- a/services/console/src/content/how_to/github-actions.mdx +++ b/services/console/src/content/how_to/github-actions.mdx @@ -4,11 +4,8 @@ heading: "How to use Bencher in GitHub Actions" sortOrder: 3 --- -import { BENCHER_VERSION } from "../../util/ext"; - -
-  
-    {`on:
+```
+on:
   push:
     branches: main
   pull_request:
@@ -24,7 +21,7 @@ jobs:
       BENCHER_TESTBED: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
-      - uses: bencherdev/bencher@v${BENCHER_VERSION}
+      - uses: bencherdev/bencher@main
       - name: Benchmark with Bencher
         run: |
           bencher run \\
@@ -33,9 +30,8 @@ jobs:
           --else-if-branch main \\
           --err \\
           --github-actions \${{ secrets.GITHUB_TOKEN }} \\
-          "bencher mock"`}
-  
-
+ "bencher mock" +``` 1. Create a GitHub Actions `workflow` (ex: `.github/workflows/benchmark.yml`). 1. Specify events the workflow should run `on`. See the [GitHub Actions `on` documentation](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#on) for a full overview. @@ -48,7 +44,7 @@ jobs: 1. Optional: Set the `--adapter` flag or the `BENCHER_ADAPTER` environment variable to the desired adapter name. If this is not set, then the `magic` Adapter will be used. See [benchmark harness adapters](/docs/explanation/adapters) for a full overview. (ex: `BENCHER_ADAPTER: json`) 1. Optional: Set the `--testbed` flag or the `BENCHER_TESTBED` environment variable to the Testbed slug or UUID. The Testbed **must** already exist. If this is not set, then the `localhost` Testbed will be used. (ex: `BENCHER_TESTBED: ubuntu-latest`) 1. Checkout your source code. (ex: `uses: actions/checkout@v3`) -1. Install the Bencher CLI using the [GitHub Action](https://github.com/marketplace/actions/bencher-cli). (ex: {`uses: bencherdev/bencher@v${BENCHER_VERSION}`}) +1. Install the Bencher CLI using the [GitHub Action](https://github.com/marketplace/actions/bencher-cli). (ex: `uses: bencherdev/bencher@main`) 1. [Track your benchmarks](/docs/how-to/track-benchmarks) with the bencher run CLI subcommand: 1. There are several options for setting the project branch. See [branch selection](/docs/how-to/branch-selection) for a full overview. The provided command uses [GitHub Action default environment variables](https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables) and it tries to: 1. Use the current branch data if it already exists. (ex: `--if-branch "$GITHUB_REF_NAME"`) @@ -60,31 +56,138 @@ jobs:
-For security reasons, secrets and a read/write `GITHUB_TOKEN` are not available in GitHub Actions for fork PRs. That is if an external contributor opens up a PR the above job will not work. There are two options to work around this: +## Fork Pull Requests + +For security reasons, secrets such as your `BENCHER_API_TOKEN` and the `GITHUB_TOKEN` are not available in GitHub Actions for fork PRs. +That is if an external contributor opens up a PR the above job will not work. There are two options to work around this. + +### Cache PR Benchmark Results + +The first and preferred option is to cache your PR benchmark results and then use a separate workflow to upload them to Bencher. -Run your benchmarks in an on `pull_request` workflow. -Save the benchmarks results to a file and upload them as an artifact. -Then chain that workflow with [the `workflow_run` event](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run) -and use `bencher run` to store your cached benchmark results for that PR. +1. Run your benchmarks in an on `pull_request` workflow. +1. Save the benchmarks results to a file and upload them as an artifact. +1. Chain that workflow with [the `workflow_run` event](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run). +1. Track the PR benchmark results with `bencher run`. + +This works because `workflow_run` runs in the context of the repository's default branch, +where secrets such as your `BENCHER_API_TOKEN` and the `GITHUB_TOKEN` are available. +Therefore, this will only run if the workflow files are on the default branch. See [using data from the triggering workflow](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#using-data-from-the-triggering-workflow) for a full overview. -The `--ci-number` argument is also required into order to know the GitHub issue number for the triggering PR. -`${{ github.event.workflow_run.pull_requests[0]?.number }}` +``` +name: Run and Cache Benchmarks + +on: pull_request -Or trigger [on the `pull_request_target` event](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target), -checkout the PR branch, and run your benchmarks using `bencher run`. +jobs: + benchmark: + name: Run Benchmarks + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Simple Benchmark + run: echo '{"my_benchmark": { "latency": { "value": 1.0 }}}' &> benchmark_results.txt + - uses: actions/upload-artifact@v3 + with: + name: benchmark_results.txt + path: ./benchmark_results.txt +``` ``` - - uses: actions/checkout@v3 - with: - ref: ${{ github.event.pull_request.head.sha }} +name: Track Benchmarks with Bencher + +on: + workflow_run: + workflows: [Run and Cache Benchmarks] + types: + - completed + +jobs: + track_with_bencher: + if: ${{ github.event.workflow_run.conclusion == 'success' }} + runs-on: ubuntu-latest + env: + BENCHER_PROJECT: benchmark-fork-prs + BENCHER_API_TOKEN: ${{ secrets.BENCHER_API_TOKEN }} + BENCHER_ADAPTER: json + steps: + - name: Download Benchmark Results + uses: actions/github-script@v6 + with: + script: | + let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({ + owner: context.repo.owner, + repo: context.repo.repo, + run_id: context.payload.workflow_run.id, + }); + let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => { + return artifact.name == "benchmark_results" + })[0]; + let download = await github.rest.actions.downloadArtifact({ + owner: context.repo.owner, + repo: context.repo.repo, + artifact_id: matchArtifact.id, + archive_format: 'zip', + }); + let fs = require('fs'); + fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/benchmark_results.zip`, Buffer.from(download.data)); + - name: Unzip Benchmark Results + run: unzip benchmark_results.zip + - uses: bencherdev/bencher@main + - name: Benchmark with Bencher + run: | + cat benchmark_results.txt | bencher run \\ + --if-branch "${{ github.event.pull_request.head.ref }}" \\ + --else-if-branch "${{ github.event.pull_request.base.ref }}" \\ + --else-if-branch main \\ + --err \\ + --github-actions \${{ secrets.GITHUB_TOKEN }} ``` + +## Benchmark PR Branch from Target Branch + +The second option is simpler, but it is much more of a security risk! + +1. Trigger [on the `pull_request_target` event](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target). +1. Checkout the PR branch. +1. Run and track your benchmarks using `bencher run` directly. + See this [GitHub Security Lab write up](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/) +and [this blog post](https://nathandavison.com/blog/github-actions-and-the-threat-of-malicious-pull-requests) on preventing pwn requests for a full overview. Avoid using any sensitive environment variables like `BENCHER_API_TOKEN`. Instead explicitly pass in the API token to `bencher run` like so `--token ${{ secrets.BENCHER_API_TOKEN }}`. -This is by far the riskier option! -`${{ github.event.pull_request.number }}` +Again, this is by far the riskier option! + +``` +name: Benchmark with Bencher and Play with Fire 🔥 + +on: pull_request_target + +jobs: + benchmark_with_bencher: + name: Track benchmarks with Bencher + runs-on: ubuntu-latest + env: + BENCHER_PROJECT: benchmark-fork-prs + BENCHER_ADAPTER: json + steps: + - uses: actions/checkout@v3 + with: + ref: ${{ github.event.pull_request.head.sha }} + - uses: bencherdev/bencher@main + - name: Benchmark with Bencher + run: | + bencher run \\ + --if-branch "${{ github.event.pull_request.head.ref }}" \\ + --else-if-branch "${{ github.event.pull_request.base.ref }}" \\ + --else-if-branch main \\ + --err \\ + --github-actions \${{ secrets.GITHUB_TOKEN }} \\ + --token ${{ secrets.BENCHER_API_TOKEN }} \\ + "bencher mock" +```