From 69c0f012310ebc4cb154e3e5f8f66402f002ac62 Mon Sep 17 00:00:00 2001 From: Sedef Date: Wed, 11 Mar 2020 10:33:58 -0700 Subject: [PATCH 1/2] Add KCP spec.version semver check --- .../v1alpha3/kubeadm_control_plane_webhook.go | 6 +++++ .../kubeadm_control_plane_webhook_test.go | 26 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go b/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go index 4fc9c941d574..f836188bb5bc 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" + "github.com/blang/semver" jsonpatch "github.com/evanphx/json-patch" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -228,6 +229,11 @@ func (in *KubeadmControlPlane) validateCommon() (allErrs field.ErrorList) { ) } + _, err := semver.ParseTolerant(in.Spec.Version) + if err != nil { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "version"), in.Spec.Version, "failed to parse kubernetes version ")) + } + return allErrs } diff --git a/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook_test.go b/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook_test.go index b7efc12ea651..e9aa250f18de 100644 --- a/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook_test.go +++ b/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook_test.go @@ -56,6 +56,7 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) { Name: "infraTemplate", }, Replicas: pointer.Int32Ptr(1), + Version: "v1.16.6", }, } invalidNamespace := valid.DeepCopy() @@ -79,6 +80,15 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) { }, } + validVersion1 := valid.DeepCopy() + validVersion1.Spec.Version = "v1.16.6" + + validVersion2 := valid.DeepCopy() + validVersion2.Spec.Version = "1.16.6" + + invalidVersion := valid.DeepCopy() + invalidVersion.Spec.Version = "vv1.16.6" + tests := []struct { name string expectErr bool @@ -114,6 +124,21 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) { expectErr: false, kcp: evenReplicasExternalEtcd, }, + { + name: "should succeed when given a valid semantic version with prepended 'v'", + expectErr: false, + kcp: validVersion1, + }, + { + name: "should succeed when given a valid semantic version without 'v'", + expectErr: false, + kcp: validVersion2, + }, + { + name: "should return error when given an invalid semantic version", + expectErr: true, + kcp: invalidVersion, + }, } for _, tt := range tests { @@ -157,6 +182,7 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) { }, }, }, + Version: "v1.16.6", }, } From e3a77b0220d62bfc969727dd9f5e9d43550e918b Mon Sep 17 00:00:00 2001 From: Sedef Date: Wed, 11 Mar 2020 12:44:59 -0700 Subject: [PATCH 2/2] Add version to all KubeadmControlPlane objects in tests since there is a validation now --- cmd/clusterctl/test/e2e/go.sum | 1 + .../v1alpha3/kubeadm_control_plane_webhook.go | 2 +- .../kubeadm_control_plane_controller_test.go | 41 +++++++++++++++++-- .../kubeadm/internal/workload_cluster_test.go | 2 +- test/infrastructure/docker/go.sum | 1 + 5 files changed, 42 insertions(+), 5 deletions(-) diff --git a/cmd/clusterctl/test/e2e/go.sum b/cmd/clusterctl/test/e2e/go.sum index 9bbf7511b037..16aa1469e38e 100644 --- a/cmd/clusterctl/test/e2e/go.sum +++ b/cmd/clusterctl/test/e2e/go.sum @@ -38,6 +38,7 @@ github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6r github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs= github.com/bifurcation/mint v0.0.0-20180715133206-93c51c6ce115/go.mod h1:zVt7zX3K/aDCk9Tj+VM7YymsX66ERvzCJzw8rFCX2JU= github.com/blang/semver v3.5.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= +github.com/blang/semver v3.5.1+incompatible h1:cQNTCjp13qL8KC3Nbxr/y2Bqb63oX6wdnnjpJbkM4JQ= github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= github.com/caddyserver/caddy v1.0.3/go.mod h1:G+ouvOY32gENkJC+jhgl62TyhvqEsFaDiZ4uw0RzP1E= github.com/cenkalti/backoff v2.1.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM= diff --git a/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go b/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go index f836188bb5bc..c5136e3b57d3 100644 --- a/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go +++ b/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go @@ -231,7 +231,7 @@ func (in *KubeadmControlPlane) validateCommon() (allErrs field.ErrorList) { _, err := semver.ParseTolerant(in.Spec.Version) if err != nil { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "version"), in.Spec.Version, "failed to parse kubernetes version ")) + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "version"), in.Spec.Version, "must be a valid semantic version")) } return allErrs diff --git a/controlplane/kubeadm/controllers/kubeadm_control_plane_controller_test.go b/controlplane/kubeadm/controllers/kubeadm_control_plane_controller_test.go index 0564de89e73e..fd092cfc4ca5 100644 --- a/controlplane/kubeadm/controllers/kubeadm_control_plane_controller_test.go +++ b/controlplane/kubeadm/controllers/kubeadm_control_plane_controller_test.go @@ -163,6 +163,9 @@ func TestReconcileKubeconfigEmptyAPIEndpoints(t *testing.T) { Name: "foo", Namespace: "test", }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + }, } clusterName := client.ObjectKey{Namespace: "test", Name: "foo"} @@ -191,6 +194,9 @@ func TestReconcileKubeconfigMissingCACertificate(t *testing.T) { Name: "foo", Namespace: "test", }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + }, } clusterName := client.ObjectKey{Namespace: "test", Name: "foo"} endpoint := clusterv1.APIEndpoint{Host: "test.local", Port: 8443} @@ -227,6 +233,9 @@ func TestReconcileKubeconfigSecretAlreadyExists(t *testing.T) { Name: "foo", Namespace: "test", }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + }, } clusterName := util.ObjectKey(cluster) endpoint := clusterv1.APIEndpoint{Host: "test.local", Port: 8443} @@ -273,6 +282,9 @@ func TestKubeadmControlPlaneReconciler_reconcileKubeconfig(t *testing.T) { Name: "foo", Namespace: "test", }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + }, } clusterName := util.ObjectKey(cluster) endpoint := clusterv1.APIEndpoint{Host: "test.local", Port: 8443} @@ -348,6 +360,9 @@ func TestReconcileNoClusterOwnerRef(t *testing.T) { Namespace: "test", Name: "foo", }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + }, } kcp.Default() g.Expect(kcp.ValidateCreate()).To(Succeed()) @@ -385,6 +400,9 @@ func TestReconcileNoCluster(t *testing.T) { }, }, }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + }, } kcp.Default() g.Expect(kcp.ValidateCreate()).To(Succeed()) @@ -431,6 +449,9 @@ func TestReconcileClusterNoEndpoints(t *testing.T) { }, }, }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + }, } kcp.Default() g.Expect(kcp.ValidateCreate()).To(Succeed()) @@ -517,7 +538,7 @@ func TestReconcileInitializeControlPlane(t *testing.T) { }, Spec: controlplanev1.KubeadmControlPlaneSpec{ Replicas: nil, - Version: "", + Version: "v1.16.6", InfrastructureTemplate: corev1.ObjectReference{ Kind: genericMachineTemplate.GetKind(), APIVersion: genericMachineTemplate.GetAPIVersion(), @@ -657,7 +678,7 @@ func TestKubeadmControlPlaneReconciler_generateMachine(t *testing.T) { Namespace: cluster.Namespace, }, Spec: controlplanev1.KubeadmControlPlaneSpec{ - Version: "my-version", + Version: "v1.16.6", }, } @@ -767,6 +788,9 @@ func TestKubeadmControlPlaneReconciler_updateStatusNoMachines(t *testing.T) { Namespace: cluster.Namespace, Name: "foo", }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + }, } kcp.Default() g.Expect(kcp.ValidateCreate()).To(Succeed()) @@ -850,6 +874,9 @@ func TestKubeadmControlPlaneReconciler_updateStatusAllMachinesNotReady(t *testin Namespace: cluster.Namespace, Name: "foo", }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + }, } kcp.Default() g.Expect(kcp.ValidateCreate()).To(Succeed()) @@ -912,6 +939,9 @@ func TestKubeadmControlPlaneReconciler_updateStatusAllMachinesReady(t *testing.T Namespace: cluster.Namespace, Name: "foo", }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + }, } kcp.Default() g.Expect(kcp.ValidateCreate()).To(Succeed()) @@ -971,6 +1001,9 @@ func TestKubeadmControlPlaneReconciler_updateStatusMachinesReadyMixed(t *testing Namespace: cluster.Namespace, Name: "foo", }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + }, } kcp.Default() g.Expect(kcp.ValidateCreate()).To(Succeed()) @@ -1032,7 +1065,7 @@ func TestKubeadmControlPlaneReconciler_updateCoreDNS(t *testing.T) { }, Spec: controlplanev1.KubeadmControlPlaneSpec{ Replicas: nil, - Version: "", + Version: "v1.16.6", KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ ClusterConfiguration: &kubeadmv1.ClusterConfiguration{ DNS: kubeadmv1.DNS{ @@ -1300,6 +1333,7 @@ func TestCloneConfigsAndGenerateMachine(t *testing.T) { Name: genericMachineTemplate.GetName(), Namespace: cluster.Namespace, }, + Version: "v1.16.6", }, } @@ -1366,6 +1400,7 @@ func createClusterWithControlPlane() (*clusterv1.Cluster, *controlplanev1.Kubead Name: "infra-foo", APIVersion: "generic.io/v1", }, + Version: "v1.16.6", }, } diff --git a/controlplane/kubeadm/internal/workload_cluster_test.go b/controlplane/kubeadm/internal/workload_cluster_test.go index fb7348d64fd1..a670b47f4c46 100644 --- a/controlplane/kubeadm/internal/workload_cluster_test.go +++ b/controlplane/kubeadm/internal/workload_cluster_test.go @@ -169,7 +169,7 @@ func TestUpdateKubeProxyImageInfo(t *testing.T) { c := &Workload{ Client: fakeClient, } - err := c.UpdateKubeProxyImageInfo(ctx, &v1alpha3.KubeadmControlPlane{Spec: v1alpha3.KubeadmControlPlaneSpec{Version: "1.16.3"}}) + err := c.UpdateKubeProxyImageInfo(ctx, &v1alpha3.KubeadmControlPlane{Spec: v1alpha3.KubeadmControlPlaneSpec{Version: "v1.16.3"}}) if err != nil && !tt.expectErr { t.Fatalf("expected no error, got %s", err) } diff --git a/test/infrastructure/docker/go.sum b/test/infrastructure/docker/go.sum index 518724997f53..62f57e2de9df 100644 --- a/test/infrastructure/docker/go.sum +++ b/test/infrastructure/docker/go.sum @@ -39,6 +39,7 @@ github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6r github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs= github.com/bifurcation/mint v0.0.0-20180715133206-93c51c6ce115/go.mod h1:zVt7zX3K/aDCk9Tj+VM7YymsX66ERvzCJzw8rFCX2JU= github.com/blang/semver v3.5.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= +github.com/blang/semver v3.5.1+incompatible h1:cQNTCjp13qL8KC3Nbxr/y2Bqb63oX6wdnnjpJbkM4JQ= github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= github.com/caddyserver/caddy v1.0.3/go.mod h1:G+ouvOY32gENkJC+jhgl62TyhvqEsFaDiZ4uw0RzP1E= github.com/cenkalti/backoff v2.1.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM=