From b55a9e7a6bcf14e57dd79351a9ac114a5f8e0478 Mon Sep 17 00:00:00 2001 From: Matt Pryor Date: Fri, 29 Apr 2022 13:46:44 +0100 Subject: [PATCH 1/3] Fix nil pointer reference --- controllers/openstackcluster_controller.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go index 25d532d39d..a71f82dae7 100644 --- a/controllers/openstackcluster_controller.go +++ b/controllers/openstackcluster_controller.go @@ -221,12 +221,12 @@ func deleteBastion(scope *scope.Scope, cluster *clusterv1.Cluster, openStackClus } } } - } - instanceSpec := bastionToInstanceSpec(openStackCluster, cluster.Name) - if err = computeService.DeleteInstance(openStackCluster, instanceSpec, instanceStatus); err != nil { - handleUpdateOSCError(openStackCluster, errors.Errorf("failed to delete bastion: %v", err)) - return errors.Errorf("failed to delete bastion: %v", err) + instanceSpec := bastionToInstanceSpec(openStackCluster, cluster.Name) + if err = computeService.DeleteInstance(openStackCluster, instanceSpec, instanceStatus); err != nil { + handleUpdateOSCError(openStackCluster, errors.Errorf("failed to delete bastion: %v", err)) + return errors.Errorf("failed to delete bastion: %v", err) + } } openStackCluster.Status.Bastion = nil From 89693ce5d9d7a3fea1bb8949d54f8e4bdd952069 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Wed, 4 May 2022 12:29:41 +0200 Subject: [PATCH 2/3] move chrischdi to emeritus_approvers --- OWNERS | 1 + OWNERS_ALIASES | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/OWNERS b/OWNERS index 107615064d..a5008f7775 100644 --- a/OWNERS +++ b/OWNERS @@ -20,3 +20,4 @@ emeritus_approvers: - m1093782566 - sbueringer - hidekazuna + - chrischdi diff --git a/OWNERS_ALIASES b/OWNERS_ALIASES index adce2c23b1..956adb9811 100644 --- a/OWNERS_ALIASES +++ b/OWNERS_ALIASES @@ -25,7 +25,6 @@ aliases: - vincepri - CecileRobertMichon cluster-api-openstack-maintainers: - - chrischdi - jichenjc - mdbooth - seanschneeweiss From 94c9af8df5dd0210d565ba56a66cb5b5db61c0c1 Mon Sep 17 00:00:00 2001 From: Mario Constanti Date: Tue, 10 May 2022 06:40:46 +0200 Subject: [PATCH 3/3] remove v1alpha4 webhooks * as we already have webhooks for v1alpha5 we don't need webhooks for older versions anymore. * remove v1alpha4 from webhook mgr setup * remove defaultIdentityRefKind const in v1alpha4 Signed-off-by: Mario Constanti --- api/v1alpha4/identity_types.go | 2 - api/v1alpha4/openstackcluster_webhook.go | 130 ------- api/v1alpha4/openstackcluster_webhook_test.go | 330 ------------------ api/v1alpha4/openstackclusterlist_webhook.go | 32 -- .../openstackclustertemplate_webhook.go | 84 ----- api/v1alpha4/openstackmachine_webhook.go | 113 ------ api/v1alpha4/openstackmachinelist_webhook.go | 32 -- .../openstackmachinetemplate_webhook.go | 75 ---- .../openstackmachinetemplate_webhook_test.go | 100 ------ .../openstackmachinetemplatelist_webhook.go | 32 -- api/v1alpha4/webhooks.go | 35 -- api/v1alpha4/zz_generated.deepcopy.go | 2 +- config/webhook/manifests.yaml | 147 -------- main.go | 30 -- 14 files changed, 1 insertion(+), 1143 deletions(-) delete mode 100644 api/v1alpha4/openstackcluster_webhook.go delete mode 100644 api/v1alpha4/openstackcluster_webhook_test.go delete mode 100644 api/v1alpha4/openstackclusterlist_webhook.go delete mode 100644 api/v1alpha4/openstackclustertemplate_webhook.go delete mode 100644 api/v1alpha4/openstackmachine_webhook.go delete mode 100644 api/v1alpha4/openstackmachinelist_webhook.go delete mode 100644 api/v1alpha4/openstackmachinetemplate_webhook.go delete mode 100644 api/v1alpha4/openstackmachinetemplate_webhook_test.go delete mode 100644 api/v1alpha4/openstackmachinetemplatelist_webhook.go delete mode 100644 api/v1alpha4/webhooks.go diff --git a/api/v1alpha4/identity_types.go b/api/v1alpha4/identity_types.go index 0b73bfe238..1a17e97b38 100644 --- a/api/v1alpha4/identity_types.go +++ b/api/v1alpha4/identity_types.go @@ -16,8 +16,6 @@ limitations under the License. package v1alpha4 -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/v1alpha4/openstackcluster_webhook.go b/api/v1alpha4/openstackcluster_webhook.go deleted file mode 100644 index e939c2ab93..0000000000 --- a/api/v1alpha4/openstackcluster_webhook.go +++ /dev/null @@ -1,130 +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 v1alpha4 - -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" -) - -// 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-v1alpha4-openstackcluster,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackclusters,versions=v1alpha4,name=validation.openstackcluster.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 -// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1alpha4-openstackcluster,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackclusters,versions=v1alpha4,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() 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) error { - var allErrs field.ErrorList - old, ok := oldRaw.(*OpenStackCluster) - if !ok { - return 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 changes to the bastion spec only if no bastion host is deployed (i.e. Spec.Bastion.Enabled=false). - if old.Status.Bastion == nil { - old.Spec.Bastion = &Bastion{} - r.Spec.Bastion = &Bastion{} - } - - // Allow toggling the bastion enabled flag. - if old.Spec.Bastion != nil && r.Spec.Bastion != nil { - old.Spec.Bastion.Enabled = true - r.Spec.Bastion.Enabled = true - } - - 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() error { - return nil -} diff --git a/api/v1alpha4/openstackcluster_webhook_test.go b/api/v1alpha4/openstackcluster_webhook_test.go deleted file mode 100644 index d2b986bb19..0000000000 --- a/api/v1alpha4/openstackcluster_webhook_test.go +++ /dev/null @@ -1,330 +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 v1alpha4 - -import ( - "testing" - - . "github.com/onsi/gomega" -) - -func TestOpenStackCluster_ValidateUpdate(t *testing.T) { - g := NewWithT(t) - - tests := []struct { - name string - oldTemplate *OpenStackCluster - newTemplate *OpenStackCluster - wantErr bool - }{ - { - name: "OpenStackCluster.Spec.IdentityRef.Kind must always be Secret", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ - Kind: "Secret", - Name: "foobar", - }, - }, - }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ - Kind: "foobar", - Name: "foobar", - }, - }, - }, - wantErr: true, - }, - { - name: "Changing OpenStackCluster.Spec.IdentityRef.Name is allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ - Kind: "Secret", - Name: "foobar", - }, - }, - }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ - Kind: "Secret", - Name: "foobarbaz", - }, - }, - }, - wantErr: false, - }, - { - name: "OpenStackCluster.Spec.IdentityRef can be changed if it was unset", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - }, - }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ - Kind: "Secret", - Name: "foobar", - }, - }, - }, - wantErr: false, - }, - { - name: "OpenStackCluster.Spec.IdentityRef must not be removed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ - Kind: "Secret", - Name: "foobar", - }, - }, - }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - }, - }, - wantErr: true, - }, - { - name: "Toggle OpenStackCluster.Spec.Bastion.Enabled flag is allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - Bastion: &Bastion{ - Instance: OpenStackMachineSpec{ - CloudName: "foobar", - Image: "foobar", - Flavor: "minimal", - }, - Enabled: false, - }, - }, - }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - Bastion: &Bastion{ - Instance: OpenStackMachineSpec{ - CloudName: "foobarbaz", - Image: "foobarbaz", - Flavor: "medium", - }, - Enabled: true, - }, - }, - }, - wantErr: false, - }, - { - name: "Changing empty OpenStackCluster.Spec.Bastion is allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - }, - }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - Bastion: &Bastion{ - Instance: OpenStackMachineSpec{ - CloudName: "foobar", - Image: "foobar", - Flavor: "medium", - }, - Enabled: true, - }, - }, - }, - wantErr: false, - }, - { - name: "Changing OpenStackCluster.Spec.Bastion with no deployed bastion host is allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - Bastion: &Bastion{ - Instance: OpenStackMachineSpec{ - CloudName: "foobar", - Image: "foobar", - Flavor: "minimal", - }, - Enabled: false, - }, - }, - Status: OpenStackClusterStatus{}, - }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - Bastion: &Bastion{ - Instance: OpenStackMachineSpec{ - CloudName: "foobarbaz", - Image: "foobarbaz", - Flavor: "medium", - }, - Enabled: true, - }, - }, - }, - wantErr: false, - }, - { - name: "Changing OpenStackCluster.Spec.Bastion with deployed bastion host is not allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - Bastion: &Bastion{ - Instance: OpenStackMachineSpec{ - CloudName: "foobar", - Image: "foobar", - Flavor: "minimal", - }, - Enabled: true, - }, - }, - Status: OpenStackClusterStatus{ - Bastion: &Instance{ - Name: "foobar", - }, - }, - }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - Bastion: &Bastion{ - Instance: OpenStackMachineSpec{ - CloudName: "foobarbaz", - Image: "foobarbaz", - Flavor: "medium", - }, - Enabled: true, - }, - }, - }, - wantErr: true, - }, - { - name: "Disabling the OpenStackCluster.Spec.Bastion while it's running is allowed", - oldTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - Bastion: &Bastion{ - Instance: OpenStackMachineSpec{ - CloudName: "foobar", - Image: "foobar", - Flavor: "minimal", - }, - Enabled: true, - }, - }, - Status: OpenStackClusterStatus{ - Bastion: &Instance{ - Name: "foobar", - }, - }, - }, - newTemplate: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - Bastion: &Bastion{ - Instance: OpenStackMachineSpec{ - CloudName: "foobar", - Image: "foobar", - Flavor: "minimal", - }, - Enabled: false, - }, - }, - Status: OpenStackClusterStatus{ - Bastion: &Instance{ - Name: "foobar", - }, - }, - }, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := tt.newTemplate.ValidateUpdate(tt.oldTemplate) - if tt.wantErr { - g.Expect(err).To(HaveOccurred()) - } else { - g.Expect(err).NotTo(HaveOccurred()) - } - }) - } -} - -func TestOpenStackCluster_ValidateCreate(t *testing.T) { - g := NewWithT(t) - - tests := []struct { - name string - template *OpenStackCluster - wantErr bool - }{ - { - name: "OpenStackCluster.Spec.IdentityRef with correct spec on create", - template: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ - Kind: "Secret", - Name: "foobar", - }, - }, - }, - wantErr: false, - }, - { - name: "OpenStackCluster.Spec.IdentityRef with faulty spec on create", - template: &OpenStackCluster{ - Spec: OpenStackClusterSpec{ - CloudName: "foobar", - IdentityRef: &OpenStackIdentityReference{ - Kind: "foobar", - Name: "foobar", - }, - }, - }, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := tt.template.ValidateCreate() - if tt.wantErr { - g.Expect(err).To(HaveOccurred()) - } else { - g.Expect(err).NotTo(HaveOccurred()) - } - }) - } -} diff --git a/api/v1alpha4/openstackclusterlist_webhook.go b/api/v1alpha4/openstackclusterlist_webhook.go deleted file mode 100644 index 01cdfe6b9d..0000000000 --- a/api/v1alpha4/openstackclusterlist_webhook.go +++ /dev/null @@ -1,32 +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 v1alpha4 - -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/v1alpha4/openstackclustertemplate_webhook.go b/api/v1alpha4/openstackclustertemplate_webhook.go deleted file mode 100644 index acc848a085..0000000000 --- a/api/v1alpha4/openstackclustertemplate_webhook.go +++ /dev/null @@ -1,84 +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 v1alpha4 - -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" -) - -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-v1alpha4-openstackclustertemplate,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackclustertemplates,versions=v1alpha4,name=default.openstackclustertemplate.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 -// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha4-openstackclustertemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackclustertemplates,versions=v1alpha4,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() 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) error { - var allErrs field.ErrorList - old, ok := oldRaw.(*OpenStackClusterTemplate) - if !ok { - return 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() error { - return nil -} diff --git a/api/v1alpha4/openstackmachine_webhook.go b/api/v1alpha4/openstackmachine_webhook.go deleted file mode 100644 index 079e9e5097..0000000000 --- a/api/v1alpha4/openstackmachine_webhook.go +++ /dev/null @@ -1,113 +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 v1alpha4 - -import ( - "reflect" - - "github.com/pkg/errors" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "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" -) - -// log is for logging in this package. -var _ = logf.Log.WithName("openstackmachine-resource") - -func (r *OpenStackMachine) SetupWebhookWithManager(mgr manager.Manager) error { - return builder.WebhookManagedBy(mgr). - For(r). - Complete() -} - -// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha4-openstackmachine,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackmachines,versions=v1alpha4,name=validation.openstackmachine.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 -// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1alpha4-openstackmachine,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackmachines,versions=v1alpha4,name=default.openstackmachine.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 - -var ( - _ webhook.Defaulter = &OpenStackMachine{} - _ webhook.Validator = &OpenStackMachine{} -) - -// Default satisfies the defaulting webhook interface. -func (r *OpenStackMachine) 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 *OpenStackMachine) ValidateCreate() 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 *OpenStackMachine) ValidateUpdate(old runtime.Object) error { - newOpenStackMachine, err := runtime.DefaultUnstructuredConverter.ToUnstructured(r) - if err != nil { - return apierrors.NewInvalid(GroupVersion.WithKind("OpenStackMachine").GroupKind(), r.Name, field.ErrorList{ - field.InternalError(nil, errors.Wrap(err, "failed to convert new OpenStackMachine to unstructured object")), - }) - } - oldOpenStackMachine, err := runtime.DefaultUnstructuredConverter.ToUnstructured(old) - if err != nil { - return apierrors.NewInvalid(GroupVersion.WithKind("OpenStackMachine").GroupKind(), r.Name, field.ErrorList{ - field.InternalError(nil, errors.Wrap(err, "failed to convert old OpenStackMachine to unstructured object")), - }) - } - - 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")) - } - - newOpenStackMachineSpec := newOpenStackMachine["spec"].(map[string]interface{}) - oldOpenStackMachineSpec := oldOpenStackMachine["spec"].(map[string]interface{}) - - // allow changes to providerID once - if oldOpenStackMachineSpec["providerID"] == nil { - delete(oldOpenStackMachineSpec, "providerID") - delete(newOpenStackMachineSpec, "providerID") - } - - // allow changes to instanceID once - if oldOpenStackMachineSpec["instanceID"] == nil { - delete(oldOpenStackMachineSpec, "instanceID") - delete(newOpenStackMachineSpec, "instanceID") - } - - if !reflect.DeepEqual(oldOpenStackMachineSpec, newOpenStackMachineSpec) { - 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 *OpenStackMachine) ValidateDelete() error { - return nil -} diff --git a/api/v1alpha4/openstackmachinelist_webhook.go b/api/v1alpha4/openstackmachinelist_webhook.go deleted file mode 100644 index cf7763edff..0000000000 --- a/api/v1alpha4/openstackmachinelist_webhook.go +++ /dev/null @@ -1,32 +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 v1alpha4 - -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() -} diff --git a/api/v1alpha4/openstackmachinetemplate_webhook.go b/api/v1alpha4/openstackmachinetemplate_webhook.go deleted file mode 100644 index 6aea3242c2..0000000000 --- a/api/v1alpha4/openstackmachinetemplate_webhook.go +++ /dev/null @@ -1,75 +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 v1alpha4 - -import ( - "fmt" - "reflect" - - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/validation/field" - "sigs.k8s.io/controller-runtime/pkg/builder" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/webhook" -) - -// 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" - -func (r *OpenStackMachineTemplate) SetupWebhookWithManager(mgr manager.Manager) error { - return builder.WebhookManagedBy(mgr). - For(r). - Complete() -} - -// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1alpha4-openstackmachinetemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=openstackmachinetemplates,versions=v1alpha4,name=validation.openstackmachinetemplate.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 - -var _ webhook.Validator = &OpenStackMachineTemplate{} - -// ValidateCreate implements webhook.Validator so a webhook will be registered for the type. -func (r *OpenStackMachineTemplate) ValidateCreate() error { - var allErrs field.ErrorList - - if r.Spec.Template.Spec.ProviderID != nil { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "template", "spec", "providerID"), "cannot be set in templates")) - } - - return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs) -} - -// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. -func (r *OpenStackMachineTemplate) ValidateUpdate(oldRaw runtime.Object) error { - var allErrs field.ErrorList - old, ok := oldRaw.(*OpenStackMachineTemplate) - if !ok { - return apierrors.NewBadRequest(fmt.Sprintf("expected an OpenStackMachineTemplate but got a %T", oldRaw)) - } - - if !reflect.DeepEqual(r.Spec.Template.Spec, old.Spec.Template.Spec) { - allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "template", "spec"), r, OpenStackMachineTemplateImmutableMsg), - ) - } - - return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs) -} - -// ValidateDelete implements webhook.Validator so a webhook will be registered for the type. -func (r *OpenStackMachineTemplate) ValidateDelete() error { - return nil -} diff --git a/api/v1alpha4/openstackmachinetemplate_webhook_test.go b/api/v1alpha4/openstackmachinetemplate_webhook_test.go deleted file mode 100644 index cd64c6ae04..0000000000 --- a/api/v1alpha4/openstackmachinetemplate_webhook_test.go +++ /dev/null @@ -1,100 +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 v1alpha4 - -import ( - "testing" - - . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { - g := NewWithT(t) - - tests := []struct { - name string - oldTemplate *OpenStackMachineTemplate - newTemplate *OpenStackMachineTemplate - wantErr bool - }{ - { - name: "OpenStackMachineTemplate with immutable spec", - oldTemplate: &OpenStackMachineTemplate{ - Spec: OpenStackMachineTemplateSpec{ - Template: OpenStackMachineTemplateResource{ - Spec: OpenStackMachineSpec{ - Flavor: "foo", - Image: "bar", - }, - }, - }, - }, - newTemplate: &OpenStackMachineTemplate{ - Spec: OpenStackMachineTemplateSpec{ - Template: OpenStackMachineTemplateResource{ - Spec: OpenStackMachineSpec{ - Flavor: "foo", - Image: "NewImage", - }, - }, - }, - }, - wantErr: true, - }, - { - name: "OpenStackMachineTemplate with mutable metadata", - oldTemplate: &OpenStackMachineTemplate{ - Spec: OpenStackMachineTemplateSpec{ - Template: OpenStackMachineTemplateResource{ - Spec: OpenStackMachineSpec{ - Flavor: "foo", - Image: "bar", - }, - }, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - }, - }, - newTemplate: &OpenStackMachineTemplate{ - Spec: OpenStackMachineTemplateSpec{ - Template: OpenStackMachineTemplateResource{ - Spec: OpenStackMachineSpec{ - Flavor: "foo", - Image: "bar", - }, - }, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "bar", - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - err := tt.newTemplate.ValidateUpdate(tt.oldTemplate) - if tt.wantErr { - g.Expect(err).To(HaveOccurred()) - } else { - g.Expect(err).NotTo(HaveOccurred()) - } - }) - } -} diff --git a/api/v1alpha4/openstackmachinetemplatelist_webhook.go b/api/v1alpha4/openstackmachinetemplatelist_webhook.go deleted file mode 100644 index a10fbd167c..0000000000 --- a/api/v1alpha4/openstackmachinetemplatelist_webhook.go +++ /dev/null @@ -1,32 +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 v1alpha4 - -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/v1alpha4/webhooks.go b/api/v1alpha4/webhooks.go deleted file mode 100644 index ca0a158e6f..0000000000 --- a/api/v1alpha4/webhooks.go +++ /dev/null @@ -1,35 +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 v1alpha4 - -import ( - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/validation/field" -) - -func aggregateObjErrors(gk schema.GroupKind, name string, allErrs field.ErrorList) error { - if len(allErrs) == 0 { - return nil - } - - return apierrors.NewInvalid( - gk, - name, - allErrs, - ) -} diff --git a/api/v1alpha4/zz_generated.deepcopy.go b/api/v1alpha4/zz_generated.deepcopy.go index a5b7798c9a..381ba8788e 100644 --- a/api/v1alpha4/zz_generated.deepcopy.go +++ b/api/v1alpha4/zz_generated.deepcopy.go @@ -23,7 +23,7 @@ package v1alpha4 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 7f0242b1a8..c8ff085770 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -69,69 +69,6 @@ webhooks: resources: - openstackmachines sideEffects: None -- admissionReviewVersions: - - v1beta1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /mutate-infrastructure-cluster-x-k8s-io-v1alpha4-openstackcluster - failurePolicy: Fail - matchPolicy: Equivalent - name: default.openstackcluster.infrastructure.cluster.x-k8s.io - rules: - - apiGroups: - - infrastructure.cluster.x-k8s.io - apiVersions: - - v1alpha4 - operations: - - CREATE - - UPDATE - resources: - - openstackclusters - sideEffects: None -- admissionReviewVersions: - - v1beta1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /mutate-infrastructure-cluster-x-k8s-io-v1alpha4-openstackclustertemplate - failurePolicy: Fail - matchPolicy: Equivalent - name: default.openstackclustertemplate.infrastructure.cluster.x-k8s.io - rules: - - apiGroups: - - infrastructure.cluster.x-k8s.io - apiVersions: - - v1alpha4 - operations: - - CREATE - - UPDATE - resources: - - openstackclustertemplates - sideEffects: None -- admissionReviewVersions: - - v1beta1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /mutate-infrastructure-cluster-x-k8s-io-v1alpha4-openstackmachine - failurePolicy: Fail - matchPolicy: Equivalent - name: default.openstackmachine.infrastructure.cluster.x-k8s.io - rules: - - apiGroups: - - infrastructure.cluster.x-k8s.io - apiVersions: - - v1alpha4 - operations: - - CREATE - - UPDATE - resources: - - openstackmachines - sideEffects: None --- apiVersion: admissionregistration.k8s.io/v1 @@ -224,87 +161,3 @@ webhooks: resources: - openstackmachinetemplates sideEffects: None -- admissionReviewVersions: - - v1beta1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /validate-infrastructure-cluster-x-k8s-io-v1alpha4-openstackcluster - failurePolicy: Fail - matchPolicy: Equivalent - name: validation.openstackcluster.infrastructure.cluster.x-k8s.io - rules: - - apiGroups: - - infrastructure.cluster.x-k8s.io - apiVersions: - - v1alpha4 - operations: - - CREATE - - UPDATE - resources: - - openstackclusters - sideEffects: None -- admissionReviewVersions: - - v1beta1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /validate-infrastructure-cluster-x-k8s-io-v1alpha4-openstackclustertemplate - failurePolicy: Fail - matchPolicy: Equivalent - name: validation.openstackclustertemplate.infrastructure.cluster.x-k8s.io - rules: - - apiGroups: - - infrastructure.cluster.x-k8s.io - apiVersions: - - v1alpha4 - operations: - - CREATE - - UPDATE - resources: - - openstackclustertemplates - sideEffects: None -- admissionReviewVersions: - - v1beta1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /validate-infrastructure-cluster-x-k8s-io-v1alpha4-openstackmachine - failurePolicy: Fail - matchPolicy: Equivalent - name: validation.openstackmachine.infrastructure.cluster.x-k8s.io - rules: - - apiGroups: - - infrastructure.cluster.x-k8s.io - apiVersions: - - v1alpha4 - operations: - - CREATE - - UPDATE - resources: - - openstackmachines - sideEffects: None -- admissionReviewVersions: - - v1beta1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /validate-infrastructure-cluster-x-k8s-io-v1alpha4-openstackmachinetemplate - failurePolicy: Fail - matchPolicy: Equivalent - name: validation.openstackmachinetemplate.infrastructure.cluster.x-k8s.io - rules: - - apiGroups: - - infrastructure.cluster.x-k8s.io - apiVersions: - - v1alpha4 - operations: - - CREATE - - UPDATE - resources: - - openstackmachinetemplates - sideEffects: None diff --git a/main.go b/main.go index a63dc3ea9f..bcfc0e53f6 100644 --- a/main.go +++ b/main.go @@ -246,36 +246,6 @@ func setupWebhooks(mgr ctrl.Manager) { setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackClusterList") os.Exit(1) } - - // infrav1alpha4 - if err := (&infrav1alpha4.OpenStackMachineTemplate{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackMachineTemplate") - os.Exit(1) - } - if err := (&infrav1alpha4.OpenStackMachineTemplateList{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackMachineTemplateList") - os.Exit(1) - } - if err := (&infrav1alpha4.OpenStackCluster{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackCluster") - os.Exit(1) - } - if err := (&infrav1alpha4.OpenStackClusterTemplate{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackClusterTemplate") - os.Exit(1) - } - if err := (&infrav1alpha4.OpenStackMachine{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackMachine") - os.Exit(1) - } - if err := (&infrav1alpha4.OpenStackMachineList{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackMachineList") - os.Exit(1) - } - if err := (&infrav1alpha4.OpenStackClusterList{}).SetupWebhookWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create webhook", "webhook", "OpenStackClusterList") - os.Exit(1) - } } func concurrency(c int) controller.Options {