From b0fccd469b2858be55844c263151f1cffcd7b04d Mon Sep 17 00:00:00 2001 From: odvarkadaniel Date: Wed, 13 Sep 2023 10:12:55 +0200 Subject: [PATCH] Move Kubeadm API v1beta1 webhooks to separate package --- .../internal/controllers/controller_test.go | 20 +-- .../internal/controllers/status_test.go | 26 ++-- controlplane/kubeadm/main.go | 5 +- internal/test/envtest/environment.go | 5 +- .../webhooks/kubeadm_control_plane.go | 120 +++++++++++------- .../webhooks/kubeadm_control_plane_test.go | 107 ++++++++++------ .../webhooks/kubeadmcontrolplanetemplate.go | 68 ++++++---- .../kubeadmcontrolplanetemplate_test.go | 61 +++++---- webhooks/alias.go | 16 +++ 9 files changed, 268 insertions(+), 160 deletions(-) rename controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go => internal/webhooks/kubeadm_control_plane.go (80%) rename controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go => internal/webhooks/kubeadm_control_plane_test.go (93%) rename controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook.go => internal/webhooks/kubeadmcontrolplanetemplate.go (61%) rename controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook_test.go => internal/webhooks/kubeadmcontrolplanetemplate_test.go (65%) diff --git a/controlplane/kubeadm/internal/controllers/controller_test.go b/controlplane/kubeadm/internal/controllers/controller_test.go index b075f89948cc..38a2fc50cc21 100644 --- a/controlplane/kubeadm/internal/controllers/controller_test.go +++ b/controlplane/kubeadm/internal/controllers/controller_test.go @@ -251,8 +251,9 @@ func TestReconcileNoClusterOwnerRef(t *testing.T) { }, }, } - kcp.Default() - _, err := kcp.ValidateCreate() + webhook := &webhooks.KubeadmControlPlane{} + g.Expect(webhook.Default(ctx, kcp)).To(Succeed()) + _, err := webhook.ValidateCreate(ctx, kcp) g.Expect(err).ToNot(HaveOccurred()) fakeClient := newFakeClient(kcp.DeepCopy()) @@ -330,8 +331,9 @@ func TestReconcileNoCluster(t *testing.T) { }, }, } - kcp.Default() - _, err := kcp.ValidateCreate() + webhook := &webhooks.KubeadmControlPlane{} + g.Expect(webhook.Default(ctx, kcp)).To(Succeed()) + _, err := webhook.ValidateCreate(ctx, kcp) g.Expect(err).ToNot(HaveOccurred()) fakeClient := newFakeClient(kcp.DeepCopy()) @@ -381,8 +383,9 @@ func TestReconcilePaused(t *testing.T) { }, }, } - kcp.Default() - _, err := kcp.ValidateCreate() + webhook := &webhooks.KubeadmControlPlane{} + g.Expect(webhook.Default(ctx, kcp)).To(Succeed()) + _, err := webhook.ValidateCreate(ctx, kcp) g.Expect(err).ToNot(HaveOccurred()) fakeClient := newFakeClient(kcp.DeepCopy(), cluster.DeepCopy()) r := &KubeadmControlPlaneReconciler{ @@ -436,8 +439,9 @@ func TestReconcileClusterNoEndpoints(t *testing.T) { }, }, } - kcp.Default() - _, err := kcp.ValidateCreate() + webhook := &webhooks.KubeadmControlPlane{} + g.Expect(webhook.Default(ctx, kcp)).To(Succeed()) + _, err := webhook.ValidateCreate(ctx, kcp) g.Expect(err).ToNot(HaveOccurred()) fakeClient := newFakeClient(kcp.DeepCopy(), cluster.DeepCopy()) diff --git a/controlplane/kubeadm/internal/controllers/status_test.go b/controlplane/kubeadm/internal/controllers/status_test.go index 37cf1d680fbd..55ebac70aa19 100644 --- a/controlplane/kubeadm/internal/controllers/status_test.go +++ b/controlplane/kubeadm/internal/controllers/status_test.go @@ -32,6 +32,7 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" + "sigs.k8s.io/cluster-api/internal/webhooks" "sigs.k8s.io/cluster-api/util/conditions" ) @@ -65,8 +66,9 @@ func TestKubeadmControlPlaneReconciler_updateStatusNoMachines(t *testing.T) { }, }, } - kcp.Default() - _, err := kcp.ValidateCreate() + webhook := &webhooks.KubeadmControlPlane{} + g.Expect(webhook.Default(ctx, kcp)).To(Succeed()) + _, err := webhook.ValidateCreate(ctx, kcp) g.Expect(err).ToNot(HaveOccurred()) fakeClient := newFakeClient(kcp.DeepCopy(), cluster.DeepCopy()) @@ -128,8 +130,9 @@ func TestKubeadmControlPlaneReconciler_updateStatusAllMachinesNotReady(t *testin }, }, } - kcp.Default() - _, err := kcp.ValidateCreate() + webhook := &webhooks.KubeadmControlPlane{} + g.Expect(webhook.Default(ctx, kcp)).To(Succeed()) + _, err := webhook.ValidateCreate(ctx, kcp) g.Expect(err).ToNot(HaveOccurred()) machines := map[string]*clusterv1.Machine{} @@ -201,8 +204,9 @@ func TestKubeadmControlPlaneReconciler_updateStatusAllMachinesReady(t *testing.T }, }, } - kcp.Default() - _, err := kcp.ValidateCreate() + webhook := &webhooks.KubeadmControlPlane{} + g.Expect(webhook.Default(ctx, kcp)).To(Succeed()) + _, err := webhook.ValidateCreate(ctx, kcp) g.Expect(err).ToNot(HaveOccurred()) objs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy(), kubeadmConfigMap()} @@ -282,8 +286,9 @@ func TestKubeadmControlPlaneReconciler_updateStatusMachinesReadyMixed(t *testing }, }, } - kcp.Default() - _, err := kcp.ValidateCreate() + webhook := &webhooks.KubeadmControlPlane{} + g.Expect(webhook.Default(ctx, kcp)).To(Succeed()) + _, err := webhook.ValidateCreate(ctx, kcp) g.Expect(err).ToNot(HaveOccurred()) machines := map[string]*clusterv1.Machine{} objs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy()} @@ -363,8 +368,9 @@ func TestKubeadmControlPlaneReconciler_machinesCreatedIsIsTrueEvenWhenTheNodesAr }, }, } - kcp.Default() - _, err := kcp.ValidateCreate() + webhook := &webhooks.KubeadmControlPlane{} + g.Expect(webhook.Default(ctx, kcp)).To(Succeed()) + _, err := webhook.ValidateCreate(ctx, kcp) g.Expect(err).ToNot(HaveOccurred()) machines := map[string]*clusterv1.Machine{} objs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy()} diff --git a/controlplane/kubeadm/main.go b/controlplane/kubeadm/main.go index ada11277e0dd..289e84a1d98f 100644 --- a/controlplane/kubeadm/main.go +++ b/controlplane/kubeadm/main.go @@ -57,6 +57,7 @@ import ( "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/util/flags" "sigs.k8s.io/cluster-api/version" + "sigs.k8s.io/cluster-api/webhooks" ) var ( @@ -342,7 +343,7 @@ func setupReconcilers(ctx context.Context, mgr ctrl.Manager) { } func setupWebhooks(mgr ctrl.Manager) { - if err := (&controlplanev1.KubeadmControlPlane{}).SetupWebhookWithManager(mgr); err != nil { + if err := (&webhooks.KubeadmControlPlane{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "KubeadmControlPlane") os.Exit(1) } @@ -354,7 +355,7 @@ func setupWebhooks(mgr ctrl.Manager) { os.Exit(1) } - if err := (&controlplanev1.KubeadmControlPlaneTemplate{}).SetupWebhookWithManager(mgr); err != nil { + if err := (&webhooks.KubeadmControlPlaneTemplate{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "KubeadmControlPlaneTemplate") os.Exit(1) } diff --git a/internal/test/envtest/environment.go b/internal/test/envtest/environment.go index e6028ce1f0ca..4ed1dab22b20 100644 --- a/internal/test/envtest/environment.go +++ b/internal/test/envtest/environment.go @@ -300,7 +300,10 @@ func newEnvironment(uncachedObjs ...client.Object) *Environment { if err := (&bootstrapv1.KubeadmConfigTemplate{}).SetupWebhookWithManager(mgr); err != nil { klog.Fatalf("unable to create webhook: %+v", err) } - if err := (&controlplanev1.KubeadmControlPlane{}).SetupWebhookWithManager(mgr); err != nil { + if err := (&webhooks.KubeadmControlPlaneTemplate{}).SetupWebhookWithManager(mgr); err != nil { + klog.Fatalf("unable to create webhook: %+v", err) + } + if err := (&webhooks.KubeadmControlPlane{}).SetupWebhookWithManager(mgr); err != nil { klog.Fatalf("unable to create webhook: %+v", err) } if err := (&addonsv1.ClusterResourceSet{}).SetupWebhookWithManager(mgr); err != nil { diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go b/internal/webhooks/kubeadm_control_plane.go similarity index 80% rename from controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go rename to internal/webhooks/kubeadm_control_plane.go index 84b55b454e44..48ad13cb06e0 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go +++ b/internal/webhooks/kubeadm_control_plane.go @@ -14,9 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1beta1 +package webhooks import ( + "context" "encoding/json" "fmt" "strings" @@ -33,30 +34,44 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" + controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" "sigs.k8s.io/cluster-api/internal/util/kubeadm" "sigs.k8s.io/cluster-api/util/container" "sigs.k8s.io/cluster-api/util/version" ) -func (in *KubeadmControlPlane) SetupWebhookWithManager(mgr ctrl.Manager) error { +func (webhook *KubeadmControlPlane) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). - For(in). + For(&controlplanev1.KubeadmControlPlane{}). + WithDefaulter(webhook). + WithValidator(webhook). Complete() } // +kubebuilder:webhook:verbs=create;update,path=/mutate-controlplane-cluster-x-k8s-io-v1beta1-kubeadmcontrolplane,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=controlplane.cluster.x-k8s.io,resources=kubeadmcontrolplanes,versions=v1beta1,name=default.kubeadmcontrolplane.controlplane.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 // +kubebuilder:webhook:verbs=create;update,path=/validate-controlplane-cluster-x-k8s-io-v1beta1-kubeadmcontrolplane,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=controlplane.cluster.x-k8s.io,resources=kubeadmcontrolplanes,versions=v1beta1,name=validation.kubeadmcontrolplane.controlplane.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 -var _ webhook.Defaulter = &KubeadmControlPlane{} -var _ webhook.Validator = &KubeadmControlPlane{} +// KubeadmControlPlane implements a validation and defaulting webhook for KubeadmControlPlane. +type KubeadmControlPlane struct{} + +var _ webhook.CustomValidator = &KubeadmControlPlane{} +var _ webhook.CustomDefaulter = &KubeadmControlPlane{} // Default implements webhook.Defaulter so a webhook will be registered for the type. -func (in *KubeadmControlPlane) Default() { - defaultKubeadmControlPlaneSpec(&in.Spec, in.Namespace) +func (webhook *KubeadmControlPlane) Default(_ context.Context, obj runtime.Object) error { + k, ok := obj.(*controlplanev1.KubeadmControlPlane) + if !ok { + return apierrors.NewBadRequest(fmt.Sprintf("expected a KubeadmControlPlane but got a %T", obj)) + } + + defaultKubeadmControlPlaneSpec(&k.Spec, k.Namespace) + + return nil } -func defaultKubeadmControlPlaneSpec(s *KubeadmControlPlaneSpec, namespace string) { +func defaultKubeadmControlPlaneSpec(s *controlplanev1.KubeadmControlPlaneSpec, namespace string) { if s.Replicas == nil { replicas := int32(1) s.Replicas = &replicas @@ -75,21 +90,21 @@ func defaultKubeadmControlPlaneSpec(s *KubeadmControlPlaneSpec, namespace string s.RolloutStrategy = defaultRolloutStrategy(s.RolloutStrategy) } -func defaultRolloutStrategy(rolloutStrategy *RolloutStrategy) *RolloutStrategy { +func defaultRolloutStrategy(rolloutStrategy *controlplanev1.RolloutStrategy) *controlplanev1.RolloutStrategy { ios1 := intstr.FromInt(1) if rolloutStrategy == nil { - rolloutStrategy = &RolloutStrategy{} + rolloutStrategy = &controlplanev1.RolloutStrategy{} } // Enforce RollingUpdate strategy and default MaxSurge if not set. if rolloutStrategy != nil { if len(rolloutStrategy.Type) == 0 { - rolloutStrategy.Type = RollingUpdateStrategyType + rolloutStrategy.Type = controlplanev1.RollingUpdateStrategyType } - if rolloutStrategy.Type == RollingUpdateStrategyType { + if rolloutStrategy.Type == controlplanev1.RollingUpdateStrategyType { if rolloutStrategy.RollingUpdate == nil { - rolloutStrategy.RollingUpdate = &RollingUpdate{} + rolloutStrategy.RollingUpdate = &controlplanev1.RollingUpdate{} } rolloutStrategy.RollingUpdate.MaxSurge = intstr.ValueOrDefault(rolloutStrategy.RollingUpdate.MaxSurge, ios1) } @@ -99,13 +114,18 @@ func defaultRolloutStrategy(rolloutStrategy *RolloutStrategy) *RolloutStrategy { } // ValidateCreate implements webhook.Validator so a webhook will be registered for the type. -func (in *KubeadmControlPlane) ValidateCreate() (admission.Warnings, error) { - spec := in.Spec - allErrs := validateKubeadmControlPlaneSpec(spec, in.Namespace, field.NewPath("spec")) - allErrs = append(allErrs, validateClusterConfiguration(spec.KubeadmConfigSpec.ClusterConfiguration, nil, field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration"))...) +func (webhook *KubeadmControlPlane) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { + k, ok := obj.(*controlplanev1.KubeadmControlPlane) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a KubeadmControlPlane but got a %T", obj)) + } + + spec := k.Spec + allErrs := validateKubeadmControlPlaneSpec(spec, k.Namespace, field.NewPath("spec")) + allErrs = append(allErrs, validateClusterConfiguration(nil, spec.KubeadmConfigSpec.ClusterConfiguration, field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration"))...) allErrs = append(allErrs, spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "kubeadmConfigSpec"))...) if len(allErrs) > 0 { - return nil, apierrors.NewInvalid(GroupVersion.WithKind("KubeadmControlPlane").GroupKind(), in.Name, allErrs) + return nil, apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("KubeadmControlPlane").GroupKind(), k.Name, allErrs) } return nil, nil } @@ -135,7 +155,7 @@ const ( const minimumCertificatesExpiryDays = 7 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. -func (in *KubeadmControlPlane) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { +func (webhook *KubeadmControlPlane) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { // add a * to indicate everything beneath is ok. // For example, {"spec", "*"} will allow any path under "spec" to change. allowedPaths := [][]string{ @@ -194,18 +214,23 @@ func (in *KubeadmControlPlane) ValidateUpdate(old runtime.Object) (admission.War {spec, "rolloutStrategy", "*"}, } - allErrs := validateKubeadmControlPlaneSpec(in.Spec, in.Namespace, field.NewPath("spec")) + oldK, ok := oldObj.(*controlplanev1.KubeadmControlPlane) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a KubeadmControlPlane but got a %T", oldObj)) + } - prev, ok := old.(*KubeadmControlPlane) + newK, ok := newObj.(*controlplanev1.KubeadmControlPlane) if !ok { - return nil, apierrors.NewBadRequest(fmt.Sprintf("expecting KubeadmControlPlane but got a %T", old)) + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a KubeadmControlPlane but got a %T", newObj)) } - originalJSON, err := json.Marshal(prev) + allErrs := validateKubeadmControlPlaneSpec(newK.Spec, newK.Namespace, field.NewPath("spec")) + + originalJSON, err := json.Marshal(oldK) if err != nil { return nil, apierrors.NewInternalError(err) } - modifiedJSON, err := json.Marshal(in) + modifiedJSON, err := json.Marshal(newK) if err != nil { return nil, apierrors.NewInternalError(err) } @@ -235,19 +260,19 @@ func (in *KubeadmControlPlane) ValidateUpdate(old runtime.Object) (admission.War } } - allErrs = append(allErrs, in.validateVersion(prev.Spec.Version)...) - allErrs = append(allErrs, validateClusterConfiguration(in.Spec.KubeadmConfigSpec.ClusterConfiguration, prev.Spec.KubeadmConfigSpec.ClusterConfiguration, field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration"))...) - allErrs = append(allErrs, in.validateCoreDNSVersion(prev)...) - allErrs = append(allErrs, in.Spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "kubeadmConfigSpec"))...) + allErrs = append(allErrs, webhook.validateVersion(oldK, newK)...) + allErrs = append(allErrs, validateClusterConfiguration(oldK.Spec.KubeadmConfigSpec.ClusterConfiguration, newK.Spec.KubeadmConfigSpec.ClusterConfiguration, field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration"))...) + allErrs = append(allErrs, webhook.validateCoreDNSVersion(oldK, newK)...) + allErrs = append(allErrs, newK.Spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "kubeadmConfigSpec"))...) if len(allErrs) > 0 { - return nil, apierrors.NewInvalid(GroupVersion.WithKind("KubeadmControlPlane").GroupKind(), in.Name, allErrs) + return nil, apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("KubeadmControlPlane").GroupKind(), newK.Name, allErrs) } return nil, nil } -func validateKubeadmControlPlaneSpec(s KubeadmControlPlaneSpec, namespace string, pathPrefix *field.Path) field.ErrorList { +func validateKubeadmControlPlaneSpec(s controlplanev1.KubeadmControlPlaneSpec, namespace string, pathPrefix *field.Path) field.ErrorList { allErrs := field.ErrorList{} if s.Replicas == nil { @@ -344,7 +369,7 @@ func validateKubeadmControlPlaneSpec(s KubeadmControlPlaneSpec, namespace string return allErrs } -func validateRolloutBefore(rolloutBefore *RolloutBefore, pathPrefix *field.Path) field.ErrorList { +func validateRolloutBefore(rolloutBefore *controlplanev1.RolloutBefore, pathPrefix *field.Path) field.ErrorList { allErrs := field.ErrorList{} if rolloutBefore == nil { @@ -360,14 +385,14 @@ func validateRolloutBefore(rolloutBefore *RolloutBefore, pathPrefix *field.Path) return allErrs } -func validateRolloutStrategy(rolloutStrategy *RolloutStrategy, replicas *int32, pathPrefix *field.Path) field.ErrorList { +func validateRolloutStrategy(rolloutStrategy *controlplanev1.RolloutStrategy, replicas *int32, pathPrefix *field.Path) field.ErrorList { allErrs := field.ErrorList{} if rolloutStrategy == nil { return allErrs } - if rolloutStrategy.Type != RollingUpdateStrategyType { + if rolloutStrategy.Type != controlplanev1.RollingUpdateStrategyType { allErrs = append( allErrs, field.Required( @@ -403,7 +428,7 @@ func validateRolloutStrategy(rolloutStrategy *RolloutStrategy, replicas *int32, return allErrs } -func validateClusterConfiguration(newClusterConfiguration, oldClusterConfiguration *bootstrapv1.ClusterConfiguration, pathPrefix *field.Path) field.ErrorList { +func validateClusterConfiguration(oldClusterConfiguration, newClusterConfiguration *bootstrapv1.ClusterConfiguration, pathPrefix *field.Path) field.ErrorList { allErrs := field.ErrorList{} if newClusterConfiguration == nil { @@ -530,22 +555,22 @@ func paths(path []string, diff map[string]interface{}) [][]string { return allPaths } -func (in *KubeadmControlPlane) validateCoreDNSVersion(prev *KubeadmControlPlane) (allErrs field.ErrorList) { - if in.Spec.KubeadmConfigSpec.ClusterConfiguration == nil || prev.Spec.KubeadmConfigSpec.ClusterConfiguration == nil { +func (webhook *KubeadmControlPlane) validateCoreDNSVersion(oldK, newK *controlplanev1.KubeadmControlPlane) (allErrs field.ErrorList) { + if newK.Spec.KubeadmConfigSpec.ClusterConfiguration == nil || oldK.Spec.KubeadmConfigSpec.ClusterConfiguration == nil { return allErrs } // return if either current or target versions is empty - if prev.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag == "" || in.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag == "" { + if newK.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag == "" || oldK.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag == "" { return allErrs } - targetDNS := &in.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS + targetDNS := &newK.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS - fromVersion, err := version.ParseMajorMinorPatchTolerant(prev.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag) + fromVersion, err := version.ParseMajorMinorPatchTolerant(oldK.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag) if err != nil { allErrs = append(allErrs, field.Invalid( field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration", "dns", "imageTag"), - prev.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag, + oldK.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag, fmt.Sprintf("failed to parse current CoreDNS version: %v", err), ), ) @@ -581,7 +606,8 @@ func (in *KubeadmControlPlane) validateCoreDNSVersion(prev *KubeadmControlPlane) return allErrs } -func (in *KubeadmControlPlane) validateVersion(previousVersion string) (allErrs field.ErrorList) { +func (webhook *KubeadmControlPlane) validateVersion(oldK, newK *controlplanev1.KubeadmControlPlane) (allErrs field.ErrorList) { + previousVersion := oldK.Spec.Version fromVersion, err := version.ParseMajorMinorPatch(previousVersion) if err != nil { allErrs = append(allErrs, @@ -593,12 +619,12 @@ func (in *KubeadmControlPlane) validateVersion(previousVersion string) (allErrs return allErrs } - toVersion, err := version.ParseMajorMinorPatch(in.Spec.Version) + toVersion, err := version.ParseMajorMinorPatch(newK.Spec.Version) if err != nil { allErrs = append(allErrs, field.InternalError( field.NewPath("spec", "version"), - errors.Wrapf(err, "failed to parse updated kubeadmcontrolplane version: %s", in.Spec.Version), + errors.Wrapf(err, "failed to parse updated kubeadmcontrolplane version: %s", newK.Spec.Version), ), ) return allErrs @@ -630,7 +656,7 @@ func (in *KubeadmControlPlane) validateVersion(previousVersion string) (allErrs allErrs = append(allErrs, field.Forbidden( field.NewPath("spec", "version"), - fmt.Sprintf("cannot update Kubernetes version from %s to %s", previousVersion, in.Spec.Version), + fmt.Sprintf("cannot update Kubernetes version from %s to %s", previousVersion, newK.Spec.Version), ), ) } @@ -643,8 +669,8 @@ func (in *KubeadmControlPlane) validateVersion(previousVersion string) (allErrs // given how the migration has been implemented in kubeadm. // // Block if imageRepository is not set (i.e. the default registry should be used), - if (in.Spec.KubeadmConfigSpec.ClusterConfiguration == nil || - in.Spec.KubeadmConfigSpec.ClusterConfiguration.ImageRepository == "") && + if (newK.Spec.KubeadmConfigSpec.ClusterConfiguration == nil || + newK.Spec.KubeadmConfigSpec.ClusterConfiguration.ImageRepository == "") && // the version changed (i.e. we have an upgrade), toVersion.NE(fromVersion) && // the version is >= v1.22.0 and < v1.26.0 @@ -664,6 +690,6 @@ func (in *KubeadmControlPlane) validateVersion(previousVersion string) (allErrs } // ValidateDelete implements webhook.Validator so a webhook will be registered for the type. -func (in *KubeadmControlPlane) ValidateDelete() (admission.Warnings, error) { +func (webhook *KubeadmControlPlane) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { return nil, nil } diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go b/internal/webhooks/kubeadm_control_plane_test.go similarity index 93% rename from controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go rename to internal/webhooks/kubeadm_control_plane_test.go index 3b26d0130fba..2787db40e94b 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go +++ b/internal/webhooks/kubeadm_control_plane_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1beta1 +package webhooks import ( "strings" @@ -29,27 +29,28 @@ import ( "k8s.io/utils/pointer" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" + controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" "sigs.k8s.io/cluster-api/feature" - utildefaulting "sigs.k8s.io/cluster-api/util/defaulting" + "sigs.k8s.io/cluster-api/internal/webhooks/util" ) func TestKubeadmControlPlaneDefault(t *testing.T) { g := NewWithT(t) - kcp := &KubeadmControlPlane{ + kcp := &controlplanev1.KubeadmControlPlane{ ObjectMeta: metav1.ObjectMeta{ Namespace: "foo", }, - Spec: KubeadmControlPlaneSpec{ + Spec: controlplanev1.KubeadmControlPlaneSpec{ Version: "v1.18.3", - MachineTemplate: KubeadmControlPlaneMachineTemplate{ + MachineTemplate: controlplanev1.KubeadmControlPlaneMachineTemplate{ InfrastructureRef: corev1.ObjectReference{ APIVersion: "test/v1alpha1", Kind: "UnknownInfraMachine", Name: "foo", }, }, - RolloutStrategy: &RolloutStrategy{}, + RolloutStrategy: &controlplanev1.RolloutStrategy{}, }, } updateDefaultingValidationKCP := kcp.DeepCopy() @@ -60,24 +61,25 @@ func TestKubeadmControlPlaneDefault(t *testing.T) { Name: "foo", Namespace: "foo", } - t.Run("for KubeadmControlPlane", utildefaulting.DefaultValidateTest(updateDefaultingValidationKCP)) - kcp.Default() + webhook := &KubeadmControlPlane{} + t.Run("for KubeadmControlPlane", util.CustomDefaultValidateTest(ctx, updateDefaultingValidationKCP, webhook)) + g.Expect(webhook.Default(ctx, kcp)).To(Succeed()) g.Expect(kcp.Spec.KubeadmConfigSpec.Format).To(Equal(bootstrapv1.CloudConfig)) g.Expect(kcp.Spec.MachineTemplate.InfrastructureRef.Namespace).To(Equal(kcp.Namespace)) g.Expect(kcp.Spec.Version).To(Equal("v1.18.3")) - g.Expect(kcp.Spec.RolloutStrategy.Type).To(Equal(RollingUpdateStrategyType)) + g.Expect(kcp.Spec.RolloutStrategy.Type).To(Equal(controlplanev1.RollingUpdateStrategyType)) g.Expect(kcp.Spec.RolloutStrategy.RollingUpdate.MaxSurge.IntVal).To(Equal(int32(1))) } func TestKubeadmControlPlaneValidateCreate(t *testing.T) { - valid := &KubeadmControlPlane{ + valid := &controlplanev1.KubeadmControlPlane{ ObjectMeta: metav1.ObjectMeta{ Name: "test", Namespace: "foo", }, - Spec: KubeadmControlPlaneSpec{ - MachineTemplate: KubeadmControlPlaneMachineTemplate{ + Spec: controlplanev1.KubeadmControlPlaneSpec{ + MachineTemplate: controlplanev1.KubeadmControlPlaneMachineTemplate{ InfrastructureRef: corev1.ObjectReference{ APIVersion: "test/v1alpha1", Kind: "UnknownInfraMachine", @@ -90,9 +92,9 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) { }, Replicas: pointer.Int32(1), Version: "v1.19.0", - RolloutStrategy: &RolloutStrategy{ - Type: RollingUpdateStrategyType, - RollingUpdate: &RollingUpdate{ + RolloutStrategy: &controlplanev1.RolloutStrategy{ + Type: controlplanev1.RollingUpdateStrategyType, + RollingUpdate: &controlplanev1.RollingUpdate{ MaxSurge: &intstr.IntOrString{ IntVal: 1, }, @@ -142,7 +144,7 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) { invalidCoreDNSVersion.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag = "v1.7" // not a valid semantic version invalidRolloutBeforeCertificateExpiryDays := valid.DeepCopy() - invalidRolloutBeforeCertificateExpiryDays.Spec.RolloutBefore = &RolloutBefore{ + invalidRolloutBeforeCertificateExpiryDays.Spec.RolloutBefore = &controlplanev1.RolloutBefore{ CertificatesExpiryDays: pointer.Int32(5), // less than minimum } @@ -167,7 +169,7 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) { name string enableIgnitionFeature bool expectErr bool - kcp *KubeadmControlPlane + kcp *controlplanev1.KubeadmControlPlane }{ { name: "should succeed when given a valid config", @@ -265,7 +267,9 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) { g := NewWithT(t) - warnings, err := tt.kcp.ValidateCreate() + webhook := &KubeadmControlPlane{} + + warnings, err := webhook.ValidateCreate(ctx, tt.kcp) if tt.expectErr { g.Expect(err).To(HaveOccurred()) } else { @@ -277,13 +281,13 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) { } func TestKubeadmControlPlaneValidateUpdate(t *testing.T) { - before := &KubeadmControlPlane{ + before := &controlplanev1.KubeadmControlPlane{ ObjectMeta: metav1.ObjectMeta{ Name: "test", Namespace: "foo", }, - Spec: KubeadmControlPlaneSpec{ - MachineTemplate: KubeadmControlPlaneMachineTemplate{ + Spec: controlplanev1.KubeadmControlPlaneSpec{ + MachineTemplate: controlplanev1.KubeadmControlPlaneMachineTemplate{ InfrastructureRef: corev1.ObjectReference{ APIVersion: "test/v1alpha1", Kind: "UnknownInfraMachine", @@ -295,9 +299,9 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) { NodeDeletionTimeout: &metav1.Duration{Duration: time.Second}, }, Replicas: pointer.Int32(1), - RolloutStrategy: &RolloutStrategy{ - Type: RollingUpdateStrategyType, - RollingUpdate: &RollingUpdate{ + RolloutStrategy: &controlplanev1.RolloutStrategy{ + Type: controlplanev1.RollingUpdateStrategyType, + RollingUpdate: &controlplanev1.RollingUpdate{ MaxSurge: &intstr.IntOrString{ IntVal: 1, }, @@ -357,7 +361,7 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) { }, }, Version: "v1.16.6", - RolloutBefore: &RolloutBefore{ + RolloutBefore: &controlplanev1.RolloutBefore{ CertificatesExpiryDays: pointer.Int32(7), }, }, @@ -426,10 +430,10 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) { validUpdate.Spec.Replicas = pointer.Int32(5) now := metav1.NewTime(time.Now()) validUpdate.Spec.RolloutAfter = &now - validUpdate.Spec.RolloutBefore = &RolloutBefore{ + validUpdate.Spec.RolloutBefore = &controlplanev1.RolloutBefore{ CertificatesExpiryDays: pointer.Int32(14), } - validUpdate.Spec.RemediationStrategy = &RemediationStrategy{ + validUpdate.Spec.RemediationStrategy = &controlplanev1.RemediationStrategy{ MaxRetry: pointer.Int32(50), MinHealthyPeriod: &metav1.Duration{Duration: 10 * time.Hour}, RetryPeriod: metav1.Duration{Duration: 10 * time.Minute}, @@ -637,7 +641,7 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) { disableNTPServers.Spec.KubeadmConfigSpec.NTP.Enabled = pointer.Bool(false) invalidRolloutBeforeCertificateExpiryDays := before.DeepCopy() - invalidRolloutBeforeCertificateExpiryDays.Spec.RolloutBefore = &RolloutBefore{ + invalidRolloutBeforeCertificateExpiryDays.Spec.RolloutBefore = &controlplanev1.RolloutBefore{ CertificatesExpiryDays: pointer.Int32(5), // less than minimum } @@ -707,8 +711,8 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) { name string enableIgnitionFeature bool expectErr bool - before *KubeadmControlPlane - kcp *KubeadmControlPlane + before *controlplanev1.KubeadmControlPlane + kcp *controlplanev1.KubeadmControlPlane }{ { name: "should succeed when given a valid config", @@ -1073,7 +1077,9 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) { g := NewWithT(t) - warnings, err := tt.kcp.ValidateUpdate(tt.before.DeepCopy()) + webhook := &KubeadmControlPlane{} + + warnings, err := webhook.ValidateUpdate(ctx, tt.before.DeepCopy(), tt.kcp) if tt.expectErr { g.Expect(err).To(HaveOccurred()) } else { @@ -1222,8 +1228,8 @@ func TestValidateVersion(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - kcp := KubeadmControlPlane{ - Spec: KubeadmControlPlaneSpec{ + kcpNew := controlplanev1.KubeadmControlPlane{ + Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ ClusterConfiguration: tt.clusterConfiguration, }, @@ -1231,7 +1237,18 @@ func TestValidateVersion(t *testing.T) { }, } - allErrs := kcp.validateVersion(tt.oldVersion) + kcpOld := controlplanev1.KubeadmControlPlane{ + Spec: controlplanev1.KubeadmControlPlaneSpec{ + KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ + ClusterConfiguration: tt.clusterConfiguration, + }, + Version: tt.oldVersion, + }, + } + + webhook := &KubeadmControlPlane{} + + allErrs := webhook.validateVersion(&kcpOld, &kcpNew) if tt.expectErr { g.Expect(allErrs).ToNot(BeEmpty()) } else { @@ -1241,14 +1258,16 @@ func TestValidateVersion(t *testing.T) { } } func TestKubeadmControlPlaneValidateUpdateAfterDefaulting(t *testing.T) { - before := &KubeadmControlPlane{ + g := NewWithT(t) + + before := &controlplanev1.KubeadmControlPlane{ ObjectMeta: metav1.ObjectMeta{ Name: "test", Namespace: "foo", }, - Spec: KubeadmControlPlaneSpec{ + Spec: controlplanev1.KubeadmControlPlaneSpec{ Version: "v1.19.0", - MachineTemplate: KubeadmControlPlaneMachineTemplate{ + MachineTemplate: controlplanev1.KubeadmControlPlaneMachineTemplate{ InfrastructureRef: corev1.ObjectReference{ APIVersion: "test/v1alpha1", Kind: "UnknownInfraMachine", @@ -1260,13 +1279,14 @@ func TestKubeadmControlPlaneValidateUpdateAfterDefaulting(t *testing.T) { } afterDefault := before.DeepCopy() - afterDefault.Default() + webhook := &KubeadmControlPlane{} + g.Expect(webhook.Default(ctx, afterDefault)).To(Succeed()) tests := []struct { name string expectErr bool - before *KubeadmControlPlane - kcp *KubeadmControlPlane + before *controlplanev1.KubeadmControlPlane + kcp *controlplanev1.KubeadmControlPlane }{ { name: "update should succeed after defaulting", @@ -1279,14 +1299,17 @@ func TestKubeadmControlPlaneValidateUpdateAfterDefaulting(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - warnings, err := tt.kcp.ValidateUpdate(tt.before.DeepCopy()) + + webhook := &KubeadmControlPlane{} + + warnings, err := webhook.ValidateUpdate(ctx, tt.before.DeepCopy(), tt.kcp) if tt.expectErr { g.Expect(err).To(HaveOccurred()) } else { g.Expect(err).To(Succeed()) g.Expect(tt.kcp.Spec.MachineTemplate.InfrastructureRef.Namespace).To(Equal(tt.before.Namespace)) g.Expect(tt.kcp.Spec.Version).To(Equal("v1.19.0")) - g.Expect(tt.kcp.Spec.RolloutStrategy.Type).To(Equal(RollingUpdateStrategyType)) + g.Expect(tt.kcp.Spec.RolloutStrategy.Type).To(Equal(controlplanev1.RollingUpdateStrategyType)) g.Expect(tt.kcp.Spec.RolloutStrategy.RollingUpdate.MaxSurge.IntVal).To(Equal(int32(1))) g.Expect(tt.kcp.Spec.Replicas).To(Equal(pointer.Int32(1))) } diff --git a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook.go b/internal/webhooks/kubeadmcontrolplanetemplate.go similarity index 61% rename from controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook.go rename to internal/webhooks/kubeadmcontrolplanetemplate.go index 02fe7da10fb3..dea55be392d3 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook.go +++ b/internal/webhooks/kubeadmcontrolplanetemplate.go @@ -14,9 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1beta1 +package webhooks import ( + "context" "fmt" "reflect" @@ -27,35 +28,52 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" + controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" "sigs.k8s.io/cluster-api/feature" ) const kubeadmControlPlaneTemplateImmutableMsg = "KubeadmControlPlaneTemplate spec.template.spec field is immutable. Please create new resource instead." -func (r *KubeadmControlPlaneTemplate) SetupWebhookWithManager(mgr ctrl.Manager) error { +func (webhook *KubeadmControlPlaneTemplate) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). - For(r). + For(&controlplanev1.KubeadmControlPlaneTemplate{}). + WithDefaulter(webhook). + WithValidator(webhook). Complete() } +// +kubebuilder:webhook:verbs=create;update,path=/validate-controlplane-cluster-x-k8s-io-v1beta1-kubeadmcontrolplanetemplate,mutating=false,failurePolicy=fail,groups=controlplane.cluster.x-k8s.io,resources=kubeadmcontrolplanetemplates,versions=v1beta1,name=validation.kubeadmcontrolplanetemplate.controlplane.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 // +kubebuilder:webhook:verbs=create;update,path=/mutate-controlplane-cluster-x-k8s-io-v1beta1-kubeadmcontrolplanetemplate,mutating=true,failurePolicy=fail,groups=controlplane.cluster.x-k8s.io,resources=kubeadmcontrolplanetemplates,versions=v1beta1,name=default.kubeadmcontrolplanetemplate.controlplane.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 -var _ webhook.Defaulter = &KubeadmControlPlaneTemplate{} +// KubeadmControlPlaneTemplate implements a validation and defaulting webhook for KubeadmControlPlaneTemplate. +type KubeadmControlPlaneTemplate struct{} + +var _ webhook.CustomValidator = &KubeadmControlPlaneTemplate{} +var _ webhook.CustomDefaulter = &KubeadmControlPlaneTemplate{} // Default implements webhook.Defaulter so a webhook will be registered for the type. -func (r *KubeadmControlPlaneTemplate) Default() { - bootstrapv1.DefaultKubeadmConfigSpec(&r.Spec.Template.Spec.KubeadmConfigSpec) +func (webhook *KubeadmControlPlaneTemplate) Default(_ context.Context, obj runtime.Object) error { + k, ok := obj.(*controlplanev1.KubeadmControlPlaneTemplate) + if !ok { + return apierrors.NewBadRequest(fmt.Sprintf("expected a KubeadmControlPlaneTemplate but got a %T", obj)) + } - r.Spec.Template.Spec.RolloutStrategy = defaultRolloutStrategy(r.Spec.Template.Spec.RolloutStrategy) -} + bootstrapv1.DefaultKubeadmConfigSpec(&k.Spec.Template.Spec.KubeadmConfigSpec) -// +kubebuilder:webhook:verbs=create;update,path=/validate-controlplane-cluster-x-k8s-io-v1beta1-kubeadmcontrolplanetemplate,mutating=false,failurePolicy=fail,groups=controlplane.cluster.x-k8s.io,resources=kubeadmcontrolplanetemplates,versions=v1beta1,name=validation.kubeadmcontrolplanetemplate.controlplane.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 + k.Spec.Template.Spec.RolloutStrategy = defaultRolloutStrategy(k.Spec.Template.Spec.RolloutStrategy) -var _ webhook.Validator = &KubeadmControlPlaneTemplate{} + return nil +} // ValidateCreate implements webhook.Validator so a webhook will be registered for the type. -func (r *KubeadmControlPlaneTemplate) ValidateCreate() (admission.Warnings, error) { +func (webhook *KubeadmControlPlaneTemplate) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { + k, ok := obj.(*controlplanev1.KubeadmControlPlaneTemplate) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a KubeadmControlPlaneTemplate but got a %T", obj)) + } + // NOTE: KubeadmControlPlaneTemplate is behind ClusterTopology feature gate flag; the web hook // must prevent creating new objects in case the feature flag is disabled. if !feature.Gates.Enabled(feature.ClusterTopology) { @@ -65,46 +83,52 @@ func (r *KubeadmControlPlaneTemplate) ValidateCreate() (admission.Warnings, erro ) } - spec := r.Spec.Template.Spec + spec := k.Spec.Template.Spec allErrs := validateKubeadmControlPlaneTemplateResourceSpec(spec, field.NewPath("spec", "template", "spec")) allErrs = append(allErrs, validateClusterConfiguration(spec.KubeadmConfigSpec.ClusterConfiguration, nil, field.NewPath("spec", "template", "spec", "kubeadmConfigSpec", "clusterConfiguration"))...) allErrs = append(allErrs, spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "template", "spec", "kubeadmConfigSpec"))...) // Validate the metadata of the KubeadmControlPlaneTemplateResource - allErrs = append(allErrs, r.Spec.Template.ObjectMeta.Validate(field.NewPath("spec", "template", "metadata"))...) + allErrs = append(allErrs, k.Spec.Template.ObjectMeta.Validate(field.NewPath("spec", "template", "metadata"))...) if len(allErrs) > 0 { - return nil, apierrors.NewInvalid(GroupVersion.WithKind("KubeadmControlPlaneTemplate").GroupKind(), r.Name, allErrs) + return nil, apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("KubeadmControlPlaneTemplate").GroupKind(), k.Name, allErrs) } return nil, nil } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. -func (r *KubeadmControlPlaneTemplate) ValidateUpdate(oldRaw runtime.Object) (admission.Warnings, error) { +func (webhook *KubeadmControlPlaneTemplate) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { var allErrs field.ErrorList - old, ok := oldRaw.(*KubeadmControlPlaneTemplate) + + oldK, ok := oldObj.(*controlplanev1.KubeadmControlPlaneTemplate) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a KubeadmControlPlaneTemplate but got a %T", oldObj)) + } + + newK, ok := newObj.(*controlplanev1.KubeadmControlPlaneTemplate) if !ok { - return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a KubeadmControlPlaneTemplate but got a %T", oldRaw)) + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a KubeadmControlPlaneTemplate but got a %T", newObj)) } - if !reflect.DeepEqual(r.Spec.Template.Spec, old.Spec.Template.Spec) { + if !reflect.DeepEqual(newK.Spec.Template.Spec, oldK.Spec.Template.Spec) { allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "template", "spec"), r, kubeadmControlPlaneTemplateImmutableMsg), + field.Invalid(field.NewPath("spec", "template", "spec"), webhook, kubeadmControlPlaneTemplateImmutableMsg), ) } if len(allErrs) == 0 { return nil, nil } - return nil, apierrors.NewInvalid(GroupVersion.WithKind("KubeadmControlPlaneTemplate").GroupKind(), r.Name, allErrs) + return nil, apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("KubeadmControlPlaneTemplate").GroupKind(), newK.Name, allErrs) } // ValidateDelete implements webhook.Validator so a webhook will be registered for the type. -func (r *KubeadmControlPlaneTemplate) ValidateDelete() (admission.Warnings, error) { +func (webhook *KubeadmControlPlaneTemplate) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { return nil, nil } // validateKubeadmControlPlaneTemplateResourceSpec is a copy of validateKubeadmControlPlaneSpec which // only validates the fields in KubeadmControlPlaneTemplateResourceSpec we care about. -func validateKubeadmControlPlaneTemplateResourceSpec(s KubeadmControlPlaneTemplateResourceSpec, pathPrefix *field.Path) field.ErrorList { +func validateKubeadmControlPlaneTemplateResourceSpec(s controlplanev1.KubeadmControlPlaneTemplateResourceSpec, pathPrefix *field.Path) field.ErrorList { allErrs := field.ErrorList{} allErrs = append(allErrs, validateRolloutBefore(s.RolloutBefore, pathPrefix.Child("rolloutBefore"))...) diff --git a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook_test.go b/internal/webhooks/kubeadmcontrolplanetemplate_test.go similarity index 65% rename from controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook_test.go rename to internal/webhooks/kubeadmcontrolplanetemplate_test.go index d02e46360715..3780ec0bb190 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook_test.go +++ b/internal/webhooks/kubeadmcontrolplanetemplate_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1beta1 +package webhooks import ( "strings" @@ -27,8 +27,9 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" + controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" "sigs.k8s.io/cluster-api/feature" - utildefaulting "sigs.k8s.io/cluster-api/util/defaulting" + "sigs.k8s.io/cluster-api/internal/webhooks/util" ) func TestKubeadmControlPlaneTemplateDefault(t *testing.T) { @@ -36,14 +37,14 @@ func TestKubeadmControlPlaneTemplateDefault(t *testing.T) { g := NewWithT(t) - kcpTemplate := &KubeadmControlPlaneTemplate{ + kcpTemplate := &controlplanev1.KubeadmControlPlaneTemplate{ ObjectMeta: metav1.ObjectMeta{ Namespace: "foo", }, - Spec: KubeadmControlPlaneTemplateSpec{ - Template: KubeadmControlPlaneTemplateResource{ - Spec: KubeadmControlPlaneTemplateResourceSpec{ - MachineTemplate: &KubeadmControlPlaneTemplateMachineTemplate{ + Spec: controlplanev1.KubeadmControlPlaneTemplateSpec{ + Template: controlplanev1.KubeadmControlPlaneTemplateResource{ + Spec: controlplanev1.KubeadmControlPlaneTemplateResourceSpec{ + MachineTemplate: &controlplanev1.KubeadmControlPlaneTemplateMachineTemplate{ NodeDrainTimeout: &metav1.Duration{Duration: 10 * time.Second}, }, }, @@ -52,11 +53,12 @@ func TestKubeadmControlPlaneTemplateDefault(t *testing.T) { } updateDefaultingValidationKCPTemplate := kcpTemplate.DeepCopy() updateDefaultingValidationKCPTemplate.Spec.Template.Spec.MachineTemplate.NodeDrainTimeout = &metav1.Duration{Duration: 20 * time.Second} - t.Run("for KubeadmControlPlaneTemplate", utildefaulting.DefaultValidateTest(updateDefaultingValidationKCPTemplate)) - kcpTemplate.Default() + webhook := &KubeadmControlPlaneTemplate{} + t.Run("for KubeadmControlPlaneTemplate", util.CustomDefaultValidateTest(ctx, updateDefaultingValidationKCPTemplate, webhook)) + g.Expect(webhook.Default(ctx, kcpTemplate)).To(Succeed()) g.Expect(kcpTemplate.Spec.Template.Spec.KubeadmConfigSpec.Format).To(Equal(bootstrapv1.CloudConfig)) - g.Expect(kcpTemplate.Spec.Template.Spec.RolloutStrategy.Type).To(Equal(RollingUpdateStrategyType)) + g.Expect(kcpTemplate.Spec.Template.Spec.RolloutStrategy.Type).To(Equal(controlplanev1.RollingUpdateStrategyType)) g.Expect(kcpTemplate.Spec.Template.Spec.RolloutStrategy.RollingUpdate.MaxSurge.IntVal).To(Equal(int32(1))) } @@ -66,22 +68,23 @@ func TestKubeadmControlPlaneTemplateValidationFeatureGateEnabled(t *testing.T) { t.Run("create kubeadmcontrolplanetemplate should pass if gate enabled and valid kubeadmcontrolplanetemplate", func(t *testing.T) { testnamespace := "test" g := NewWithT(t) - kcpTemplate := &KubeadmControlPlaneTemplate{ + kcpTemplate := &controlplanev1.KubeadmControlPlaneTemplate{ ObjectMeta: metav1.ObjectMeta{ Name: "kubeadmcontrolplanetemplate-test", Namespace: testnamespace, }, - Spec: KubeadmControlPlaneTemplateSpec{ - Template: KubeadmControlPlaneTemplateResource{ - Spec: KubeadmControlPlaneTemplateResourceSpec{ - MachineTemplate: &KubeadmControlPlaneTemplateMachineTemplate{ + Spec: controlplanev1.KubeadmControlPlaneTemplateSpec{ + Template: controlplanev1.KubeadmControlPlaneTemplateResource{ + Spec: controlplanev1.KubeadmControlPlaneTemplateResourceSpec{ + MachineTemplate: &controlplanev1.KubeadmControlPlaneTemplateMachineTemplate{ NodeDrainTimeout: &metav1.Duration{Duration: time.Second}, }, }, }, }, } - warnings, err := kcpTemplate.ValidateCreate() + webhook := &KubeadmControlPlaneTemplate{} + warnings, err := webhook.ValidateCreate(ctx, kcpTemplate) g.Expect(err).ToNot(HaveOccurred()) g.Expect(warnings).To(BeEmpty()) }) @@ -92,22 +95,23 @@ func TestKubeadmControlPlaneTemplateValidationFeatureGateDisabled(t *testing.T) t.Run("create kubeadmcontrolplanetemplate should not pass if gate disabled and valid kubeadmcontrolplanetemplate", func(t *testing.T) { testnamespace := "test" g := NewWithT(t) - kcpTemplate := &KubeadmControlPlaneTemplate{ + kcpTemplate := &controlplanev1.KubeadmControlPlaneTemplate{ ObjectMeta: metav1.ObjectMeta{ Name: "kubeadmcontrolplanetemplate-test", Namespace: testnamespace, }, - Spec: KubeadmControlPlaneTemplateSpec{ - Template: KubeadmControlPlaneTemplateResource{ - Spec: KubeadmControlPlaneTemplateResourceSpec{ - MachineTemplate: &KubeadmControlPlaneTemplateMachineTemplate{ + Spec: controlplanev1.KubeadmControlPlaneTemplateSpec{ + Template: controlplanev1.KubeadmControlPlaneTemplateResource{ + Spec: controlplanev1.KubeadmControlPlaneTemplateResourceSpec{ + MachineTemplate: &controlplanev1.KubeadmControlPlaneTemplateMachineTemplate{ NodeDrainTimeout: &metav1.Duration{Duration: time.Second}, }, }, }, }, } - warnings, err := kcpTemplate.ValidateCreate() + webhook := &KubeadmControlPlaneTemplate{} + warnings, err := webhook.ValidateCreate(ctx, kcpTemplate) g.Expect(err).To(HaveOccurred()) g.Expect(warnings).To(BeEmpty()) }) @@ -116,9 +120,9 @@ func TestKubeadmControlPlaneTemplateValidationFeatureGateDisabled(t *testing.T) func TestKubeadmControlPlaneTemplateValidationMetadata(t *testing.T) { t.Run("create kubeadmcontrolplanetemplate should not pass if metadata is invalid", func(t *testing.T) { g := NewWithT(t) - kcpTemplate := &KubeadmControlPlaneTemplate{ - Spec: KubeadmControlPlaneTemplateSpec{ - Template: KubeadmControlPlaneTemplateResource{ + kcpTemplate := &controlplanev1.KubeadmControlPlaneTemplate{ + Spec: controlplanev1.KubeadmControlPlaneTemplateSpec{ + Template: controlplanev1.KubeadmControlPlaneTemplateResource{ ObjectMeta: clusterv1.ObjectMeta{ Labels: map[string]string{ "foo": "$invalid-key", @@ -129,8 +133,8 @@ func TestKubeadmControlPlaneTemplateValidationMetadata(t *testing.T) { "/invalid-key": "foo", }, }, - Spec: KubeadmControlPlaneTemplateResourceSpec{ - MachineTemplate: &KubeadmControlPlaneTemplateMachineTemplate{ + Spec: controlplanev1.KubeadmControlPlaneTemplateResourceSpec{ + MachineTemplate: &controlplanev1.KubeadmControlPlaneTemplateMachineTemplate{ ObjectMeta: clusterv1.ObjectMeta{ Labels: map[string]string{ "foo": "$invalid-key", @@ -146,7 +150,8 @@ func TestKubeadmControlPlaneTemplateValidationMetadata(t *testing.T) { }, }, } - warnings, err := kcpTemplate.ValidateCreate() + webhook := &KubeadmControlPlaneTemplate{} + warnings, err := webhook.ValidateCreate(ctx, kcpTemplate) g.Expect(err).To(HaveOccurred()) g.Expect(warnings).To(BeEmpty()) }) diff --git a/webhooks/alias.go b/webhooks/alias.go index ee0646bdc19e..35b5e5ae1ddd 100644 --- a/webhooks/alias.go +++ b/webhooks/alias.go @@ -83,3 +83,19 @@ type MachineHealthCheck struct{} func (webhook *MachineHealthCheck) SetupWebhookWithManager(mgr ctrl.Manager) error { return (&webhooks.MachineHealthCheck{}).SetupWebhookWithManager(mgr) } + +// KubeadmControlPlane implements a validating and defaulting webhook for KubeadmControlPlane. +type KubeadmControlPlane struct{} + +// SetupWebhookWithManager sets up KubeadmControlPlane webhooks. +func (webhook *KubeadmControlPlane) SetupWebhookWithManager(mgr ctrl.Manager) error { + return (&webhooks.KubeadmControlPlane{}).SetupWebhookWithManager(mgr) +} + +// KubeadmControlPlaneTemplate implements a validating and defaulting webhook for KubeadmControlPlaneTemplate. +type KubeadmControlPlaneTemplate struct{} + +// SetupWebhookWithManager sets up KubeadmControlPlaneTemplate webhooks. +func (webhook *KubeadmControlPlaneTemplate) SetupWebhookWithManager(mgr ctrl.Manager) error { + return (&webhooks.KubeadmControlPlaneTemplate{}).SetupWebhookWithManager(mgr) +}