Branch protection WAR: #605 Reprise

NVIDIA · May 3, 2024 · 27b79af · 27b79af
1 parent 1bb4b03
commit 27b79af
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 25 deletions.
diff --git a/.github/workflows/ci-workflow-nightly.yml b/.github/workflows/ci-workflow-nightly.yml
@@ -61,10 +61,11 @@ jobs:
       name: ${{ matrix.name }}
       jobs: ${{ toJSON(fromJSON(needs.build-workflow.outputs.workflow)[matrix.name]) }}
 
-  # Check all other job statuses. This job gates branch protection checks.
-  ci:
-    name: CI
-    if: ${{ always() || !cancelled() }}
+  # This job acts as a sentry and will fail if any leaf job in the workflow tree fails, as
+  # run-workflow always succeeds. Use this job when checking for successful matrix workflow job completion.
+  verify-workflow:
+    name: Verify and summarize workflow results
+    if: ${{ always() && !cancelled() }}
     needs:
       - build-workflow
       - run-workflow
@@ -78,9 +79,19 @@ jobs:
       - name: Check workflow success
         id: check-workflow
         uses: ./.github/actions/workflow-results
-        with:
-          github_token: ${{ secrets.GITHUB_TOKEN }}
 
+  # Check all other job statuses. This job gates branch protection checks.
+  ci:
+    name: CI
+    # !! Important: This job is used for branch protection checks.
+    # !! Need to use always() instead of !cancelled() because skipped jobs count as success
+    # !! for Github branch protection checks. Yes, really: by default, branch protections
+    # !! can be bypassed by cancelling CI. See NVIDIA/cccl#605.
+    if: ${{ always() }}
+    needs:
+      - verify-workflow
+    runs-on: ubuntu-latest
+    steps:
       - name: Check results
         run: |
           status="passed"
@@ -98,10 +109,8 @@ jobs:
             fi
           }
 
-          # Note that run-workflow is different:
-          check_result "build-workflow" "success" "${{needs.build-workflow.result}}"
-          check_result "run-workflow" "true" "${{steps.check-workflow.outputs.success}}"
+          check_result "verify-workflow"      "success" "${{needs.verify-workflow.result}}"
 
-          if [[ "$status" == "failed" ]]; then
+          if [[ "$status" != "success" ]]; then
             exit 1
           fi
diff --git a/.github/workflows/ci-workflow-pull-request.yml b/.github/workflows/ci-workflow-pull-request.yml
@@ -73,21 +73,14 @@ jobs:
       name: ${{ matrix.name }}
       jobs: ${{ toJSON(fromJSON(needs.build-workflow.outputs.workflow)[matrix.name]) }}
 
-  verify-devcontainers:
-    name: Verify Dev Containers
-    permissions:
-      id-token: write
-      contents: read
-    uses: ./.github/workflows/verify-devcontainers.yml
-
-  # Check all other job statuses. This job gates branch protection checks.
-  ci:
-    name: CI
+  # This job acts as a sentry and will fail if any leaf job in the workflow tree fails, as
+  # run-workflow always succeeds. Use this job when checking for successful matrix workflow job completion.
+  verify-workflow:
+    name: Verify and summarize workflow results
     if: ${{ always() && !cancelled() }}
     needs:
       - build-workflow
       - run-workflow
-      - verify-devcontainers
     permissions:
       contents: read
       pull-requests: write # Posts a comment back to the PR.
@@ -107,6 +100,26 @@ jobs:
           github_token: ${{ secrets.GITHUB_TOKEN }}
           pr_number: ${{ fromJSON(steps.get-pr-info.outputs.pr-info).number }}
 
+  verify-devcontainers:
+    name: Verify Dev Containers
+    permissions:
+      id-token: write
+      contents: read
+    uses: ./.github/workflows/verify-devcontainers.yml
+
+  # Check all other job statuses. This job gates branch protection checks.
+  ci:
+    name: CI
+    # !! Important: This job is used for branch protection checks.
+    # !! Need to use always() instead of !cancelled() because skipped jobs count as success
+    # !! for Github branch protection checks. Yes, really: by default, branch protections
+    # !! can be bypassed by cancelling CI. See NVIDIA/cccl#605.
+    if: ${{ always() }}
+    needs:
+      - verify-workflow
+      - verify-devcontainers
+    runs-on: ubuntu-latest
+    steps:
       - name: Check results
         run: |
           status="passed"
@@ -124,11 +137,9 @@ jobs:
             fi
           }
 
-          # Note that run-workflow is different:
-          check_result "build-workflow" "success" "${{needs.build-workflow.result}}"
-          check_result "run-workflow" "true" "${{steps.check-workflow.outputs.success}}"
+          check_result "verify-workflow"      "success" "${{needs.verify-workflow.result}}"
           check_result "verify-devcontainers" "success" "${{needs.verify-devcontainers.result}}"
 
-          if [[ "$status" == "failed" ]]; then
+          if [[ "$status" != "success" ]]; then
             exit 1
           fi