Merge pull request #2674 from sedefsavas/machinesemver

🏃Add semver version validation rules to Machine type and fix KCP version validation
kubernetes-sigs · Mar 16, 2020 · 21c07c9 · 21c07c9
2 parents 3af8e0a + 84f0b3b
commit 21c07c9
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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"))
 	}