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