Add mutations to conversion tests

Add optional "hub after" and "spoke after" mutation functions to the conversion tests to support things like removing fields that were added during the conversion process that will cause the equality check to fail. Signed-off-by: Andy Goldstein <[email protected]>
kubernetes-sigs · Mar 9, 2021 · ae92f28 · ae92f28
1 parent 762d6f1
commit ae92f28
Show file tree

Hide file tree

Showing 5 changed files with 118 additions and 35 deletions.
diff --git a/api/v1alpha3/conversion.go b/api/v1alpha3/conversion.go
@@ -48,6 +48,7 @@ func (dst *Cluster) ConvertFrom(srcRaw conversion.Hub) error {
 		return err
 	}
 
+	// Set the v1alpha3 boolean status field if the v1alpha4 condition was true
 	if conditions.IsTrue(src, v1alpha4.ControlPlaneInitializedCondition) {
 		dst.Status.ControlPlaneInitialized = true
 	}

diff --git a/api/v1alpha3/conversion_test.go b/api/v1alpha3/conversion_test.go
@@ -21,6 +21,8 @@ import (
 
 	fuzz "github.com/google/gofuzz"
 	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
+	"sigs.k8s.io/controller-runtime/pkg/conversion"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer"
@@ -34,10 +36,33 @@ func TestFuzzyConversion(t *testing.T) {
 	g.Expect(AddToScheme(scheme)).To(Succeed())
 	g.Expect(v1alpha4.AddToScheme(scheme)).To(Succeed())
 
-	t.Run("for Cluster", utilconversion.FuzzTestFunc(scheme, &v1alpha4.Cluster{}, &Cluster{}))
-	t.Run("for Machine", utilconversion.FuzzTestFunc(scheme, &v1alpha4.Machine{}, &Machine{}, BootstrapFuzzFuncs))
-	t.Run("for MachineSet", utilconversion.FuzzTestFunc(scheme, &v1alpha4.MachineSet{}, &MachineSet{}, BootstrapFuzzFuncs))
-	t.Run("for MachineDeployment", utilconversion.FuzzTestFunc(scheme, &v1alpha4.MachineDeployment{}, &MachineDeployment{}, BootstrapFuzzFuncs))
+	t.Run("for Cluster", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
+		Scheme:             scheme,
+		Hub:                &v1alpha4.Cluster{},
+		Spoke:              &Cluster{},
+		SpokeAfterMutation: clusterSpokeAfterMutation,
+	}))
+
+	t.Run("for Machine", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
+		Scheme:      scheme,
+		Hub:         &v1alpha4.Machine{},
+		Spoke:       &Machine{},
+		FuzzerFuncs: []fuzzer.FuzzerFuncs{BootstrapFuzzFuncs},
+	}))
+
+	t.Run("for MachineSet", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
+		Scheme:      scheme,
+		Hub:         &v1alpha4.MachineSet{},
+		Spoke:       &MachineSet{},
+		FuzzerFuncs: []fuzzer.FuzzerFuncs{BootstrapFuzzFuncs},
+	}))
+
+	t.Run("for MachineDeployment", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
+		Scheme:      scheme,
+		Hub:         &v1alpha4.MachineDeployment{},
+		Spoke:       &MachineDeployment{},
+		FuzzerFuncs: []fuzzer.FuzzerFuncs{BootstrapFuzzFuncs},
+	}))
 }
 
 func BootstrapFuzzFuncs(_ runtimeserializer.CodecFactory) []interface{} {
@@ -52,3 +77,25 @@ func BootstrapFuzzer(obj *Bootstrap, c fuzz.Continue) {
 	// Bootstrap.Data has been removed in v1alpha4, so setting it to nil in order to avoid v1alpha3 --> v1alpha4 --> v1alpha3 round trip errors.
 	obj.Data = nil
 }
+
+// clusterSpokeAfterMutation modifies the spoke version of the Cluster such that it can pass an equality test in the
+// spoke-hub-spoke conversion scenario.
+func clusterSpokeAfterMutation(c conversion.Convertible) {
+	cluster := c.(*Cluster)
+
+	// Create a temporary 0-length slice using the same underlying array as cluster.Status.Conditions to avoid
+	// allocations.
+	tmp := cluster.Status.Conditions[:0]
+
+	for i := range cluster.Status.Conditions {
+		condition := cluster.Status.Conditions[i]
+
+		// Keep everything that is not ControlPlaneInitializedCondition
+		if condition.Type != ConditionType(v1alpha4.ControlPlaneInitializedCondition) {
+			tmp = append(tmp, condition)
+		}
+	}
+
+	// Point cluster.Status.Conditions and our slice that does not have ControlPlaneInitializedCondition
+	cluster.Status.Conditions = tmp
+}
diff --git a/bootstrap/kubeadm/api/v1alpha3/conversion_test.go b/bootstrap/kubeadm/api/v1alpha3/conversion_test.go
@@ -21,6 +21,7 @@ import (
 
 	fuzz "github.com/google/gofuzz"
 	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
 	runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer"
 
 	"k8s.io/apimachinery/pkg/runtime"
@@ -34,8 +35,17 @@ func TestFuzzyConversion(t *testing.T) {
 	g.Expect(AddToScheme(scheme)).To(Succeed())
 	g.Expect(v1alpha4.AddToScheme(scheme)).To(Succeed())
 
-	t.Run("for KubeadmConfig", utilconversion.FuzzTestFunc(scheme, &v1alpha4.KubeadmConfig{}, &KubeadmConfig{}, KubeadmConfigStatusFuzzFuncs))
-	t.Run("for KubeadmConfigTemplate", utilconversion.FuzzTestFunc(scheme, &v1alpha4.KubeadmConfigTemplate{}, &KubeadmConfigTemplate{}))
+	t.Run("for KubeadmConfig", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
+		Scheme:      scheme,
+		Hub:         &v1alpha4.KubeadmConfig{},
+		Spoke:       &KubeadmConfig{},
+		FuzzerFuncs: []fuzzer.FuzzerFuncs{KubeadmConfigStatusFuzzFuncs},
+	}))
+	t.Run("for KubeadmConfigTemplate", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
+		Scheme: scheme,
+		Hub:    &v1alpha4.KubeadmConfigTemplate{},
+		Spoke:  &KubeadmConfigTemplate{},
+	}))
 }
 
 func KubeadmConfigStatusFuzzFuncs(_ runtimeserializer.CodecFactory) []interface{} {

diff --git a/controlplane/kubeadm/api/v1alpha3/conversion_test.go b/controlplane/kubeadm/api/v1alpha3/conversion_test.go
@@ -21,6 +21,7 @@ import (
 
 	fuzz "github.com/google/gofuzz"
 	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer"
@@ -35,25 +36,29 @@ func TestFuzzyConversion(t *testing.T) {
 	g.Expect(AddToScheme(scheme)).To(Succeed())
 	g.Expect(v1alpha4.AddToScheme(scheme)).To(Succeed())
 
-	t.Run("for KubeadmControlPLane", utilconversion.FuzzTestFunc(
-		scheme, &v1alpha4.KubeadmControlPlane{}, &KubeadmControlPlane{},
-		func(codecs runtimeserializer.CodecFactory) []interface{} {
-			return []interface{}{
-				// This custom function is needed when ConvertTo/ConvertFrom functions
-				// uses the json package to unmarshal the bootstrap token string.
-				//
-				// The Kubeadm v1beta1.BootstrapTokenString type ships with a custom
-				// json string representation, in particular it supplies a customized
-				// UnmarshalJSON function that can return an error if the string
-				// isn't in the correct form.
-				//
-				// This function effectively disables any fuzzing for the token by setting
-				// the values for ID and Secret to working alphanumeric values.
-				func(in *kubeadmv1.BootstrapTokenString, c fuzz.Continue) {
-					in.ID = "abcdef"
-					in.Secret = "abcdef0123456789"
-				},
-			}
+	t.Run("for KubeadmControlPLane", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
+		Scheme: scheme,
+		Hub:    &v1alpha4.KubeadmControlPlane{},
+		Spoke:  &KubeadmControlPlane{},
+		FuzzerFuncs: []fuzzer.FuzzerFuncs{
+			func(codecs runtimeserializer.CodecFactory) []interface{} {
+				return []interface{}{
+					// This custom function is needed when ConvertTo/ConvertFrom functions
+					// uses the json package to unmarshal the bootstrap token string.
+					//
+					// The Kubeadm v1beta1.BootstrapTokenString type ships with a custom
+					// json string representation, in particular it supplies a customized
+					// UnmarshalJSON function that can return an error if the string
+					// isn't in the correct form.
+					//
+					// This function effectively disables any fuzzing for the token by setting
+					// the values for ID and Secret to working alphanumeric values.
+					func(in *kubeadmv1.BootstrapTokenString, c fuzz.Continue) {
+						in.ID = "abcdef"
+						in.Secret = "abcdef0123456789"
+					},
+				}
+			},
 		},
-	))
+	}))
 }
diff --git a/util/conversion/conversion.go b/util/conversion/conversion.go
@@ -136,51 +136,71 @@ func GetFuzzer(scheme *runtime.Scheme, funcs ...fuzzer.FuzzerFuncs) *fuzz.Fuzzer
 	)
 }
 
