ci: Upload benchmark report from separate workflow (#23)

## 📝 Summary CI for PRs should (and do) run in a context that doesn't have access to secrets. This PR separates the upload of the report into a separate workflow. See also https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ --- ## ✅ I have completed the following steps: * [x] Run `make lint` * [x] Run `make test` * [ ] Added tests (if applicable) --------- Co-authored-by: Anton <[email protected]>
flashbots · Jul 4, 2024 · df90080 · df90080
1 parent 3915ec4
commit df90080
Show file tree

Hide file tree

Showing 4 changed files with 109 additions and 79 deletions.
diff --git a/.github/workflows/bench.yaml b/.github/workflows/bench.yaml
@@ -1,7 +1,6 @@
 name: Benchmarks
 
 on:
-  workflow_dispatch:
   pull_request:
   push:
     branches: [develop]
@@ -16,7 +15,6 @@ jobs:
           - stable
     env:
       PR_NUMBER: ${{ github.event.number }}
-      BENCH_AGAINST_BASE: 1
     steps:
       - name: Checkout sources
         uses: actions/checkout@v4
@@ -40,84 +38,32 @@ jobs:
         run: |
           source scripts/ci/env-vars.sh
 
-          echo "HEAD_SHA: ${HEAD_SHA}"
-          echo "HEAD_SHA_SHORT: ${HEAD_SHA_SHORT}"
-          echo "BASE_SHA: ${BASE_SHA}"
-          echo "BASE_SHA_SHORT: ${BASE_SHA_SHORT}"
+          # Keep important variables around for the upload workflow that comes afterwards
+          fn_vars="vars.txt"
+          echo "PR_NUMBER=${PR_NUMBER}" >> $fn_vars
+          echo "HEAD_SHA=${HEAD_SHA}" >> $fn_vars
+          echo "HEAD_SHA_SHORT=${HEAD_SHA_SHORT}" >> $fn_vars
+          echo "BASE_SHA=${BASE_SHA}" >> $fn_vars
+          echo "BASE_SHA_SHORT=${BASE_SHA_SHORT}" >> $fn_vars
+          cat $fn_vars
 
-          echo "HEAD_SHA=${HEAD_SHA}" >> "$GITHUB_OUTPUT"
-          echo "HEAD_SHA_SHORT=${HEAD_SHA_SHORT}" >> "$GITHUB_OUTPUT"
-          echo "BASE_SHA=${BASE_SHA}" >> "$GITHUB_OUTPUT"
-          echo "BASE_SHA_SHORT=${BASE_SHA_SHORT}" >> "$GITHUB_OUTPUT"
-
-          # S3 upload directory, depending if it includes a comparison or not
-          if [ "$HEAD_SHA" == "$BASE_SHA" ]; then
-            # No comparison (i.e. running on a branch like develop directly)
-            S3_UPLOAD_DIR="benchmark/${HEAD_SHA_SHORT}"
-          else
-            # Comparison (i.e. running on a PR)
-            S3_UPLOAD_DIR="benchmark/${HEAD_SHA_SHORT}-${BASE_SHA_SHORT}"
-          fi
-          echo "S3_UPLOAD_DIR: ${S3_UPLOAD_DIR}"
-          echo "S3_UPLOAD_DIR=${S3_UPLOAD_DIR}" >> "$GITHUB_OUTPUT"
-          echo "S3_UPLOAD_DIR=${S3_UPLOAD_DIR}" >> "$GITHUB_ENV"
-
-      #
-      # RUN BENCHMARKS (and upload the report)
-      #
+      # RUN BENCHMARKS
       - run: make bench-in-ci
 
-      # Upload as artifact first
-      - name: Upload report as artifact
-        uses: actions/[email protected]
-        with:
-          name: benchmark-report
-          path: target/benchmark-in-ci/benchmark-report/
-
-      - name: Zip the report and add to folder (for S3 upload)
-        working-directory: target/benchmark-in-ci
+      - name: Zip the report
         run: |
-          zip_fn="report.zip"
-          zip -r $zip_fn benchmark-report
-          mv $zip_fn benchmark-report/
+          cp vars.txt target/benchmark-in-ci/benchmark-report/
 
-      # Upload S3 (using https://github.com/shallwefootball/upload-s3-action)
-      - name: Upload report to S3
-        uses: shallwefootball/s3-upload-action@master
-        id: S3
-        with:
-          aws_key_id: ${{secrets.AWS_KEY_ID}}
-          aws_secret_access_key: ${{secrets.AWS_SECRET_ACCESS_KEY}}
-          aws_bucket: flashbots-rbuilder-ci-stats
-          source_dir: target/benchmark-in-ci/benchmark-report
-          destination_dir: ${{ steps.vars.outputs.S3_UPLOAD_DIR }}
+          cd target/benchmark-in-ci
+          zip -r benchmark-report.zip benchmark-report
+          mv benchmark-report.zip ../../
 
-      #
-      # POST SUMMARY (to PR comment and CI job summary)
-      #
-      - name: Add summary to CI job summary
-        run: |
-          BENCH_URL="https://flashbots-rbuilder-ci-stats.s3.us-east-2.amazonaws.com/${{steps.S3.outputs.object_key}}/report/index.html"
-          sed -i "s|__BENCH_URL__|${BENCH_URL}|" target/benchmark-in-ci/benchmark-summary.md
-          sed -i "s|__BENCH_URL__|${BENCH_URL}|" target/benchmark-in-ci/benchmark-pr-comment.md
-          cat target/benchmark-in-ci/benchmark-summary.md >> $GITHUB_STEP_SUMMARY
-
-      # https://github.com/peter-evans/find-comment
-      - name: Find previous PR comment
-        uses: peter-evans/find-comment@v3
-        if: github.event_name == 'pull_request'
-        id: fc
+      - name: Upload report as artifact
+        uses: actions/upload-artifact@v4
         with:
-          issue-number: ${{ github.event.pull_request.number }}
-          comment-author: 'github-actions[bot]'
-          body-includes: Benchmark results
+          name: benchmark-report.zip
+          path: benchmark-report.zip
 
-      # https://github.com/peter-evans/create-or-update-comment
-      - name: Create or update PR comment
-        uses: peter-evans/create-or-update-comment@v4
-        if: github.event_name == 'pull_request'
-        with:
-          comment-id: ${{ steps.fc.outputs.comment-id }}
-          issue-number: ${{ github.event.pull_request.number }}
-          edit-mode: replace
-          body-path: target/benchmark-in-ci/benchmark-pr-comment.md
+      - name: Add details to CI job summary
+        run: |
+          cat target/benchmark-in-ci/benchmark-report/benchmark-summary.md >> $GITHUB_STEP_SUMMARY
diff --git a/.github/workflows/bench_upload_for_pr.yaml b/.github/workflows/bench_upload_for_pr.yaml
@@ -0,0 +1,86 @@
+name: Benchmark upload
+
+#
+# This workflow runs after every benchmark, to upload the report to S3.
+#
+# It is its own workflow, because PRs can contain malicious code and run in a context with secrets..
+#
+# See also https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
+#
+
+on:
+  workflow_run:
+    workflows: ["Benchmarks"]
+    types:
+      - completed
+
+jobs:
+  upload:
+    name: Upload benchmark report
+    runs-on: warp-ubuntu-latest-x64-16x
+    if: github.event.workflow_run.conclusion == 'success'
+    steps:
+      # https://github.com/actions/download-artifact
+      - uses: actions/download-artifact@v4
+        with:
+          name: benchmark-report.zip
+          run-id: ${{ github.event.workflow_run.id }}
+          github-token: ${{ github.token }}
+
+      - name: Extract report
+        run: |
+          unzip benchmark-report.zip
+          ls -alh benchmark-report/
+
+      - name: Prepare variables
+        id: prepare
+        run: |
+          cat benchmark-report/vars.txt
+          source benchmark-report/vars.txt
+
+          S3_UPLOAD_DIR="benchmark/${HEAD_SHA_SHORT}-${BASE_SHA_SHORT}"
+          echo "S3_UPLOAD_DIR=${S3_UPLOAD_DIR}" >> "$GITHUB_OUTPUT"
+          echo "PR_NUMBER=${PR_NUMBER}" >> "$GITHUB_OUTPUT"
+
+      # Upload S3 (using https://github.com/shallwefootball/upload-s3-action)
+      - name: Upload to S3
+        uses: shallwefootball/s3-upload-action@master
+        id: S3
+        with:
+          aws_key_id: ${{secrets.AWS_KEY_ID}}
+          aws_secret_access_key: ${{secrets.AWS_SECRET_ACCESS_KEY}}
+          aws_bucket: flashbots-rbuilder-ci-stats
+          source_dir: benchmark-report
+          destination_dir: ${{ steps.prepare.outputs.S3_UPLOAD_DIR }}
+
+      #
+      # POST SUMMARY (to PR comment and CI job summary)
+      #
+      - name: Add summary to CI job summary
+        run: |
+          BENCH_URL="https://flashbots-rbuilder-ci-stats.s3.us-east-2.amazonaws.com/${{steps.S3.outputs.object_key}}/report/index.html"
+          sed -i "s|__BENCH_URL__|${BENCH_URL}|" benchmark-report/benchmark-pr-comment.md
+          cat benchmark-report/benchmark-pr-comment.md
+
+          cat benchmark-report/benchmark-pr-comment.md >> $GITHUB_STEP_SUMMARY
+          cat benchmark-report/benchmark-summary.md >> $GITHUB_STEP_SUMMARY
+
+      # https://github.com/peter-evans/find-comment
+      - name: Find previous PR comment
+        if: steps.prepare.outputs.PR_NUMBER != ''
+        uses: peter-evans/find-comment@v3
+        id: fc
+        with:
+          issue-number: ${{ steps.prepare.outputs.PR_NUMBER }}
+          comment-author: 'github-actions[bot]'
+          body-includes: Benchmark results
+
+      # https://github.com/peter-evans/create-or-update-comment
+      - name: Create or update PR comment
+        if: steps.prepare.outputs.PR_NUMBER != ''
+        uses: peter-evans/create-or-update-comment@v4
+        with:
+          comment-id: ${{ steps.fc.outputs.comment-id }}
+          issue-number: ${{ steps.prepare.outputs.PR_NUMBER }}
+          edit-mode: replace
+          body-path: benchmark-report/benchmark-pr-comment.md
diff --git a/scripts/ci/benchmark-in-ci.sh b/scripts/ci/benchmark-in-ci.sh
@@ -90,11 +90,11 @@ mkdir -p target/benchmark-in-ci
 echo "Saved report: target/benchmark-in-ci/benchmark-report/report/index.html"
 
 # Create summary markdown
-fn="target/benchmark-in-ci/benchmark-summary.md"
+fn="target/benchmark-in-ci/benchmark-report/benchmark-summary.md"
 envsubst < scripts/ci/templates/benchmark-summary.md > $fn
 echo "Wrote summary: $fn"
 
 # Create summary pr comment
-fn="target/benchmark-in-ci/benchmark-pr-comment.md"
+fn="target/benchmark-in-ci/benchmark-report/benchmark-pr-comment.md"
 envsubst < scripts/ci/templates/benchmark-pr-comment.md > $fn
 echo "Wrote PR comment: $fn"
diff --git a/scripts/ci/templates/benchmark-summary.md b/scripts/ci/templates/benchmark-summary.md
@@ -1,7 +1,5 @@
 # Benchmark results for `${HEAD_SHA_SHORT}`
 
-Full report: __BENCH_URL__
-
 |                |                      |
 | -------------- | -------------------- |
 | Date (UTC)     | ${DATE}              |