From 5fefee34b740c821e6179d8e1f3acc7b2bff920d Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Wed, 23 Nov 2022 15:48:14 +0000 Subject: [PATCH] build: Improve fuzz tests' reliability Establish conventions which aligns with what is supported upstream today, whilst expanding on documentation to ensure folks have pointers on how to debug/check for issues going forwards. Signed-off-by: Paulo Gomes --- Makefile | 4 +- controllers/controllers_fuzzer_test.go | 5 +- .../imageupdateautomation_controller_test.go | 16 ++ tests/fuzz/README.md | 82 +++++++++++ tests/fuzz/compile_native_go_fuzzer | 56 ++----- tests/fuzz/oss_fuzz_build.sh | 137 ++++++------------ tests/fuzz/oss_fuzz_prebuild.sh | 73 ++++++++++ 7 files changed, 226 insertions(+), 147 deletions(-) create mode 100644 tests/fuzz/README.md create mode 100755 tests/fuzz/oss_fuzz_prebuild.sh diff --git a/Makefile b/Makefile index b92b97bf..6ade0a7e 100644 --- a/Makefile +++ b/Makefile @@ -276,9 +276,9 @@ ifneq ($(shell grep -o 'LIBGIT2_IMG ?= \w.*' Makefile | cut -d ' ' -f 3):$(shell exit 1; \ } endif -ifneq ($(shell grep -o 'LIBGIT2_TAG ?= \w.*' Makefile | cut -d ' ' -f 3), $(shell grep -o "LIBGIT2_TAG=.*" tests/fuzz/oss_fuzz_build.sh | sed 's;LIBGIT2_TAG="$${LIBGIT2_TAG:-;;g' | sed 's;}";;g')) +ifneq ($(shell grep -o 'LIBGIT2_TAG ?= \w.*' Makefile | cut -d ' ' -f 3), $(shell grep -o "LIBGIT2_TAG=.*" tests/fuzz/oss_fuzz_prebuild.sh | sed 's;LIBGIT2_TAG="$${LIBGIT2_TAG:-;;g' | sed 's;}";;g')) @{ \ - echo "LIBGIT2_TAG must match in both Makefile and tests/fuzz/oss_fuzz_build.sh"; \ + echo "LIBGIT2_TAG must match in both Makefile and tests/fuzz/oss_fuzz_prebuild.sh"; \ exit 1; \ } endif diff --git a/controllers/controllers_fuzzer_test.go b/controllers/controllers_fuzzer_test.go index 635f653c..50eae8ee 100644 --- a/controllers/controllers_fuzzer_test.go +++ b/controllers/controllers_fuzzer_test.go @@ -1,5 +1,5 @@ -//go:build gofuzz -// +build gofuzz +//go:build gofuzz_libfuzzer +// +build gofuzz_libfuzzer /* Copyright 2021 The Flux authors @@ -32,7 +32,6 @@ import ( "time" fuzz "github.com/AdaLogics/go-fuzz-headers" - "github.com/fluxcd/image-automation-controller/controllers" "github.com/fluxcd/image-automation-controller/pkg/update" "github.com/fluxcd/pkg/gittestserver" "github.com/fluxcd/pkg/runtime/testenv" diff --git a/controllers/imageupdateautomation_controller_test.go b/controllers/imageupdateautomation_controller_test.go index 91400521..09c23057 100644 --- a/controllers/imageupdateautomation_controller_test.go +++ b/controllers/imageupdateautomation_controller_test.go @@ -1,3 +1,19 @@ +/* +Copyright 2022 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 controllers import ( diff --git a/tests/fuzz/README.md b/tests/fuzz/README.md new file mode 100644 index 00000000..98e1d265 --- /dev/null +++ b/tests/fuzz/README.md @@ -0,0 +1,82 @@ +# fuzz testing + +Flux is part of Google's [oss fuzz] program which provides continuous fuzzing for +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]. + +### Testing locally + +Build fuzzers: + +```bash +make fuzz-build +``` +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 +``` + +Run fuzzer locally: +```bash +./build/fuzz/out/fuzz_conditions_match +``` + +Run fuzzer inside a container: + +```bash + docker run --rm -ti \ + -v "$(pwd)/build/fuzz/out":/out \ + gcr.io/oss-fuzz/fluxcd \ + /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/compile_native_go_fuzzer b/tests/fuzz/compile_native_go_fuzzer index 82f3bdfc..447c7477 100755 --- a/tests/fuzz/compile_native_go_fuzzer +++ b/tests/fuzz/compile_native_go_fuzzer @@ -18,27 +18,7 @@ # This is a copy of the upstream script which is only needed to link # additional static libraries. Orignal source: # -# https://github.com/google/oss-fuzz/blob/81326f0a39eadebfa9b7a98aa9f0553707875696/infra/base-images/base-builder/compile_go_fuzzer - -# Rewrites a copy of the fuzzer to allow for -# libFuzzer instrumentation. -function rewrite_go_fuzz_harness() { - fuzzer_filename=$1 - fuzz_function=$2 - - # Create a copy of the fuzzer to not modify the existing fuzzer. - cp $fuzzer_filename "${fuzzer_filename}"_fuzz_.go - mv $fuzzer_filename /tmp/ - fuzzer_fn="${fuzzer_filename}"_fuzz_.go - - # Replace *testing.F with *go118fuzzbuildutils.F. - echo "replacing *testing.F" - sed -i "s/func $fuzz_function(\([a-zA-Z0-9]*\) \*testing\.F)/func $fuzz_function(\1 \*go118fuzzbuildutils\.F)/g" "${fuzzer_fn}" - - # Import https://github.com/AdamKorcz/go-118-fuzz-build. - # This changes the line numbers from the original fuzzer. - addimport -path "${fuzzer_fn}" -} +# https://github.com/google/oss-fuzz/blob/9e8dd47cb902545efc60a5580126adc36d70bae3/infra/base-images/base-builder/compile_native_go_fuzzer function build_native_go_fuzzer() { fuzzer=$1 @@ -46,28 +26,16 @@ function build_native_go_fuzzer() { path=$3 tags="-tags gofuzz" - if [[ $SANITIZER = *coverage* ]]; then - echo "here we perform coverage build" - fuzzed_package=`go list $tags -f '{{.Name}}' $path` - abspath=`go list $tags -f {{.Dir}} $path` - cd $abspath - cp $GOPATH/native_ossfuzz_coverage_runner.go ./"${function,,}"_test.go - sed -i -e 's/FuzzFunction/'$function'/' ./"${function,,}"_test.go - sed -i -e 's/mypackagebeingfuzzed/'$fuzzed_package'/' ./"${function,,}"_test.go - sed -i -e 's/TestFuzzCorpus/Test'$function'Corpus/' ./"${function,,}"_test.go - - # The repo is the module path/name, which is already created above - # in case it doesn't exist, but not always the same as the module - # path. This is necessary to handle SIV properly. - fuzzed_repo=$(go list $tags -f {{.Module}} "$path") - abspath_repo=`go list -m $tags -f {{.Dir}} $fuzzed_repo || go list $tags -f {{.Dir}} $fuzzed_repo` - # give equivalence to absolute paths in another file, as go test -cover uses golangish pkg.Dir - echo "s=$fuzzed_repo"="$abspath_repo"= > $OUT/$fuzzer.gocovpath - go test -run Test${function}Corpus -v $tags -coverpkg $fuzzed_repo/... -c -o $OUT/$fuzzer $path - - rm ./"${function,,}"_test.go + if [[ $SANITIZER == *coverage* ]]; then + current_dir=$(pwd) + mkdir $OUT/rawfuzzers || true + cd $abs_file_dir + go test -c -run $fuzzer -o $OUT/$fuzzer -cover + cp "${fuzzer_filename}" "${OUT}/rawfuzzers/${fuzzer}" + cd $current_dir else go-118-fuzz-build -o $fuzzer.a -func $function $abs_file_dir + # TODO: upstream support for linking $ADDITIONAL_LIBS $CXX $CXXFLAGS $LIB_FUZZING_ENGINE $fuzzer.a -o $OUT/$fuzzer \ $ADDITIONAL_LIBS fi @@ -88,13 +56,7 @@ fuzzer_filename=$(grep -r -l --include='*.go' -s "$function" "${abs_file_dir}") # Test if file contains a line with "func $function" and "testing.F". if [ $(grep -r "func $function" $fuzzer_filename | grep "testing.F" | wc -l) -eq 1 ] then - - rewrite_go_fuzz_harness $fuzzer_filename $function build_native_go_fuzzer $fuzzer $function $abs_file_dir - - # Clean up. - rm "${fuzzer_filename}_fuzz_.go" - mv /tmp/$(basename $fuzzer_filename) $fuzzer_filename else echo "Could not find the function: func ${function}(f *testing.F)" fi diff --git a/tests/fuzz/oss_fuzz_build.sh b/tests/fuzz/oss_fuzz_build.sh index cc243352..153eaea8 100755 --- a/tests/fuzz/oss_fuzz_build.sh +++ b/tests/fuzz/oss_fuzz_build.sh @@ -16,118 +16,65 @@ set -euxo pipefail -LIBGIT2_TAG="${LIBGIT2_TAG:-v0.4.0}" +# This file aims for: +# - Dynamically discover and build all fuzz tests within the repository. +# - Work for both local make fuzz-smoketest and the upstream oss-fuzz. + GOPATH="${GOPATH:-/root/go}" GO_SRC="${GOPATH}/src" PROJECT_PATH="github.com/fluxcd/image-automation-controller" -TMP_DIR=$(mktemp -d /tmp/oss_fuzz-XXXXXX) - -cleanup(){ - rm -rf "${TMP_DIR}" -} -trap cleanup EXIT +# install_deps installs all dependencies needed for upstream oss-fuzz. +# Unfortunately we can't pin versions here, as we want to always +# have the latest, so that we can reproduce errors occuring upstream. install_deps(){ - if ! command -v go-118-fuzz-build &> /dev/null || ! command -v addimport &> /dev/null; then - mkdir -p "${TMP_DIR}/go-118-fuzz-build" - - git clone https://github.com/AdamKorcz/go-118-fuzz-build "${TMP_DIR}/go-118-fuzz-build" - cd "${TMP_DIR}/go-118-fuzz-build" - go build -o "${GOPATH}/bin/go-118-fuzz-build" - - cd addimport - go build -o "${GOPATH}/bin/addimport" - fi - - if ! command -v goimports &> /dev/null; then - go install golang.org/x/tools/cmd/goimports@latest + if ! command -v go-118-fuzz-build &> /dev/null; then + go install github.com/AdamKorcz/go-118-fuzz-build@latest fi } -# Removes the content of test funcs which could cause the Fuzz -# tests to break. -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}" -export TARGET_DIR="$(/bin/pwd)/build/libgit2/${LIBGIT2_TAG}" - -# For most cases, libgit2 will already be present. -# The exception being at the oss-fuzz integration. -if [ ! -d "${TARGET_DIR}" ]; then - curl -o output.tar.gz -LO "https://github.com/fluxcd/golang-with-libgit2/releases/download/${LIBGIT2_TAG}/linux-$(uname -m)-libgit2-only.tar.gz" - - DIR=linux-libgit2-only - NEW_DIR="$(/bin/pwd)/build/libgit2/${LIBGIT2_TAG}" - INSTALLED_DIR="/home/runner/work/golang-with-libgit2/golang-with-libgit2/build/${DIR}" - - mkdir -p ./build/libgit2 - - tar -xf output.tar.gz - rm output.tar.gz - mv "${DIR}" "${LIBGIT2_TAG}" - mv "${LIBGIT2_TAG}/" "./build/libgit2" - - # Update the prefix paths included in the .pc files. - # This will make it easier to update to the location in which they will be used. - find "${NEW_DIR}" -type f -name "*.pc" | xargs -I {} sed -i "s;${INSTALLED_DIR};${NEW_DIR};g" {} +# Ensure any project-specific requirements are catered for ahead of +# the generic build process. +if [ -f "tests/fuzz/oss_fuzz_prebuild.sh" ]; then + . tests/fuzz/oss_fuzz_prebuild.sh fi -go get github.com/AdamKorcz/go-118-fuzz-build/utils - -# Setup files to be embedded into controllers_fuzzer.go's testFiles variable. -mkdir -p controllers/testdata/crd -cp config/crd/bases/*.yaml controllers/testdata/crd - -export CGO_ENABLED=1 -export LIBRARY_PATH="${TARGET_DIR}/lib" -export PKG_CONFIG_PATH="${TARGET_DIR}/lib/pkgconfig" -export CGO_CFLAGS="-I${TARGET_DIR}/include" -export CGO_LDFLAGS="$(pkg-config --libs --static --cflags libgit2)" +modules=$(find . -mindepth 1 -maxdepth 4 -type f -name 'go.mod' | cut -c 3- | sed 's|/[^/]*$$||' | sort -u | sed 's;/go.mod;;g' | sed 's;go.mod;.;g') -# Version of the source-controller from which to get the GitRepository CRD. -# Pulls source-controller/api's version set in go.mod. -SOURCE_VER=$(go list -m github.com/fluxcd/source-controller/api | awk '{print $2}') +for module in ${modules}; do -# Version of the image-reflector-controller from which to get the ImagePolicy CRD. -# Pulls image-reflector-controller/api's version set in go.mod. -REFLECTOR_VER=$(go list -m github.com/fluxcd/image-reflector-controller/api | awk '{print $2}') + cd "${GO_SRC}/${PROJECT_PATH}/${module}" -if [ -d "../../controllers/testdata/crds" ]; then - cp ../../controllers/testdata/crds/*.yaml testdata/crds -else - # Fetch the CRDs if not present since we need them when running fuzz tests on CI. - curl -s --fail https://raw.githubusercontent.com/fluxcd/source-controller/${SOURCE_VER}/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml -o controllers/testdata/crd/gitrepositories.yaml - curl -s --fail https://raw.githubusercontent.com/fluxcd/image-reflector-controller/${REFLECTOR_VER}/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml -o controllers/testdata/crd/imagepolicies.yaml -fi - -export ADDITIONAL_LIBS="${TARGET_DIR}/lib/libgit2.a" - -# Iterate through all Go Fuzz targets, compiling each into a fuzzer. -test_files=$(grep -r --include='**_test.go' --files-with-matches 'func Fuzz' .) -for file in ${test_files} -do - remove_test_funcs "${file}" - - targets=$(grep -oP 'func \K(Fuzz\w*)' "${file}") - for target_name in ${targets} - do - fuzzer_name=$(echo "${target_name}" | tr '[:upper:]' '[:lower:]') - target_dir=$(dirname "${file}") + test_files=$(grep -r --include='**_test.go' --files-with-matches 'func Fuzz' . || echo "") + if [ -z "${test_files}" ]; then + continue + fi - echo "Building ${file}.${target_name} into ${fuzzer_name}" - compile_native_go_fuzzer "${target_dir}" "${target_name}" "${fuzzer_name}" + go get github.com/AdamKorcz/go-118-fuzz-build/testing + + # Iterate through all Go Fuzz targets, compiling each into a fuzzer. + for file in ${test_files}; do + # If the subdir is a module, skip this file, as it will be handled + # at the next iteration of the outer loop. + if [ -f "$(dirname "${file}")/go.mod" ]; then + continue + fi + + 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). + module_name=$(echo ${module} | tr / _) + # Compose fuzzer name based on the lowercase version of the func names. + # The module name is added after the fuzz prefix, for better discoverability. + fuzzer_name=$(echo "${target_name}" | tr '[:upper:]' '[:lower:]' | sed "s;fuzz_;fuzz_${module_name}_;g") + target_dir=$(dirname "${file}") + + echo "Building ${file}.${target_name} into ${fuzzer_name}" + compile_native_go_fuzzer "${target_dir}" "${target_name}" "${fuzzer_name}" + done done done diff --git a/tests/fuzz/oss_fuzz_prebuild.sh b/tests/fuzz/oss_fuzz_prebuild.sh new file mode 100755 index 00000000..c4d66c65 --- /dev/null +++ b/tests/fuzz/oss_fuzz_prebuild.sh @@ -0,0 +1,73 @@ +#!/usr/bin/env bash + +# Copyright 2022 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. + +set -euxo pipefail + +# This file is executed by upstream oss-fuzz for any requirements that +# are specific for building this project. + +# Some tests requires embedded resources. Embedding does not allow +# for traversing into ascending dirs, therefore we copy those contents here: +mkdir -p controllers/testdata/crd +cp config/crd/bases/*.yaml controllers/testdata/crd + +LIBGIT2_TAG="${LIBGIT2_TAG:-v0.4.0}" + +export TARGET_DIR="$(/bin/pwd)/build/libgit2/${LIBGIT2_TAG}" +export CGO_ENABLED=1 +export LIBRARY_PATH="${TARGET_DIR}/lib" +export PKG_CONFIG_PATH="${TARGET_DIR}/lib/pkgconfig" +export CGO_CFLAGS="-I${TARGET_DIR}/include" +export CGO_LDFLAGS="$(pkg-config --libs --static --cflags libgit2)" + +# For most cases, libgit2 will already be present. +# The exception being at the oss-fuzz integration. +if [ ! -d "${TARGET_DIR}" ]; then + curl -o output.tar.gz -LO "https://github.com/fluxcd/golang-with-libgit2/releases/download/${LIBGIT2_TAG}/linux-$(uname -m)-libgit2-only.tar.gz" + + DIR=linux-libgit2-only + NEW_DIR="$(/bin/pwd)/build/libgit2/${LIBGIT2_TAG}" + INSTALLED_DIR="/home/runner/work/golang-with-libgit2/golang-with-libgit2/build/${DIR}" + + mkdir -p ./build/libgit2 + + tar -xf output.tar.gz + rm output.tar.gz + mv "${DIR}" "${LIBGIT2_TAG}" + mv "${LIBGIT2_TAG}/" "./build/libgit2" + + # Update the prefix paths included in the .pc files. + # This will make it easier to update to the location in which they will be used. + find "${NEW_DIR}" -type f -name "*.pc" | xargs -I {} sed -i "s;${INSTALLED_DIR};${NEW_DIR};g" {} +fi + +# Version of the source-controller from which to get the GitRepository CRD. +# Pulls source-controller/api's version set in go.mod. +SOURCE_VER=$(go list -m github.com/fluxcd/source-controller/api | awk '{print $2}') + +# Version of the image-reflector-controller from which to get the ImagePolicy CRD. +# Pulls image-reflector-controller/api's version set in go.mod. +REFLECTOR_VER=$(go list -m github.com/fluxcd/image-reflector-controller/api | awk '{print $2}') + +if [ -d "../../controllers/testdata/crds" ]; then + cp ../../controllers/testdata/crds/*.yaml testdata/crds +else + # Fetch the CRDs if not present since we need them when running fuzz tests on CI. + curl -s --fail https://raw.githubusercontent.com/fluxcd/source-controller/${SOURCE_VER}/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml -o controllers/testdata/crd/gitrepositories.yaml + curl -s --fail https://raw.githubusercontent.com/fluxcd/image-reflector-controller/${REFLECTOR_VER}/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml -o controllers/testdata/crd/imagepolicies.yaml +fi + +export ADDITIONAL_LIBS="${TARGET_DIR}/lib/libgit2.a"