From 253333b8a3212496b03d0cef5805967015fdad12 Mon Sep 17 00:00:00 2001
From: David Anderson <danderson@tailscale.com>
Date: Sat, 11 Feb 2023 17:45:01 -0800
Subject: [PATCH] .github/workflows: support disabling fuzz testing safely

OSS-Fuzz doesn't update their version of Go as quickly as we do, so
we sometimes end up with OSS-Fuzz being unable to build our code for
a few weeks. We don't want CI to be red for that entire time, but
we also don't want to forget to reenable fuzzing when OSS-Fuzz does
start working again.

This change makes two configurations worthy of a CI pass:
 - Fuzzing works, and we expected it to work. This is a normal
   happy state.
 - Fuzzing didn't compile, and we expected it to not compile. This
   is the "OSS-Fuzz temporarily broken" state.

If fuzzing is unexpectedly broken, or unexpectedly not broken, that's
a CI failure because we need to either address a fuzz finding, or
update TS_FUZZ_CURRENTLY_BROKEN to reflect the state of OSS-Fuzz.

Signed-off-by: David Anderson <danderson@tailscale.com>
---
 .github/workflows/test.yml | 56 +++++++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 1b8061741..f2d62a057 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -2,6 +2,20 @@
 # both PRs and merged commits, and for the latter reports failures to slack.
 name: CI
 
+env:
+  # Our fuzz job, powered by OSS-Fuzz, fails periodically because we upgrade to
+  # new Go versions very eagerly. OSS-Fuzz is a little more conservative, and
+  # ends up being unable to compile our code.
+  #
+  # When this happens, we want to disable the fuzz target until OSS-Fuzz catches
+  # up. However, we also don't want to forget to turn it back on when OSS-Fuzz
+  # can once again build our code.
+  #
+  # This variable toggles the fuzz job between two modes:
+  #  - false: we expect fuzzing to be happy, and should report failure if it's not.
+  #  - true: we expect fuzzing is broken, and should report failure if it start working.
+  TS_FUZZ_CURRENTLY_BROKEN: true
+
 on:
   push:
     branches:
@@ -215,17 +229,51 @@ jobs:
         ./tool/go run ./cmd/tsconnect --fast-compression build-pkg
 
   fuzz:
+    # This target periodically breaks (see TS_FUZZ_CURRENTLY_BROKEN at the top
+    # of the file), so it's more complex than usual: the 'build fuzzers' step
+    # might fail, and depending on the value of 'TS_FUZZ_CURRENTLY_BROKEN', that
+    # might or might not be fine. The steps after the build figure out whether
+    # the success/failure is expected, and appropriately pass/fail the job
+    # overall accordingly.
+    #
+    # Practically, this means that all steps after 'build fuzzers' must have an
+    # explicit 'if' condition, because the default condition for steps is
+    # 'success()', meaning "only run this if no previous steps failed".
     if: github.event_name == 'pull_request'
     runs-on: ubuntu-22.04
     steps:
     - name: build fuzzers
       id: build
       uses: google/oss-fuzz/infra/cifuzz/actions/build_fuzzers@master
+      # continue-on-error makes steps.build.conclusion be 'success' even if
+      # steps.build.outcome is 'failure'. This means this step does not
+      # contribute to the job's overall pass/fail evaluation.
+      continue-on-error: true
       with:
-        oss-fuzz-project-name: 'tailscale'
-        dry-run: false
-        language: go
+       oss-fuzz-project-name: 'tailscale'
+       dry-run: false
+       language: go
+    - name: report unexpectedly broken fuzz build
+      if: steps.build.outcome == 'failure' && env.TS_FUZZ_CURRENTLY_BROKEN != 'true'
+      run: |
+        echo "fuzzer build failed, see above for why"
+        echo "if the failure is due to OSS-Fuzz not being on the latest Go yet,"
+        echo "set TS_FUZZ_CURRENTLY_BROKEN=true in .github/workflows/test.yml"
+        echo "to temporarily disable fuzzing until OSS-Fuzz works again."
+        exit 1
+    - name: report unexpectedly working fuzz build
+      if: steps.build.outcome == 'success' && env.TS_FUZZ_CURRENTLY_BROKEN == 'true'
+      run: |
+        echo "fuzzer build succeeded, but we expect it to be broken"
+        echo "please set TS_FUZZ_CURRENTLY_BROKEN=false in .github/workflows/test.yml"
+        echo "to reenable fuzz testing"
+        exit 1
     - name: run fuzzers
+      id: run
+      # Run the fuzzers whenever they're able to build, even if we're going to
+      # report a failure because TS_FUZZ_CURRENTLY_BROKEN is set to the wrong
+      # value.
+      if: steps.build.outcome == 'success'
       uses: google/oss-fuzz/infra/cifuzz/actions/run_fuzzers@master
       with:
         oss-fuzz-project-name: 'tailscale'
@@ -234,7 +282,7 @@ jobs:
         language: go
     - name: upload crash
       uses: actions/upload-artifact@v3
-      if: failure() && steps.build.outcome == 'success'
+      if: steps.run.outcome != 'success' && steps.build.outcome == 'success'
       with:
         name: artifacts
         path: ./out/artifacts