Skip to content

Commit

Permalink
kcp: Add additional validation of versioning
Browse files Browse the repository at this point in the history
Signed-off-by: Naadir Jeewa <[email protected]>
  • Loading branch information
Naadir Jeewa committed Mar 16, 2020
1 parent bb840d9 commit 6a920ef
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 9 deletions.
64 changes: 56 additions & 8 deletions controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ package v1alpha3

import (
"encoding/json"
"fmt"

"github.com/blang/semver"
jsonpatch "github.com/evanphx/json-patch"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"sigs.k8s.io/cluster-api/util"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)
Expand All @@ -50,6 +52,8 @@ func (in *KubeadmControlPlane) Default() {
if in.Spec.InfrastructureTemplate.Namespace == "" {
in.Spec.InfrastructureTemplate.Namespace = in.Namespace
}

in.Spec.Version = util.NormalizedKubernetesBuildVersion(in.Spec.Version)
}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
Expand Down Expand Up @@ -88,8 +92,6 @@ func (in *KubeadmControlPlane) ValidateUpdate(old runtime.Object) error {

prev := old.(*KubeadmControlPlane)

allErrs = append(allErrs, in.validateEtcd(prev)...)

originalJSON, err := json.Marshal(prev)
if err != nil {
return apierrors.NewInternalError(err)
Expand Down Expand Up @@ -234,6 +236,29 @@ func (in *KubeadmControlPlane) validateCommon() (allErrs field.ErrorList) {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "version"), in.Spec.Version, "must be a valid semantic version"))
}

if !util.KubernetesVersionIsValid(in.Spec.Version) {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "version"), in.Spec.Version, "must be a valid Kubernetes version"))
}

allErrs = append(allErrs, in.validateEtcd(nil)...)
allErrs = append(allErrs, in.validateCoreDNS()...)

return allErrs
}

func (in *KubeadmControlPlane) validateCoreDNS() (allErrs field.ErrorList) {
if in.Spec.KubeadmConfigSpec.ClusterConfiguration == nil {
return allErrs
}
if !util.ImageTagIsValid(in.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag) {
allErrs = append(
allErrs,
field.Forbidden(
field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration", "dns", "imageTag"),
fmt.Sprintf("tag %s is invalid", in.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag),
),
)
}
return allErrs
}

Expand All @@ -242,26 +267,49 @@ func (in *KubeadmControlPlane) validateEtcd(prev *KubeadmControlPlane) (allErrs
return allErrs
}

if in.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.External != nil && prev.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local != nil {
if in.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local != nil && !util.ImageTagIsValid(in.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local.ImageTag) {
allErrs = append(
allErrs,
field.Forbidden(
field.NewPath("spec", "kubeadmConfigSpec", "initConfiguration", "etcd", "external"),
"cannot have both local and external etcd at the same time",
field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration", "etcd", "local", "imageTag"),
fmt.Sprintf("tag %s is invalid", in.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local.ImageTag),
),
)
}

if in.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local != nil && prev.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.External != nil {
if in.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local != nil && in.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.External != nil {
allErrs = append(
allErrs,
field.Forbidden(
field.NewPath("spec", "kubeadmConfigSpec", "initConfiguration", "etcd", "local"),
"cannot have both local and external etcd at the same time",
field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration", "etcd", "local"),
"cannot have both external and local etcd",
),
)
}

// update validations
if prev != nil {
if in.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.External != nil && prev.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local != nil {
allErrs = append(
allErrs,
field.Forbidden(
field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration", "etcd", "external"),
"cannot change between external and local etcd",
),
)
}

if in.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local != nil && prev.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.External != nil {
allErrs = append(
allErrs,
field.Forbidden(
field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration", "etcd", "local"),
"cannot change between external and local etcd",
),
)
}
}

return allErrs
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,21 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
ImageTag: "v9.1.1",
},
}

etcdLocalImageBuildTag := before.DeepCopy()
etcdLocalImageBuildTag.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local = &kubeadmv1beta1.LocalEtcd{
ImageMeta: kubeadmv1beta1.ImageMeta{
ImageTag: "v9.1.1_invalidBuild1",
},
}

etcdLocalImageInvalidTag := before.DeepCopy()
etcdLocalImageInvalidTag.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local = &kubeadmv1beta1.LocalEtcd{
ImageMeta: kubeadmv1beta1.ImageMeta{
ImageTag: "v9.1.1+invalidBuild1",
},
}

unsetEtcd := etcdLocalImageTag.DeepCopy()
unsetEtcd.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local = nil

Expand Down Expand Up @@ -256,6 +271,22 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
},
}

dnsBuildTag := before.DeepCopy()
dnsBuildTag.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS = kubeadmv1beta1.DNS{
ImageMeta: kubeadmv1beta1.ImageMeta{
ImageRepository: "gcr.io/capi-test",
ImageTag: "v0.20.0_build1",
},
}

dnsInvalidTag := before.DeepCopy()
dnsInvalidTag.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS = kubeadmv1beta1.DNS{
ImageMeta: kubeadmv1beta1.ImageMeta{
ImageRepository: "gcr.io/capi-test",
ImageTag: "v0.20.0+invalidBuild1",
},
}

certificatesDir := before.DeepCopy()
certificatesDir.Spec.KubeadmConfigSpec.ClusterConfiguration.CertificatesDir = "a new certificates directory"

Expand Down Expand Up @@ -392,6 +423,18 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
before: before,
kcp: etcdLocalImageTag,
},
{
name: "should succeed when making a change to the local etcd image tag",
expectErr: false,
before: before,
kcp: etcdLocalImageBuildTag,
},
{
name: "should fail when using an invalid etcd image tag",
expectErr: true,
before: before,
kcp: etcdLocalImageInvalidTag,
},
{
name: "should fail when making a change to the cluster config's networking struct",
expectErr: true,
Expand Down Expand Up @@ -429,11 +472,23 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
kcp: scheduler,
},
{
name: "should fail when making a change to the cluster config's dns",
name: "should succeed when making a change to the cluster config's dns",
expectErr: false,
before: before,
kcp: dns,
},
{
name: "should succeed when using an valid DNS build",
expectErr: false,
before: before,
kcp: dnsBuildTag,
},
{
name: "should fail when using an invalid DNS build",
expectErr: false,
before: before,
kcp: dnsInvalidTag,
},
{
name: "should fail when making a change to the cluster config's certificatesDir",
expectErr: true,
Expand Down

0 comments on commit 6a920ef

Please sign in to comment.