Skip to content

Commit

Permalink
Prevent deletion (#315)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
TalShorSap authored Aug 8, 2023
1 parent 08b1da4 commit c69d155
Show file tree
Hide file tree
Showing 9 changed files with 196 additions and 23 deletions.
1 change: 1 addition & 0 deletions api/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 4 additions & 6 deletions api/v1/servicebinding_validating_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
57 changes: 57 additions & 0 deletions api/v1/serviceinstance_validating_webhook.go
Original file line number Diff line number Diff line change
@@ -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
}
43 changes: 43 additions & 0 deletions api/v1/serviceinstance_validating_webhook_test.go
Original file line number Diff line number Diff line change
@@ -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())
})
})
})
})
22 changes: 20 additions & 2 deletions config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
metadata:
creationTimestamp: null
name: mutating-webhook-configuration
webhooks:
- admissionReviewVersions:
Expand Down Expand Up @@ -51,7 +50,6 @@ webhooks:
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
creationTimestamp: null
name: validating-webhook-configuration
webhooks:
- admissionReviewVersions:
Expand All @@ -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
30 changes: 27 additions & 3 deletions controllers/serviceinstance_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
}
}
26 changes: 14 additions & 12 deletions controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,23 @@ 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"
v1 "k8s.io/api/core/v1"
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"

Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand Down
4 changes: 4 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
26 changes: 26 additions & 0 deletions sapbtp-operator-charts/templates/webhook.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit c69d155

Please sign in to comment.