From f1fa4e1b9dee64eda25365dbd36b80299ce86466 Mon Sep 17 00:00:00 2001 From: swamyan Date: Fri, 15 Sep 2023 15:02:22 +0530 Subject: [PATCH] Move experimental addons API v1beta1 webhooks to separate package --- Makefile | 1 + .../api/v1beta1/clusterresourceset_webhook.go | 118 ------------- .../clusterresourcesetbinding_webhook.go | 79 --------- .../webhooks/clusterresourceset_webhook.go | 163 ++++++++++++++++++ .../clusterresourceset_webhook_test.go | 63 ++++--- .../clusterresourcesetbinding_webhook.go | 99 +++++++++++ .../clusterresourcesetbinding_webhook_test.go | 19 +- exp/addons/internal/webhooks/doc.go | 18 ++ exp/addons/webhooks/alias.go | 39 +++++ exp/addons/webhooks/doc.go | 18 ++ internal/test/envtest/environment.go | 5 +- main.go | 5 +- 12 files changed, 391 insertions(+), 236 deletions(-) delete mode 100644 exp/addons/api/v1beta1/clusterresourceset_webhook.go delete mode 100644 exp/addons/api/v1beta1/clusterresourcesetbinding_webhook.go create mode 100644 exp/addons/internal/webhooks/clusterresourceset_webhook.go rename exp/addons/{api/v1beta1 => internal/webhooks}/clusterresourceset_webhook_test.go (67%) create mode 100644 exp/addons/internal/webhooks/clusterresourcesetbinding_webhook.go rename exp/addons/{api/v1beta1 => internal/webhooks}/clusterresourcesetbinding_webhook_test.go (75%) create mode 100644 exp/addons/internal/webhooks/doc.go create mode 100644 exp/addons/webhooks/alias.go create mode 100644 exp/addons/webhooks/doc.go diff --git a/Makefile b/Makefile index 45e91dd4a9a7..0a261b2d6da2 100644 --- a/Makefile +++ b/Makefile @@ -287,6 +287,7 @@ generate-manifests-core: $(CONTROLLER_GEN) $(KUSTOMIZE) ## Generate manifests e. paths=./$(EXP_DIR)/internal/webhooks/... \ paths=./$(EXP_DIR)/addons/api/... \ paths=./$(EXP_DIR)/addons/internal/controllers/... \ + paths=./$(EXP_DIR)/addons/internal/webhooks/... \ paths=./$(EXP_DIR)/ipam/api/... \ paths=./$(EXP_DIR)/ipam/internal/webhooks/... \ paths=./$(EXP_DIR)/runtime/api/... \ diff --git a/exp/addons/api/v1beta1/clusterresourceset_webhook.go b/exp/addons/api/v1beta1/clusterresourceset_webhook.go deleted file mode 100644 index 61f8f6dc5c02..000000000000 --- a/exp/addons/api/v1beta1/clusterresourceset_webhook.go +++ /dev/null @@ -1,118 +0,0 @@ -/* -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 v1beta1 - -import ( - "fmt" - "reflect" - - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "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" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "sigs.k8s.io/cluster-api/feature" -) - -func (m *ClusterResourceSet) SetupWebhookWithManager(mgr ctrl.Manager) error { - return ctrl.NewWebhookManagedBy(mgr). - For(m). - Complete() -} - -// +kubebuilder:webhook:verbs=create;update,path=/validate-addons-cluster-x-k8s-io-v1beta1-clusterresourceset,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=addons.cluster.x-k8s.io,resources=clusterresourcesets,versions=v1beta1,name=validation.clusterresourceset.addons.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 -// +kubebuilder:webhook:verbs=create;update,path=/mutate-addons-cluster-x-k8s-io-v1beta1-clusterresourceset,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=addons.cluster.x-k8s.io,resources=clusterresourcesets,versions=v1beta1,name=default.clusterresourceset.addons.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 - -var _ webhook.Defaulter = &ClusterResourceSet{} -var _ webhook.Validator = &ClusterResourceSet{} - -// Default implements webhook.Defaulter so a webhook will be registered for the type. -func (m *ClusterResourceSet) Default() { - // ClusterResourceSet Strategy defaults to ApplyOnce. - if m.Spec.Strategy == "" { - m.Spec.Strategy = string(ClusterResourceSetStrategyApplyOnce) - } -} - -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type. -func (m *ClusterResourceSet) ValidateCreate() (admission.Warnings, error) { - return nil, m.validate(nil) -} - -// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. -func (m *ClusterResourceSet) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { - oldCRS, ok := old.(*ClusterResourceSet) - if !ok { - return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterResourceSet but got a %T", old)) - } - return nil, m.validate(oldCRS) -} - -// ValidateDelete implements webhook.Validator so a webhook will be registered for the type. -func (m *ClusterResourceSet) ValidateDelete() (admission.Warnings, error) { - return nil, nil -} - -func (m *ClusterResourceSet) validate(old *ClusterResourceSet) error { - // NOTE: ClusterResourceSet is behind ClusterResourceSet feature gate flag; the web hook - // must prevent creating new objects when the feature flag is disabled. - if !feature.Gates.Enabled(feature.ClusterResourceSet) { - return field.Forbidden( - field.NewPath("spec"), - "can be set only if the ClusterResourceSet feature flag is enabled", - ) - } - var allErrs field.ErrorList - // Validate selector parses as Selector - selector, err := metav1.LabelSelectorAsSelector(&m.Spec.ClusterSelector) - if err != nil { - allErrs = append( - allErrs, - field.Invalid(field.NewPath("spec", "clusterSelector"), m.Spec.ClusterSelector, err.Error()), - ) - } - - // Validate that the selector isn't empty as null selectors do not select any objects. - if selector != nil && selector.Empty() { - allErrs = append( - allErrs, - field.Invalid(field.NewPath("spec", "clusterSelector"), m.Spec.ClusterSelector, "selector must not be empty"), - ) - } - - if old != nil && old.Spec.Strategy != "" && old.Spec.Strategy != m.Spec.Strategy { - allErrs = append( - allErrs, - field.Invalid(field.NewPath("spec", "strategy"), m.Spec.Strategy, "field is immutable"), - ) - } - - if old != nil && !reflect.DeepEqual(old.Spec.ClusterSelector, m.Spec.ClusterSelector) { - allErrs = append( - allErrs, - field.Invalid(field.NewPath("spec", "clusterSelector"), m.Spec.ClusterSelector, "field is immutable"), - ) - } - - if len(allErrs) == 0 { - return nil - } - return apierrors.NewInvalid(GroupVersion.WithKind("ClusterResourceSet").GroupKind(), m.Name, allErrs) -} diff --git a/exp/addons/api/v1beta1/clusterresourcesetbinding_webhook.go b/exp/addons/api/v1beta1/clusterresourcesetbinding_webhook.go deleted file mode 100644 index 1f51795173b5..000000000000 --- a/exp/addons/api/v1beta1/clusterresourcesetbinding_webhook.go +++ /dev/null @@ -1,79 +0,0 @@ -/* -Copyright 2022 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 v1beta1 - -import ( - "fmt" - - apierrors "k8s.io/apimachinery/pkg/api/errors" - "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" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "sigs.k8s.io/cluster-api/feature" -) - -func (c *ClusterResourceSetBinding) SetupWebhookWithManager(mgr ctrl.Manager) error { - return ctrl.NewWebhookManagedBy(mgr). - For(c). - Complete() -} - -// +kubebuilder:webhook:verbs=create;update,path=/validate-addons-cluster-x-k8s-io-v1beta1-clusterresourcesetbinding,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=addons.cluster.x-k8s.io,resources=clusterresourcesetbindings,versions=v1beta1,name=validation.clusterresourcesetbinding.addons.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 - -var _ webhook.Validator = &ClusterResourceSetBinding{} - -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type. -func (c *ClusterResourceSetBinding) ValidateCreate() (admission.Warnings, error) { - return nil, c.validate(nil) -} - -// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. -func (c *ClusterResourceSetBinding) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { - oldBinding, ok := old.(*ClusterResourceSetBinding) - if !ok { - return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterResourceSetBinding but got a %T", old)) - } - return nil, c.validate(oldBinding) -} - -// ValidateDelete implements webhook.Validator so a webhook will be registered for the type. -func (c *ClusterResourceSetBinding) ValidateDelete() (admission.Warnings, error) { - return nil, nil -} - -func (c *ClusterResourceSetBinding) validate(old *ClusterResourceSetBinding) error { - // NOTE: ClusterResourceSet is behind ClusterResourceSet feature gate flag; the web hook - // must prevent creating new objects in case the feature flag is disabled. - if !feature.Gates.Enabled(feature.ClusterResourceSet) { - return field.Forbidden( - field.NewPath("spec"), - "can be set only if the ClusterResourceSet feature flag is enabled", - ) - } - var allErrs field.ErrorList - if old != nil && old.Spec.ClusterName != "" && old.Spec.ClusterName != c.Spec.ClusterName { - allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "clusterName"), c.Spec.ClusterName, "field is immutable")) - } - if len(allErrs) == 0 { - return nil - } - return apierrors.NewInvalid(GroupVersion.WithKind("ClusterResourceSetBinding").GroupKind(), c.Name, allErrs) -} diff --git a/exp/addons/internal/webhooks/clusterresourceset_webhook.go b/exp/addons/internal/webhooks/clusterresourceset_webhook.go new file mode 100644 index 000000000000..a0aff87ab1aa --- /dev/null +++ b/exp/addons/internal/webhooks/clusterresourceset_webhook.go @@ -0,0 +1,163 @@ +/* +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 webhooks + +import ( + "context" + "fmt" + "reflect" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "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" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1" + "sigs.k8s.io/cluster-api/feature" +) + +// ClusterResourceSet implements a validation and defaulting webhook for ClusterResourceSet. +type ClusterResourceSet struct{} + +func (webhook *ClusterResourceSet) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&addonsv1.ClusterResourceSet{}). + WithDefaulter(webhook). + WithValidator(webhook). + Complete() +} + +// +kubebuilder:webhook:verbs=create;update,path=/validate-addons-cluster-x-k8s-io-v1beta1-clusterresourceset,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=addons.cluster.x-k8s.io,resources=clusterresourcesets,versions=v1beta1,name=validation.clusterresourceset.addons.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 +// +kubebuilder:webhook:verbs=create;update,path=/mutate-addons-cluster-x-k8s-io-v1beta1-clusterresourceset,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=addons.cluster.x-k8s.io,resources=clusterresourcesets,versions=v1beta1,name=default.clusterresourceset.addons.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 + +var _ webhook.CustomDefaulter = &ClusterResourceSet{} +var _ webhook.CustomValidator = &ClusterResourceSet{} + +// Default implements webhook.Defaulter so a webhook will be registered for the type. +func (webhook *ClusterResourceSet) Default(_ context.Context, oldObj runtime.Object) error { + oldCRS, ok := oldObj.(*addonsv1.ClusterResourceSet) + if !ok { + return apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterResourceSet but got a %T", oldObj)) + } + // ClusterResourceSet Strategy defaults to ApplyOnce. + if oldCRS.Spec.Strategy == "" { + oldCRS.Spec.Strategy = string(addonsv1.ClusterResourceSetStrategyApplyOnce) + } + return nil +} + +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type. +func (webhook *ClusterResourceSet) ValidateCreate(_ context.Context, oldObj runtime.Object) (admission.Warnings, error) { + oldCRS, ok := oldObj.(*addonsv1.ClusterResourceSet) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterResourceSet but got a %T", oldObj)) + } + return nil, webhook.validate(oldCRS, nil) +} + +// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. +func (webhook *ClusterResourceSet) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { + oldCRS, ok := oldObj.(*addonsv1.ClusterResourceSet) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterResourceSet but got a %T", oldObj)) + } + newCRS, ok := newObj.(*addonsv1.ClusterResourceSet) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterResourceSet but got a %T", newObj)) + } + return nil, webhook.validate(oldCRS, newCRS) +} + +// ValidateDelete implements webhook.Validator so a webhook will be registered for the type. +func (webhook *ClusterResourceSet) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { + return nil, nil +} + +func (webhook *ClusterResourceSet) validate(oldCRS, newCRS *addonsv1.ClusterResourceSet) error { + // NOTE: ClusterResourceSet is behind ClusterResourceSet feature gate flag; the web hook + // must prevent creating newCRS objects when the feature flag is disabled. + if !feature.Gates.Enabled(feature.ClusterResourceSet) { + return field.Forbidden( + field.NewPath("spec"), + "can be set only if the ClusterResourceSet feature flag is enabled", + ) + } + var ( + allErrs field.ErrorList + selector labels.Selector + err error + ) + + // Validate selector parses as Selector + if newCRS != nil { + selector, err = metav1.LabelSelectorAsSelector(&newCRS.Spec.ClusterSelector) + if err != nil { + allErrs = append( + allErrs, + field.Invalid(field.NewPath("spec", "clusterSelector"), newCRS.Spec.ClusterSelector, err.Error()), + ) + } + } + + // Validate selector parses as Selector + if oldCRS != nil { + selector, err = metav1.LabelSelectorAsSelector(&oldCRS.Spec.ClusterSelector) + if err != nil { + allErrs = append( + allErrs, + field.Invalid(field.NewPath("spec", "clusterSelector"), oldCRS.Spec.ClusterSelector, err.Error()), + ) + } + } + + // Validate that the selector isn't empty as null selectors do not select any objects. + if selector != nil && selector.Empty() { + allErrs = append( + allErrs, + field.Invalid(field.NewPath("spec", "clusterSelector"), newCRS.Spec.ClusterSelector, "selector must not be empty"), + ) + } + + if oldCRS != nil && newCRS != nil && oldCRS.Spec.Strategy != "" && oldCRS.Spec.Strategy != newCRS.Spec.Strategy { + allErrs = append( + allErrs, + field.Invalid(field.NewPath("spec", "strategy"), newCRS.Spec.Strategy, "field is immutable"), + ) + } + + if oldCRS != nil && newCRS != nil && !reflect.DeepEqual(oldCRS.Spec.ClusterSelector, newCRS.Spec.ClusterSelector) { + allErrs = append( + allErrs, + field.Invalid(field.NewPath("spec", "clusterSelector"), newCRS.Spec.ClusterSelector, "field is immutable"), + ) + } + + if len(allErrs) == 0 { + return nil + } + if oldCRS != nil { + return apierrors.NewInvalid(addonsv1.GroupVersion.WithKind("ClusterResourceSet").GroupKind(), oldCRS.Name, allErrs) + } + if newCRS != nil { + return apierrors.NewInvalid(addonsv1.GroupVersion.WithKind("ClusterResourceSet").GroupKind(), newCRS.Name, allErrs) + } + return nil +} diff --git a/exp/addons/api/v1beta1/clusterresourceset_webhook_test.go b/exp/addons/internal/webhooks/clusterresourceset_webhook_test.go similarity index 67% rename from exp/addons/api/v1beta1/clusterresourceset_webhook_test.go rename to exp/addons/internal/webhooks/clusterresourceset_webhook_test.go index 525300b0cc43..68a59f07ad7c 100644 --- a/exp/addons/api/v1beta1/clusterresourceset_webhook_test.go +++ b/exp/addons/internal/webhooks/clusterresourceset_webhook_test.go @@ -14,28 +14,33 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1beta1 +package webhooks import ( "testing" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" - utildefaulting "sigs.k8s.io/cluster-api/util/defaulting" + addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1" + "sigs.k8s.io/cluster-api/internal/webhooks/util" ) +var ctx = ctrl.SetupSignalHandler() + func TestClusterResourcesetDefault(t *testing.T) { g := NewWithT(t) - clusterResourceSet := &ClusterResourceSet{} + clusterResourceSet := &addonsv1.ClusterResourceSet{} defaultingValidationCRS := clusterResourceSet.DeepCopy() defaultingValidationCRS.Spec.ClusterSelector = metav1.LabelSelector{ MatchLabels: map[string]string{"foo": "bar"}, } - t.Run("for ClusterResourceSet", utildefaulting.DefaultValidateTest(defaultingValidationCRS)) - clusterResourceSet.Default() + webhook := ClusterResourceSet{} + t.Run("for ClusterResourceSet", util.CustomDefaultValidateTest(ctx, defaultingValidationCRS, &webhook)) + g.Expect(webhook.Default(ctx, defaultingValidationCRS)).To(Succeed()) - g.Expect(clusterResourceSet.Spec.Strategy).To(Equal(string(ClusterResourceSetStrategyApplyOnce))) + g.Expect(defaultingValidationCRS.Spec.Strategy).To(Equal(string(addonsv1.ClusterResourceSetStrategyApplyOnce))) } func TestClusterResourceSetLabelSelectorAsSelectorValidation(t *testing.T) { @@ -59,25 +64,26 @@ func TestClusterResourceSetLabelSelectorAsSelectorValidation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - clusterResourceSet := &ClusterResourceSet{ - Spec: ClusterResourceSetSpec{ + clusterResourceSet := &addonsv1.ClusterResourceSet{ + Spec: addonsv1.ClusterResourceSetSpec{ ClusterSelector: metav1.LabelSelector{ MatchLabels: tt.selectors, }, }, } + webhook := ClusterResourceSet{} if tt.expectErr { - warnings, err := clusterResourceSet.ValidateCreate() + warnings, err := webhook.ValidateCreate(ctx, clusterResourceSet) g.Expect(err).To(HaveOccurred()) g.Expect(warnings).To(BeEmpty()) - warnings, err = clusterResourceSet.ValidateUpdate(clusterResourceSet) + warnings, err = webhook.ValidateUpdate(ctx, clusterResourceSet, clusterResourceSet) g.Expect(err).To(HaveOccurred()) g.Expect(warnings).To(BeEmpty()) } else { - warnings, err := clusterResourceSet.ValidateCreate() + warnings, err := webhook.ValidateCreate(ctx, clusterResourceSet) g.Expect(err).ToNot(HaveOccurred()) g.Expect(warnings).To(BeEmpty()) - warnings, err = clusterResourceSet.ValidateUpdate(clusterResourceSet) + warnings, err = webhook.ValidateUpdate(ctx, clusterResourceSet, clusterResourceSet) g.Expect(err).ToNot(HaveOccurred()) g.Expect(warnings).To(BeEmpty()) } @@ -94,13 +100,13 @@ func TestClusterResourceSetStrategyImmutable(t *testing.T) { }{ { name: "when the Strategy has not changed", - oldStrategy: string(ClusterResourceSetStrategyApplyOnce), - newStrategy: string(ClusterResourceSetStrategyApplyOnce), + oldStrategy: string(addonsv1.ClusterResourceSetStrategyApplyOnce), + newStrategy: string(addonsv1.ClusterResourceSetStrategyApplyOnce), expectErr: false, }, { name: "when the Strategy has changed", - oldStrategy: string(ClusterResourceSetStrategyApplyOnce), + oldStrategy: string(addonsv1.ClusterResourceSetStrategyApplyOnce), newStrategy: "", expectErr: true, }, @@ -110,8 +116,8 @@ func TestClusterResourceSetStrategyImmutable(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - newClusterResourceSet := &ClusterResourceSet{ - Spec: ClusterResourceSetSpec{ + newClusterResourceSet := &addonsv1.ClusterResourceSet{ + Spec: addonsv1.ClusterResourceSetSpec{ ClusterSelector: metav1.LabelSelector{ MatchLabels: map[string]string{ "test": "test", @@ -121,8 +127,8 @@ func TestClusterResourceSetStrategyImmutable(t *testing.T) { }, } - oldClusterResourceSet := &ClusterResourceSet{ - Spec: ClusterResourceSetSpec{ + oldClusterResourceSet := &addonsv1.ClusterResourceSet{ + Spec: addonsv1.ClusterResourceSetSpec{ ClusterSelector: metav1.LabelSelector{ MatchLabels: map[string]string{ "test": "test", @@ -131,8 +137,9 @@ func TestClusterResourceSetStrategyImmutable(t *testing.T) { Strategy: tt.oldStrategy, }, } + webhook := ClusterResourceSet{} - warnings, err := newClusterResourceSet.ValidateUpdate(oldClusterResourceSet) + warnings, err := webhook.ValidateUpdate(ctx, oldClusterResourceSet, newClusterResourceSet) if tt.expectErr { g.Expect(err).To(HaveOccurred()) g.Expect(warnings).To(BeEmpty()) @@ -169,23 +176,24 @@ func TestClusterResourceSetClusterSelectorImmutable(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - newClusterResourceSet := &ClusterResourceSet{ - Spec: ClusterResourceSetSpec{ + newClusterResourceSet := &addonsv1.ClusterResourceSet{ + Spec: addonsv1.ClusterResourceSetSpec{ ClusterSelector: metav1.LabelSelector{ MatchLabels: tt.newClusterSelector, }, }, } - oldClusterResourceSet := &ClusterResourceSet{ - Spec: ClusterResourceSetSpec{ + oldClusterResourceSet := &addonsv1.ClusterResourceSet{ + Spec: addonsv1.ClusterResourceSetSpec{ ClusterSelector: metav1.LabelSelector{ MatchLabels: tt.oldClusterSelector, }, }, } + webhook := ClusterResourceSet{} - warnings, err := newClusterResourceSet.ValidateUpdate(oldClusterResourceSet) + warnings, err := webhook.ValidateUpdate(ctx, oldClusterResourceSet, newClusterResourceSet) if tt.expectErr { g.Expect(err).To(HaveOccurred()) g.Expect(warnings).To(BeEmpty()) @@ -199,8 +207,9 @@ func TestClusterResourceSetClusterSelectorImmutable(t *testing.T) { func TestClusterResourceSetSelectorNotEmptyValidation(t *testing.T) { g := NewWithT(t) - clusterResourceSet := &ClusterResourceSet{} - err := clusterResourceSet.validate(nil) + clusterResourceSet := &addonsv1.ClusterResourceSet{} + webhook := ClusterResourceSet{} + err := webhook.validate(clusterResourceSet, clusterResourceSet) g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring("selector must not be empty")) } diff --git a/exp/addons/internal/webhooks/clusterresourcesetbinding_webhook.go b/exp/addons/internal/webhooks/clusterresourcesetbinding_webhook.go new file mode 100644 index 000000000000..0d74ebf00df9 --- /dev/null +++ b/exp/addons/internal/webhooks/clusterresourcesetbinding_webhook.go @@ -0,0 +1,99 @@ +/* +Copyright 2022 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 webhooks + +import ( + "context" + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "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" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1" + "sigs.k8s.io/cluster-api/feature" +) + +func (webhook *ClusterResourceSetBinding) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&addonsv1.ClusterResourceSetBinding{}). + WithValidator(webhook). + Complete() +} + +// +kubebuilder:webhook:verbs=create;update,path=/validate-addons-cluster-x-k8s-io-v1beta1-clusterresourcesetbinding,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=addons.cluster.x-k8s.io,resources=clusterresourcesetbindings,versions=v1beta1,name=validation.clusterresourcesetbinding.addons.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 + +// ClusterResourceSetBinding implements a validation webhook for ClusterResourceSetBinding. +type ClusterResourceSetBinding struct{} + +var _ webhook.CustomValidator = &ClusterResourceSetBinding{} + +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type. +func (webhook *ClusterResourceSetBinding) ValidateCreate(_ context.Context, oldObj runtime.Object) (admission.Warnings, error) { + oldBinding, ok := oldObj.(*addonsv1.ClusterResourceSetBinding) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterResourceSetBinding but got a %T", oldObj)) + } + return nil, webhook.validate(oldBinding, nil) +} + +// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. +func (webhook *ClusterResourceSetBinding) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { + oldBinding, ok := oldObj.(*addonsv1.ClusterResourceSetBinding) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterResourceSetBinding but got a %T", oldObj)) + } + newBinding, ok := newObj.(*addonsv1.ClusterResourceSetBinding) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterResourceSetBinding but got a %T", newObj)) + } + return nil, webhook.validate(oldBinding, newBinding) +} + +// ValidateDelete implements webhook.Validator so a webhook will be registered for the type. +func (webhook *ClusterResourceSetBinding) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { + return nil, nil +} + +func (webhook *ClusterResourceSetBinding) validate(oldCRSB, newCRSB *addonsv1.ClusterResourceSetBinding) error { + // NOTE: ClusterResourceSet is behind ClusterResourceSet feature gate flag; the web hook + // must prevent creating newCRSB objects in case the feature flag is disabled. + if !feature.Gates.Enabled(feature.ClusterResourceSet) { + return field.Forbidden( + field.NewPath("spec"), + "can be set only if the ClusterResourceSet feature flag is enabled", + ) + } + var allErrs field.ErrorList + if oldCRSB != nil && newCRSB != nil && oldCRSB.Spec.ClusterName != "" && oldCRSB.Spec.ClusterName != newCRSB.Spec.ClusterName { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "clusterName"), newCRSB.Spec.ClusterName, "field is immutable")) + } + if len(allErrs) == 0 { + return nil + } + if oldCRSB != nil { + return apierrors.NewInvalid(addonsv1.GroupVersion.WithKind("ClusterResourceSetBinding").GroupKind(), oldCRSB.Name, allErrs) + } + if newCRSB != nil { + return apierrors.NewInvalid(addonsv1.GroupVersion.WithKind("ClusterResourceSetBinding").GroupKind(), newCRSB.Name, allErrs) + } + return nil +} diff --git a/exp/addons/api/v1beta1/clusterresourcesetbinding_webhook_test.go b/exp/addons/internal/webhooks/clusterresourcesetbinding_webhook_test.go similarity index 75% rename from exp/addons/api/v1beta1/clusterresourcesetbinding_webhook_test.go rename to exp/addons/internal/webhooks/clusterresourcesetbinding_webhook_test.go index 7c28b418d20d..bd2282b52d7d 100644 --- a/exp/addons/api/v1beta1/clusterresourcesetbinding_webhook_test.go +++ b/exp/addons/internal/webhooks/clusterresourcesetbinding_webhook_test.go @@ -14,12 +14,14 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1beta1 +package webhooks import ( "testing" . "github.com/onsi/gomega" + + addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1" ) func TestClusterResourceSetBindingClusterNameImmutable(t *testing.T) { @@ -65,27 +67,28 @@ func TestClusterResourceSetBindingClusterNameImmutable(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - newClusterResourceSetBinding := &ClusterResourceSetBinding{ - Spec: ClusterResourceSetBindingSpec{ + newClusterResourceSetBinding := &addonsv1.ClusterResourceSetBinding{ + Spec: addonsv1.ClusterResourceSetBindingSpec{ ClusterName: tt.newClusterName, }, } - oldClusterResourceSetBinding := &ClusterResourceSetBinding{ - Spec: ClusterResourceSetBindingSpec{ + oldClusterResourceSetBinding := &addonsv1.ClusterResourceSetBinding{ + Spec: addonsv1.ClusterResourceSetBindingSpec{ ClusterName: tt.oldClusterName, }, } + webhook := ClusterResourceSetBinding{} - warnings, err := newClusterResourceSetBinding.ValidateCreate() + warnings, err := webhook.ValidateCreate(ctx, oldClusterResourceSetBinding) g.Expect(err).ToNot(HaveOccurred()) g.Expect(warnings).To(BeEmpty()) if tt.expectErr { - warnings, err = newClusterResourceSetBinding.ValidateUpdate(oldClusterResourceSetBinding) + warnings, err = webhook.ValidateUpdate(ctx, oldClusterResourceSetBinding, newClusterResourceSetBinding) g.Expect(err).To(HaveOccurred()) g.Expect(warnings).To(BeEmpty()) } else { - warnings, err = newClusterResourceSetBinding.ValidateUpdate(oldClusterResourceSetBinding) + warnings, err = webhook.ValidateUpdate(ctx, oldClusterResourceSetBinding, newClusterResourceSetBinding) g.Expect(err).ToNot(HaveOccurred()) g.Expect(warnings).To(BeEmpty()) } diff --git a/exp/addons/internal/webhooks/doc.go b/exp/addons/internal/webhooks/doc.go new file mode 100644 index 000000000000..68d6dfa18d25 --- /dev/null +++ b/exp/addons/internal/webhooks/doc.go @@ -0,0 +1,18 @@ +/* +Copyright 2023 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 webhooks implements experimental addon webhooks. +package webhooks diff --git a/exp/addons/webhooks/alias.go b/exp/addons/webhooks/alias.go new file mode 100644 index 000000000000..293eb074567c --- /dev/null +++ b/exp/addons/webhooks/alias.go @@ -0,0 +1,39 @@ +/* +Copyright 2023 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 webhooks + +import ( + ctrl "sigs.k8s.io/controller-runtime" + + "sigs.k8s.io/cluster-api/exp/addons/internal/webhooks" +) + +// ClusterResourceSet implements a validating and defaulting webhook for ClusterResourceSet. +type ClusterResourceSet struct{} + +// SetupWebhookWithManager sets up ClusterResourceSet webhooks. +func (webhook *ClusterResourceSet) SetupWebhookWithManager(mgr ctrl.Manager) error { + return (&webhooks.ClusterResourceSet{}).SetupWebhookWithManager(mgr) +} + +// ClusterResourceSetBinding implements a validating webhook for ClusterResourceSetBinding. +type ClusterResourceSetBinding struct{} + +// SetupWebhookWithManager sets up ClusterResourceSet webhooks. +func (webhook *ClusterResourceSetBinding) SetupWebhookWithManager(mgr ctrl.Manager) error { + return (&webhooks.ClusterResourceSetBinding{}).SetupWebhookWithManager(mgr) +} diff --git a/exp/addons/webhooks/doc.go b/exp/addons/webhooks/doc.go new file mode 100644 index 000000000000..b465a94bca14 --- /dev/null +++ b/exp/addons/webhooks/doc.go @@ -0,0 +1,18 @@ +/* +Copyright 2023 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 webhooks contains external addons webhook implementations for some of our API types. +package webhooks diff --git a/internal/test/envtest/environment.go b/internal/test/envtest/environment.go index 29389a14c612..23f46978275e 100644 --- a/internal/test/envtest/environment.go +++ b/internal/test/envtest/environment.go @@ -55,6 +55,7 @@ import ( "sigs.k8s.io/cluster-api/cmd/clusterctl/log" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1" + addonswebhooks "sigs.k8s.io/cluster-api/exp/addons/webhooks" expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" ipamv1 "sigs.k8s.io/cluster-api/exp/ipam/api/v1alpha1" expipamwebhooks "sigs.k8s.io/cluster-api/exp/ipam/webhooks" @@ -304,10 +305,10 @@ func newEnvironment(uncachedObjs ...client.Object) *Environment { if err := (&controlplanev1.KubeadmControlPlane{}).SetupWebhookWithManager(mgr); err != nil { klog.Fatalf("unable to create webhook: %+v", err) } - if err := (&addonsv1.ClusterResourceSet{}).SetupWebhookWithManager(mgr); err != nil { + if err := (&addonswebhooks.ClusterResourceSet{}).SetupWebhookWithManager(mgr); err != nil { klog.Fatalf("unable to create webhook for crs: %+v", err) } - if err := (&addonsv1.ClusterResourceSetBinding{}).SetupWebhookWithManager(mgr); err != nil { + if err := (&addonswebhooks.ClusterResourceSetBinding{}).SetupWebhookWithManager(mgr); err != nil { klog.Fatalf("unable to create webhook for ClusterResourceSetBinding: %+v", err) } if err := (&expapiwebhooks.MachinePool{}).SetupWebhookWithManager(mgr); err != nil { diff --git a/main.go b/main.go index 947c41b2f346..e0768440ea3e 100644 --- a/main.go +++ b/main.go @@ -53,6 +53,7 @@ import ( addonsv1alpha4 "sigs.k8s.io/cluster-api/exp/addons/api/v1alpha4" addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1" addonscontrollers "sigs.k8s.io/cluster-api/exp/addons/controllers" + addonswebhooks "sigs.k8s.io/cluster-api/exp/addons/webhooks" expv1alpha4 "sigs.k8s.io/cluster-api/exp/api/v1alpha4" expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" expcontrollers "sigs.k8s.io/cluster-api/exp/controllers" @@ -599,13 +600,13 @@ func setupWebhooks(mgr ctrl.Manager) { // NOTE: ClusterResourceSet is behind ClusterResourceSet feature gate flag; the webhook // is going to prevent creating or updating new objects in case the feature flag is disabled - if err := (&addonsv1.ClusterResourceSet{}).SetupWebhookWithManager(mgr); err != nil { + if err := (&addonswebhooks.ClusterResourceSet{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "ClusterResourceSet") os.Exit(1) } // NOTE: ClusterResourceSetBinding is behind ClusterResourceSet feature gate flag; the webhook // is going to prevent creating or updating new objects in case the feature flag is disabled - if err := (&addonsv1.ClusterResourceSetBinding{}).SetupWebhookWithManager(mgr); err != nil { + if err := (&addonswebhooks.ClusterResourceSetBinding{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "ClusterResourceSetBinding") os.Exit(1) }