From c69d1559eaf8ed41af5d8ee16f74c760f25795f6 Mon Sep 17 00:00:00 2001 From: TalShorSap <108805932+TalShorSap@users.noreply.github.com> Date: Tue, 8 Aug 2023 15:48:17 +0300 Subject: [PATCH] Prevent deletion (#315) * prevent deletion init * prevent deletion init * prevent deletion init * prevent deletion init * prevent deletion init * prevent deletion init * prevent deletion init * prevent deletion init * remove prevent deletion from spec to annotation * test decription * fix test * fix test * fix test * fix lint * fix value of prevent to true * fix * fix * register webhook * daniel * fix --- api/common.go | 1 + api/v1/servicebinding_validating_webhook.go | 10 ++-- api/v1/serviceinstance_validating_webhook.go | 57 +++++++++++++++++++ ...serviceinstance_validating_webhook_test.go | 43 ++++++++++++++ config/webhook/manifests.yaml | 22 ++++++- .../serviceinstance_controller_test.go | 30 +++++++++- controllers/suite_test.go | 26 +++++---- main.go | 4 ++ sapbtp-operator-charts/templates/webhook.yml | 26 +++++++++ 9 files changed, 196 insertions(+), 23 deletions(-) create mode 100644 api/v1/serviceinstance_validating_webhook.go create mode 100644 api/v1/serviceinstance_validating_webhook_test.go 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/servicebinding_validating_webhook.go b/api/v1/servicebinding_validating_webhook.go index 495c8b16..04df9b93 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 new file mode 100644 index 00000000..54f668ba --- /dev/null +++ b/api/v1/serviceinstance_validating_webhook.go @@ -0,0 +1,57 @@ +/* + + +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 ( + "fmt" + "strings" + + "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 { + return ctrl.NewWebhookManagedBy(mgr). + For(si). + Complete() +} + +// +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.Annotations != nil { + preventDeletion, ok := si.Annotations[api.PreventDeletion] + if ok && strings.ToLower(preventDeletion) == "true" { + return nil, fmt.Errorf("service instance '%s' is marked with \"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 new file mode 100644 index 00000000..cdd8d8c8 --- /dev/null +++ b/api/v1/serviceinstance_validating_webhook_test.go @@ -0,0 +1,43 @@ +package v1 + +import ( + "github.com/SAP/sap-btp-service-operator/api" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Service Instance 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 return error from webhook", func() { + instance.Annotations = map[string]string{ + api.PreventDeletion: "true", + } + _, err := instance.ValidateDelete() + Expect(err).To(HaveOccurred()) + }) + }) + + When("service instance is not marked as prevent deletion", func() { + It("should not return error from webhook", func() { + _, err := instance.ValidateDelete() + Expect(err).ToNot(HaveOccurred()) + }) + }) + + 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", + } + _, err := instance.ValidateDelete() + Expect(err).ToNot(HaveOccurred()) + }) + }) + }) +}) diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 9c2ca8a2..9d245ee1 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/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go index 005fd2ce..148eabdb 100644 --- a/controllers/serviceinstance_controller_test.go +++ b/controllers/serviceinstance_controller_test.go @@ -3,11 +3,10 @@ package controllers import ( "context" "fmt" - "net/http" - "strings" - "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" @@ -629,6 +628,25 @@ var _ = Describe("ServiceInstance controller", func() { }) }) + When("instance is marked for prevent deletion", func() { + BeforeEach(func() { + fakeClient.UpdateInstanceReturns(nil, "", nil) + fakeClient.DeprovisionReturns("", nil) + }) + It("should fail deleting the instance because of the webhook delete validation", func() { + markInstanceAsPreventDeletion(serviceInstance) + + Expect(k8sClient.Update(ctx, serviceInstance)).To(Succeed()) + err := k8sClient.Delete(ctx, serviceInstance) + Expect(err.Error()).To(ContainSubstring("is marked with \"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) + }) + }) + When("delete without instance id", func() { JustBeforeEach(func() { fakeClient.DeprovisionReturns("", nil) @@ -1231,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", + } +} 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(), 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 diff --git a/sapbtp-operator-charts/templates/webhook.yml b/sapbtp-operator-charts/templates/webhook.yml index ef64ce21..9c17ec89 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