From 84f0b3b2cbcf2a6ca8a8c8461a15cf878e455e9c Mon Sep 17 00:00:00 2001
From: Sedef <ssavas@vmware.com>
Date: Fri, 13 Mar 2020 04:48:09 -0700
Subject: [PATCH] Add semver version validation rules to Machine type

[KCP] Fix semver version validation for KCP spec.version
---
 api/v1alpha3/machine_webhook.go               |  8 +++
 api/v1alpha3/machine_webhook_test.go          | 55 +++++++++++++++++++
 .../v1alpha3/kubeadm_control_plane_webhook.go |  4 +-
 3 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/api/v1alpha3/machine_webhook.go b/api/v1alpha3/machine_webhook.go
index 9a891e29586d..d2f34dce1403 100644
--- a/api/v1alpha3/machine_webhook.go
+++ b/api/v1alpha3/machine_webhook.go
@@ -18,7 +18,9 @@ package v1alpha3
 
 import (
 	"fmt"
+	"strings"
 
+	"github.com/blang/semver"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	runtime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -114,6 +116,12 @@ func (m *Machine) validate(old *Machine) error {
 		)
 	}
 
+	if m.Spec.Version != nil {
+		if _, err := semver.Parse(strings.TrimPrefix(strings.TrimSpace(*m.Spec.Version), "v")); err != nil {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "version"), *m.Spec.Version, "must be a valid semantic version"))
+		}
+	}
+
 	if len(allErrs) == 0 {
 		return nil
 	}
diff --git a/api/v1alpha3/machine_webhook_test.go b/api/v1alpha3/machine_webhook_test.go
index f2323f0fdb2c..5c29bbbe0c32 100644
--- a/api/v1alpha3/machine_webhook_test.go
+++ b/api/v1alpha3/machine_webhook_test.go
@@ -189,3 +189,58 @@ func TestMachineClusterNameImmutable(t *testing.T) {
 		})
 	}
 }
+
+func TestMachineVersionValidation(t *testing.T) {
+	tests := []struct {
+		name      string
+		version   string
+		expectErr bool
+	}{
+		{
+			name:      "should succeed when given a valid semantic version with prepended 'v'",
+			version:   "v1.17.2",
+			expectErr: false,
+		},
+		{
+			name:      "should succeed when given a valid semantic version without 'v'",
+			version:   "1.17.2",
+			expectErr: false,
+		},
+		{
+			name:      "should return error when given an invalid semantic version",
+			version:   "1",
+			expectErr: true,
+		},
+		{
+			name:      "should return error when given an invalid semantic version",
+			version:   "v1",
+			expectErr: true,
+		},
+		{
+			name:      "should return error when given an invalid semantic version",
+			version:   "wrong_version",
+			expectErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			m := &Machine{
+				Spec: MachineSpec{
+					Version:   &tt.version,
+					Bootstrap: Bootstrap{ConfigRef: nil, DataSecretName: pointer.StringPtr("test")},
+				},
+			}
+
+			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())
+			}
+		})
+	}
+}
diff --git a/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go b/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go
index 8cbbb94ee0db..79aea2a1298e 100644
--- a/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go
+++ b/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go
@@ -19,6 +19,7 @@ package v1alpha3
 import (
 	"encoding/json"
 	"fmt"
+	"strings"
 
 	"github.com/blang/semver"
 	jsonpatch "github.com/evanphx/json-patch"
@@ -232,8 +233,7 @@ func (in *KubeadmControlPlane) validateCommon() (allErrs field.ErrorList) {
 		)
 	}
 
-	_, err := semver.ParseTolerant(in.Spec.Version)
-	if err != nil {
+	if _, err := semver.Parse(strings.TrimPrefix(strings.TrimSpace(in.Spec.Version), "v")); err != nil {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "version"), in.Spec.Version, "must be a valid semantic version"))
 	}