From d645938c2af582a36e032eeee6d5d7b41e81ff2f Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 26 Jun 2025 10:15:02 +0100 Subject: [PATCH] Move webhooks into pkg/webhooks Moves webhooks from api to pkg/webhooks making only mechanical code changes except for the removal of the defaulting webhooks, because they weren't used. This results in there now being no mutating webhook configured. NOTE(stephenfin): There were a lot of conflicts here. These were mostly mitigated by faking the addition of v1alpha8, which moved the webhooks to the 'api/v1alpha8' package (commit 750b84de45), followed by the subsequent rename of this package to v1beta1 (commit e9fb53c937), for the webhook files and tests. This still resulted in some merge conflicts due the v1alpha8 changes such as 564b6bd89 and 4368c4f2be (which we obviously don't want to include here) but it made the backport much simpler. Signed-off-by: Stephen Finucane (cherry picked from commit 750b84de45c2e884daa690143a56091e2b79a3f8) --- Makefile | 4 +- api/v1alpha7/identity_types.go | 2 - api/v1alpha7/openstackcluster_webhook.go | 153 -------------- api/v1alpha7/openstackclusterlist_webhook.go | 32 --- .../openstackclustertemplate_webhook.go | 85 -------- .../openstackmachinetemplatelist_webhook.go | 32 --- api/v1alpha7/zz_generated.deepcopy.go | 2 +- config/webhook/manifests.yaml | 69 ------- main.go | 32 +-- .../webhooks.go => pkg/webhooks/errors.go | 2 +- .../webhooks/identity_types.go | 17 +- pkg/webhooks/openstackcluster_webhook.go | 161 +++++++++++++++ .../openstackcluster_webhook_test.go | 195 +++++++++--------- .../openstackclustertemplate_webhook.go | 116 +++++++++++ .../webhooks}/openstackmachine_webhook.go | 86 +++++--- .../openstackmachinetemplate_webhook.go | 61 +++--- .../openstackmachinetemplate_webhook_test.go | 74 +++---- pkg/webhooks/register.go | 63 ++++++ 18 files changed, 581 insertions(+), 605 deletions(-) delete mode 100644 api/v1alpha7/openstackcluster_webhook.go delete mode 100644 api/v1alpha7/openstackclusterlist_webhook.go delete mode 100644 api/v1alpha7/openstackclustertemplate_webhook.go delete mode 100644 api/v1alpha7/openstackmachinetemplatelist_webhook.go rename api/v1alpha7/webhooks.go => pkg/webhooks/errors.go (98%) rename api/v1alpha7/openstackmachinelist_webhook.go => pkg/webhooks/identity_types.go (57%) create mode 100644 pkg/webhooks/openstackcluster_webhook.go rename {api/v1alpha7 => pkg/webhooks}/openstackcluster_webhook_test.go (59%) create mode 100644 pkg/webhooks/openstackclustertemplate_webhook.go rename {api/v1alpha7 => pkg/webhooks}/openstackmachine_webhook.go (52%) rename {api/v1alpha7 => pkg/webhooks}/openstackmachinetemplate_webhook.go (55%) rename {api/v1alpha7 => pkg/webhooks}/openstackmachinetemplate_webhook_test.go (61%) create mode 100644 pkg/webhooks/register.go diff --git a/Makefile b/Makefile index 479e71c9d6..0fb2fb8476 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ include $(ROOT_DIR_RELATIVE)/common.mk # If you update this file, please follow # https://www.thapaliya.com/en/writings/well-documented-makefiles/ -export GOTOOLCHAIN=go1.22.8 +# export GOTOOLCHAIN=go1.22.8 # Active module mode, as we use go modules to manage dependencies export GO111MODULE=on @@ -267,7 +267,7 @@ generate-go: $(MOCKGEN) .PHONY: generate-manifests generate-manifests: $(CONTROLLER_GEN) ## Generate manifests e.g. CRD, RBAC etc. $(CONTROLLER_GEN) \ - paths=./api/... \ + paths=./pkg/webhooks/... \ crd:crdVersions=v1 \ output:crd:dir=$(CRD_ROOT) \ output:webhook:dir=$(WEBHOOK_ROOT) \ diff --git a/api/v1alpha7/identity_types.go b/api/v1alpha7/identity_types.go index 15cd58562b..1ea5f2fb9b 100644 --- a/api/v1alpha7/identity_types.go +++ b/api/v1alpha7/identity_types.go @@ -16,8 +16,6 @@ limitations under the License. package v1alpha7 -const defaultIdentityRefKind = "Secret" - // OpenStackIdentityReference is a reference to an infrastructure // provider identity to be used to provision cluster resources. type OpenStackIdentityReference struct { diff --git a/api/v1alpha7/openstackcluster_webhook.go b/api/v1alpha7/openstackcluster_webhook.go deleted file mode 100644 index 706d10a8c6..0000000000 --- a/api/v1alpha7/openstackcluster_webhook.go +++ /dev/null @@ -1,153 +0,0 @@ -/* -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 v1alpha7 - -import ( - "fmt" - "reflect" - - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/validation/field" - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/controller-runtime/pkg/builder" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/webhook" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" -) - -// log is for logging in this package. -var _ = logf.Log.WithName("openstackcluster-resource") - -func (r *OpenStackCluster) SetupWebhookWithManager(mgr manager.Manager) error { - return builder.WebhookManagedBy(mgr). - For(r). - Complete() -} - -// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackcluster,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackclusters,versions=v1alpha7,name=validation.openstackcluster.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 -// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackcluster,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackclusters,versions=v1alpha7,name=default.openstackcluster.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 - -var ( - _ webhook.Defaulter = &OpenStackCluster{} - _ webhook.Validator = &OpenStackCluster{} -) - -// Default satisfies the defaulting webhook interface. -func (r *OpenStackCluster) Default() { - if r.Spec.IdentityRef != nil && r.Spec.IdentityRef.Kind == "" { - r.Spec.IdentityRef.Kind = defaultIdentityRefKind - } -} - -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type. -func (r *OpenStackCluster) ValidateCreate() (admission.Warnings, error) { - var allErrs field.ErrorList - - if r.Spec.IdentityRef != nil && r.Spec.IdentityRef.Kind != defaultIdentityRefKind { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret")) - } - - return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs) -} - -// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. -func (r *OpenStackCluster) ValidateUpdate(oldRaw runtime.Object) (admission.Warnings, error) { - var allErrs field.ErrorList - old, ok := oldRaw.(*OpenStackCluster) - if !ok { - return nil, apierrors.NewBadRequest(fmt.Sprintf("expected an OpenStackCluster but got a %T", oldRaw)) - } - - if r.Spec.IdentityRef != nil && r.Spec.IdentityRef.Kind != defaultIdentityRefKind { - allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "identityRef", "kind"), - r.Spec.IdentityRef, "must be a Secret"), - ) - } - - // Allow changes to Spec.IdentityRef.Name. - if old.Spec.IdentityRef != nil && r.Spec.IdentityRef != nil { - old.Spec.IdentityRef.Name = "" - r.Spec.IdentityRef.Name = "" - } - - // Allow changes to Spec.IdentityRef if it was unset. - if old.Spec.IdentityRef == nil && r.Spec.IdentityRef != nil { - old.Spec.IdentityRef = &OpenStackIdentityReference{} - r.Spec.IdentityRef = &OpenStackIdentityReference{} - } - - if old.Spec.IdentityRef != nil && r.Spec.IdentityRef == nil { - allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "identityRef"), - r.Spec.IdentityRef, "field cannot be set to nil"), - ) - } - - // Allow change only for the first time. - if old.Spec.ControlPlaneEndpoint.Host == "" { - old.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{} - r.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{} - } - - // Allow change only for the first time. - if old.Spec.DisableAPIServerFloatingIP && old.Spec.APIServerFixedIP == "" { - r.Spec.APIServerFixedIP = "" - } - - // If API Server floating IP is disabled, allow the change of the API Server port only for the first time. - if old.Spec.DisableAPIServerFloatingIP && old.Spec.APIServerPort == 0 && r.Spec.APIServerPort > 0 { - r.Spec.APIServerPort = 0 - } - - // Allow changes to the bastion spec. - old.Spec.Bastion = &Bastion{} - r.Spec.Bastion = &Bastion{} - - // Allow changes on AllowedCIDRs - if r.Spec.APIServerLoadBalancer.Enabled { - old.Spec.APIServerLoadBalancer.AllowedCIDRs = []string{} - r.Spec.APIServerLoadBalancer.AllowedCIDRs = []string{} - } - - // Allow changes to the availability zones. - old.Spec.ControlPlaneAvailabilityZones = []string{} - r.Spec.ControlPlaneAvailabilityZones = []string{} - - // Allow change to the allowAllInClusterTraffic. - old.Spec.AllowAllInClusterTraffic = false - r.Spec.AllowAllInClusterTraffic = false - - // Allow change on the spec.APIServerFloatingIP only if it matches the current api server loadbalancer IP. - if old.Status.APIServerLoadBalancer != nil && r.Spec.APIServerFloatingIP == old.Status.APIServerLoadBalancer.IP { - r.Spec.APIServerFloatingIP = "" - old.Spec.APIServerFloatingIP = "" - } - - if !reflect.DeepEqual(old.Spec, r.Spec) { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "cannot be modified")) - } - - return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs) -} - -// ValidateDelete implements webhook.Validator so a webhook will be registered for the type. -func (r *OpenStackCluster) ValidateDelete() (admission.Warnings, error) { - return nil, nil -} diff --git a/api/v1alpha7/openstackclusterlist_webhook.go b/api/v1alpha7/openstackclusterlist_webhook.go deleted file mode 100644 index f7749b7ade..0000000000 --- a/api/v1alpha7/openstackclusterlist_webhook.go +++ /dev/null @@ -1,32 +0,0 @@ -/* -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 v1alpha7 - -import ( - "sigs.k8s.io/controller-runtime/pkg/builder" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/manager" -) - -// log is for logging in this package. -var _ = logf.Log.WithName("openstackclusterlist-resource") - -func (r *OpenStackClusterList) SetupWebhookWithManager(mgr manager.Manager) error { - return builder.WebhookManagedBy(mgr). - For(r). - Complete() -} diff --git a/api/v1alpha7/openstackclustertemplate_webhook.go b/api/v1alpha7/openstackclustertemplate_webhook.go deleted file mode 100644 index 337928a3f1..0000000000 --- a/api/v1alpha7/openstackclustertemplate_webhook.go +++ /dev/null @@ -1,85 +0,0 @@ -/* -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 v1alpha7 - -import ( - "fmt" - "reflect" - - 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" -) - -const openStackClusterTemplateImmutableMsg = "OpenStackClusterTemplate spec.template.spec field is immutable. Please create new resource instead." - -func (r *OpenStackClusterTemplate) SetupWebhookWithManager(mgr ctrl.Manager) error { - return ctrl.NewWebhookManagedBy(mgr). - For(r). - Complete() -} - -// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackclustertemplate,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackclustertemplates,versions=v1alpha7,name=default.openstackclustertemplate.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 -// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackclustertemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackclustertemplates,versions=v1alpha7,name=validation.openstackclustertemplate.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 - -var ( - _ webhook.Defaulter = &OpenStackClusterTemplate{} - _ webhook.Validator = &OpenStackClusterTemplate{} -) - -// Default implements webhook.Defaulter so a webhook will be registered for the type. -func (r *OpenStackClusterTemplate) Default() { - if r.Spec.Template.Spec.IdentityRef != nil && r.Spec.Template.Spec.IdentityRef.Kind == "" { - r.Spec.Template.Spec.IdentityRef.Kind = defaultIdentityRefKind - } -} - -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type. -func (r *OpenStackClusterTemplate) ValidateCreate() (admission.Warnings, error) { - var allErrs field.ErrorList - - if r.Spec.Template.Spec.IdentityRef != nil && r.Spec.Template.Spec.IdentityRef.Kind != defaultIdentityRefKind { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "identityRef", "kind"), "must be a Secret")) - } - - return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs) -} - -// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. -func (r *OpenStackClusterTemplate) ValidateUpdate(oldRaw runtime.Object) (admission.Warnings, error) { - var allErrs field.ErrorList - old, ok := oldRaw.(*OpenStackClusterTemplate) - if !ok { - return nil, apierrors.NewBadRequest(fmt.Sprintf("expected an OpenStackClusterTemplate but got a %T", oldRaw)) - } - - if !reflect.DeepEqual(r.Spec.Template.Spec, old.Spec.Template.Spec) { - allErrs = append(allErrs, - field.Invalid(field.NewPath("OpenStackClusterTemplate", "spec", "template", "spec"), r, openStackClusterTemplateImmutableMsg), - ) - } - - return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs) -} - -// ValidateDelete implements webhook.Validator so a webhook will be registered for the type. -func (r *OpenStackClusterTemplate) ValidateDelete() (admission.Warnings, error) { - return nil, nil -} diff --git a/api/v1alpha7/openstackmachinetemplatelist_webhook.go b/api/v1alpha7/openstackmachinetemplatelist_webhook.go deleted file mode 100644 index c1ff00c3dd..0000000000 --- a/api/v1alpha7/openstackmachinetemplatelist_webhook.go +++ /dev/null @@ -1,32 +0,0 @@ -/* -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 v1alpha7 - -import ( - "sigs.k8s.io/controller-runtime/pkg/builder" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/manager" -) - -// log is for logging in this package. -var _ = logf.Log.WithName("openstackmachinetemplatelist-resource") - -func (r *OpenStackMachineTemplateList) SetupWebhookWithManager(mgr manager.Manager) error { - return builder.WebhookManagedBy(mgr). - For(r). - Complete() -} diff --git a/api/v1alpha7/zz_generated.deepcopy.go b/api/v1alpha7/zz_generated.deepcopy.go index 1d81a83410..2569c2d1b3 100644 --- a/api/v1alpha7/zz_generated.deepcopy.go +++ b/api/v1alpha7/zz_generated.deepcopy.go @@ -22,7 +22,7 @@ package v1alpha7 import ( "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/runtime" + runtime "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/errors" ) diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index b2e5ad4c2b..195ce4a34c 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -1,74 +1,5 @@ --- apiVersion: admissionregistration.k8s.io/v1 -kind: MutatingWebhookConfiguration -metadata: - name: mutating-webhook-configuration -webhooks: -- admissionReviewVersions: - - v1beta1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /mutate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackcluster - failurePolicy: Fail - matchPolicy: Equivalent - name: default.openstackcluster.infrastructure.cluster.x-k8s.io - rules: - - apiGroups: - - infrastructure.cluster.x-k8s.io - apiVersions: - - v1alpha7 - operations: - - CREATE - - UPDATE - resources: - - openstackclusters - sideEffects: None -- admissionReviewVersions: - - v1beta1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /mutate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackclustertemplate - failurePolicy: Fail - matchPolicy: Equivalent - name: default.openstackclustertemplate.infrastructure.cluster.x-k8s.io - rules: - - apiGroups: - - infrastructure.cluster.x-k8s.io - apiVersions: - - v1alpha7 - operations: - - CREATE - - UPDATE - resources: - - openstackclustertemplates - sideEffects: None -- admissionReviewVersions: - - v1beta1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /mutate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackmachine - failurePolicy: Fail - matchPolicy: Equivalent - name: default.openstackmachine.infrastructure.cluster.x-k8s.io - rules: - - apiGroups: - - infrastructure.cluster.x-k8s.io - apiVersions: - - v1alpha7 - operations: - - CREATE - - UPDATE - resources: - - openstackmachines - sideEffects: None ---- -apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: name: validating-webhook-configuration diff --git a/main.go b/main.go index 771fc4c323..770a0e280e 100644 --- a/main.go +++ b/main.go @@ -49,6 +49,7 @@ import ( "sigs.k8s.io/cluster-api-provider-openstack/pkg/metrics" "sigs.k8s.io/cluster-api-provider-openstack/pkg/record" "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/webhooks" "sigs.k8s.io/cluster-api-provider-openstack/version" ) @@ -297,32 +298,11 @@ func setupReconcilers(ctx context.Context, mgr ctrl.Manager, caCerts []byte, sco } func setupWebhooks(mgr ctrl.Manager) { - if err := (&infrav1.OpenStackMachineTemplateWebhook{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackMachineTemplate") - os.Exit(1) - } - if err := (&infrav1.OpenStackMachineTemplateList{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackMachineTemplateList") - os.Exit(1) - } - if err := (&infrav1.OpenStackCluster{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackCluster") - os.Exit(1) - } - if err := (&infrav1.OpenStackClusterTemplate{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackClusterTemplate") - os.Exit(1) - } - if err := (&infrav1.OpenStackMachine{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackMachine") - os.Exit(1) - } - if err := (&infrav1.OpenStackMachineList{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackMachineList") - os.Exit(1) - } - if err := (&infrav1.OpenStackClusterList{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackClusterList") + errs := webhooks.RegisterAllWithManager(mgr) + if len(errs) > 0 { + for i := range errs { + setupLog.Error(errs[i], "unable to register webhook") + } os.Exit(1) } } diff --git a/api/v1alpha7/webhooks.go b/pkg/webhooks/errors.go similarity index 98% rename from api/v1alpha7/webhooks.go rename to pkg/webhooks/errors.go index d58fdbb155..ad2c53aae4 100644 --- a/api/v1alpha7/webhooks.go +++ b/pkg/webhooks/errors.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1alpha7 +package webhooks import ( apierrors "k8s.io/apimachinery/pkg/api/errors" diff --git a/api/v1alpha7/openstackmachinelist_webhook.go b/pkg/webhooks/identity_types.go similarity index 57% rename from api/v1alpha7/openstackmachinelist_webhook.go rename to pkg/webhooks/identity_types.go index 0122e1b52c..e3fcb8f45d 100644 --- a/api/v1alpha7/openstackmachinelist_webhook.go +++ b/pkg/webhooks/identity_types.go @@ -14,19 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1alpha7 +package webhooks -import ( - "sigs.k8s.io/controller-runtime/pkg/builder" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/manager" -) - -// log is for logging in this package. -var _ = logf.Log.WithName("openstackmachinelist-resource") - -func (r *OpenStackMachineList) SetupWebhookWithManager(mgr manager.Manager) error { - return builder.WebhookManagedBy(mgr). - For(r). - Complete() -} +const defaultIdentityRefKind = "Secret" diff --git a/pkg/webhooks/openstackcluster_webhook.go b/pkg/webhooks/openstackcluster_webhook.go new file mode 100644 index 0000000000..4827a5daa5 --- /dev/null +++ b/pkg/webhooks/openstackcluster_webhook.go @@ -0,0 +1,161 @@ +/* +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 ( + "context" + "fmt" + "reflect" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7" +) + +// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackcluster,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackclusters,versions=v1alpha7,name=validation.openstackcluster.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 + +func SetupOpenStackClusterWebhook(mgr manager.Manager) error { + return builder.WebhookManagedBy(mgr). + For(&infrav1.OpenStackCluster{}). + WithValidator(&openStackClusterWebhook{}). + Complete() +} + +type openStackClusterWebhook struct{} + +// Compile-time assertion that openStackClusterWebhook implements webhook.CustomValidator. +var _ webhook.CustomValidator = &openStackClusterWebhook{} + +// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type. +func (*openStackClusterWebhook) ValidateCreate(_ context.Context, objRaw runtime.Object) (admission.Warnings, error) { + var allErrs field.ErrorList + + newObj, err := castToOpenStackCluster(objRaw) + if err != nil { + return nil, err + } + + if newObj.Spec.IdentityRef != nil && newObj.Spec.IdentityRef.Kind != defaultIdentityRefKind { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret")) + } + + return aggregateObjErrors(newObj.GroupVersionKind().GroupKind(), newObj.Name, allErrs) +} + +// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type. +func (*openStackClusterWebhook) ValidateUpdate(_ context.Context, oldObjRaw, newObjRaw runtime.Object) (admission.Warnings, error) { + var allErrs field.ErrorList + oldObj, err := castToOpenStackCluster(oldObjRaw) + if err != nil { + return nil, err + } + newObj, err := castToOpenStackCluster(newObjRaw) + if err != nil { + return nil, err + } + + if newObj.Spec.IdentityRef != nil && newObj.Spec.IdentityRef.Kind != defaultIdentityRefKind { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "identityRef", "kind"), + newObj.Spec.IdentityRef, "must be a Secret"), + ) + } + + // Allow changes to Spec.IdentityRef.Name. + if oldObj.Spec.IdentityRef != nil && newObj.Spec.IdentityRef != nil { + oldObj.Spec.IdentityRef.Name = "" + newObj.Spec.IdentityRef.Name = "" + } + + // Allow changes to Spec.IdentityRef if it was unset. + if oldObj.Spec.IdentityRef == nil && newObj.Spec.IdentityRef != nil { + oldObj.Spec.IdentityRef = &infrav1.OpenStackIdentityReference{} + newObj.Spec.IdentityRef = &infrav1.OpenStackIdentityReference{} + } + + if oldObj.Spec.IdentityRef != nil && newObj.Spec.IdentityRef == nil { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "identityRef"), + newObj.Spec.IdentityRef, "field cannot be set to nil"), + ) + } + + // Allow change only for the first time. + if oldObj.Spec.ControlPlaneEndpoint.Host == "" { + oldObj.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{} + newObj.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{} + } + + // Allow change only for the first time. + if oldObj.Spec.DisableAPIServerFloatingIP && oldObj.Spec.APIServerFixedIP == "" { + newObj.Spec.APIServerFixedIP = "" + } + + // If API Server floating IP is disabled, allow the change of the API Server port only for the first time. + if oldObj.Spec.DisableAPIServerFloatingIP && oldObj.Spec.APIServerPort == 0 && newObj.Spec.APIServerPort > 0 { + newObj.Spec.APIServerPort = 0 + } + + // Allow changes to the bastion spec. + oldObj.Spec.Bastion = &infrav1.Bastion{} + newObj.Spec.Bastion = &infrav1.Bastion{} + + // Allow changes on AllowedCIDRs + if newObj.Spec.APIServerLoadBalancer.Enabled { + oldObj.Spec.APIServerLoadBalancer.AllowedCIDRs = []string{} + newObj.Spec.APIServerLoadBalancer.AllowedCIDRs = []string{} + } + + // Allow changes to the availability zones. + oldObj.Spec.ControlPlaneAvailabilityZones = []string{} + newObj.Spec.ControlPlaneAvailabilityZones = []string{} + + // Allow change to the allowAllInClusterTraffic. + oldObj.Spec.AllowAllInClusterTraffic = false + newObj.Spec.AllowAllInClusterTraffic = false + + // Allow change on the spec.APIServerFloatingIP only if it matches the current api server loadbalancer IP. + if oldObj.Status.APIServerLoadBalancer != nil && newObj.Spec.APIServerFloatingIP == oldObj.Status.APIServerLoadBalancer.IP { + newObj.Spec.APIServerFloatingIP = "" + oldObj.Spec.APIServerFloatingIP = "" + } + + if !reflect.DeepEqual(oldObj.Spec, newObj.Spec) { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "cannot be modified")) + } + + return aggregateObjErrors(newObj.GroupVersionKind().GroupKind(), newObj.Name, allErrs) +} + +// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type. +func (*openStackClusterWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { + return nil, nil +} + +func castToOpenStackCluster(obj runtime.Object) (*infrav1.OpenStackCluster, error) { + cast, ok := obj.(*infrav1.OpenStackCluster) + if !ok { + return nil, fmt.Errorf("expected an OpenStackCluster but got a %T", obj) + } + return cast, nil +} diff --git a/api/v1alpha7/openstackcluster_webhook_test.go b/pkg/webhooks/openstackcluster_webhook_test.go similarity index 59% rename from api/v1alpha7/openstackcluster_webhook_test.go rename to pkg/webhooks/openstackcluster_webhook_test.go index 24e1407f2c..c3e57bae38 100644 --- a/api/v1alpha7/openstackcluster_webhook_test.go +++ b/pkg/webhooks/openstackcluster_webhook_test.go @@ -14,12 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1alpha7 +package webhooks import ( + "context" "testing" . "github.com/onsi/gomega" + + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7" ) func TestOpenStackCluster_ValidateUpdate(t *testing.T) { @@ -27,25 +30,25 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { tests := []struct { name string - oldTemplate *OpenStackCluster - newTemplate *OpenStackCluster + oldTemplate *infrav1.OpenStackCluster + newTemplate *infrav1.OpenStackCluster wantErr bool }{ { name: "OpenStackCluster.Spec.IdentityRef.Kind must always be Secret", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ + IdentityRef: &infrav1.OpenStackIdentityReference{ Kind: "Secret", Name: "foobar", }, }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ + IdentityRef: &infrav1.OpenStackIdentityReference{ Kind: "foobar", Name: "foobar", }, @@ -55,19 +58,19 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, { name: "Changing OpenStackCluster.Spec.IdentityRef.Name is allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ + IdentityRef: &infrav1.OpenStackIdentityReference{ Kind: "Secret", Name: "foobar", }, }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ + IdentityRef: &infrav1.OpenStackIdentityReference{ Kind: "Secret", Name: "foobarbaz", }, @@ -77,15 +80,15 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, { name: "OpenStackCluster.Spec.IdentityRef can be changed if it was unset", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ + IdentityRef: &infrav1.OpenStackIdentityReference{ Kind: "Secret", Name: "foobar", }, @@ -95,17 +98,17 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, { name: "OpenStackCluster.Spec.IdentityRef must not be removed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ + IdentityRef: &infrav1.OpenStackIdentityReference{ Kind: "Secret", Name: "foobar", }, }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", }, }, @@ -113,11 +116,11 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, { name: "Changing OpenStackCluster.Spec.Bastion is allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", - Bastion: &Bastion{ - Instance: OpenStackMachineSpec{ + Bastion: &infrav1.Bastion{ + Instance: infrav1.OpenStackMachineSpec{ CloudName: "foobar", Image: "foobar", Flavor: "minimal", @@ -125,17 +128,17 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Enabled: true, }, }, - Status: OpenStackClusterStatus{ - Bastion: &BastionStatus{ + Status: infrav1.OpenStackClusterStatus{ + Bastion: &infrav1.BastionStatus{ Name: "foobar", }, }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", - Bastion: &Bastion{ - Instance: OpenStackMachineSpec{ + Bastion: &infrav1.Bastion{ + Instance: infrav1.OpenStackMachineSpec{ CloudName: "foobarbaz", Image: "foobarbaz", Flavor: "medium", @@ -148,10 +151,10 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, { name: "Changing CIDRs on the OpenStackCluster.Spec.APIServerLoadBalancer.AllowedCIDRs is allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", - APIServerLoadBalancer: APIServerLoadBalancer{ + APIServerLoadBalancer: infrav1.APIServerLoadBalancer{ Enabled: true, AllowedCIDRs: []string{ "0.0.0.0/0", @@ -160,10 +163,10 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", - APIServerLoadBalancer: APIServerLoadBalancer{ + APIServerLoadBalancer: infrav1.APIServerLoadBalancer{ Enabled: true, AllowedCIDRs: []string{ "0.0.0.0/0", @@ -177,13 +180,13 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, { name: "Adding OpenStackCluster.Spec.ControlPlaneAvailabilityZones is allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", ControlPlaneAvailabilityZones: []string{ "alice", @@ -195,8 +198,8 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, { name: "Modifying OpenStackCluster.Spec.ControlPlaneAvailabilityZones is allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", ControlPlaneAvailabilityZones: []string{ "alice", @@ -204,8 +207,8 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", ControlPlaneAvailabilityZones: []string{ "alice", @@ -218,8 +221,8 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, { name: "Removing OpenStackCluster.Spec.ControlPlaneAvailabilityZones is allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", ControlPlaneAvailabilityZones: []string{ "alice", @@ -227,8 +230,8 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", }, }, @@ -236,13 +239,13 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, { name: "Changing OpenStackCluster.Spec.APIServerFixedIP is allowed when API Server Floating IP is disabled", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ DisableAPIServerFloatingIP: true, }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ DisableAPIServerFloatingIP: true, APIServerFixedIP: "20.1.56.1", }, @@ -251,13 +254,13 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, { name: "Changing OpenStackCluster.Spec.APIServerFixedIP is not allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ DisableAPIServerFloatingIP: false, }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ DisableAPIServerFloatingIP: false, APIServerFixedIP: "20.1.56.1", }, @@ -267,13 +270,13 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { { name: "Changing OpenStackCluster.Spec.APIServerPort is allowed when API Server Floating IP is disabled", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ DisableAPIServerFloatingIP: true, }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ DisableAPIServerFloatingIP: true, APIServerPort: 8443, }, @@ -282,13 +285,13 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, { name: "Changing OpenStackCluster.Spec.APIServerPort is not allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ DisableAPIServerFloatingIP: false, }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ DisableAPIServerFloatingIP: false, APIServerPort: 8443, }, @@ -297,22 +300,22 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, { name: "Changing OpenStackCluster.Spec.APIServerFloatingIP is allowed when it matches the current api server loadbalancer IP", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ APIServerFloatingIP: "", }, - Status: OpenStackClusterStatus{ - APIServerLoadBalancer: &LoadBalancer{ + Status: infrav1.OpenStackClusterStatus{ + APIServerLoadBalancer: &infrav1.LoadBalancer{ IP: "1.2.3.4", }, }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ APIServerFloatingIP: "1.2.3.4", }, - Status: OpenStackClusterStatus{ - APIServerLoadBalancer: &LoadBalancer{ + Status: infrav1.OpenStackClusterStatus{ + APIServerLoadBalancer: &infrav1.LoadBalancer{ IP: "1.2.3.4", }, }, @@ -321,22 +324,22 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, { name: "Changing OpenStackCluster.Spec.APIServerFloatingIP is not allowed when it doesn't matches the current api server loadbalancer IP", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + oldTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ APIServerFloatingIP: "", }, - Status: OpenStackClusterStatus{ - APIServerLoadBalancer: &LoadBalancer{ + Status: infrav1.OpenStackClusterStatus{ + APIServerLoadBalancer: &infrav1.LoadBalancer{ IP: "1.2.3.4", }, }, }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + newTemplate: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ APIServerFloatingIP: "5.6.7.8", }, - Status: OpenStackClusterStatus{ - APIServerLoadBalancer: &LoadBalancer{ + Status: infrav1.OpenStackClusterStatus{ + APIServerLoadBalancer: &infrav1.LoadBalancer{ IP: "1.2.3.4", }, }, @@ -346,7 +349,9 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - warn, err := tt.newTemplate.ValidateUpdate(tt.oldTemplate) + ctx := context.TODO() + webhook := &openStackClusterWebhook{} + warn, err := webhook.ValidateUpdate(ctx, tt.oldTemplate, tt.newTemplate) if tt.wantErr { g.Expect(err).To(HaveOccurred()) } else { @@ -363,15 +368,15 @@ func TestOpenStackCluster_ValidateCreate(t *testing.T) { tests := []struct { name string - template *OpenStackCluster + template *infrav1.OpenStackCluster wantErr bool }{ { name: "OpenStackCluster.Spec.IdentityRef with correct spec on create", - template: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + template: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ + IdentityRef: &infrav1.OpenStackIdentityReference{ Kind: "Secret", Name: "foobar", }, @@ -381,10 +386,10 @@ func TestOpenStackCluster_ValidateCreate(t *testing.T) { }, { name: "OpenStackCluster.Spec.IdentityRef with faulty spec on create", - template: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ + template: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ + IdentityRef: &infrav1.OpenStackIdentityReference{ Kind: "foobar", Name: "foobar", }, @@ -395,7 +400,9 @@ func TestOpenStackCluster_ValidateCreate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - warn, err := tt.template.ValidateCreate() + ctx := context.TODO() + webhook := &openStackClusterWebhook{} + warn, err := webhook.ValidateCreate(ctx, tt.template) if tt.wantErr { g.Expect(err).To(HaveOccurred()) } else { diff --git a/pkg/webhooks/openstackclustertemplate_webhook.go b/pkg/webhooks/openstackclustertemplate_webhook.go new file mode 100644 index 0000000000..fb059f2f58 --- /dev/null +++ b/pkg/webhooks/openstackclustertemplate_webhook.go @@ -0,0 +1,116 @@ +/* +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 ( + "context" + "fmt" + "reflect" + + 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" + + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7" +) + +// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackclustertemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackclustertemplates,versions=v1alpha7,name=validation.openstackclustertemplate.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 + +func SetupOpenStackClusterTemplateWebhook(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&infrav1.OpenStackClusterTemplate{}). + WithValidator(&openStackClusterTemplateWebhook{}). + Complete() +} + +type openStackClusterTemplateWebhook struct{} + +// Compile-time assertion that openStackClusterTemplateWebhook implements webhook.CustomValidator. +var ( + _ webhook.CustomValidator = &openStackClusterTemplateWebhook{} + _ webhook.CustomDefaulter = &openStackClusterTemplateWebhook{} +) + +func (*openStackClusterTemplateWebhook) Default(_ context.Context, objRaw runtime.Object) error { + var allErrs field.ErrorList + newObj, err := castToOpenStackClusterTemplate(objRaw) + if err != nil { + return err + } + + if newObj.Spec.Template.Spec.IdentityRef != nil && newObj.Spec.Template.Spec.IdentityRef.Kind != defaultIdentityRefKind { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "identityRef", "kind"), "must be a Secret")) + } + + if len(allErrs) == 0 { + return nil + } + + return apierrors.NewInvalid(newObj.GroupVersionKind().GroupKind(), newObj.Name, allErrs) +} + +// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type. +func (*openStackClusterTemplateWebhook) ValidateCreate(_ context.Context, objRaw runtime.Object) (admission.Warnings, error) { + var allErrs field.ErrorList + newObj, err := castToOpenStackClusterTemplate(objRaw) + if err != nil { + return nil, err + } + + if newObj.Spec.Template.Spec.IdentityRef != nil && newObj.Spec.Template.Spec.IdentityRef.Kind != defaultIdentityRefKind { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "identityRef", "kind"), "must be a Secret")) + } + + return aggregateObjErrors(newObj.GroupVersionKind().GroupKind(), newObj.Name, allErrs) +} + +// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type. +func (*openStackClusterTemplateWebhook) ValidateUpdate(_ context.Context, oldObjRaw, newObjRaw runtime.Object) (admission.Warnings, error) { + var allErrs field.ErrorList + oldObj, err := castToOpenStackClusterTemplate(oldObjRaw) + if err != nil { + return nil, err + } + newObj, err := castToOpenStackClusterTemplate(newObjRaw) + if err != nil { + return nil, err + } + + if !reflect.DeepEqual(newObj.Spec.Template.Spec, oldObj.Spec.Template.Spec) { + allErrs = append(allErrs, + field.Invalid(field.NewPath("OpenStackClusterTemplate", "spec", "template", "spec"), newObj, "OpenStackClusterTemplate spec.template.spec field is immutable. Please create new resource instead."), + ) + } + + return aggregateObjErrors(newObj.GroupVersionKind().GroupKind(), newObj.Name, allErrs) +} + +// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type. +func (*openStackClusterTemplateWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { + return nil, nil +} + +func castToOpenStackClusterTemplate(obj runtime.Object) (*infrav1.OpenStackClusterTemplate, error) { + cast, ok := obj.(*infrav1.OpenStackClusterTemplate) + if !ok { + return nil, fmt.Errorf("expected an OpenStackClusterTemplate but got a %T", obj) + } + return cast, nil +} diff --git a/api/v1alpha7/openstackmachine_webhook.go b/pkg/webhooks/openstackmachine_webhook.go similarity index 52% rename from api/v1alpha7/openstackmachine_webhook.go rename to pkg/webhooks/openstackmachine_webhook.go index 6470589a76..5a1ee496b3 100644 --- a/api/v1alpha7/openstackmachine_webhook.go +++ b/pkg/webhooks/openstackmachine_webhook.go @@ -14,9 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1alpha7 +package webhooks import ( + "context" "fmt" "reflect" @@ -24,73 +25,90 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/builder" - logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7" ) -// log is for logging in this package. -var _ = logf.Log.WithName("openstackmachine-resource") +// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackmachine,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackmachines,versions=v1alpha7,name=validation.openstackmachine.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 -func (r *OpenStackMachine) SetupWebhookWithManager(mgr manager.Manager) error { +func SetupOpenStackMachineWebhook(mgr manager.Manager) error { return builder.WebhookManagedBy(mgr). - For(r). + For(&infrav1.OpenStackMachine{}). + WithValidator(&openStackMachineWebhook{}). Complete() } -// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackmachine,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackmachines,versions=v1alpha7,name=validation.openstackmachine.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 -// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackmachine,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackmachines,versions=v1alpha7,name=default.openstackmachine.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 +type openStackMachineWebhook struct{} +// Compile-time assertion that openStackMachineWebhook implements webhook.CustomValidator. var ( - _ webhook.Defaulter = &OpenStackMachine{} - _ webhook.Validator = &OpenStackMachine{} + _ webhook.CustomValidator = &openStackMachineWebhook{} + _ webhook.CustomDefaulter = &openStackMachineWebhook{} ) -// Default satisfies the defaulting webhook interface. -func (r *OpenStackMachine) Default() { - if r.Spec.IdentityRef != nil && r.Spec.IdentityRef.Kind == "" { - r.Spec.IdentityRef.Kind = defaultIdentityRefKind +// Default implements webhook.CustomDefaulter. +func (*openStackMachineWebhook) Default(_ context.Context, objRaw runtime.Object) error { + newObj, err := castToOpenStackMachine(objRaw) + if err != nil { + return err + } + + if newObj.Spec.IdentityRef != nil && newObj.Spec.IdentityRef.Kind != defaultIdentityRefKind { + newObj.Spec.IdentityRef.Kind = defaultIdentityRefKind } + + return nil } -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type. -func (r *OpenStackMachine) ValidateCreate() (admission.Warnings, error) { +// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type. +func (*openStackMachineWebhook) ValidateCreate(_ context.Context, objRaw runtime.Object) (admission.Warnings, error) { var allErrs field.ErrorList + newObj, err := castToOpenStackMachine(objRaw) + if err != nil { + return nil, err + } - if r.Spec.IdentityRef != nil && r.Spec.IdentityRef.Kind != defaultIdentityRefKind { + if newObj.Spec.IdentityRef != nil && newObj.Spec.IdentityRef.Kind != defaultIdentityRefKind { allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret")) } - if r.Spec.RootVolume != nil && r.Spec.AdditionalBlockDevices != nil { - for _, device := range r.Spec.AdditionalBlockDevices { + if newObj.Spec.RootVolume != nil && newObj.Spec.AdditionalBlockDevices != nil { + for _, device := range newObj.Spec.AdditionalBlockDevices { if device.Name == "root" { allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "additionalBlockDevices"), "cannot contain a device named \"root\" when rootVolume is set")) } } } - return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs) + return aggregateObjErrors(newObj.GroupVersionKind().GroupKind(), newObj.Name, allErrs) } -// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. -func (r *OpenStackMachine) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { - newOpenStackMachine, err := runtime.DefaultUnstructuredConverter.ToUnstructured(r) +// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type. +func (*openStackMachineWebhook) ValidateUpdate(_ context.Context, oldObjRaw, newObjRaw runtime.Object) (admission.Warnings, error) { + newObj, err := castToOpenStackMachine(newObjRaw) + if err != nil { + return nil, err + } + + newOpenStackMachine, err := runtime.DefaultUnstructuredConverter.ToUnstructured(newObj) if err != nil { - return nil, apierrors.NewInvalid(GroupVersion.WithKind("OpenStackMachine").GroupKind(), r.Name, field.ErrorList{ + return nil, apierrors.NewInvalid(infrav1.GroupVersion.WithKind("OpenStackMachine").GroupKind(), newObj.Name, field.ErrorList{ field.InternalError(nil, fmt.Errorf("failed to convert new OpenStackMachine to unstructured object: %w", err)), }) } - oldOpenStackMachine, err := runtime.DefaultUnstructuredConverter.ToUnstructured(old) + oldOpenStackMachine, err := runtime.DefaultUnstructuredConverter.ToUnstructured(oldObjRaw) if err != nil { - return nil, apierrors.NewInvalid(GroupVersion.WithKind("OpenStackMachine").GroupKind(), r.Name, field.ErrorList{ + return nil, apierrors.NewInvalid(infrav1.GroupVersion.WithKind("OpenStackMachine").GroupKind(), newObj.Name, field.ErrorList{ field.InternalError(nil, fmt.Errorf("failed to convert old OpenStackMachine to unstructured object: %w", err)), }) } var allErrs field.ErrorList - if r.Spec.IdentityRef != nil && r.Spec.IdentityRef.Kind != defaultIdentityRefKind { + if newObj.Spec.IdentityRef != nil && newObj.Spec.IdentityRef.Kind != defaultIdentityRefKind { allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret")) } @@ -113,10 +131,18 @@ func (r *OpenStackMachine) ValidateUpdate(old runtime.Object) (admission.Warning allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "cannot be modified")) } - return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs) + return aggregateObjErrors(newObj.GroupVersionKind().GroupKind(), newObj.Name, allErrs) } -// ValidateDelete implements webhook.Validator so a webhook will be registered for the type. -func (r *OpenStackMachine) ValidateDelete() (admission.Warnings, error) { +// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type. +func (*openStackMachineWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { return nil, nil } + +func castToOpenStackMachine(obj runtime.Object) (*infrav1.OpenStackMachine, error) { + cast, ok := obj.(*infrav1.OpenStackMachine) + if !ok { + return nil, fmt.Errorf("expected an OpenStackMachine but got a %T", obj) + } + return cast, nil +} diff --git a/api/v1alpha7/openstackmachinetemplate_webhook.go b/pkg/webhooks/openstackmachinetemplate_webhook.go similarity index 55% rename from api/v1alpha7/openstackmachinetemplate_webhook.go rename to pkg/webhooks/openstackmachinetemplate_webhook.go index b74d056ba2..a96d1a2938 100644 --- a/api/v1alpha7/openstackmachinetemplate_webhook.go +++ b/pkg/webhooks/openstackmachinetemplate_webhook.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1alpha7 +package webhooks import ( "context" @@ -29,52 +29,51 @@ import ( "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" -) -// OpenStackMachineTemplateImmutableMsg ... -const OpenStackMachineTemplateImmutableMsg = "OpenStackMachineTemplate spec.template.spec field is immutable. Please create a new resource instead. Ref doc: https://cluster-api.sigs.k8s.io/tasks/change-machine-template.html" + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7" +) -// +kubebuilder:object:generate=false -type OpenStackMachineTemplateWebhook struct{} +// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackmachinetemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackmachinetemplates,versions=v1alpha7,name=validation.openstackmachinetemplate.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 -func (r *OpenStackMachineTemplateWebhook) SetupWebhookWithManager(mgr manager.Manager) error { +func SetupOpenStackMachineTemplateWebhook(mgr manager.Manager) error { return builder.WebhookManagedBy(mgr). - For(&OpenStackMachineTemplate{}). - WithValidator(r). + For(&infrav1.OpenStackMachineTemplate{}). + WithValidator(&openStackMachineTemplateWebhook{}). Complete() } -// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha7-openstackmachinetemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackmachinetemplates,versions=v1alpha7,name=validation.openstackmachinetemplate.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 +type openStackMachineTemplateWebhook struct{} -var _ webhook.CustomValidator = &OpenStackMachineTemplateWebhook{} +// Compile-time assertion that openStackMachineTemplateWebhook implements webhook.CustomValidator. +var _ webhook.CustomValidator = &openStackMachineTemplateWebhook{} // ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type. -func (r *OpenStackMachineTemplateWebhook) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { - openStackMachineTemplate, ok := obj.(*OpenStackMachineTemplate) - if !ok { - return nil, apierrors.NewBadRequest(fmt.Sprintf("expected an OpenStackMachineTemplate but got a %T", obj)) +func (*openStackMachineTemplateWebhook) ValidateCreate(_ context.Context, objRaw runtime.Object) (admission.Warnings, error) { + newObj, err := castToOpenStackMachineTemplate(objRaw) + if err != nil { + return nil, err } var allErrs field.ErrorList - if openStackMachineTemplate.Spec.Template.Spec.ProviderID != nil { + if newObj.Spec.Template.Spec.ProviderID != nil { allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "providerID"), "cannot be set in templates")) } - return aggregateObjErrors(openStackMachineTemplate.GroupVersionKind().GroupKind(), openStackMachineTemplate.Name, allErrs) + return aggregateObjErrors(newObj.GroupVersionKind().GroupKind(), newObj.Name, allErrs) } // ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type. -func (r *OpenStackMachineTemplateWebhook) ValidateUpdate(ctx context.Context, oldRaw runtime.Object, newRaw runtime.Object) (admission.Warnings, error) { +func (*openStackMachineTemplateWebhook) ValidateUpdate(ctx context.Context, oldObjRaw, newObjRaw runtime.Object) (admission.Warnings, error) { var allErrs field.ErrorList - old, ok := oldRaw.(*OpenStackMachineTemplate) - if !ok { - return nil, apierrors.NewBadRequest(fmt.Sprintf("expected an OpenStackMachineTemplate but got a %T", oldRaw)) + oldObj, err := castToOpenStackMachineTemplate(oldObjRaw) + if err != nil { + return nil, err } - newObj, ok := newRaw.(*OpenStackMachineTemplate) - if !ok { - return nil, apierrors.NewBadRequest(fmt.Sprintf("expected an OpenStackMachineTemplate but got a %T", oldRaw)) + newObj, err := castToOpenStackMachineTemplate(newObjRaw) + if err != nil { + return nil, err } req, err := admission.RequestFromContext(ctx) @@ -83,9 +82,9 @@ func (r *OpenStackMachineTemplateWebhook) ValidateUpdate(ctx context.Context, ol } if !topology.ShouldSkipImmutabilityChecks(req, newObj) && - !reflect.DeepEqual(newObj.Spec.Template.Spec, old.Spec.Template.Spec) { + !reflect.DeepEqual(newObj.Spec.Template.Spec, oldObj.Spec.Template.Spec) { allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "template", "spec"), r, OpenStackMachineTemplateImmutableMsg), + field.Invalid(field.NewPath("spec", "template", "spec"), newObj.Spec.Template.Spec, "OpenStackMachineTemplate spec.template.spec field is immutable. Please create a new resource instead. Ref doc: https://cluster-api.sigs.k8s.io/tasks/change-machine-template.html"), ) } @@ -93,6 +92,14 @@ func (r *OpenStackMachineTemplateWebhook) ValidateUpdate(ctx context.Context, ol } // ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type. -func (r *OpenStackMachineTemplateWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { +func (*openStackMachineTemplateWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { return nil, nil } + +func castToOpenStackMachineTemplate(obj runtime.Object) (*infrav1.OpenStackMachineTemplate, error) { + cast, ok := obj.(*infrav1.OpenStackMachineTemplate) + if !ok { + return nil, fmt.Errorf("expected an OpenStackMachineTemplate but got a %T", obj) + } + return cast, nil +} diff --git a/api/v1alpha7/openstackmachinetemplate_webhook_test.go b/pkg/webhooks/openstackmachinetemplate_webhook_test.go similarity index 61% rename from api/v1alpha7/openstackmachinetemplate_webhook_test.go rename to pkg/webhooks/openstackmachinetemplate_webhook_test.go index d9a53adc17..891ad58f13 100644 --- a/api/v1alpha7/openstackmachinetemplate_webhook_test.go +++ b/pkg/webhooks/openstackmachinetemplate_webhook_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1alpha7 +package webhooks import ( "context" @@ -26,6 +26,8 @@ import ( "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7" ) func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { @@ -33,27 +35,27 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { tests := []struct { name string - oldTemplate *OpenStackMachineTemplate - newTemplate *OpenStackMachineTemplate + oldTemplate *infrav1.OpenStackMachineTemplate + newTemplate *infrav1.OpenStackMachineTemplate req *admission.Request wantErr bool }{ { name: "OpenStackMachineTemplate with immutable spec", - oldTemplate: &OpenStackMachineTemplate{ - Spec: OpenStackMachineTemplateSpec{ - Template: OpenStackMachineTemplateResource{ - Spec: OpenStackMachineSpec{ + oldTemplate: &infrav1.OpenStackMachineTemplate{ + Spec: infrav1.OpenStackMachineTemplateSpec{ + Template: infrav1.OpenStackMachineTemplateResource{ + Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", Image: "bar", }, }, }, }, - newTemplate: &OpenStackMachineTemplate{ - Spec: OpenStackMachineTemplateSpec{ - Template: OpenStackMachineTemplateResource{ - Spec: OpenStackMachineSpec{ + newTemplate: &infrav1.OpenStackMachineTemplate{ + Spec: infrav1.OpenStackMachineTemplateSpec{ + Template: infrav1.OpenStackMachineTemplateResource{ + Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", Image: "NewImage", }, @@ -65,10 +67,10 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { }, { name: "OpenStackMachineTemplate with mutable metadata", - oldTemplate: &OpenStackMachineTemplate{ - Spec: OpenStackMachineTemplateSpec{ - Template: OpenStackMachineTemplateResource{ - Spec: OpenStackMachineSpec{ + oldTemplate: &infrav1.OpenStackMachineTemplate{ + Spec: infrav1.OpenStackMachineTemplateSpec{ + Template: infrav1.OpenStackMachineTemplateResource{ + Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", Image: "bar", }, @@ -78,10 +80,10 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { Name: "foo", }, }, - newTemplate: &OpenStackMachineTemplate{ - Spec: OpenStackMachineTemplateSpec{ - Template: OpenStackMachineTemplateResource{ - Spec: OpenStackMachineSpec{ + newTemplate: &infrav1.OpenStackMachineTemplate{ + Spec: infrav1.OpenStackMachineTemplateSpec{ + Template: infrav1.OpenStackMachineTemplateResource{ + Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", Image: "bar", }, @@ -95,20 +97,20 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { }, { name: "don't allow modification, dry run, no skip immutability annotation set", - oldTemplate: &OpenStackMachineTemplate{ - Spec: OpenStackMachineTemplateSpec{ - Template: OpenStackMachineTemplateResource{ - Spec: OpenStackMachineSpec{ + oldTemplate: &infrav1.OpenStackMachineTemplate{ + Spec: infrav1.OpenStackMachineTemplateSpec{ + Template: infrav1.OpenStackMachineTemplateResource{ + Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", Image: "bar", }, }, }, }, - newTemplate: &OpenStackMachineTemplate{ - Spec: OpenStackMachineTemplateSpec{ - Template: OpenStackMachineTemplateResource{ - Spec: OpenStackMachineSpec{ + newTemplate: &infrav1.OpenStackMachineTemplate{ + Spec: infrav1.OpenStackMachineTemplateSpec{ + Template: infrav1.OpenStackMachineTemplateResource{ + Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", Image: "NewImage", }, @@ -120,25 +122,25 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { }, { name: "allow modification, dry run, skip immutability annotation set", - oldTemplate: &OpenStackMachineTemplate{ - Spec: OpenStackMachineTemplateSpec{ - Template: OpenStackMachineTemplateResource{ - Spec: OpenStackMachineSpec{ + oldTemplate: &infrav1.OpenStackMachineTemplate{ + Spec: infrav1.OpenStackMachineTemplateSpec{ + Template: infrav1.OpenStackMachineTemplateResource{ + Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", Image: "bar", }, }, }, }, - newTemplate: &OpenStackMachineTemplate{ + newTemplate: &infrav1.OpenStackMachineTemplate{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ clusterv1.TopologyDryRunAnnotation: "", }, }, - Spec: OpenStackMachineTemplateSpec{ - Template: OpenStackMachineTemplateResource{ - Spec: OpenStackMachineSpec{ + Spec: infrav1.OpenStackMachineTemplateSpec{ + Template: infrav1.OpenStackMachineTemplateResource{ + Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", Image: "NewImage", }, @@ -154,7 +156,7 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - webhook := &OpenStackMachineTemplateWebhook{} + webhook := &openStackMachineTemplateWebhook{} ctx := admission.NewContextWithRequest(context.Background(), *tt.req) warn, err := webhook.ValidateUpdate(ctx, tt.oldTemplate, tt.newTemplate) diff --git a/pkg/webhooks/register.go b/pkg/webhooks/register.go new file mode 100644 index 0000000000..6eae031ff0 --- /dev/null +++ b/pkg/webhooks/register.go @@ -0,0 +1,63 @@ +/* +Copyright 2024 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 ( + "fmt" + + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/conversion" + "sigs.k8s.io/controller-runtime/pkg/manager" + + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7" +) + +func RegisterAllWithManager(mgr manager.Manager) []error { + var errs []error + + // Register webhooks for all types with custom validators. + for _, webhook := range []struct { + name string + setup func(ctrl.Manager) error + }{ + {"OpenStackCluster", SetupOpenStackClusterWebhook}, + {"OpenStackClusterTemplate", SetupOpenStackClusterTemplateWebhook}, + {"OpenStackMachine", SetupOpenStackMachineWebhook}, + {"OpenStackMachineTemplate", SetupOpenStackMachineTemplateWebhook}, + } { + if err := webhook.setup(mgr); err != nil { + errs = append(errs, fmt.Errorf("creating webhook for %s: %v", webhook.name, err)) + } + } + + // Additionally register webhooks for other types so they get conversion webhooks. + for _, conversionOnlyType := range []conversion.Hub{ + &infrav1.OpenStackClusterList{}, + &infrav1.OpenStackClusterTemplateList{}, + &infrav1.OpenStackMachineList{}, + &infrav1.OpenStackMachineTemplateList{}, + } { + if err := builder.WebhookManagedBy(mgr). + For(conversionOnlyType). + Complete(); err != nil { + errs = append(errs, fmt.Errorf("creating webhook for %T: %v", conversionOnlyType, err)) + } + } + + return errs +}