Merge pull request #6311 from CecileRobertMichon/machinepool-version-…

…validation 🐛 Ensure MachinePool Version is a valid Kubernetes version
kubernetes-sigs · Mar 28, 2022 · 93821f9 · 93821f9
2 parents d61b7b9 + 8e04778
commit 93821f9
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 0 deletions.
diff --git a/exp/api/v1beta1/machinepool_webhook.go b/exp/api/v1beta1/machinepool_webhook.go
@@ -18,6 +18,7 @@ package v1beta1
 
 import (
 	"fmt"
+	"strings"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -27,6 +28,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/cluster-api/util/version"
 )
 
 func (m *MachinePool) SetupWebhookWithManager(mgr ctrl.Manager) error {
@@ -63,6 +65,12 @@ func (m *MachinePool) Default() {
 	if m.Spec.Template.Spec.InfrastructureRef.Namespace == "" {
 		m.Spec.Template.Spec.InfrastructureRef.Namespace = m.Namespace
 	}
+
+	// tolerate version strings without a "v" prefix: prepend it if it's not there.
+	if m.Spec.Template.Spec.Version != nil && !strings.HasPrefix(*m.Spec.Template.Spec.Version, "v") {
+		normalizedVersion := "v" + *m.Spec.Template.Spec.Version
+		m.Spec.Template.Spec.Version = &normalizedVersion
+	}
 }
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
@@ -125,6 +133,12 @@ func (m *MachinePool) validate(old *MachinePool) error {
 		)
 	}
 
+	if m.Spec.Template.Spec.Version != nil {
+		if !version.KubeSemver.MatchString(*m.Spec.Template.Spec.Version) {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "template", "spec", "version"), *m.Spec.Template.Spec.Version, "must be a valid semantic version"))
+		}
+	}
+
 	if len(allErrs) == 0 {
 		return nil
 	}

diff --git a/exp/api/v1beta1/machinepool_webhook_test.go b/exp/api/v1beta1/machinepool_webhook_test.go
@@ -39,6 +39,7 @@ func TestMachinePoolDefault(t *testing.T) {
 			Template: clusterv1.MachineTemplateSpec{
 				Spec: clusterv1.MachineSpec{
 					Bootstrap: clusterv1.Bootstrap{ConfigRef: &corev1.ObjectReference{}},
+					Version:   pointer.StringPtr("1.20.0"),
 				},
 			},
 		},
@@ -51,6 +52,7 @@ func TestMachinePoolDefault(t *testing.T) {
 	g.Expect(m.Spec.MinReadySeconds).To(Equal(pointer.Int32Ptr(0)))
 	g.Expect(m.Spec.Template.Spec.Bootstrap.ConfigRef.Namespace).To(Equal(m.Namespace))
 	g.Expect(m.Spec.Template.Spec.InfrastructureRef.Namespace).To(Equal(m.Namespace))
+	g.Expect(m.Spec.Template.Spec.Version).To(Equal(pointer.StringPtr("v1.20.0")))
 }
 
 func TestMachinePoolBootstrapValidation(t *testing.T) {
@@ -219,3 +221,57 @@ func TestMachinePoolClusterNameImmutable(t *testing.T) {
 		})
 	}
 }
+
+func TestMachinePoolVersionValidation(t *testing.T) {
+	tests := []struct {
+		name      string
+		expectErr bool
+		version   string
+	}{
+		{
+			name:      "should succeed version is a valid kube semver",
+			expectErr: false,
+			version:   "v1.23.3",
+		},
+		{
+			name:      "should succeed version is a valid pre-release",
+			expectErr: false,
+			version:   "v1.19.0-alpha.1",
+		},
+		{
+			name:      "should fail if version is not a valid semver",
+			expectErr: true,
+			version:   "v1.1",
+		},
+		{
+			name:      "should fail if version is missing a v prefix",
+			expectErr: true,
+			version:   "1.20.0",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			m := &MachinePool{
+				Spec: MachinePoolSpec{
+					Template: clusterv1.MachineTemplateSpec{
+						Spec: clusterv1.MachineSpec{
+							Bootstrap: clusterv1.Bootstrap{ConfigRef: &corev1.ObjectReference{}},
+							Version:   &tt.version,
+						},
+					},
+				},
+			}
+
+			if tt.expectErr {
+				g.Expect(m.ValidateCreate()).NotTo(Succeed())
+				g.Expect(m.ValidateUpdate(m)).NotTo(Succeed())
+			} else {
+				g.Expect(m.ValidateCreate()).To(Succeed())
+				g.Expect(m.ValidateUpdate(m)).To(Succeed())
+			}
+		})
+	}
+}