Merge pull request #517 from pjbgf/fix-fuzz

fuzz: Refactor Fuzz tests based on Go native fuzzing

Makefile

@@ -7,6 +7,10 @@ CRD_OPTIONS ?= crd:crdVersions=v1
 REPOSITORY_ROOT := $(shell git rev-parse --show-toplevel)
 BUILD_DIR := $(REPOSITORY_ROOT)/build
 
+# FUZZ_TIME defines the max amount of time, in Go Duration,
+# each fuzzer should run for.
+FUZZ_TIME ?= 1m
+
 # If gobin not set, create one on ./build and add to path.
 ifeq (,$(shell go env GOBIN))
 GOBIN=$(BUILD_DIR)/gobin
@@ -142,7 +146,7 @@ rm -rf $$TMP_DIR ;\
 }
 endef
 
-# Build fuzzers
+# Build fuzzers used by oss-fuzz.
 fuzz-build:
 	rm -rf $(BUILD_DIR)/fuzz/
 	mkdir -p $(BUILD_DIR)/fuzz/out/
@@ -154,10 +158,16 @@ fuzz-build:
 		-v "$(BUILD_DIR)/fuzz/out":/out \
 		local-fuzzing:latest
 
-# Run each fuzzer once to ensure they are working
+# Run each fuzzer once to ensure they will work when executed by oss-fuzz.
 fuzz-smoketest: fuzz-build
 	docker run --rm \
 		-v "$(BUILD_DIR)/fuzz/out":/out \
 		-v "$(REPOSITORY_ROOT)/tests/fuzz/oss_fuzz_run.sh":/runner.sh \
 		local-fuzzing:latest \
 		bash -c "/runner.sh"
+
+# Run fuzz tests for the duration set in FUZZ_TIME.
+fuzz-native: 
+	KUBEBUILDER_ASSETS=$(KUBEBUILDER_ASSETS) \
+	FUZZ_TIME=$(FUZZ_TIME) \
+		./tests/fuzz/native_go_run.sh

controllers/helmrelease_controller_test.go

@@ -34,6 +34,7 @@ import (
 	"sigs.k8s.io/yaml"
 
 	v2 "github.com/fluxcd/helm-controller/api/v2beta1"
+	sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
 )
 
 func TestHelmReleaseReconciler_composeValues(t *testing.T) {
@@ -446,6 +447,208 @@ func TestValuesReferenceValidation(t *testing.T) {
 	}
 }
 
+func FuzzHelmReleaseReconciler_composeValues(f *testing.F) {
+	scheme := testScheme()
+
+	tests := []struct {
+		targetPath   string
+		valuesKey    string
+		hrValues     string
+		createObject bool
+		secretData   []byte
+		configData   string
+	}{
+		{
+			targetPath: "flat",
+			valuesKey:  "custom-values.yaml",
+			secretData: []byte(`flat:
+  nested: value
+nested: value
+`),
+			configData: `flat: value
+nested:
+  configuration: value
+`,
+			hrValues: `
+other: values
+`,
+			createObject: true,
+		},
+		{
+			targetPath: "'flat'",
+			valuesKey:  "custom-values.yaml",
+			secretData: []byte(`flat:
+  nested: value
+nested: value
+`),
+			configData: `flat: value
+nested:
+  configuration: value
+`,
+			hrValues: `
+other: values
+`,
+			createObject: true,
+		},
+		{
+			targetPath: "flat[0]",
+			secretData: []byte(``),
+			configData: `flat: value`,
+			hrValues: `
+other: values
+`,
+			createObject: true,
+		},
+		{
+			secretData: []byte(`flat:
+  nested: value
+nested: value
+`),
+			configData: `flat: value
+nested:
+  configuration: value
+`,
+			hrValues: `
+other: values
+`,
+			createObject: true,
+		},
+		{
+			targetPath: "some-value",
+			hrValues: `
+other: values
+`,
+			createObject: false,
+		},
+	}
+
+	for _, tt := range tests {
+		f.Add(tt.targetPath, tt.valuesKey, tt.hrValues, tt.createObject, tt.secretData, tt.configData)
+	}
+
+	f.Fuzz(func(t *testing.T,
+		targetPath, valuesKey, hrValues string, createObject bool, secretData []byte, configData string) {
+
+		// objectName represents a core Kubernetes name (Secret/ConfigMap) which is validated
+		// upstream, and also validated by us in the OpenAPI-based validation set in
+		// v2.ValuesReference. Therefore a static value here suffices, and instead we just
+		// play with the objects presence/absence.
+		objectName := "values"
+		resources := []runtime.Object{}
+
+		if createObject {
+			resources = append(resources,
+				valuesConfigMap(objectName, map[string]string{valuesKey: configData}),
+				valuesSecret(objectName, map[string][]byte{valuesKey: secretData}),
+			)
+		}
+
+		references := []v2.ValuesReference{
+			{
+				Kind:       "ConfigMap",
+				Name:       objectName,
+				ValuesKey:  valuesKey,
+				TargetPath: targetPath,
+			},
+			{
+				Kind:       "Secret",
+				Name:       objectName,
+				ValuesKey:  valuesKey,
+				TargetPath: targetPath,
+			},
+		}
+
+		c := fake.NewFakeClientWithScheme(scheme, resources...)
+		r := &HelmReleaseReconciler{Client: c}
+		var values *apiextensionsv1.JSON
+		if hrValues != "" {
+			v, _ := yaml.YAMLToJSON([]byte(hrValues))
+			values = &apiextensionsv1.JSON{Raw: v}
+		}
+
+		hr := v2.HelmRelease{
+			Spec: v2.HelmReleaseSpec{
+				ValuesFrom: references,
+				Values:     values,
+			},
+		}
+
+		// OpenAPI-based validation on schema is not verified here.
+		// Therefore some false positives may be arise, as the apiserver
+		// would not allow such values to make their way into the control plane.
+		//
+		// Testenv could be used so the fuzzing covers the entire E2E.
+		// The downsize being the resource and time cost per test would be a lot higher.
+		//
+		// Another approach could be to add validation to reject invalid inputs before
+		// the r.composeValues call.
+		_, _ = r.composeValues(logr.NewContext(context.TODO(), logr.Discard()), hr)
+	})
+}
+
+func FuzzHelmReleaseReconciler_reconcile(f *testing.F) {
+	scheme := testScheme()
+	tests := []struct {
+		valuesKey  string
+		hrValues   string
+		secretData []byte
+		configData string
+	}{
+		{
+			valuesKey: "custom-values.yaml",
+			secretData: []byte(`flat:
+  nested: value
+nested: value
+`),
+			configData: `flat: value
+nested:
+  configuration: value
+`,
+			hrValues: `
+other: values
+`,
+		},
+	}
+
+	for _, tt := range tests {
+		f.Add(tt.valuesKey, tt.hrValues, tt.secretData, tt.configData)
+	}
+
+	f.Fuzz(func(t *testing.T,
+		valuesKey, hrValues string, secretData []byte, configData string) {
+
+		var values *apiextensionsv1.JSON
+		if hrValues != "" {
+			v, _ := yaml.YAMLToJSON([]byte(hrValues))
+			values = &apiextensionsv1.JSON{Raw: v}
+		}
+
+		hr := v2.HelmRelease{
+			Spec: v2.HelmReleaseSpec{
+				Values: values,
+			},
+		}
+
+		hc := sourcev1.HelmChart{}
+		hc.ObjectMeta.Name = hr.GetHelmChartName()
+		hc.ObjectMeta.Namespace = hr.Spec.Chart.GetNamespace(hr.Namespace)
+
+		resources := []runtime.Object{
+			valuesConfigMap("values", map[string]string{valuesKey: configData}),
+			valuesSecret("values", map[string][]byte{valuesKey: secretData}),
+			&hc,
+		}
+
+		c := fake.NewFakeClientWithScheme(scheme, resources...)
+		r := &HelmReleaseReconciler{
+			Client:        c,
+			EventRecorder: &DummyRecorder{},
+		}
+
+		_, _, _ = r.reconcile(logr.NewContext(context.TODO(), logr.Discard()), hr)
+	})
+}
+
 func valuesSecret(name string, data map[string][]byte) *corev1.Secret {
 	return &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{Name: name},
@@ -459,3 +662,24 @@ func valuesConfigMap(name string, data map[string]string) *corev1.ConfigMap {
 		Data:       data,
 	}
 }
