From 4386e9204bbb65ddfd57ea0a90f3cb8070bf31ea Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Wed, 23 Nov 2022 11:59:13 +0000 Subject: [PATCH] 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 --- runtime/conditions/patch_fuzz_test.go | 55 ++++++++++++++++++++ runtime/conditions/patch_test.go | 47 ----------------- runtime/conditions/unstructured_fuzz_test.go | 49 +++++++++++++++++ runtime/conditions/unstructured_test.go | 24 --------- runtime/events/recorder_fuzzer_test.go | 5 +- tests/fuzz/README.md | 45 ++++++++++++++-- tests/fuzz/oss_fuzz_build.sh | 18 +------ 7 files changed, 149 insertions(+), 94 deletions(-) create mode 100644 runtime/conditions/patch_fuzz_test.go create mode 100644 runtime/conditions/unstructured_fuzz_test.go diff --git a/runtime/conditions/patch_fuzz_test.go b/runtime/conditions/patch_fuzz_test.go new file mode 100644 index 00000000..c3b576b0 --- /dev/null +++ b/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 +} diff --git a/runtime/conditions/patch_test.go b/runtime/conditions/patch_test.go index 35374e8c..27e997ad 100644 --- a/runtime/conditions/patch_test.go +++ b/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 -} diff --git a/runtime/conditions/unstructured_fuzz_test.go b/runtime/conditions/unstructured_fuzz_test.go new file mode 100644 index 00000000..b6fbdb07 --- /dev/null +++ b/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() + }) +} diff --git a/runtime/conditions/unstructured_test.go b/runtime/conditions/unstructured_test.go index 072f834a..04442c7e 100644 --- a/runtime/conditions/unstructured_test.go +++ b/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() - }) -} diff --git a/runtime/events/recorder_fuzzer_test.go b/runtime/events/recorder_fuzzer_test.go index bd133e01..50ae7271 100644 --- a/runtime/events/recorder_fuzzer_test.go +++ b/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 { diff --git a/tests/fuzz/README.md b/tests/fuzz/README.md index f2d23396..98e1d265 100644 --- a/tests/fuzz/README.md +++ b/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 \ No newline at end of file diff --git a/tests/fuzz/oss_fuzz_build.sh b/tests/fuzz/oss_fuzz_build.sh index c84a5979..409584c6 100755 --- a/tests/fuzz/oss_fuzz_build.sh +++ b/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).