+type FuzzTestFuncInput struct {
+	Scheme *runtime.Scheme
+
+	Hub conversion.Hub
+	HubAfterMutation func(conversion.Hub)
+
+	Spoke conversion.Convertible
+	SpokeAfterMutation func(convertible conversion.Convertible)
+
+	FuzzerFuncs []fuzzer.FuzzerFuncs
+}
+
 // FuzzTestFunc returns a new testing function to be used in tests to make sure conversions between
 // the Hub version of an object and an older version aren't lossy.
-func FuzzTestFunc(scheme *runtime.Scheme, hub conversion.Hub, dst conversion.Convertible, funcs ...fuzzer.FuzzerFuncs) func(*testing.T) {
+func FuzzTestFunc(input FuzzTestFuncInput) func(*testing.T) {
 	return func(t *testing.T) {
 		t.Run("spoke-hub-spoke", func(t *testing.T) {
 			g := gomega.NewWithT(t)
-			fuzzer := GetFuzzer(scheme, funcs...)
+			fuzzer := GetFuzzer(input.Scheme, input.FuzzerFuncs...)
 
 			for i := 0; i < 10000; i++ {
 				// Create the spoke and fuzz it
-				spokeBefore := dst.DeepCopyObject().(conversion.Convertible)
+				spokeBefore := input.Spoke.DeepCopyObject().(conversion.Convertible)
 				fuzzer.Fuzz(spokeBefore)
 
 				// First convert spoke to hub
-				hubCopy := hub.DeepCopyObject().(conversion.Hub)
+				hubCopy := input.Hub.DeepCopyObject().(conversion.Hub)
 				g.Expect(spokeBefore.ConvertTo(hubCopy)).To(gomega.Succeed())
 
 				// Convert hub back to spoke and check if the resulting spoke is equal to the spoke before the round trip
-				spokeAfter := dst.DeepCopyObject().(conversion.Convertible)
+				spokeAfter := input.Spoke.DeepCopyObject().(conversion.Convertible)
 				g.Expect(spokeAfter.ConvertFrom(hubCopy)).To(gomega.Succeed())
 
 				// Remove data annotation eventually added by ConvertFrom for avoiding data loss in hub-spoke-hub round trips
 				metaAfter := spokeAfter.(metav1.Object)
 				delete(metaAfter.GetAnnotations(), DataAnnotation)
 
+				if input.SpokeAfterMutation != nil {
+					input.SpokeAfterMutation(spokeAfter)
+				}
+
 				g.Expect(apiequality.Semantic.DeepEqual(spokeBefore, spokeAfter)).To(gomega.BeTrue(), cmp.Diff(spokeBefore, spokeAfter))
 			}
 		})
 		t.Run("hub-spoke-hub", func(t *testing.T) {
 			g := gomega.NewWithT(t)
-			fuzzer := GetFuzzer(scheme, funcs...)
+			fuzzer := GetFuzzer(input.Scheme, input.FuzzerFuncs...)
 
 			for i := 0; i < 10000; i++ {
 				// Create the hub and fuzz it
-				hubBefore := hub.DeepCopyObject().(conversion.Hub)
+				hubBefore := input.Hub.DeepCopyObject().(conversion.Hub)
 				fuzzer.Fuzz(hubBefore)
 
 				// First convert hub to spoke
-				dstCopy := dst.DeepCopyObject().(conversion.Convertible)
+				dstCopy := input.Spoke.DeepCopyObject().(conversion.Convertible)
 				g.Expect(dstCopy.ConvertFrom(hubBefore)).To(gomega.Succeed())
 
 				// Convert spoke back to hub and check if the resulting hub is equal to the hub before the round trip
-				hubAfter := hub.DeepCopyObject().(conversion.Hub)
+				hubAfter := input.Hub.DeepCopyObject().(conversion.Hub)
 				g.Expect(dstCopy.ConvertTo(hubAfter)).To(gomega.Succeed())
 
+				if input.HubAfterMutation != nil {
+					input.HubAfterMutation(hubAfter)
+				}
+
 				g.Expect(apiequality.Semantic.DeepEqual(hubBefore, hubAfter)).To(gomega.BeTrue(), cmp.Diff(hubBefore, hubAfter))
 			}
 		})