+
+func testScheme() *runtime.Scheme {
+	scheme := runtime.NewScheme()
+	_ = corev1.AddToScheme(scheme)
+	_ = v2.AddToScheme(scheme)
+	_ = sourcev1.AddToScheme(scheme)
+	return scheme
+}
+
+// DummyRecorder serves as a dummy for kuberecorder.EventRecorder.
+type DummyRecorder struct{}
+
+func (r *DummyRecorder) Event(object runtime.Object, eventtype, reason, message string) {
+}
+
+func (r *DummyRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) {
+}
+
+func (r *DummyRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string,
+	eventtype, reason string, messageFmt string, args ...interface{}) {
+}

tests/fuzz/Dockerfile.builder

@@ -1,6 +1,16 @@
+FROM golang:1.18 AS go
+
 FROM gcr.io/oss-fuzz-base/base-builder-go
 
+# ensures golang 1.18 to enable go native fuzzing.
+COPY --from=go /usr/local/go /usr/local/
+
 COPY ./ $GOPATH/src/github.com/fluxcd/helm-controller/
 COPY ./tests/fuzz/oss_fuzz_build.sh $SRC/build.sh
 
+# Temporarily overrides compile_native_go_fuzzer.
+# Pending upstream merge: https://github.com/google/oss-fuzz/pull/8285
+COPY tests/fuzz/compile_native_go_fuzzer.sh /usr/local/bin/compile_native_go_fuzzer
+RUN go install golang.org/x/tools/cmd/goimports@latest
+
 WORKDIR $SRC

tests/fuzz/compile_native_go_fuzzer.sh

@@ -0,0 +1,102 @@
+#!/bin/bash -eu
+# Copyright 2022 Google LLC
+#
+# 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.
+#
+################################################################################
+
+# 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
+
+	# Remove the body of go testing funcs that may be co-located.
+	echo "removing *testing.T"
+	sed -i -e '/testing.T) {$/ {:r;/\n}/!{N;br}; s/\n.*\n/\n/}' "${fuzzer_fn}"
+	# After removing the body of the go testing funcs, consolidate the imports.
+	if command -v goimports; then
+		goimports -w "${fuzzer_fn}"
+	fi
+
+	# 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}"
+}
+
+function build_native_go_fuzzer() {
+	fuzzer=$1
+	function=$2
+	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
+		gotip test -run Test${function}Corpus -v $tags -coverpkg $fuzzed_repo/... -c -o $OUT/$fuzzer $path
+
+		rm ./"${function,,}"_test.go
+	else
+		go-118-fuzz-build -o $fuzzer.a -func $function $abs_file_dir
+		$CXX $CXXFLAGS $LIB_FUZZING_ENGINE $fuzzer.a -o $OUT/$fuzzer
+	fi
+}
+
+
+path=$1
+function=$2
+fuzzer=$3
+tags="-tags gofuzz"
+
+# Get absolute path.
+abs_file_dir=$(go list $tags -f {{.Dir}} $path)
+
+# TODO(adamkorcz): Get rid of "-r" flag here.
+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