Merge pull request #4452 from randomvariable/automated-cherry-pick-of…

…-#4448-release-0.3 🐛 Adds a test helper to all mutating webhooks to ensure defaulting passes validation (0.3 backport)
kubernetes-sigs · Apr 9, 2021 · c4082b2 · c4082b2
2 parents 4232d5d + 03f1869
commit c4082b2
Show file tree

Hide file tree

Showing 10 changed files with 102 additions and 16 deletions.
diff --git a/api/v1alpha3/cluster_webhook_test.go b/api/v1alpha3/cluster_webhook_test.go
@@ -23,6 +23,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	utildefaulting "sigs.k8s.io/cluster-api/util/defaulting"
 )
 
 func TestClusterDefault(t *testing.T) {
@@ -37,6 +38,8 @@ func TestClusterDefault(t *testing.T) {
 			ControlPlaneRef:   &corev1.ObjectReference{},
 		},
 	}
+
+	t.Run("for Cluster", utildefaulting.DefaultValidateTest(c))
 	c.Default()
 
 	g.Expect(c.Spec.InfrastructureRef.Namespace).To(Equal(c.Namespace))

diff --git a/api/v1alpha3/machine_webhook_test.go b/api/v1alpha3/machine_webhook_test.go
@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	. "github.com/onsi/gomega"
+	utildefaulting "sigs.k8s.io/cluster-api/util/defaulting"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -38,7 +39,7 @@ func TestMachineDefault(t *testing.T) {
 			Version:   pointer.StringPtr("1.17.5"),
 		},
 	}
-
+	t.Run("for Machine", utildefaulting.DefaultValidateTest(m))
 	m.Default()
 
 	g.Expect(m.Labels[ClusterLabelName]).To(Equal(m.Spec.ClusterName))

diff --git a/api/v1alpha3/machinedeployment_webhook_test.go b/api/v1alpha3/machinedeployment_webhook_test.go
@@ -23,6 +23,7 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/pointer"
+	utildefaulting "sigs.k8s.io/cluster-api/util/defaulting"
 )
 
 func TestMachineDeploymentDefault(t *testing.T) {
@@ -32,7 +33,7 @@ func TestMachineDeploymentDefault(t *testing.T) {
 			Name: "test-md",
 		},
 	}
-
+	t.Run("for MachineDeployment", utildefaulting.DefaultValidateTest(md))
 	md.Default()
 
 	g.Expect(md.Labels[ClusterLabelName]).To(Equal(md.Spec.ClusterName))

