From 26e8c0746d931b11891ff31a477da38998102937 Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Thu, 28 Mar 2024 11:23:05 +0000 Subject: [PATCH] Consolidate and fix v1beta1 fuzzer funcs Firstly this change consolidates common fuzzer funcs for fuzzing v1beta1 between v1alpha6 and v1alpha7. Secondly, we fix a couple of bugs where we were generating invalid output: In OpenStackClusterSpec we were creating a second subnet with FuzzNoCustom, which doesn't use our custom functions for generating valid output. When generating filters we were appending a second tag after validating tags, which meant we occasionally got invalid tags. We now add tags before validation, and also add tags to all tag fields instead of just 'Tags'. We also consolidate tag validation in a FilterByNeutronTags func instead of individually for each Filter. --- api/v1alpha6/conversion_test.go | 91 ++------------------------ api/v1alpha7/conversion_test.go | 111 ++------------------------------ test/helpers/fuzzerfuncs.go | 104 ++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 194 deletions(-) create mode 100644 test/helpers/fuzzerfuncs.go diff --git a/api/v1alpha6/conversion_test.go b/api/v1alpha6/conversion_test.go index 3e5484cafd..a5cacce26b 100644 --- a/api/v1alpha6/conversion_test.go +++ b/api/v1alpha6/conversion_test.go @@ -17,7 +17,7 @@ limitations under the License. package v1alpha6 import ( - "strings" + "slices" "testing" "github.com/google/go-cmp/cmp" @@ -57,19 +57,8 @@ func TestFuzzyConversion(t *testing.T) { delete(obj.GetAnnotations(), utilconversion.DataAnnotation) } - filterInvalidTags := func(tags []infrav1.NeutronTag) []infrav1.NeutronTag { - var ret []infrav1.NeutronTag - for i := range tags { - s := string(tags[i]) - if len(s) > 0 && !strings.Contains(s, ",") { - ret = append(ret, tags[i]) - } - } - return ret - } - fuzzerFuncs := func(_ runtimeserializer.CodecFactory) []interface{} { - return []interface{}{ + v1alpha6FuzzerFuncs := []interface{}{ func(instance *Instance, c fuzz.Continue) { c.FuzzNoCustom(instance) @@ -102,19 +91,6 @@ func TestFuzzyConversion(t *testing.T) { } }, - func(spec *infrav1.OpenStackClusterSpec, c fuzz.Continue) { - c.FuzzNoCustom(spec) - - // The fuzzer only seems to generate Subnets of - // length 1, but we need to also test length 2. - // Ensure it is occasionally generated. - if len(spec.Subnets) == 1 && c.RandBool() { - subnet := infrav1.SubnetFilter{} - c.FuzzNoCustom(&subnet) - spec.Subnets = append(spec.Subnets, subnet) - } - }, - func(spec *OpenStackMachineSpec, c fuzz.Continue) { c.FuzzNoCustom(spec) @@ -145,68 +121,9 @@ func TestFuzzyConversion(t *testing.T) { } } }, - - func(spec *infrav1.SubnetSpec, c fuzz.Continue) { - c.FuzzNoCustom(spec) - - // CIDR is required and API validates that it's present, so - // we force it to always be set. - for spec.CIDR == "" { - spec.CIDR = c.RandString() - } - }, - - func(pool *infrav1.AllocationPool, c fuzz.Continue) { - c.FuzzNoCustom(pool) - - // Start and End are required properties, let's make sure both are set - for pool.Start == "" { - pool.Start = c.RandString() - } - - for pool.End == "" { - pool.End = c.RandString() - } - }, - - // v1beta1 filter tags cannot contain commas and can't be empty. - - func(filter *infrav1.SubnetFilter, c fuzz.Continue) { - c.FuzzNoCustom(filter) - - filter.Tags = filterInvalidTags(filter.Tags) - filter.TagsAny = filterInvalidTags(filter.TagsAny) - filter.NotTags = filterInvalidTags(filter.NotTags) - filter.NotTagsAny = filterInvalidTags(filter.NotTagsAny) - }, - - func(filter *infrav1.NetworkFilter, c fuzz.Continue) { - c.FuzzNoCustom(filter) - - filter.Tags = filterInvalidTags(filter.Tags) - filter.TagsAny = filterInvalidTags(filter.TagsAny) - filter.NotTags = filterInvalidTags(filter.NotTags) - filter.NotTagsAny = filterInvalidTags(filter.NotTagsAny) - }, - - func(filter *infrav1.RouterFilter, c fuzz.Continue) { - c.FuzzNoCustom(filter) - - filter.Tags = filterInvalidTags(filter.Tags) - filter.TagsAny = filterInvalidTags(filter.TagsAny) - filter.NotTags = filterInvalidTags(filter.NotTags) - filter.NotTagsAny = filterInvalidTags(filter.NotTagsAny) - }, - - func(filter *infrav1.SecurityGroupFilter, c fuzz.Continue) { - c.FuzzNoCustom(filter) - - filter.Tags = filterInvalidTags(filter.Tags) - filter.TagsAny = filterInvalidTags(filter.TagsAny) - filter.NotTags = filterInvalidTags(filter.NotTags) - filter.NotTagsAny = filterInvalidTags(filter.NotTagsAny) - }, } + + return slices.Concat(v1alpha6FuzzerFuncs, testhelpers.InfraV1FuzzerFuncs()) } t.Run("for OpenStackCluster", runParallel(utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{ diff --git a/api/v1alpha7/conversion_test.go b/api/v1alpha7/conversion_test.go index a2c770984f..57364304b4 100644 --- a/api/v1alpha7/conversion_test.go +++ b/api/v1alpha7/conversion_test.go @@ -17,7 +17,7 @@ limitations under the License. package v1alpha7 import ( - "strings" + "slices" "testing" "github.com/google/go-cmp/cmp" @@ -57,32 +57,8 @@ func TestFuzzyConversion(t *testing.T) { delete(obj.GetAnnotations(), utilconversion.DataAnnotation) } - filterInvalidTags := func(tags []infrav1.NeutronTag) []infrav1.NeutronTag { - var ret []infrav1.NeutronTag - for i := range tags { - s := string(tags[i]) - if len(s) > 0 && !strings.Contains(s, ",") { - ret = append(ret, tags[i]) - } - } - return ret - } - fuzzerFuncs := func(_ runtimeserializer.CodecFactory) []interface{} { - return []interface{}{ - func(spec *infrav1.OpenStackClusterSpec, c fuzz.Continue) { - c.FuzzNoCustom(spec) - - // The fuzzer only seems to generate Subnets of - // length 1, but we need to also test length 2. - // Ensure it is occasionally generated. - if len(spec.Subnets) == 1 && c.RandBool() { - subnet := infrav1.SubnetFilter{} - c.FuzzNoCustom(&subnet) - spec.Subnets = append(spec.Subnets, subnet) - } - }, - + v1alpha7FuzzerFuncs := []interface{}{ func(spec *OpenStackMachineSpec, c fuzz.Continue) { c.FuzzNoCustom(spec) @@ -113,88 +89,9 @@ func TestFuzzyConversion(t *testing.T) { } } }, - - func(spec *infrav1.SubnetSpec, c fuzz.Continue) { - c.FuzzNoCustom(spec) - - // CIDR is required and API validates that it's present, so - // we force it to always be set. - for spec.CIDR == "" { - spec.CIDR = c.RandString() - } - }, - - func(pool *infrav1.AllocationPool, c fuzz.Continue) { - c.FuzzNoCustom(pool) - - // Start and End are required properties, let's make sure both are set - for pool.Start == "" { - pool.Start = c.RandString() - } - - for pool.End == "" { - pool.End = c.RandString() - } - }, - - // v1beta1 filter tags cannot contain commas and can't be empty. - - func(filter *infrav1.SubnetFilter, c fuzz.Continue) { - c.FuzzNoCustom(filter) - - // Sometimes add an additional tag to ensure we get test coverage of multiple tags - if c.RandBool() { - filter.Tags = append(filter.Tags, infrav1.NeutronTag(c.RandString())) - } - - filter.Tags = filterInvalidTags(filter.Tags) - filter.TagsAny = filterInvalidTags(filter.TagsAny) - filter.NotTags = filterInvalidTags(filter.NotTags) - filter.NotTagsAny = filterInvalidTags(filter.NotTagsAny) - }, - - func(filter *infrav1.NetworkFilter, c fuzz.Continue) { - c.FuzzNoCustom(filter) - - // Sometimes add an additional tag to ensure we get test coverage of multiple tags - if c.RandBool() { - filter.Tags = append(filter.Tags, infrav1.NeutronTag(c.RandString())) - } - - filter.Tags = filterInvalidTags(filter.Tags) - filter.TagsAny = filterInvalidTags(filter.TagsAny) - filter.NotTags = filterInvalidTags(filter.NotTags) - filter.NotTagsAny = filterInvalidTags(filter.NotTagsAny) - }, - - func(filter *infrav1.RouterFilter, c fuzz.Continue) { - c.FuzzNoCustom(filter) - - // Sometimes add an additional tag to ensure we get test coverage of multiple tags - if c.RandBool() { - filter.Tags = append(filter.Tags, infrav1.NeutronTag(c.RandString())) - } - - filter.Tags = filterInvalidTags(filter.Tags) - filter.TagsAny = filterInvalidTags(filter.TagsAny) - filter.NotTags = filterInvalidTags(filter.NotTags) - filter.NotTagsAny = filterInvalidTags(filter.NotTagsAny) - }, - - func(filter *infrav1.SecurityGroupFilter, c fuzz.Continue) { - c.FuzzNoCustom(filter) - - // Sometimes add an additional tag to ensure we get test coverage of multiple tags - if c.RandBool() { - filter.Tags = append(filter.Tags, infrav1.NeutronTag(c.RandString())) - } - - filter.Tags = filterInvalidTags(filter.Tags) - filter.TagsAny = filterInvalidTags(filter.TagsAny) - filter.NotTags = filterInvalidTags(filter.NotTags) - filter.NotTagsAny = filterInvalidTags(filter.NotTagsAny) - }, } + + return slices.Concat(v1alpha7FuzzerFuncs, testhelpers.InfraV1FuzzerFuncs()) } t.Run("for OpenStackCluster", runParallel(utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{ diff --git a/test/helpers/fuzzerfuncs.go b/test/helpers/fuzzerfuncs.go new file mode 100644 index 0000000000..6657ed3c73 --- /dev/null +++ b/test/helpers/fuzzerfuncs.go @@ -0,0 +1,104 @@ +/* +Copyright 2024 The Kubernetes 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 helpers + +import ( + "strings" + + fuzz "github.com/google/gofuzz" + + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" +) + +func filterInvalidTags(tags []infrav1.NeutronTag) []infrav1.NeutronTag { + var ret []infrav1.NeutronTag + for i := range tags { + s := string(tags[i]) + if len(s) > 0 && !strings.Contains(s, ",") { + ret = append(ret, tags[i]) + } + } + return ret +} + +// InfraV1FuzzerFuncs returns fuzzer funcs for v1beta1 OpenStack types which: +// * Constrain the output in ways which are validated by the API server +// * Add additional test coverage where it is not generated by the default fuzzer. +func InfraV1FuzzerFuncs() []interface{} { + return []interface{}{ + func(spec *infrav1.OpenStackClusterSpec, c fuzz.Continue) { + c.FuzzNoCustom(spec) + + // The fuzzer only seems to generate Subnets of + // length 1, but we need to also test length 2. + // Ensure it is occasionally generated. + if len(spec.Subnets) == 1 && c.RandBool() { + subnet := infrav1.SubnetFilter{} + c.Fuzz(&subnet) + spec.Subnets = append(spec.Subnets, subnet) + } + }, + + func(spec *infrav1.SubnetSpec, c fuzz.Continue) { + c.FuzzNoCustom(spec) + + // CIDR is required and API validates that it's present, so + // we force it to always be set. + for spec.CIDR == "" { + spec.CIDR = c.RandString() + } + }, + + func(pool *infrav1.AllocationPool, c fuzz.Continue) { + c.FuzzNoCustom(pool) + + // Start and End are required properties, let's make sure both are set + for pool.Start == "" { + pool.Start = c.RandString() + } + + for pool.End == "" { + pool.End = c.RandString() + } + }, + + // v1beta1 filter tags cannot contain commas and can't be empty. + func(filter *infrav1.FilterByNeutronTags, c fuzz.Continue) { + c.FuzzNoCustom(filter) + + // Sometimes add an additional tag to ensure we get test coverage of multiple tags + if c.RandBool() { + filter.Tags = append(filter.Tags, infrav1.NeutronTag(c.RandString())) + } + if c.RandBool() { + filter.TagsAny = append(filter.TagsAny, infrav1.NeutronTag(c.RandString())) + } + if c.RandBool() { + filter.NotTags = append(filter.NotTags, infrav1.NeutronTag(c.RandString())) + } + if c.RandBool() { + filter.NotTagsAny = append(filter.NotTagsAny, infrav1.NeutronTag(c.RandString())) + } + + // Remove empty tags and tags with commas + filter.Tags = filterInvalidTags(filter.Tags) + filter.TagsAny = filterInvalidTags(filter.TagsAny) + filter.NotTags = filterInvalidTags(filter.NotTags) + filter.NotTagsAny = filterInvalidTags(filter.NotTagsAny) + }, + } +}