ci_number

bencherdev · Sep 12, 2023 · 3c05a52 · 3c05a52
1 parent caa5fc2
commit 3c05a52
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 39 deletions.
diff --git a/services/cli/src/bencher/sub/project/run/ci.rs b/services/cli/src/bencher/sub/project/run/ci.rs
@@ -39,13 +39,15 @@ impl TryFrom<CliRunCi> for Option<Ci> {
             ci_only_thresholds,
             ci_only_on_alert,
             ci_id,
+            ci_number,
             github_actions,
         } = ci;
         Ok(github_actions.map(|github_actions| {
             Ci::GitHubActions(GitHubActions::new(
                 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<NonEmpty>,
+    ci_number: Option<u64>,
     token: String,
 }
 
@@ -73,12 +76,14 @@ impl GitHubActions {
         ci_only_thresholds: bool,
         ci_only_on_alert: bool,
         ci_id: Option<NonEmpty>,
+        ci_number: Option<u64>,
         token: String,
     ) -> Self {
         Self {
             ci_only_thresholds,
             ci_only_on_alert,
             ci_id,
+            ci_number,
             token,
         }
     }
@@ -100,52 +105,61 @@ 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() {
             Some(repository) => {
                 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/<pr_number>/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::<u64>().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/<pr_number>/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::<u64>().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
@@ -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<NonEmpty>,
+    /// Issue number for posting results to CI (requires: `--github-actions`)
+    #[clap(long, requires = "ci_cd")]
+    pub ci_number: Option<u64>,
     /// GitHub API authentication token for GitHub Actions to comment on PRs (ie `--github-actions ${{ secrets.GITHUB_TOKEN }}`)
     #[clap(long)]
     pub github_actions: Option<String>,

diff --git 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`
 
 <br />
 
-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`
 
+<br/>
+
+### `--ci-number`
+
+<br />
+
+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`
+
 <br />
 
 ### `--github-actions`

diff --git 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:
 
 <br/>
 
-> 🐰 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 }}`
 
 <br/>
 <br/>