diff --git a/api/v1alpha3/machinehealthcheck_webhook_test.go b/api/v1alpha3/machinehealthcheck_webhook_test.go
@@ -24,12 +24,19 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	utildefaulting "sigs.k8s.io/cluster-api/util/defaulting"
 )
 
 func TestMachineHealthCheckDefault(t *testing.T) {
 	g := NewWithT(t)
-	mhc := &MachineHealthCheck{}
-
+	mhc := &MachineHealthCheck{
+		Spec: MachineHealthCheckSpec{
+			Selector: metav1.LabelSelector{
+				MatchLabels: map[string]string{"foo": "bar"},
+			},
+		},
+	}
+	t.Run("for MachineHealthCheck", utildefaulting.DefaultValidateTest(mhc))
 	mhc.Default()
 
 	g.Expect(mhc.Labels[ClusterLabelName]).To(Equal(mhc.Spec.ClusterName))

diff --git a/api/v1alpha3/machineset_webhook_test.go b/api/v1alpha3/machineset_webhook_test.go
@@ -22,24 +22,23 @@ import (
 	. "github.com/onsi/gomega"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/utils/pointer"
+	utildefaulting "sigs.k8s.io/cluster-api/util/defaulting"
 )
 
 func TestMachineSetDefault(t *testing.T) {
 	g := NewWithT(t)
-	md := &MachineSet{
+	ms := &MachineSet{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "test-ms",
 		},
 	}
+	t.Run("for MachineSet", utildefaulting.DefaultValidateTest(ms))
+	ms.Default()
 
-	md.Default()
-
-	g.Expect(md.Labels[ClusterLabelName]).To(Equal(md.Spec.ClusterName))
-	g.Expect(md.Spec.Replicas).To(Equal(pointer.Int32Ptr(1)))
-	g.Expect(md.Spec.DeletePolicy).To(Equal(string(RandomMachineSetDeletePolicy)))
-	g.Expect(md.Spec.Selector.MatchLabels).To(HaveKeyWithValue(MachineSetLabelName, "test-ms"))
-	g.Expect(md.Spec.Template.Labels).To(HaveKeyWithValue(MachineSetLabelName, "test-ms"))
+	g.Expect(ms.Labels[ClusterLabelName]).To(Equal(ms.Spec.ClusterName))
+	g.Expect(ms.Spec.DeletePolicy).To(Equal(string(RandomMachineSetDeletePolicy)))
+	g.Expect(ms.Spec.Selector.MatchLabels).To(HaveKeyWithValue(MachineSetLabelName, "test-ms"))
+	g.Expect(ms.Spec.Template.Labels).To(HaveKeyWithValue(MachineSetLabelName, "test-ms"))
 }
 
 func TestMachineSetLabelSelectorMatchValidation(t *testing.T) {

diff --git a/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook_test.go b/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook_test.go
@@ -28,6 +28,7 @@ import (
 	"k8s.io/utils/pointer"
 	bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha3"
 	kubeadmv1beta1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1"
+	utildefaulting "sigs.k8s.io/cluster-api/util/defaulting"
 )
 
 func TestKubeadmControlPlaneDefault(t *testing.T) {
@@ -43,6 +44,12 @@ func TestKubeadmControlPlaneDefault(t *testing.T) {
 			RolloutStrategy:        &RolloutStrategy{},
 		},
 	}
+	updateDefaultingValidationKCP := kcp.DeepCopy()
+	updateDefaultingValidationKCP.Spec.Version = "v1.18.3"
+	updateDefaultingValidationKCP.Spec.InfrastructureTemplate = corev1.ObjectReference{
+		Namespace: "foo",
+	}
+	t.Run("for KubeadmControlPLane", utildefaulting.DefaultValidateTest(updateDefaultingValidationKCP))
 	kcp.Default()
 
 	g.Expect(kcp.Spec.InfrastructureTemplate.Namespace).To(Equal(kcp.Namespace))

diff --git a/exp/addons/api/v1alpha3/clusterresourceset_webhook.go b/exp/addons/api/v1alpha3/clusterresourceset_webhook.go
@@ -87,7 +87,7 @@ func (m *ClusterResourceSet) validate(old *ClusterResourceSet) error {
 		)
 	}
 
-	if old != nil && old.Spec.Strategy != m.Spec.Strategy {
+	if old != nil && old.Spec.Strategy != "" && old.Spec.Strategy != m.Spec.Strategy {
 		allErrs = append(
 			allErrs,
 			field.Invalid(field.NewPath("spec", "strategy"), m.Spec.Strategy, "field is immutable"),

diff --git a/exp/addons/api/v1alpha3/clusterresourceset_webhook_test.go b/exp/addons/api/v1alpha3/clusterresourceset_webhook_test.go
@@ -22,12 +22,17 @@ import (
 	. "github.com/onsi/gomega"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	utildefaulting "sigs.k8s.io/cluster-api/util/defaulting"
 )
 
 func TestClusterResourcesetDefault(t *testing.T) {
 	g := NewWithT(t)
 	clusterResourceSet := &ClusterResourceSet{}
-
+	defaultingValidationCRS := clusterResourceSet.DeepCopy()
+	defaultingValidationCRS.Spec.ClusterSelector = metav1.LabelSelector{
+		MatchLabels: map[string]string{"foo": "bar"},
+	}
+	t.Run("for ClusterResourceSet", utildefaulting.DefaultValidateTest(defaultingValidationCRS))
 	clusterResourceSet.Default()
 
 	g.Expect(clusterResourceSet.Spec.Strategy).To(Equal(string(ClusterResourceSetStrategyApplyOnce)))

diff --git a/exp/api/v1alpha3/machinepool_webhook_test.go b/exp/api/v1alpha3/machinepool_webhook_test.go
@@ -25,6 +25,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/pointer"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
+	utildefaulting "sigs.k8s.io/cluster-api/util/defaulting"
 )
 
 func TestMachinePoolDefault(t *testing.T) {
@@ -42,7 +43,7 @@ func TestMachinePoolDefault(t *testing.T) {
 			},
 		},
 	}
-
+	t.Run("for MachinePool", utildefaulting.DefaultValidateTest(m))
 	m.Default()
 
 	g.Expect(m.Labels[clusterv1.ClusterLabelName]).To(Equal(m.Spec.ClusterName))

diff --git a/util/defaulting/defaulting.go b/util/defaulting/defaulting.go
@@ -0,0 +1,62 @@
+/*
+Copyright 2021 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 defaulting
+
+import (
+	"testing"
+
+	"github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+// DefaultingValidator interface is for objects that define both defaulting
+// and validating webhooks.
+type DefaultingValidator interface {
+	runtime.Object
+	Default()
+	ValidateCreate() error
+	ValidateUpdate(old runtime.Object) error
+	ValidateDelete() error
+}
+
+// DefaultValidateTest returns a new testing function to be used in tests to
+// make sure defaulting webhooks also pass validation tests on create,
+// update and delete.
+func DefaultValidateTest(object DefaultingValidator) func(*testing.T) {
+	return func(t *testing.T) {
+		createCopy := object.DeepCopyObject().(DefaultingValidator)
+		updateCopy := object.DeepCopyObject().(DefaultingValidator)
+		deleteCopy := object.DeepCopyObject().(DefaultingValidator)
+		defaultingUpdateCopy := updateCopy.DeepCopyObject().(DefaultingValidator)
+
+		t.Run("validate-on-create", func(t *testing.T) {
+			g := gomega.NewWithT(t)
+			createCopy.Default()
+			g.Expect(createCopy.ValidateCreate()).To(gomega.Succeed())
+		})
+		t.Run("validate-on-update", func(t *testing.T) {
+			g := gomega.NewWithT(t)
+			defaultingUpdateCopy.Default()
+			g.Expect(defaultingUpdateCopy.ValidateUpdate(updateCopy)).To(gomega.Succeed())
+		})
+		t.Run("validate-on-delete", func(t *testing.T) {
+			g := gomega.NewWithT(t)
+			deleteCopy.Default()
+			g.Expect(deleteCopy.ValidateDelete()).To(gomega.Succeed())
+		})
+	}
+}