fuzz: Refactor Fuzz tests based on Go native fuzzing.

Moving into Go Native, the adhoc changes and on-demand build is no
longer necessary.

Previously calls to r.EventRecorder.AnnotatedEventf resulted in panic.
The new dummy recorder resolves the problem without impacting
resource consumption.

A new make target `fuzz-native` was introduced, to loop through all
fuzz tests for the duration of time specified via the environment
variable `FUZZ_TIME`.

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

Author: Paulo Gomes

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,5 +1,10 @@
+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