Skip to content

Commit

Permalink
Merge pull request #4163 from CecileRobertMichon/version-util
Browse files Browse the repository at this point in the history
🌱 Clean up kube version parsing
  • Loading branch information
k8s-ci-robot authored Feb 9, 2021
2 parents 4a6cccd + 67618d8 commit 51a6d64
Show file tree
Hide file tree
Showing 9 changed files with 223 additions and 99 deletions.
8 changes: 3 additions & 5 deletions api/v1alpha4/machine_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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"))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package v1alpha4
import (
"encoding/json"
"fmt"
"regexp"
"strings"

"github.com/blang/semver"
Expand All @@ -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"
)
Expand All @@ -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 {
Expand Down Expand Up @@ -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"))
}

Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
8 changes: 4 additions & 4 deletions controlplane/kubeadm/internal/workload_cluster_coredns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion exp/api/v1alpha3/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
41 changes: 10 additions & 31 deletions util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import (
"fmt"
"math"
"math/rand"
"regexp"
"strconv"
"strings"
"time"

Expand All @@ -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"
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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.
Expand Down
48 changes: 0 additions & 48 deletions util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
74 changes: 74 additions & 0 deletions util/version/version.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading

0 comments on commit 51a6d64

Please sign in to comment.