From b877c3dabb360b8f5cd420fc43882ddf1864f414 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 11 Mar 2024 13:21:49 +0200
Subject: [PATCH 1/4] ci: Safe commenting on PRs

This generalizes and refactors what the QNS workflow did.
---
 .../actions/pr-comment-data-export/action.yml | 37 +++++++++++++++++++
 .github/actions/pr-comment/action.yml         | 25 +++++++++++++
 .../actions/quic-interop-runner/action.yml    | 32 +++++++---------
 .github/workflows/bench.yml                   | 12 +++---
 .github/workflows/check.yml                   |  7 ----
 .github/workflows/qns-comment.yml             | 34 +----------------
 6 files changed, 84 insertions(+), 63 deletions(-)
 create mode 100644 .github/actions/pr-comment-data-export/action.yml
 create mode 100644 .github/actions/pr-comment/action.yml

diff --git a/.github/actions/pr-comment-data-export/action.yml b/.github/actions/pr-comment-data-export/action.yml
new file mode 100644
index 0000000000..60ad4b9bb3
--- /dev/null
+++ b/.github/actions/pr-comment-data-export/action.yml
@@ -0,0 +1,37 @@
+name: 'Export data for PR comment'
+description: 'Exports the neccessary data to post a PR comment securely.'
+
+# This action might be running off of a fork and would thus not have write
+# permissions on the origin repository. In order to allow a separate
+# priviledged action to post a comment on a pull request, upload the
+# necessary metadata.
+
+inputs:
+  name:
+    description: 'A unique name for the artifact used for exporting.'
+    required: true
+  contents:
+    description: 'A filename with a comment (in Markdown) to be added to the PR.'
+    required: true
+  log-url:
+    description: 'A URL to a log to be linked from the PR comment.'
+    required: false
+
+runs:
+  using: composite
+  steps:
+    - if: github.event_name == 'pull_request'
+      shell: bash
+      run: |
+        mkdir comment-data
+        cp "${{ inputs.contents }}" comment-data/contents
+        echo "${{ inputs.name }}"" > comment-data/name
+        echo "${{ inputs.log-url }}"" > comment-data/log-url
+        echo "${{ github.event.number }}" > comment-data/pr-number
+
+    - if: github.event_name == 'pull_request'
+      uses: actions/upload-artifact@v4
+      with:
+        name: ${{ inputs.name }}
+        path: comment-data
+        retention-days: 1
diff --git a/.github/actions/pr-comment/action.yml b/.github/actions/pr-comment/action.yml
new file mode 100644
index 0000000000..37cde6838f
--- /dev/null
+++ b/.github/actions/pr-comment/action.yml
@@ -0,0 +1,25 @@
+name: 'Comment on PR'
+description: 'Post a PR comment securely.'
+
+inputs:
+  name:
+    description: 'Artifact name to import comment data from.'
+    required: true
+
+runs:
+  using: composite
+  steps:
+    - uses: actions/download-artifact@v4
+      with:
+        run-id: ${{ github.event.workflow_run.id }}
+        name: ${{ inputs.name }}
+
+    - id: pr-number
+      shell: bash
+      run: echo "number=$(cat pr-number)" >> "$GITHUB_OUTPUT"
+
+    - uses: thollander/actions-comment-pull-request@v2
+      with:
+        filePath: contents
+        pr_number: ${{ steps.pr-number.outputs.number }}
+        comment_tag: ${{ inputs.name }}-comment
diff --git a/.github/actions/quic-interop-runner/action.yml b/.github/actions/quic-interop-runner/action.yml
index 6e79b97cfe..0704198b7e 100644
--- a/.github/actions/quic-interop-runner/action.yml
+++ b/.github/actions/quic-interop-runner/action.yml
@@ -88,24 +88,20 @@ runs:
         name: logs
         path: quic-interop-runner/logs
 
-    # This action might be running off of a fork and would thus not have write
-    # permissions on the origin repository. In order to allow a separate
-    # priviledged action to post a comment on a pull request, upload the
-    # necessary metadata.
-    - name: store comment-data
-      shell: bash
-      if: github.event_name == 'pull_request'
-      env:
-        PULL_REQUEST_NUMBER: ${{ github.event.number }}
+    - name: Format GitHub comment
       run: |
