🌱 Clean up kube version parsing

api/v1alpha4/machine_webhook.go

@@ -18,11 +18,11 @@ package v1alpha4
 
 import (
 	"fmt"
-	"regexp"
+	"sigs.k8s.io/cluster-api/util/version"
 	"strings"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	runtime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -40,8 +40,6 @@ func (m *Machine) SetupWebhookWithManager(mgr ctrl.Manager) error {
 var _ webhook.Validator = &Machine{}
 var _ webhook.Defaulter = &Machine{}
 
-var kubeSemver = regexp.MustCompile(`^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)([-0-9a-zA-Z_\.+]*)?$`)
-
 // Default implements webhook.Defaulter so a webhook will be registered for the type
 func (m *Machine) Default() {
 	if m.Labels == nil {
@@ -124,7 +122,7 @@ func (m *Machine) validate(old *Machine) error {
 	}
 
 	if m.Spec.Version != nil {
-		if !kubeSemver.MatchString(*m.Spec.Version) {
+		if !version.KubeSemver.MatchString(*m.Spec.Version) {
 			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "version"), *m.Spec.Version, "must be a valid semantic version"))
 		}
 	}

controlplane/kubeadm/api/v1alpha4/kubeadm_control_plane_webhook.go

@@ -19,7 +19,6 @@ package v1alpha4
 import (
 	"encoding/json"
 	"fmt"
-	"regexp"
 	"strings"
 
 	"github.com/blang/semver"
@@ -30,8 +29,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	kubeadmv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1"
-	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/container"
+	"sigs.k8s.io/cluster-api/util/version"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 )
@@ -48,8 +47,6 @@ func (in *KubeadmControlPlane) SetupWebhookWithManager(mgr ctrl.Manager) error {
 var _ webhook.Defaulter = &KubeadmControlPlane{}
 var _ webhook.Validator = &KubeadmControlPlane{}
 
-var kubeSemver = regexp.MustCompile(`^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)([-0-9a-zA-Z_\.+]*)?$`)
-
 // Default implements webhook.Defaulter so a webhook will be registered for the type
 func (in *KubeadmControlPlane) Default() {
 	if in.Spec.Replicas == nil {
@@ -268,7 +265,7 @@ func (in *KubeadmControlPlane) validateCommon() (allErrs field.ErrorList) {
 		)
 	}
 
-	if !kubeSemver.MatchString(in.Spec.Version) {
+	if !version.KubeSemver.MatchString(in.Spec.Version) {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "version"), in.Spec.Version, "must be a valid semantic version"))
 	}
 
@@ -308,7 +305,7 @@ func (in *KubeadmControlPlane) validateCoreDNSVersion(prev *KubeadmControlPlane)
 		return allErrs
 	}
 
-	fromVersion, err := util.ParseMajorMinorPatch(prev.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag)
+	fromVersion, err := version.ParseMajorMinorPatchTolerant(prev.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag)
 	if err != nil {
 		allErrs = append(allErrs,
 			field.InternalError(
@@ -319,7 +316,7 @@ func (in *KubeadmControlPlane) validateCoreDNSVersion(prev *KubeadmControlPlane)
 		return allErrs
 	}
 
-	toVersion, err := util.ParseMajorMinorPatch(targetDNS.ImageTag)
+	toVersion, err := version.ParseMajorMinorPatchTolerant(targetDNS.ImageTag)
 	if err != nil {
 		allErrs = append(allErrs,
 			field.Invalid(
@@ -397,7 +394,7 @@ func (in *KubeadmControlPlane) validateEtcd(prev *KubeadmControlPlane) (allErrs
 }
 
 func (in *KubeadmControlPlane) validateVersion(previousVersion string) (allErrs field.ErrorList) {
-	fromVersion, err := util.ParseMajorMinorPatch(previousVersion)
+	fromVersion, err := version.ParseMajorMinorPatch(previousVersion)
 	if err != nil {
 		allErrs = append(allErrs,
 			field.InternalError(
@@ -408,7 +405,7 @@ func (in *KubeadmControlPlane) validateVersion(previousVersion string) (allErrs
 		return allErrs
 	}
 
-	toVersion, err := util.ParseMajorMinorPatch(in.Spec.Version)
+	toVersion, err := version.ParseMajorMinorPatch(in.Spec.Version)
 	if err != nil {
 		allErrs = append(allErrs,
 			field.InternalError(

controlplane/kubeadm/api/v1alpha4/kubeadm_control_plane_webhook_test.go

@@ -133,7 +133,7 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) {
 			kcp:       validVersion,
 		},
 		{
-			name:      "should succeed when given a valid semantic version without 'v'",
+			name:      "should error when given a valid semantic version without 'v'",
 			expectErr: true,
 			kcp:       invalidVersion2,
 		},

controlplane/kubeadm/internal/workload_cluster_coredns.go

@@ -19,6 +19,7 @@ package internal
 import (
 	"context"
 	"fmt"
+	"sigs.k8s.io/cluster-api/util/version"
 
 	"github.com/coredns/corefile-migration/migration"
 	"github.com/pkg/errors"
@@ -28,7 +29,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kubeadmv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1"
 	controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha4"
-	"sigs.k8s.io/cluster-api/util"
 	containerutil "sigs.k8s.io/cluster-api/util/container"
 	"sigs.k8s.io/cluster-api/util/patch"
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
@@ -298,7 +298,7 @@ func patchCoreDNSDeploymentImage(deployment *appsv1.Deployment, image string) {
 }
 
 func extractImageVersion(tag string) (string, error) {
-	ver, err := util.ParseMajorMinorPatch(tag)
+	ver, err := version.ParseMajorMinorPatchTolerant(tag)
 	if err != nil {
 		return "", err
 	}
@@ -309,11 +309,11 @@ func extractImageVersion(tag string) (string, error) {
 // Some of the checks come from
 // https://github.com/coredns/corefile-migration/blob/v1.0.6/migration/migrate.go#L414
 func validateCoreDNSImageTag(fromTag, toTag string) error {
-	from, err := util.ParseMajorMinorPatch(fromTag)
+	from, err := version.ParseMajorMinorPatchTolerant(fromTag)
 	if err != nil {
 		return errors.Wrapf(err, "failed to parse CoreDNS current version %q", fromTag)
 	}
-	to, err := util.ParseMajorMinorPatch(toTag)
+	to, err := version.ParseMajorMinorPatchTolerant(toTag)
 	if err != nil {
 		return errors.Wrapf(err, "failed to parse CoreDNS target version %q", toTag)
 	}

exp/api/v1alpha3/conversion.go

@@ -24,4 +24,4 @@ import (
 // Convert_v1alpha3_MachinePoolSpec_To_v1alpha4_MachinePoolSpec is an autogenerated conversion function.
 func Convert_v1alpha3_MachinePoolSpec_To_v1alpha4_MachinePoolSpec(in *MachinePoolSpec, out *v1alpha4.MachinePoolSpec, s conversion.Scope) error {
 	return autoConvert_v1alpha3_MachinePoolSpec_To_v1alpha4_MachinePoolSpec(in, out, s)
-}
+}

util/util.go

@@ -22,8 +22,6 @@ import (
 	"fmt"
 	"math"
 	"math/rand"
-	"regexp"
-	"strconv"
 	"strings"
 	"time"
 
@@ -38,14 +36,15 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/version"
+	k8sversion "k8s.io/apimachinery/pkg/version"
 	"k8s.io/client-go/metadata"
 	"k8s.io/client-go/rest"
 	"k8s.io/klog/klogr"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
 	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/container"
 	"sigs.k8s.io/cluster-api/util/predicates"
+	"sigs.k8s.io/cluster-api/util/version"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
@@ -67,35 +66,8 @@ var (
 	rnd                          = rand.New(rand.NewSource(time.Now().UnixNano()))
 	ErrNoCluster                 = fmt.Errorf("no %q label present", clusterv1.ClusterLabelName)
 	ErrUnstructuredFieldNotFound = fmt.Errorf("field not found")
-	kubeSemver                   = regexp.MustCompile(`^v?(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)([-0-9a-zA-Z_\.+]*)?$`)
 )
 
-// ParseMajorMinorPatch returns a semver.Version from the string provided
-// by looking only at major.minor.patch and stripping everything else out.
-func ParseMajorMinorPatch(version string) (semver.Version, error) {
-	groups := kubeSemver.FindStringSubmatch(version)
-	if len(groups) < 4 {
-		return semver.Version{}, errors.Errorf("failed to parse major.minor.patch from %q", version)
-	}
-	major, err := strconv.ParseUint(groups[1], 10, 64)
-	if err != nil {
-		return semver.Version{}, errors.Wrapf(err, "failed to parse major version from %q", version)
-	}
-	minor, err := strconv.ParseUint(groups[2], 10, 64)
-	if err != nil {
-		return semver.Version{}, errors.Wrapf(err, "failed to parse minor version from %q", version)
-	}
-	patch, err := strconv.ParseUint(groups[3], 10, 64)
-	if err != nil {
-		return semver.Version{}, errors.Wrapf(err, "failed to parse patch version from %q", version)
-	}
-	return semver.Version{
-		Major: major,
-		Minor: minor,
-		Patch: patch,
-	}, nil
-}
-
 // RandomString returns a random alphanumeric string.
 func RandomString(n int) string {
 	result := make([]byte, n)
@@ -128,6 +100,13 @@ func Ordinalize(n int) string {
 	return fmt.Sprintf("%d%s", n, m[an%10])
 }
 
+// ParseMajorMinorPatch returns a semver.Version from the string provided
+// by looking only at major.minor.patch and stripping everything else out.
+// Deprecated: Please use the function in util/version
+func ParseMajorMinorPatch(v string) (semver.Version, error) {
+	return version.ParseMajorMinorPatchTolerant(v)
+}
+
 // ModifyImageRepository takes an imageName (e.g., repository/image:tag), and returns an image name with updated repository
 // Deprecated: Please use the functions in util/container
 func ModifyImageRepository(imageName, repositoryName string) (string, error) {
@@ -597,7 +576,7 @@ type KubeAwareAPIVersions []string
 func (k KubeAwareAPIVersions) Len() int      { return len(k) }
 func (k KubeAwareAPIVersions) Swap(i, j int) { k[i], k[j] = k[j], k[i] }
 func (k KubeAwareAPIVersions) Less(i, j int) bool {
-	return version.CompareKubeAwareVersionStrings(k[i], k[j]) < 0
+	return k8sversion.CompareKubeAwareVersionStrings(k[i], k[j]) < 0
 }
 
 // MachinesByCreationTimestamp sorts a list of Machine by creation timestamp, using their names as a tie breaker.

util/util_test.go

@@ -39,54 +39,6 @@ var (
 	ctx = ctrl.SetupSignalHandler()
 )
 
-func TestParseMajorMinorPatch(t *testing.T) {
-	g := NewWithT(t)
-
-	var testcases = []struct {
-		name        string
-		input       string
-		output      semver.Version
-		expectError bool
-	}{
-		{
-			name:  "should parse an OCI compliant string",
-			input: "v1.2.16_foo-1",
-			output: semver.Version{
-				Major: 1,
-				Minor: 2,
-				Patch: 16,
-			},
-		},
-		{
-			name:  "should parse a valid semver",
-			input: "v1.16.6+foobar-0",
-			output: semver.Version{
-				Major: 1,
-				Minor: 16,
-				Patch: 6,
-			},
-		},
-		{
-			name:        "should error if there is no patch version",
-			input:       "v1.16+foobar-0",
-			expectError: true,
-		},
-		{
-			name:        "should error if there is no minor and patch",
-			input:       "v1+foobar-0",
-			expectError: true,
-		},
-	}
-
-	for _, tc := range testcases {
-		t.Run(tc.name, func(t *testing.T) {
-			out, err := ParseMajorMinorPatch(tc.input)
-			g.Expect(err != nil).To(Equal(tc.expectError))
-			g.Expect(out).To(Equal(tc.output))
-		})
-	}
-}
-
 func TestMachineToInfrastructureMapFunc(t *testing.T) {
 	g := NewWithT(t)
 

util/version/version.go

@@ -0,0 +1,74 @@
+/*
+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 version
+
+import (
+	"github.com/blang/semver"
+	"github.com/pkg/errors"
+	"regexp"
+	"strconv"
+)
+
+var (
+	// KubeSemver is the regex for Kubernetes versions. It requires the "v" prefix.
+	KubeSemver = regexp.MustCompile(`^v(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)([-0-9a-zA-Z_\.+]*)?$`)
+	// KubeSemverTolerant is the regex for Kubernetes versions with an optional "v" prefix.
+	KubeSemverTolerant = regexp.MustCompile(`^v?(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)([-0-9a-zA-Z_\.+]*)?$`)
+)
+
+// ParseMajorMinorPatch returns a semver.Version from the string provided
+// by looking only at major.minor.patch and stripping everything else out.
+// It requires the version to have a "v" prefix.
+func ParseMajorMinorPatch(version string) (semver.Version, error) {
+	return parseMajorMinorPatch(version, false)
+}
+
+// ParseMajorMinorPatchTolerant returns a semver.Version from the string provided
+// by looking only at major.minor.patch and stripping everything else out.
+// It does not require the version to have a "v" prefix.
+func ParseMajorMinorPatchTolerant(version string) (semver.Version, error) {
+	return parseMajorMinorPatch(version, true)
+}
+
+// parseMajorMinorPatch returns a semver.Version from the string provided
+// by looking only at major.minor.patch and stripping everything else out.
+func parseMajorMinorPatch(version string, tolerant bool) (semver.Version, error) {
+	groups := KubeSemver.FindStringSubmatch(version)
+	if tolerant {
+		groups = KubeSemverTolerant.FindStringSubmatch(version)
+	}
+	if len(groups) < 4 {
+		return semver.Version{}, errors.Errorf("failed to parse major.minor.patch from %q", version)
+	}
+	major, err := strconv.ParseUint(groups[1], 10, 64)
+	if err != nil {
+		return semver.Version{}, errors.Wrapf(err, "failed to parse major version from %q", version)
+	}
+	minor, err := strconv.ParseUint(groups[2], 10, 64)
+	if err != nil {
+		return semver.Version{}, errors.Wrapf(err, "failed to parse minor version from %q", version)
+	}
+	patch, err := strconv.ParseUint(groups[3], 10, 64)
+	if err != nil {
+		return semver.Version{}, errors.Wrapf(err, "failed to parse patch version from %q", version)
+	}
+	return semver.Version{
+		Major: major,
+		Minor: minor,
+		Patch: patch,
+	}, nil
+}