From 226f978888a511d4bbb3086684f26aff30a75790 Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Wed, 2 Aug 2023 15:22:50 +0300 Subject: [PATCH 01/20] prevent deletion init --- api/v1/serviceinstance_types.go | 6 ++++++ api/v1alpha1/serviceinstance_types.go | 5 +++++ client/sm/types/service_instance.go | 7 ++++--- sapbtp-operator-charts/templates/crd.yml | 6 ++++++ 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/api/v1/serviceinstance_types.go b/api/v1/serviceinstance_types.go index fd223bcb..e4c659ff 100644 --- a/api/v1/serviceinstance_types.go +++ b/api/v1/serviceinstance_types.go @@ -56,6 +56,11 @@ type ServiceInstanceSpec struct { // +kubebuilder:default={} Shared *bool `json:"shared,omitempty"` + // Indicates if the instance deletion will be prevented + // +optional + // +kubebuilder:default=false + PreventDeletion bool `json:"prevent_deletion"` + // Provisioning parameters for the instance. // // The Parameters field is NOT secret or secured in any way and should @@ -121,6 +126,7 @@ type ServiceInstanceStatus struct { // +kubebuilder:printcolumn:JSONPath=".spec.serviceOfferingName",name="Offering",type=string // +kubebuilder:printcolumn:JSONPath=".spec.servicePlanName",name="Plan",type=string // +kubebuilder:printcolumn:JSONPath=".spec.shared",name="shared",type=boolean +// +kubebuilder:printcolumn:JSONPath=".spec.preventDeletion",name="PreventDeletion",type=boolean // +kubebuilder:printcolumn:JSONPath=".spec.dataCenter",name="dataCenter",type=string // +kubebuilder:printcolumn:JSONPath=".status.conditions[0].reason",name="Status",type=string // +kubebuilder:printcolumn:JSONPath=".status.ready",name="Ready",type=string diff --git a/api/v1alpha1/serviceinstance_types.go b/api/v1alpha1/serviceinstance_types.go index 04879bb2..1ba7c00e 100644 --- a/api/v1alpha1/serviceinstance_types.go +++ b/api/v1alpha1/serviceinstance_types.go @@ -52,6 +52,11 @@ type ServiceInstanceSpec struct { // +kubebuilder:default={} Shared *bool `json:"shared,omitempty"` + // Indicates if the instance deletion will be prevented + // +optional + // +kubebuilder:default=false + PreventDeletion bool `json:"prevent_deletion"` + // Provisioning parameters for the instance. // // The Parameters field is NOT secret or secured in any way and should diff --git a/client/sm/types/service_instance.go b/client/sm/types/service_instance.go index 3b8e4729..bc31cef5 100644 --- a/client/sm/types/service_instance.go +++ b/client/sm/types/service_instance.go @@ -40,9 +40,10 @@ type ServiceInstance struct { Context json.RawMessage `json:"context,omitempty" yaml:"context,omitempty"` PreviousValues json.RawMessage `json:"-" yaml:"-"` - Ready bool `json:"ready" yaml:"ready"` - Usable bool `json:"usable" yaml:"usable"` - Shared bool `json:"shared,omitempty" yaml:"shared,omitempty"` + Ready bool `json:"ready" yaml:"ready"` + Usable bool `json:"usable" yaml:"usable"` + Shared bool `json:"shared,omitempty" yaml:"shared,omitempty"` + PreventDeletion bool `json:"prevent_deletion,omitempty" yaml:"prevent_deletion,omitempty"` LastOperation *Operation `json:"last_operation,omitempty" yaml:"last_operation,omitempty"` } diff --git a/sapbtp-operator-charts/templates/crd.yml b/sapbtp-operator-charts/templates/crd.yml index 9c4a49d6..31ef6946 100644 --- a/sapbtp-operator-charts/templates/crd.yml +++ b/sapbtp-operator-charts/templates/crd.yml @@ -645,6 +645,9 @@ spec: shared: description: Indicates if the instance aim to be shared, default is false type: boolean + preventDeletion: + description: Indicates if the instance deletion will be prevented + type: boolean userInfo: description: UserInfo contains information about the user that last modified this instance. This field is set by the API server and @@ -892,6 +895,9 @@ spec: shared: description: Indicates if the instance aim to be shared, default is false type: boolean + preventDeletion: + description: Indicates if the instance deletion will be prevented + type: boolean userInfo: description: UserInfo contains information about the user that last modified this instance. This field is set by the API server and From dedc0fe336d174f75100f7758799b10c3a239af4 Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Wed, 2 Aug 2023 15:37:56 +0300 Subject: [PATCH 02/20] prevent deletion init --- api/v1/serviceinstance_types.go | 17 ++++++++ api/v1/serviceinstance_validating_webhook.go | 39 +++++++++++++++++++ ...serviceinstance_validating_webhook_test.go | 30 ++++++++++++++ config/webhook/manifests.yaml | 22 ++++++++++- sapbtp-operator-charts/templates/webhook.yml | 26 +++++++++++++ 5 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 api/v1/serviceinstance_validating_webhook.go create mode 100644 api/v1/serviceinstance_validating_webhook_test.go diff --git a/api/v1/serviceinstance_types.go b/api/v1/serviceinstance_types.go index e4c659ff..713c8713 100644 --- a/api/v1/serviceinstance_types.go +++ b/api/v1/serviceinstance_types.go @@ -17,11 +17,13 @@ limitations under the License. package v1 import ( + "fmt" "github.com/SAP/sap-btp-service-operator/api" "github.com/SAP/sap-btp-service-operator/client/sm/types" v1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) // EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! @@ -142,6 +144,21 @@ type ServiceInstance struct { Status ServiceInstanceStatus `json:"status,omitempty"` } +func (si *ServiceInstance) ValidateCreate() (warnings admission.Warnings, err error) { + return nil, nil +} + +func (si *ServiceInstance) ValidateUpdate(old runtime.Object) (warnings admission.Warnings, err error) { + return nil, nil +} + +func (si *ServiceInstance) ValidateDelete() (warnings admission.Warnings, err error) { + if si.Spec.PreventDeletion { + return nil, fmt.Errorf("service instance %s marked as prevent deletion", si.Name) + } + return nil, nil +} + func (si *ServiceInstance) GetConditions() []metav1.Condition { return si.Status.Conditions } diff --git a/api/v1/serviceinstance_validating_webhook.go b/api/v1/serviceinstance_validating_webhook.go new file mode 100644 index 00000000..c245c5f5 --- /dev/null +++ b/api/v1/serviceinstance_validating_webhook.go @@ -0,0 +1,39 @@ +/* + + +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 v1 + +import ( + ctrl "sigs.k8s.io/controller-runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook" +) + +// log is for logging in this package. +var serviceinstancelog = logf.Log.WithName("serviceinstance-resource") + +func (si *ServiceInstance) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(si). + Complete() +} + +// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! + +// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation. +// +kubebuilder:webhook:verbs=delete,path=/validate-services-cloud-sap-com-v1-serviceinstance,mutating=false,failurePolicy=fail,groups=services.cloud.sap.com,resources=serviceinstances,versions=v1,name=vserviceinstance.kb.io,sideEffects=None,admissionReviewVersions=v1beta1;v1 + +var _ webhook.Validator = &ServiceInstance{} diff --git a/api/v1/serviceinstance_validating_webhook_test.go b/api/v1/serviceinstance_validating_webhook_test.go new file mode 100644 index 00000000..2b4497f6 --- /dev/null +++ b/api/v1/serviceinstance_validating_webhook_test.go @@ -0,0 +1,30 @@ +package v1 + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Service Binding Webhook Test", func() { + var instance *ServiceInstance + BeforeEach(func() { + instance = getInstance() + }) + + Context("Validate Delete", func() { + When("service instance is marked as prevent deletion", func() { + It("should not delete the instance", func() { + instance.Spec.PreventDeletion = true + _, err := instance.ValidateDelete() + Expect(err).To(HaveOccurred()) + }) + }) + + When("service instance is not marked as prevent deletion", func() { + It("should delete the instance", func() { + _, err := instance.ValidateDelete() + Expect(err).ToNot(HaveOccurred()) + }) + }) + }) +}) diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 9c2ca8a2..ea55ad0d 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -2,7 +2,6 @@ apiVersion: admissionregistration.k8s.io/v1 kind: MutatingWebhookConfiguration metadata: - creationTimestamp: null name: mutating-webhook-configuration webhooks: - admissionReviewVersions: @@ -51,7 +50,6 @@ webhooks: apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: - creationTimestamp: null name: validating-webhook-configuration webhooks: - admissionReviewVersions: @@ -75,3 +73,23 @@ webhooks: resources: - servicebindings sideEffects: None +- admissionReviewVersions: + - v1beta1 + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-services-cloud-sap-com-v1-serviceinstance + failurePolicy: Fail + name: vserviceinstance.kb.io + rules: + - apiGroups: + - services.cloud.sap.com + apiVersions: + - v1 + operations: + - DELETE + resources: + - serviceinstances + sideEffects: None diff --git a/sapbtp-operator-charts/templates/webhook.yml b/sapbtp-operator-charts/templates/webhook.yml index ef64ce21..0e081042 100644 --- a/sapbtp-operator-charts/templates/webhook.yml +++ b/sapbtp-operator-charts/templates/webhook.yml @@ -97,4 +97,30 @@ webhooks: - UPDATE resources: - servicebindings + sideEffects: None + - admissionReviewVersions: + - v1beta1 + - v1 + clientConfig: + service: + name: sap-btp-operator-webhook-service + namespace: {{.Release.Namespace}} + path: /validate-services-cloud-sap-com-v1-serviceinstance + {{- if .Values.manager.certificates.selfSigned }} + caBundle: {{.Values.manager.certificates.selfSigned.caBundle }} + {{- end }} + {{- if .Values.manager.certificates.gardenerCertManager }} + caBundle: {{.Values.manager.certificates.gardenerCertManager.caBundle }} + {{- end }} + failurePolicy: Fail + name: vserviceinstance.kb.io + rules: + - apiGroups: + - services.cloud.sap.com + apiVersions: + - v1 + operations: + - DELETE + resources: + - serviceinstances sideEffects: None \ No newline at end of file From 8a5d682cab55e3a0b42a47b5cb79e0f714fe360a Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Wed, 2 Aug 2023 15:40:06 +0300 Subject: [PATCH 03/20] prevent deletion init --- api/v1/serviceinstance_types.go | 17 ----------------- api/v1/serviceinstance_validating_webhook.go | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/api/v1/serviceinstance_types.go b/api/v1/serviceinstance_types.go index 713c8713..e4c659ff 100644 --- a/api/v1/serviceinstance_types.go +++ b/api/v1/serviceinstance_types.go @@ -17,13 +17,11 @@ limitations under the License. package v1 import ( - "fmt" "github.com/SAP/sap-btp-service-operator/api" "github.com/SAP/sap-btp-service-operator/client/sm/types" v1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) // EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! @@ -144,21 +142,6 @@ type ServiceInstance struct { Status ServiceInstanceStatus `json:"status,omitempty"` } -func (si *ServiceInstance) ValidateCreate() (warnings admission.Warnings, err error) { - return nil, nil -} - -func (si *ServiceInstance) ValidateUpdate(old runtime.Object) (warnings admission.Warnings, err error) { - return nil, nil -} - -func (si *ServiceInstance) ValidateDelete() (warnings admission.Warnings, err error) { - if si.Spec.PreventDeletion { - return nil, fmt.Errorf("service instance %s marked as prevent deletion", si.Name) - } - return nil, nil -} - func (si *ServiceInstance) GetConditions() []metav1.Condition { return si.Status.Conditions } diff --git a/api/v1/serviceinstance_validating_webhook.go b/api/v1/serviceinstance_validating_webhook.go index c245c5f5..ab69a837 100644 --- a/api/v1/serviceinstance_validating_webhook.go +++ b/api/v1/serviceinstance_validating_webhook.go @@ -17,9 +17,12 @@ limitations under the License. package v1 import ( + "fmt" + "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) // log is for logging in this package. @@ -37,3 +40,18 @@ func (si *ServiceInstance) SetupWebhookWithManager(mgr ctrl.Manager) error { // +kubebuilder:webhook:verbs=delete,path=/validate-services-cloud-sap-com-v1-serviceinstance,mutating=false,failurePolicy=fail,groups=services.cloud.sap.com,resources=serviceinstances,versions=v1,name=vserviceinstance.kb.io,sideEffects=None,admissionReviewVersions=v1beta1;v1 var _ webhook.Validator = &ServiceInstance{} + +func (si *ServiceInstance) ValidateCreate() (warnings admission.Warnings, err error) { + return nil, nil +} + +func (si *ServiceInstance) ValidateUpdate(old runtime.Object) (warnings admission.Warnings, err error) { + return nil, nil +} + +func (si *ServiceInstance) ValidateDelete() (warnings admission.Warnings, err error) { + if si.Spec.PreventDeletion { + return nil, fmt.Errorf("service instance %s marked as prevent deletion", si.Name) + } + return nil, nil +} From 814035fc017c8ecd2f780dc8208b9067410cf4ba Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Wed, 2 Aug 2023 15:48:00 +0300 Subject: [PATCH 04/20] prevent deletion init --- api/v1/serviceinstance_validating_webhook.go | 7 ------- controllers/serviceinstance_controller_test.go | 3 ++- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/api/v1/serviceinstance_validating_webhook.go b/api/v1/serviceinstance_validating_webhook.go index ab69a837..55083058 100644 --- a/api/v1/serviceinstance_validating_webhook.go +++ b/api/v1/serviceinstance_validating_webhook.go @@ -20,23 +20,16 @@ import ( "fmt" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" - logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -// log is for logging in this package. -var serviceinstancelog = logf.Log.WithName("serviceinstance-resource") - func (si *ServiceInstance) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). For(si). Complete() } -// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! - -// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation. // +kubebuilder:webhook:verbs=delete,path=/validate-services-cloud-sap-com-v1-serviceinstance,mutating=false,failurePolicy=fail,groups=services.cloud.sap.com,resources=serviceinstances,versions=v1,name=vserviceinstance.kb.io,sideEffects=None,admissionReviewVersions=v1beta1;v1 var _ webhook.Validator = &ServiceInstance{} diff --git a/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go index 9c4dffe2..d253acda 100644 --- a/controllers/serviceinstance_controller_test.go +++ b/controllers/serviceinstance_controller_test.go @@ -54,6 +54,7 @@ var _ = Describe("ServiceInstance controller", func() { instanceSpec := v1.ServiceInstanceSpec{ ExternalName: fakeInstanceExternalName, + PreventDeletion: true, ServicePlanName: fakePlanName, ServiceOfferingName: fakeOfferingName, Parameters: &runtime.RawExtension{ @@ -624,7 +625,7 @@ var _ = Describe("ServiceInstance controller", func() { BeforeEach(func() { fakeClient.DeprovisionReturns("", nil) }) - It("should delete the k8s instance", func() { + FIt("should delete the k8s instance", func() { deleteInstance(ctx, serviceInstance, true) }) }) From f07d401d769ed47f0f92ade17066f6d6d433c800 Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Wed, 2 Aug 2023 15:48:10 +0300 Subject: [PATCH 05/20] prevent deletion init --- .../bases/services.cloud.sap.com_serviceinstances.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml b/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml index 352b08ec..927b1f35 100644 --- a/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml +++ b/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml @@ -25,6 +25,9 @@ spec: - jsonPath: .spec.shared name: shared type: boolean + - jsonPath: .spec.preventDeletion + name: preventDeletion + type: boolean - jsonPath: .spec.dataCenter name: dataCenter type: string @@ -128,6 +131,9 @@ spec: shared: description: Indicates the desired shared state type: boolean + preventDeletion: + description: Indicates if the instance deletion will be prevented + type: boolean userInfo: description: UserInfo contains information about the user that last modified this instance. This field is set by the API server and @@ -372,6 +378,9 @@ spec: shared: description: Indicates the desired shared state type: boolean + preventDeletion: + description: Indicates if the instance deletion will be prevented + type: boolean userInfo: description: UserInfo contains information about the user that last modified this instance. This field is set by the API server and From 87b57b8ac8dcb66da4ad1735925a4f39d852b6e8 Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Wed, 2 Aug 2023 16:02:28 +0300 Subject: [PATCH 06/20] prevent deletion init --- .../serviceinstance_controller_test.go | 3 +-- controllers/suite_test.go | 26 ++++++++++--------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go index d253acda..9c4dffe2 100644 --- a/controllers/serviceinstance_controller_test.go +++ b/controllers/serviceinstance_controller_test.go @@ -54,7 +54,6 @@ var _ = Describe("ServiceInstance controller", func() { instanceSpec := v1.ServiceInstanceSpec{ ExternalName: fakeInstanceExternalName, - PreventDeletion: true, ServicePlanName: fakePlanName, ServiceOfferingName: fakeOfferingName, Parameters: &runtime.RawExtension{ @@ -625,7 +624,7 @@ var _ = Describe("ServiceInstance controller", func() { BeforeEach(func() { fakeClient.DeprovisionReturns("", nil) }) - FIt("should delete the k8s instance", func() { + It("should delete the k8s instance", func() { deleteInstance(ctx, serviceInstance, true) }) }) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index e838e0dc..e1aabb50 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -19,15 +19,16 @@ package controllers import ( "context" "crypto/tls" + "github.com/SAP/sap-btp-service-operator/api/v1/webhooks" "net" "path/filepath" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "testing" "time" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - + "fmt" "github.com/SAP/sap-btp-service-operator/api" - "github.com/SAP/sap-btp-service-operator/api/v1/webhooks" "github.com/SAP/sap-btp-service-operator/client/sm" "github.com/SAP/sap-btp-service-operator/client/sm/smfakes" "github.com/SAP/sap-btp-service-operator/internal/config" @@ -35,9 +36,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" - "sigs.k8s.io/controller-runtime/pkg/webhook" - - "fmt" ctrl "sigs.k8s.io/controller-runtime" @@ -113,6 +111,16 @@ var _ = BeforeSuite(func(done Done) { testConfig := config.Get() testConfig.SyncPeriod = syncPeriod testConfig.PollInterval = pollInterval + + k8sManager.GetWebhookServer().Register("/mutate-services-cloud-sap-com-v1-serviceinstance", &webhook.Admission{Handler: &webhooks.ServiceInstanceDefaulter{Decoder: admission.NewDecoder(k8sManager.GetScheme())}}) + k8sManager.GetWebhookServer().Register("/mutate-services-cloud-sap-com-v1-servicebinding", &webhook.Admission{Handler: &webhooks.ServiceBindingDefaulter{Decoder: admission.NewDecoder(k8sManager.GetScheme())}}) + + err = (&servicesv1.ServiceBinding{}).SetupWebhookWithManager(k8sManager) + Expect(err).ToNot(HaveOccurred()) + + err = (&servicesv1.ServiceInstance{}).SetupWebhookWithManager(k8sManager) + Expect(err).ToNot(HaveOccurred()) + err = (&ServiceInstanceReconciler{ BaseReconciler: &BaseReconciler{ Client: k8sManager.GetClient(), @@ -125,12 +133,6 @@ var _ = BeforeSuite(func(done Done) { }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) - k8sManager.GetWebhookServer().Register("/mutate-services-cloud-sap-com-v1-serviceinstance", &webhook.Admission{Handler: &webhooks.ServiceInstanceDefaulter{Decoder: admission.NewDecoder(k8sManager.GetScheme())}}) - k8sManager.GetWebhookServer().Register("/mutate-services-cloud-sap-com-v1-servicebinding", &webhook.Admission{Handler: &webhooks.ServiceBindingDefaulter{Decoder: admission.NewDecoder(k8sManager.GetScheme())}}) - - err = (&servicesv1.ServiceBinding{}).SetupWebhookWithManager(k8sManager) - Expect(err).ToNot(HaveOccurred()) - err = (&ServiceBindingReconciler{ BaseReconciler: &BaseReconciler{ Client: k8sManager.GetClient(), From a0f6bbfe1cfa1a586dff637b3b91a25bbc473b1d Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Wed, 2 Aug 2023 16:12:59 +0300 Subject: [PATCH 07/20] prevent deletion init --- controllers/serviceinstance_controller_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go index 9c4dffe2..906f2465 100644 --- a/controllers/serviceinstance_controller_test.go +++ b/controllers/serviceinstance_controller_test.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "strings" + "time" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/utils/pointer" @@ -629,6 +630,21 @@ var _ = Describe("ServiceInstance controller", func() { }) }) + When("instance is marked for prevent deletion", func() { + BeforeEach(func() { + fakeClient.UpdateInstanceReturns(nil, "", nil) + fakeClient.DeprovisionReturns("", nil) + }) + It("should not delete the instance", func() { + serviceInstance.Spec.PreventDeletion = true + Expect(k8sClient.Update(ctx, serviceInstance)).To(Succeed()) + deleteInstance(ctx, serviceInstance, false) + time.Sleep(10) + err := k8sClient.Get(ctx, defaultLookupKey, serviceInstance) + Expect(err).ToNot(HaveOccurred()) + }) + }) + When("delete without instance id", func() { JustBeforeEach(func() { fakeClient.DeprovisionReturns("", nil) From fd11783198316a40403c15b1fe15cf30e53b4e48 Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Wed, 2 Aug 2023 16:27:01 +0300 Subject: [PATCH 08/20] prevent deletion init --- config/webhook/manifests.yaml | 20 +++++++------- sapbtp-operator-charts/templates/webhook.yml | 28 ++++++++++---------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index ea55ad0d..9d245ee1 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -74,8 +74,8 @@ webhooks: - servicebindings sideEffects: None - admissionReviewVersions: - - v1beta1 - - v1 + - v1beta1 + - v1 clientConfig: service: name: webhook-service @@ -84,12 +84,12 @@ webhooks: failurePolicy: Fail name: vserviceinstance.kb.io rules: - - apiGroups: - - services.cloud.sap.com - apiVersions: - - v1 - operations: - - DELETE - resources: - - serviceinstances + - apiGroups: + - services.cloud.sap.com + apiVersions: + - v1 + operations: + - DELETE + resources: + - serviceinstances sideEffects: None diff --git a/sapbtp-operator-charts/templates/webhook.yml b/sapbtp-operator-charts/templates/webhook.yml index 0e081042..9c17ec89 100644 --- a/sapbtp-operator-charts/templates/webhook.yml +++ b/sapbtp-operator-charts/templates/webhook.yml @@ -102,25 +102,25 @@ webhooks: - v1beta1 - v1 clientConfig: - service: - name: sap-btp-operator-webhook-service - namespace: {{.Release.Namespace}} - path: /validate-services-cloud-sap-com-v1-serviceinstance + service: + name: sap-btp-operator-webhook-service + namespace: {{.Release.Namespace}} + path: /validate-services-cloud-sap-com-v1-serviceinstance {{- if .Values.manager.certificates.selfSigned }} - caBundle: {{.Values.manager.certificates.selfSigned.caBundle }} + caBundle: {{.Values.manager.certificates.selfSigned.caBundle }} {{- end }} {{- if .Values.manager.certificates.gardenerCertManager }} - caBundle: {{.Values.manager.certificates.gardenerCertManager.caBundle }} + caBundle: {{.Values.manager.certificates.gardenerCertManager.caBundle }} {{- end }} failurePolicy: Fail name: vserviceinstance.kb.io rules: - - apiGroups: - - services.cloud.sap.com - apiVersions: - - v1 - operations: - - DELETE - resources: - - serviceinstances + - apiGroups: + - services.cloud.sap.com + apiVersions: + - v1 + operations: + - DELETE + resources: + - serviceinstances sideEffects: None \ No newline at end of file From 05d4ea600c12ac1547de26ff634d65f92daad134 Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Thu, 3 Aug 2023 09:04:29 +0300 Subject: [PATCH 09/20] remove prevent deletion from spec to annotation --- api/common.go | 1 + api/v1/serviceinstance_types.go | 6 ------ api/v1/serviceinstance_validating_webhook.go | 7 +++++-- ...serviceinstance_validating_webhook_test.go | 5 ++++- api/v1alpha1/serviceinstance_types.go | 5 ----- client/sm/types/service_instance.go | 7 +++---- ...rvices.cloud.sap.com_serviceinstances.yaml | 9 --------- .../serviceinstance_controller_test.go | 20 ++++++++++--------- sapbtp-operator-charts/templates/crd.yml | 6 ------ 9 files changed, 24 insertions(+), 42 deletions(-) diff --git a/api/common.go b/api/common.go index ef48488e..b87046dc 100644 --- a/api/common.go +++ b/api/common.go @@ -17,6 +17,7 @@ const ( StaleBindingIDLabel string = "services.cloud.sap.com/stale" StaleBindingRotationOfLabel string = "services.cloud.sap.com/rotationOf" ForceRotateAnnotation string = "services.cloud.sap.com/forceRotate" + PreventDeletion string = "services.cloud.sap.com/preventDeletion" ) type HTTPStatusCodeError struct { diff --git a/api/v1/serviceinstance_types.go b/api/v1/serviceinstance_types.go index e4c659ff..fd223bcb 100644 --- a/api/v1/serviceinstance_types.go +++ b/api/v1/serviceinstance_types.go @@ -56,11 +56,6 @@ type ServiceInstanceSpec struct { // +kubebuilder:default={} Shared *bool `json:"shared,omitempty"` - // Indicates if the instance deletion will be prevented - // +optional - // +kubebuilder:default=false - PreventDeletion bool `json:"prevent_deletion"` - // Provisioning parameters for the instance. // // The Parameters field is NOT secret or secured in any way and should @@ -126,7 +121,6 @@ type ServiceInstanceStatus struct { // +kubebuilder:printcolumn:JSONPath=".spec.serviceOfferingName",name="Offering",type=string // +kubebuilder:printcolumn:JSONPath=".spec.servicePlanName",name="Plan",type=string // +kubebuilder:printcolumn:JSONPath=".spec.shared",name="shared",type=boolean -// +kubebuilder:printcolumn:JSONPath=".spec.preventDeletion",name="PreventDeletion",type=boolean // +kubebuilder:printcolumn:JSONPath=".spec.dataCenter",name="dataCenter",type=string // +kubebuilder:printcolumn:JSONPath=".status.conditions[0].reason",name="Status",type=string // +kubebuilder:printcolumn:JSONPath=".status.ready",name="Ready",type=string diff --git a/api/v1/serviceinstance_validating_webhook.go b/api/v1/serviceinstance_validating_webhook.go index 55083058..b1ff832b 100644 --- a/api/v1/serviceinstance_validating_webhook.go +++ b/api/v1/serviceinstance_validating_webhook.go @@ -18,6 +18,7 @@ package v1 import ( "fmt" + "github.com/SAP/sap-btp-service-operator/api" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -43,8 +44,10 @@ func (si *ServiceInstance) ValidateUpdate(old runtime.Object) (warnings admissio } func (si *ServiceInstance) ValidateDelete() (warnings admission.Warnings, err error) { - if si.Spec.PreventDeletion { - return nil, fmt.Errorf("service instance %s marked as prevent deletion", si.Name) + if si.Annotations != nil { + if _, ok := si.Annotations[api.PreventDeletion]; ok { + return nil, fmt.Errorf("service instance %s marked as prevent deletion", si.Name) + } } return nil, nil } diff --git a/api/v1/serviceinstance_validating_webhook_test.go b/api/v1/serviceinstance_validating_webhook_test.go index 2b4497f6..ae8f704d 100644 --- a/api/v1/serviceinstance_validating_webhook_test.go +++ b/api/v1/serviceinstance_validating_webhook_test.go @@ -1,6 +1,7 @@ package v1 import ( + "github.com/SAP/sap-btp-service-operator/api" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -14,7 +15,9 @@ var _ = Describe("Service Binding Webhook Test", func() { Context("Validate Delete", func() { When("service instance is marked as prevent deletion", func() { It("should not delete the instance", func() { - instance.Spec.PreventDeletion = true + instance.Annotations = map[string]string{ + api.PreventDeletion: "true", + } _, err := instance.ValidateDelete() Expect(err).To(HaveOccurred()) }) diff --git a/api/v1alpha1/serviceinstance_types.go b/api/v1alpha1/serviceinstance_types.go index 1ba7c00e..04879bb2 100644 --- a/api/v1alpha1/serviceinstance_types.go +++ b/api/v1alpha1/serviceinstance_types.go @@ -52,11 +52,6 @@ type ServiceInstanceSpec struct { // +kubebuilder:default={} Shared *bool `json:"shared,omitempty"` - // Indicates if the instance deletion will be prevented - // +optional - // +kubebuilder:default=false - PreventDeletion bool `json:"prevent_deletion"` - // Provisioning parameters for the instance. // // The Parameters field is NOT secret or secured in any way and should diff --git a/client/sm/types/service_instance.go b/client/sm/types/service_instance.go index bc31cef5..3b8e4729 100644 --- a/client/sm/types/service_instance.go +++ b/client/sm/types/service_instance.go @@ -40,10 +40,9 @@ type ServiceInstance struct { Context json.RawMessage `json:"context,omitempty" yaml:"context,omitempty"` PreviousValues json.RawMessage `json:"-" yaml:"-"` - Ready bool `json:"ready" yaml:"ready"` - Usable bool `json:"usable" yaml:"usable"` - Shared bool `json:"shared,omitempty" yaml:"shared,omitempty"` - PreventDeletion bool `json:"prevent_deletion,omitempty" yaml:"prevent_deletion,omitempty"` + Ready bool `json:"ready" yaml:"ready"` + Usable bool `json:"usable" yaml:"usable"` + Shared bool `json:"shared,omitempty" yaml:"shared,omitempty"` LastOperation *Operation `json:"last_operation,omitempty" yaml:"last_operation,omitempty"` } diff --git a/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml b/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml index 927b1f35..352b08ec 100644 --- a/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml +++ b/config/crd/bases/services.cloud.sap.com_serviceinstances.yaml @@ -25,9 +25,6 @@ spec: - jsonPath: .spec.shared name: shared type: boolean - - jsonPath: .spec.preventDeletion - name: preventDeletion - type: boolean - jsonPath: .spec.dataCenter name: dataCenter type: string @@ -131,9 +128,6 @@ spec: shared: description: Indicates the desired shared state type: boolean - preventDeletion: - description: Indicates if the instance deletion will be prevented - type: boolean userInfo: description: UserInfo contains information about the user that last modified this instance. This field is set by the API server and @@ -378,9 +372,6 @@ spec: shared: description: Indicates the desired shared state type: boolean - preventDeletion: - description: Indicates if the instance deletion will be prevented - type: boolean userInfo: description: UserInfo contains information about the user that last modified this instance. This field is set by the API server and diff --git a/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go index 906f2465..5522546a 100644 --- a/controllers/serviceinstance_controller_test.go +++ b/controllers/serviceinstance_controller_test.go @@ -3,12 +3,10 @@ package controllers import ( "context" "fmt" - "net/http" - "strings" - "time" - "k8s.io/apimachinery/pkg/api/meta" "k8s.io/utils/pointer" + "net/http" + "strings" "github.com/SAP/sap-btp-service-operator/api" v1 "github.com/SAP/sap-btp-service-operator/api/v1" @@ -636,12 +634,16 @@ var _ = Describe("ServiceInstance controller", func() { fakeClient.DeprovisionReturns("", nil) }) It("should not delete the instance", func() { - serviceInstance.Spec.PreventDeletion = true + serviceInstance.Annotations = map[string]string{ + api.PreventDeletion: "true", + } Expect(k8sClient.Update(ctx, serviceInstance)).To(Succeed()) - deleteInstance(ctx, serviceInstance, false) - time.Sleep(10) - err := k8sClient.Get(ctx, defaultLookupKey, serviceInstance) - Expect(err).ToNot(HaveOccurred()) + err := k8sClient.Delete(ctx, serviceInstance) + strError := fmt.Sprintf("%v", err) + Expect(strError).To(ContainSubstring("marked as prevent deletion")) + serviceInstance.Annotations = nil + Expect(k8sClient.Update(ctx, serviceInstance)).To(Succeed()) + deleteInstance(ctx, serviceInstance, true) }) }) diff --git a/sapbtp-operator-charts/templates/crd.yml b/sapbtp-operator-charts/templates/crd.yml index 31ef6946..9c4a49d6 100644 --- a/sapbtp-operator-charts/templates/crd.yml +++ b/sapbtp-operator-charts/templates/crd.yml @@ -645,9 +645,6 @@ spec: shared: description: Indicates if the instance aim to be shared, default is false type: boolean - preventDeletion: - description: Indicates if the instance deletion will be prevented - type: boolean userInfo: description: UserInfo contains information about the user that last modified this instance. This field is set by the API server and @@ -895,9 +892,6 @@ spec: shared: description: Indicates if the instance aim to be shared, default is false type: boolean - preventDeletion: - description: Indicates if the instance deletion will be prevented - type: boolean userInfo: description: UserInfo contains information about the user that last modified this instance. This field is set by the API server and From 6bfcc9b3abbe2e3df2f6f2134e518f6ec9b2d56b Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Thu, 3 Aug 2023 09:05:51 +0300 Subject: [PATCH 10/20] test decription --- api/v1/serviceinstance_validating_webhook_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/v1/serviceinstance_validating_webhook_test.go b/api/v1/serviceinstance_validating_webhook_test.go index ae8f704d..b5847bb1 100644 --- a/api/v1/serviceinstance_validating_webhook_test.go +++ b/api/v1/serviceinstance_validating_webhook_test.go @@ -14,7 +14,7 @@ var _ = Describe("Service Binding Webhook Test", func() { Context("Validate Delete", func() { When("service instance is marked as prevent deletion", func() { - It("should not delete the instance", func() { + It("should return error from webhook", func() { instance.Annotations = map[string]string{ api.PreventDeletion: "true", } @@ -24,7 +24,7 @@ var _ = Describe("Service Binding Webhook Test", func() { }) When("service instance is not marked as prevent deletion", func() { - It("should delete the instance", func() { + It("should not return error from webhook", func() { _, err := instance.ValidateDelete() Expect(err).ToNot(HaveOccurred()) }) From eb933e9c9f004ccecdd19e80fd615d1203d9e77f Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Thu, 3 Aug 2023 09:09:59 +0300 Subject: [PATCH 11/20] fix test --- controllers/serviceinstance_controller_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go index 5522546a..b3ef42d4 100644 --- a/controllers/serviceinstance_controller_test.go +++ b/controllers/serviceinstance_controller_test.go @@ -639,8 +639,8 @@ var _ = Describe("ServiceInstance controller", func() { } Expect(k8sClient.Update(ctx, serviceInstance)).To(Succeed()) err := k8sClient.Delete(ctx, serviceInstance) - strError := fmt.Sprintf("%v", err) - Expect(strError).To(ContainSubstring("marked as prevent deletion")) + Expect(err.Error()).To(ContainSubstring("marked as prevent deletion")) + serviceInstance.Annotations = nil Expect(k8sClient.Update(ctx, serviceInstance)).To(Succeed()) deleteInstance(ctx, serviceInstance, true) From 518bb63456fb8e62ce15ab931643e0784c601631 Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Thu, 3 Aug 2023 09:15:03 +0300 Subject: [PATCH 12/20] fix test --- controllers/serviceinstance_controller_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go index b3ef42d4..deceaa95 100644 --- a/controllers/serviceinstance_controller_test.go +++ b/controllers/serviceinstance_controller_test.go @@ -633,7 +633,7 @@ var _ = Describe("ServiceInstance controller", func() { fakeClient.UpdateInstanceReturns(nil, "", nil) fakeClient.DeprovisionReturns("", nil) }) - It("should not delete the instance", func() { + It("should fail deleting the instance because of the webhook delete validation", func() { serviceInstance.Annotations = map[string]string{ api.PreventDeletion: "true", } @@ -641,6 +641,7 @@ var _ = Describe("ServiceInstance controller", func() { err := k8sClient.Delete(ctx, serviceInstance) Expect(err.Error()).To(ContainSubstring("marked as prevent deletion")) + /* After annotation is removed the instance should be deleted properly */ serviceInstance.Annotations = nil Expect(k8sClient.Update(ctx, serviceInstance)).To(Succeed()) deleteInstance(ctx, serviceInstance, true) From 63f90136c5d4c73f29cf84bd04ff7df789034353 Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Thu, 3 Aug 2023 09:15:59 +0300 Subject: [PATCH 13/20] fix test --- controllers/serviceinstance_controller_test.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go index deceaa95..6bb8e54e 100644 --- a/controllers/serviceinstance_controller_test.go +++ b/controllers/serviceinstance_controller_test.go @@ -634,9 +634,8 @@ var _ = Describe("ServiceInstance controller", func() { fakeClient.DeprovisionReturns("", nil) }) It("should fail deleting the instance because of the webhook delete validation", func() { - serviceInstance.Annotations = map[string]string{ - api.PreventDeletion: "true", - } + markInstanceAsPreventDeletion(serviceInstance) + Expect(k8sClient.Update(ctx, serviceInstance)).To(Succeed()) err := k8sClient.Delete(ctx, serviceInstance) Expect(err.Error()).To(ContainSubstring("marked as prevent deletion")) @@ -1250,3 +1249,9 @@ func isInstanceShared(serviceInstance *v1.ServiceInstance) bool { return sharedCond.Status == metav1.ConditionTrue } + +func markInstanceAsPreventDeletion(serviceInstance *v1.ServiceInstance) { + serviceInstance.Annotations = map[string]string{ + api.PreventDeletion: "true", + } +} From 09e8179b8ee3a41ba3f7d5352c67a63c997642d8 Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Thu, 3 Aug 2023 09:49:13 +0300 Subject: [PATCH 14/20] fix lint --- api/v1/servicebinding_validating_webhook.go | 10 ++++------ api/v1/serviceinstance_validating_webhook.go | 4 +++- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/api/v1/servicebinding_validating_webhook.go b/api/v1/servicebinding_validating_webhook.go index 58bea906..fa40bccb 100644 --- a/api/v1/servicebinding_validating_webhook.go +++ b/api/v1/servicebinding_validating_webhook.go @@ -17,19 +17,17 @@ limitations under the License. package v1 import ( + "fmt" "reflect" "time" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - - "fmt" - - "github.com/SAP/sap-btp-service-operator/api" - "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/SAP/sap-btp-service-operator/api" ) // log is for logging in this package. diff --git a/api/v1/serviceinstance_validating_webhook.go b/api/v1/serviceinstance_validating_webhook.go index b1ff832b..9e0c4d99 100644 --- a/api/v1/serviceinstance_validating_webhook.go +++ b/api/v1/serviceinstance_validating_webhook.go @@ -18,11 +18,13 @@ package v1 import ( "fmt" - "github.com/SAP/sap-btp-service-operator/api" + "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/SAP/sap-btp-service-operator/api" ) func (si *ServiceInstance) SetupWebhookWithManager(mgr ctrl.Manager) error { From 4cd9900a6f9abcaf5372c7599d373ebe342fce96 Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Thu, 3 Aug 2023 10:36:47 +0300 Subject: [PATCH 15/20] fix value of prevent to true --- api/v1/serviceinstance_validating_webhook.go | 4 +++- api/v1/serviceinstance_validating_webhook_test.go | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/api/v1/serviceinstance_validating_webhook.go b/api/v1/serviceinstance_validating_webhook.go index 9e0c4d99..761240fa 100644 --- a/api/v1/serviceinstance_validating_webhook.go +++ b/api/v1/serviceinstance_validating_webhook.go @@ -18,6 +18,7 @@ package v1 import ( "fmt" + "strings" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" @@ -47,7 +48,8 @@ func (si *ServiceInstance) ValidateUpdate(old runtime.Object) (warnings admissio func (si *ServiceInstance) ValidateDelete() (warnings admission.Warnings, err error) { if si.Annotations != nil { - if _, ok := si.Annotations[api.PreventDeletion]; ok { + preventDeletion, ok := si.Annotations[api.PreventDeletion] + if ok && strings.ToLower(preventDeletion) == "true" { return nil, fmt.Errorf("service instance %s marked as prevent deletion", si.Name) } } diff --git a/api/v1/serviceinstance_validating_webhook_test.go b/api/v1/serviceinstance_validating_webhook_test.go index b5847bb1..ffbafcb7 100644 --- a/api/v1/serviceinstance_validating_webhook_test.go +++ b/api/v1/serviceinstance_validating_webhook_test.go @@ -29,5 +29,15 @@ var _ = Describe("Service Binding Webhook Test", func() { Expect(err).ToNot(HaveOccurred()) }) }) + + When("service instance is marked as prevent deletion, but not with true as value", func() { + It("should not return error from webhook", func() { + instance.Annotations = map[string]string{ + api.PreventDeletion: "not-true", + } + _, err := instance.ValidateDelete() + Expect(err).ToNot(HaveOccurred()) + }) + }) }) }) From 2d28f87540d0c5f17bcf1bc77771c20c9adcfbd3 Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Thu, 3 Aug 2023 10:38:52 +0300 Subject: [PATCH 16/20] fix --- api/v1/serviceinstance_validating_webhook_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1/serviceinstance_validating_webhook_test.go b/api/v1/serviceinstance_validating_webhook_test.go index ffbafcb7..93557412 100644 --- a/api/v1/serviceinstance_validating_webhook_test.go +++ b/api/v1/serviceinstance_validating_webhook_test.go @@ -6,7 +6,7 @@ import ( . "github.com/onsi/gomega" ) -var _ = Describe("Service Binding Webhook Test", func() { +var _ = Describe("Service Instance Webhook Test", func() { var instance *ServiceInstance BeforeEach(func() { instance = getInstance() From d3e05c7d75d34790518592f7b470d832fef175bc Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Thu, 3 Aug 2023 10:39:59 +0300 Subject: [PATCH 17/20] fix --- api/v1/serviceinstance_validating_webhook_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1/serviceinstance_validating_webhook_test.go b/api/v1/serviceinstance_validating_webhook_test.go index 93557412..cdd8d8c8 100644 --- a/api/v1/serviceinstance_validating_webhook_test.go +++ b/api/v1/serviceinstance_validating_webhook_test.go @@ -30,7 +30,7 @@ var _ = Describe("Service Instance Webhook Test", func() { }) }) - When("service instance is marked as prevent deletion, but not with true as value", func() { + When("service instance prevent deletion annotation is not set with true", func() { It("should not return error from webhook", func() { instance.Annotations = map[string]string{ api.PreventDeletion: "not-true", From 0033fe991d9ae2e77c627c14c21b510e3025cdd0 Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Tue, 8 Aug 2023 10:24:10 +0300 Subject: [PATCH 18/20] register webhook --- main.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/main.go b/main.go index 004506aa..ea50ffd9 100644 --- a/main.go +++ b/main.go @@ -138,6 +138,10 @@ func main() { setupLog.Error(err, "unable to create webhook", "webhook", "ServiceBinding") os.Exit(1) } + if err = (&servicesv1.ServiceInstance{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ServiceInstance") + os.Exit(1) + } } // +kubebuilder:scaffold:builder From f3498a16ccb268938c9c4cde472ae1df76155749 Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Tue, 8 Aug 2023 11:28:50 +0300 Subject: [PATCH 19/20] daniel --- api/v1/serviceinstance_validating_webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1/serviceinstance_validating_webhook.go b/api/v1/serviceinstance_validating_webhook.go index 761240fa..54f668ba 100644 --- a/api/v1/serviceinstance_validating_webhook.go +++ b/api/v1/serviceinstance_validating_webhook.go @@ -50,7 +50,7 @@ func (si *ServiceInstance) ValidateDelete() (warnings admission.Warnings, err er if si.Annotations != nil { preventDeletion, ok := si.Annotations[api.PreventDeletion] if ok && strings.ToLower(preventDeletion) == "true" { - return nil, fmt.Errorf("service instance %s marked as prevent deletion", si.Name) + return nil, fmt.Errorf("service instance '%s' is marked with \"prevent deletion\"", si.Name) } } return nil, nil From ccb8fb962f0e56ade28ee2499b234523d161065a Mon Sep 17 00:00:00 2001 From: Tal Shor Date: Tue, 8 Aug 2023 11:44:54 +0300 Subject: [PATCH 20/20] fix --- controllers/serviceinstance_controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go index 6bb8e54e..69f56807 100644 --- a/controllers/serviceinstance_controller_test.go +++ b/controllers/serviceinstance_controller_test.go @@ -638,7 +638,7 @@ var _ = Describe("ServiceInstance controller", func() { Expect(k8sClient.Update(ctx, serviceInstance)).To(Succeed()) err := k8sClient.Delete(ctx, serviceInstance) - Expect(err.Error()).To(ContainSubstring("marked as prevent deletion")) + Expect(err.Error()).To(ContainSubstring("is marked with \"prevent deletion\"")) /* After annotation is removed the instance should be deleted properly */ serviceInstance.Annotations = nil