-        mkdir comment-data
-        mv quic-interop-runner/summary comment-data/summary
-        echo $PULL_REQUEST_NUMBER > comment-data/pr-number
-        echo '${{ steps.artifact-upload-step.outputs.artifact-url }}' > comment-data/logs-url
+        echo '[**QUIC Interop Runner**](https://github.com/quic-interop/quic-interop-runner)' >> comment
+        echo '' >> comment
+        echo '```' >> comment
+        cat quic-interop-runner/summary >> comment
+        echo '```' >> comment
+        echo '' >> comment
+      shell: bash
 
-    - name: Upload comment data
-      uses: actions/upload-artifact@v4
-      if: github.event_name == 'pull_request'
+    - name: Export PR comment data
+      if: ${{ github.event_name == 'pull_request' }}
+      uses: ./.github/actions/pr-comment-data-export
       with:
-        name: comment-data
-        path: ./comment-data
+        name: qns
+        contents: comment
+        log-url: ${{ steps.artifact-upload-step.outputs.artifact-url }}
diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml
index 176a80deac..a4b6900a8c 100644
--- a/.github/workflows/bench.yml
+++ b/.github/workflows/bench.yml
@@ -142,10 +142,10 @@ jobs:
           echo '' >> results.md
           echo '[:arrow_down: Download full results](${{ steps.export.outputs.artifact-url }})' >> results.md
 
-      - name: "Post results to PR"
-        uses: thollander/actions-comment-pull-request@v2
+      - name: Export PR comment data
+        if: ${{ github.event_name == 'pull_request' }}
+        uses: ./.github/actions/pr-comment-data-export
         with:
-          filePath: results.md
-          pr_number: ${{ github.event.pull_request.number }}
-          comment_tag: bench-results
-
+          name: bench
+          contents: results.md
+          log-url: ${{ steps.export.outputs.artifact-url }}
diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml
index e96466e2e1..4cfa81ee7f 100644
--- a/.github/workflows/check.yml
+++ b/.github/workflows/check.yml
@@ -152,10 +152,3 @@ jobs:
           fail_ci_if_error: false
           token: ${{ secrets.CODECOV_TOKEN }}
         if: matrix.type == 'debug' && matrix.rust-toolchain == 'stable'
-
-  bench:
-    name: "Benchmark"
-    needs: [check]
-    permissions:
-      pull-requests: write
-    uses: ./.github/workflows/bench.yml
diff --git a/.github/workflows/qns-comment.yml b/.github/workflows/qns-comment.yml
index 8b897b259a..2ceaac1bd4 100644
--- a/.github/workflows/qns-comment.yml
+++ b/.github/workflows/qns-comment.yml
@@ -21,36 +21,6 @@ jobs:
       github.event.workflow_run.event == 'pull_request' &&
       github.event.workflow_run.conclusion == 'failure'
     steps:
-      - name: Download comment-data
-        uses: actions/download-artifact@v4
+      - uses: ./.github/actions/pr-comment
         with:
-          run-id: ${{ github.event.workflow_run.id }}
-          name: comment-data
-          github-token: ${{ secrets.GITHUB_TOKEN }}
-
-      - name: Format GitHub comment
-        run: |
-          pwd
-          ls -la
-          echo '[**QUIC Interop Runner**](https://github.com/quic-interop/quic-interop-runner)' >> comment
-          echo '' >> comment
-          echo '```' >> comment
-          cat summary >> comment
-          echo '```' >> comment
-          echo '' >> comment
-          echo 'Download artifacts [here](' >> comment
-          cat logs-url >> comment
-          echo ').' >> comment
-        shell: bash
-
-      - name: Read PR Number
-        id: pr-number
-        run: echo "::set-output name=number::$(cat pr-number)"
-        shell: bash
-
-      - name: Comment PR
-        uses: thollander/actions-comment-pull-request@v2
-        with:
-          filePath: comment
-          pr_number: ${{ steps.pr-number.outputs.number }}
-          comment_tag: quic-network-simulator-comment
+          name: qns

From 4742a5a1758ea5e7e07b085a385e6c77ffb7095f Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 11 Mar 2024 13:36:57 +0200
Subject: [PATCH 2/4] Test

