diff --git a/services/cli/src/bencher/sub/project/run/ci.rs b/services/cli/src/bencher/sub/project/run/ci.rs index 9d43aa238..5a6cee510 100644 --- a/services/cli/src/bencher/sub/project/run/ci.rs +++ b/services/cli/src/bencher/sub/project/run/ci.rs @@ -39,6 +39,7 @@ impl TryFrom for Option { ci_only_thresholds, ci_only_on_alert, ci_id, + ci_number, github_actions, } = ci; Ok(github_actions.map(|github_actions| { @@ -46,6 +47,7 @@ impl TryFrom for Option { ci_only_thresholds, ci_only_on_alert, ci_id, + ci_number, github_actions, )) })) @@ -65,6 +67,7 @@ pub struct GitHubActions { ci_only_thresholds: bool, ci_only_on_alert: bool, ci_id: Option, + ci_number: Option, token: String, } @@ -73,12 +76,14 @@ 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, } } @@ -100,16 +105,16 @@ impl GitHubActions { } // The name of the event that triggered the workflow. For example, `workflow_dispatch`. - match std::env::var("GITHUB_EVENT_NAME").ok() { - Some(event_name) - if event_name == "pull_request" || event_name == "pull_request_target" => {}, + 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. Skipping CI integration." + "Not running as a GitHub Action pull request or as another branch. Skipping CI integration." ); return Ok(()); }, - } + }; // The owner and repository name. For example, octocat/Hello-World. let (owner, repo) = match std::env::var("GITHUB_REPOSITORY").ok() { @@ -117,35 +122,44 @@ impl GitHubActions { if let Some((owner, repo)) = repository.split_once('/') { (owner.to_owned(), repo.to_owned()) } else { - cli_eprintln!("GitHub Action running on a pull request but repository is not in the form `owner/repo` ({repository}). Skipping CI integration."); + cli_eprintln!("Repository is not in the form `owner/repo` ({repository}). Skipping CI integration."); return Err(CiError::GitHubRepository(repository)); } }, _ => { - cli_eprintln!("GitHub Action running on a pull request but failed to get repository. Skipping CI integration."); + cli_eprintln!("Failed to get repository. Skipping CI integration."); return Err(CiError::NoGithubRepository); }, }; - // For workflows triggered by `pull_request`, this is the pull request merge branch. - // for pull requests it is `refs/pull//merge` - let issue_number = 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); - }, + let issue_number = if let Some(number) = self.ci_number { + number + } 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); + }, + } }; let github_client = Octocrab::builder() diff --git a/services/cli/src/parser/project/run.rs b/services/cli/src/parser/project/run.rs index 80cf97233..82cbb804f 100644 --- a/services/cli/src/parser/project/run.rs +++ b/services/cli/src/parser/project/run.rs @@ -207,9 +207,12 @@ pub struct CliRunCi { /// Only start posting results to CI if an Alert is generated (requires: `--github-actions`) #[clap(long, requires = "ci_cd")] pub ci_only_on_alert: bool, - /// Custom ID for for posting results to CI (requires: `--github-actions`) + /// 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 c2550f8d1..1f83a45b9 100644 --- a/services/console/src/content/explanation/bencher-run.mdx +++ b/services/console/src/content/explanation/bencher-run.mdx @@ -172,11 +172,22 @@ Requires: `--github-actions`
-Optional: Custom ID for for posting results to CI. +Optional: Custom ID for posting results to CI. By default, Bencher will automatically segment out results by the combination of: Project, Branch, Testbed, and [Adapter](/docs/explanation/adapters). Setting a custom ID is useful when Bencher is being run multiple times in the same CI workflow for the same Project, Branch, Testbed, and Adapter combination. 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` diff --git a/services/console/src/content/how_to/github-actions.mdx b/services/console/src/content/how_to/github-actions.mdx index 17a716f44..189cadd49 100644 --- a/services/console/src/content/how_to/github-actions.mdx +++ b/services/console/src/content/how_to/github-actions.mdx @@ -60,17 +60,31 @@ jobs:
-> 🐰 In order to run a GitHub Action job safely for a fork PR (that is from an external contributor), -> you will need to trigger [on the `pull_request_target` event](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target) and then checkout the PR branch. -> ``` -> - uses: actions/checkout@v3 -> with: -> ref: ${{ github.event.pull_request.head.sha }} -> ``` -> See this [GitHub Security Lab write up](https://securitylab.github.com/research/github-actions-preventing-pwn-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 }}`. +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: + +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. +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 }}` + + +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`. + +``` + - uses: actions/checkout@v3 + with: + ref: ${{ github.event.pull_request.head.sha }} +``` +See this [GitHub Security Lab write up](https://securitylab.github.com/research/github-actions-preventing-pwn-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 }}`