build: Improve fuzz tests' reliability

Establish conventions which aligns with what is supported upstream
today, whilst expanding on documentation to ensure other contributors
have pointers on how to debug/check for issues going forwards.

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>

runtime/conditions/patch_fuzz_test.go

@@ -0,0 +1,55 @@
+package conditions
+
+import (
+ "testing"
+
+ fuzz "github.com/AdaLogics/go-fuzz-headers"
+ "github.com/fluxcd/pkg/runtime/conditions/testdata"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func Fuzz_PatchApply(f *testing.F) {
+ f.Fuzz(func(t *testing.T,
+ beforeData, afterData, setterData []byte) {
+
+ before, err := newFake(fuzz.NewConsumer(beforeData))
+ if err != nil {
+ return
+ }
+
+ after, err := newFake(fuzz.NewConsumer(afterData))
+ if err != nil {
+ return
+ }
+
+ patch := NewPatch(before, after)
+ setter, err := newFake(fuzz.NewConsumer(setterData))
+ if err != nil {
+ return
+ }
+
+ _ = patch.Apply(setter)
+ })
+}
+
+func newFake(fc *fuzz.ConsumeFuzzer) (*testdata.Fake, error) {
+ obj := &testdata.Fake{}
+ noOfConditions, err := fc.GetInt()
+ if err != nil {
+ return obj, err
+ }
+
+ maxNoOfConditions := 30
+ conditions := make([]metav1.Condition, 0)
+ for i := 0; i < noOfConditions%maxNoOfConditions; i++ {
+ c := metav1.Condition{}
+ err = fc.GenerateStruct(&c)
+ if err != nil {
+ return obj, err
+ }
+
+ conditions = append(conditions, c)
+ }
+ obj.SetConditions(conditions)
+ return obj, nil
+}

runtime/conditions/patch_test.go

@@ -27,7 +27,6 @@ import (
  "testing"
  "time"
 
- fuzz "github.com/AdaLogics/go-fuzz-headers"
  . "github.com/onsi/gomega"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -290,49 +289,3 @@ func TestApplyDoesNotAlterLastTransitionTime(t *testing.T) {
  g.Expect(err).ToNot(HaveOccurred())
  g.Expect(latest.GetConditions()).To(Equal(after.GetConditions()))
 }
-
-func Fuzz_PatchApply(f *testing.F) {
- f.Fuzz(func(t *testing.T,
- beforeData, afterData, setterData []byte) {
-
- before, err := newFake(fuzz.NewConsumer(beforeData))
- if err != nil {
- return
- }
-
- after, err := newFake(fuzz.NewConsumer(afterData))
- if err != nil {
- return
- }
-
- patch := NewPatch(before, after)
- setter, err := newFake(fuzz.NewConsumer(setterData))
- if err != nil {
- return
- }
-
- _ = patch.Apply(setter)
- })
-}
-
-func newFake(fc *fuzz.ConsumeFuzzer) (*testdata.Fake, error) {
- obj := &testdata.Fake{}
- noOfConditions, err := fc.GetInt()
- if err != nil {
- return obj, err
- }
-
- maxNoOfConditions := 30
- conditions := make([]metav1.Condition, 0)
- for i := 0; i < noOfConditions%maxNoOfConditions; i++ {
- c := metav1.Condition{}
- err = fc.GenerateStruct(&c)
- if err != nil {
- return obj, err
- }
-
- conditions = append(conditions, c)
- }
- obj.SetConditions(conditions)
- return obj, nil
-}

runtime/conditions/unstructured_fuzz_test.go

@@ -0,0 +1,49 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+Copyright 2021 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package conditions
+
+import (
+ "testing"
+
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+func Fuzz_Unstructured(f *testing.F) {
+ f.Add("type", "reason true", "condition message")
+
+ f.Fuzz(func(t *testing.T,
+ ct, reason, message string) {
+
+ cs := []metav1.Condition{{
+ Type: ct,
+ Status: metav1.ConditionUnknown,
+ Reason: reason,
+ Message: message,
+ }}
+
+ u := &unstructured.Unstructured{
+ Object: map[string]interface{}{},
+ }
+ s := UnstructuredSetter(u)
+ s.SetConditions(cs)
+
+ g := UnstructuredGetter(u)
+ _ = g.GetConditions()
+ })
+}

runtime/conditions/unstructured_test.go

@@ -102,27 +102,3 @@ func TestUnstructuredSetConditions(t *testing.T) {
  s.SetConditions(conditions)
  g.Expect(s.GetConditions()).To(Equal(conditions))
 }
-
-func Fuzz_Unstructured(f *testing.F) {
- f.Add("type", "reason true", "condition message")
-
- f.Fuzz(func(t *testing.T,
- ct, reason, message string) {
-
- cs := []metav1.Condition{{
- Type: ct,
- Status: metav1.ConditionUnknown,
- Reason: reason,
- Message: message,
- }}
-
- u := &unstructured.Unstructured{
- Object: map[string]interface{}{},
- }
- s := UnstructuredSetter(u)
- s.SetConditions(cs)
-
- g := UnstructuredGetter(u)
- _ = g.GetConditions()
- })
-}

runtime/events/recorder_fuzzer_test.go

@@ -1,5 +1,5 @@
-//go:build gofuzz
-// +build gofuzz
+//go:build gofuzz_libfuzzer
+// +build gofuzz_libfuzzer
 
 /*
 Copyright 2021 The Flux authors
@@ -54,6 +54,7 @@ const defaultBinVersion = "1.24"
 //
 // TODO: refactor and remove build tag.
 func Fuzz_Eventf(f *testing.F) {
+
  f.Fuzz(func(t *testing.T, data []byte) {
  doOnce.Do(func() {
  if err := ensureDependencies(); err != nil {

tests/fuzz/README.md

@@ -6,8 +6,6 @@ open source projects.
 The long running fuzzing execution is configured in the [oss-fuzz repository].
 Shorter executions are done on a per-PR basis, configured as a [github workflow].
 
-For fuzzers to be called, they must be compiled within [oss_fuzz_build.sh](./oss_fuzz_build.sh).
-
 ### Testing locally
 
 Build fuzzers:
@@ -19,12 +17,12 @@ All fuzzers will be built into `./build/fuzz/out`.
 
 Smoke test fuzzers:
 
+All the fuzzers will be built and executed once, to ensure they are fully functional.
+
 ```bash
 make fuzz-smoketest
 ```
 
-The smoke test runs each fuzzer once to ensure they are fully functional.
-
 Run fuzzer locally:
 ```bash
 ./build/fuzz/out/fuzz_conditions_match
@@ -39,7 +37,46 @@ Run fuzzer inside a container:
  /out/fuzz_conditions_match
 ```
 
+### Caveats of creating oss-fuzz compatible tests
+
+#### Segregate fuzz tests
+
+OSS-Fuzz does not properly support mixed `*_test.go` files, in which there is a combination
+of fuzz and non-fuzz tests. To mitigate this problem, ensure your fuzz tests are not in the
+same file as other Go tests. As a pattern, call your fuzz test files `*_fuzz_test.go`.
+
+#### Build tags to avoid conflicts when running Go tests
+
+Due to the issue above, code duplication will occur when creating fuzz tests that rely on
+helper functions that are shared with other tests. To avoid build issues, add a conditional
+build tag at the top of the `*_fuzz_test.go` file:
+```go
+//go:build gofuzz_libfuzzer
+// +build gofuzz_libfuzzer
+```
+
+The build tag above is set at [go-118-fuzz-build].
+At this point in time we can't pass on specific tags from [compile_native_go_fuzzer].
+
+### Running oss-fuzz locally
+
+The `make fuzz-smoketest` is meant to be an easy way to reproduce errors that may occur
+upstream. If our checks ever run out of sync with upstream, the upstream tests can be
+executed locally with:
+
+```
+git clone --depth 1 https://github.com/google/oss-fuzz
+cd oss-fuzz
+python infra/helper.py build_image fluxcd
+python infra/helper.py build_fuzzers --sanitizer address --architecture x86_64 fluxcd
+python infra/helper.py check_build --sanitizer address --architecture x86_64 fluxcd
+```
+
+For latest info on testing oss-fuzz locally, refer to the [upstream guide].
 
 [oss fuzz]: https://github.com/google/oss-fuzz
 [oss-fuzz repository]: https://github.com/google/oss-fuzz/tree/master/projects/fluxcd
 [github workflow]: .github/workflows/cifuzz.yaml
+[upstream guide]: https://google.github.io/oss-fuzz/getting-started/new-project-guide/#testing-locally
+[go-118-fuzz-build]: https://github.com/AdamKorcz/go-118-fuzz-build/blob/b2031950a318d4f2dcf3ec3e128f904d5cf84623/main.go#L40
+[compile_native_go_fuzzer]: https://github.com/google/oss-fuzz/blob/c2d827cb78529fdc757c9b0b4fea0f1238a54814/infra/base-images/base-builder/compile_native_go_fuzzer#L32

tests/fuzz/oss_fuzz_build.sh

@@ -44,20 +44,6 @@ install_deps(){
  fi
 }
 
-# Removes the content of test funcs which could cause the Fuzz
-# tests to break. This is not supported natively upstream.
-remove_test_funcs(){
- filename=$1
-
- echo "removing co-located *testing.T"
- sed -i -e '/func Test.*testing.T) {$/ {:r;/\n}/!{N;br}; s/\n.*\n/\n/}' "${filename}"
- # Remove gomega reference as it is not used by Fuzz tests.
- sed -i 's;. "github.com/onsi/gomega";;g' "${filename}"
-
- # After removing the body of the go testing funcs, consolidate the imports.
- goimports -w "${filename}"
-}
-
 install_deps
 
 cd "${GO_SRC}/${PROJECT_PATH}"
@@ -68,7 +54,7 @@ for module in ${modules}; do
  cd "${GO_SRC}/${PROJECT_PATH}/${module}"
 
  # TODO: stop ignoring recorder_fuzzer_test.go. Temporary fix for fuzzing building issues.
- test_files=$(grep -r --include='**_test.go' --files-with-matches 'func Fuzz' . | grep -v recorder_fuzzer_test.go || echo "")
+ test_files=$(grep -r --include='**_test.go' --files-with-matches 'func Fuzz' . || echo "")
  if [ -z "${test_files}" ]; then
  continue
  fi
@@ -83,8 +69,6 @@ for module in ${modules}; do
  continue
  fi
 
- remove_test_funcs "${file}"
-
  targets=$(grep -oP 'func \K(Fuzz\w*)' "${file}")
  for target_name in ${targets}; do
  # Transform module path into module name (e.g. git/libgit2 to git_libgit2).