---
 .github/actions/pr-comment/action.yml | 4 ++++
 .github/workflows/bench.yml           | 1 -
 .github/workflows/qns-comment.yml     | 5 +++--
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/.github/actions/pr-comment/action.yml b/.github/actions/pr-comment/action.yml
index 37cde6838f..ff46d40310 100644
--- a/.github/actions/pr-comment/action.yml
+++ b/.github/actions/pr-comment/action.yml
@@ -18,6 +18,10 @@ runs:
       shell: bash
       run: echo "number=$(cat pr-number)" >> "$GITHUB_OUTPUT"
 
+    - shell: bash
+      run: |
+        [ -s log-url ] && echo "" >> contents && echo "[:arrow_down: Download logs]($(cat log-url))" >> contents
+
     - uses: thollander/actions-comment-pull-request@v2
       with:
         filePath: contents
diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml
index a4b6900a8c..47d9619a0e 100644
--- a/.github/workflows/bench.yml
+++ b/.github/workflows/bench.yml
@@ -140,7 +140,6 @@ jobs:
                     -e 's/(change:[^%]*%)([^%]*%)(.*)/\1**\2**\3/gi' \
               > results.md
           echo '' >> results.md
-          echo '[:arrow_down: Download full results](${{ steps.export.outputs.artifact-url }})' >> results.md
 
       - name: Export PR comment data
         if: ${{ github.event_name == 'pull_request' }}
diff --git a/.github/workflows/qns-comment.yml b/.github/workflows/qns-comment.yml
index 2ceaac1bd4..36632c0a01 100644
--- a/.github/workflows/qns-comment.yml
+++ b/.github/workflows/qns-comment.yml
@@ -18,8 +18,9 @@ jobs:
       pull-requests: write
     runs-on: ubuntu-latest
     if: >
-      github.event.workflow_run.event == 'pull_request' &&
-      github.event.workflow_run.conclusion == 'failure'
+      github.event.workflow_run.event == 'pull_request'
+  # &&
+  # github.event.workflow_run.conclusion == 'failure'
     steps:
       - uses: ./.github/actions/pr-comment
         with:

From 280c27504cb0ae3ea3663bb9589dae4791c883a7 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 11 Mar 2024 13:41:49 +0200
Subject: [PATCH 3/4] Add missing file

---
 .github/workflows/bench-comment.yml | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 .github/workflows/bench-comment.yml

diff --git a/.github/workflows/bench-comment.yml b/.github/workflows/bench-comment.yml
new file mode 100644
index 0000000000..e5e46de887
--- /dev/null
+++ b/.github/workflows/bench-comment.yml
@@ -0,0 +1,24 @@
+# Post test results as pull request comment.
+#
+# This is done as a separate workflow as it requires write permissions. The
+# tests itself might run off of a fork, i.e. an untrusted environment and should
+# thus not be granted write permissions.
+
+name: Benchmark Comment
+
+on:
+  workflow_run:
+    workflows: ["Bench"]
+    types:
+      - completed
+
+jobs:
+  comment:
+    permissions:
+      pull-requests: write
+    runs-on: ubuntu-latest
+    if: github.event.workflow_run.event == 'pull_request'
+    steps:
+      - uses: ./.github/actions/pr-comment
+        with:
+          name: bench

From 62c387b7d4c6a42b640235315eda083e01c5914e Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 11 Mar 2024 13:46:48 +0200
Subject: [PATCH 4/4] Finalize

---
 .github/workflows/qns-comment.yml | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/.github/workflows/qns-comment.yml b/.github/workflows/qns-comment.yml
index 36632c0a01..bb5eeaef00 100644
--- a/.github/workflows/qns-comment.yml
+++ b/.github/workflows/qns-comment.yml
@@ -17,10 +17,7 @@ jobs:
     permissions:
       pull-requests: write
     runs-on: ubuntu-latest
-    if: >
-      github.event.workflow_run.event == 'pull_request'
-  # &&
-  # github.event.workflow_run.conclusion == 'failure'
+    if: github.event.workflow_run.event == 'pull_request'
     steps:
       - uses: ./.github/actions/pr-comment
         with: