fix security vulnerabilities

api/v1/servicebinding_validating_webhook.go

@@ -20,6 +20,8 @@ import (
 	"reflect"
 	"time"
 
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
 	"fmt"
 
 	"github.com/SAP/sap-btp-service-operator/api"
@@ -47,22 +49,22 @@ func (sb *ServiceBinding) SetupWebhookWithManager(mgr ctrl.Manager) error {
 var _ webhook.Validator = &ServiceBinding{}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
-func (sb *ServiceBinding) ValidateCreate() error {
+func (sb *ServiceBinding) ValidateCreate() (admission.Warnings, error) {
 	servicebindinglog.Info("validate create", "name", sb.Name)
 	if sb.Spec.CredRotationPolicy != nil {
 		if err := sb.validateCredRotatingConfig(); err != nil {
-			return err
+			return nil, err
 		}
 	}
-	return nil
+	return nil, nil
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
-func (sb *ServiceBinding) ValidateUpdate(old runtime.Object) error {
+func (sb *ServiceBinding) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
 	servicebindinglog.Info("validate update", "name", sb.Name)
 	if sb.Spec.CredRotationPolicy != nil {
 		if err := sb.validateCredRotatingConfig(); err != nil {
-			return err
+			return nil, err
 		}
 	}
 
@@ -75,9 +77,9 @@ func (sb *ServiceBinding) ValidateUpdate(old runtime.Object) error {
 	}
 
 	if specChanged && (sb.Status.BindingID != "" || isStale) {
-		return fmt.Errorf("updating service bindings is not supported")
+		return nil, fmt.Errorf("updating service bindings is not supported")
 	}
-	return nil
+	return nil, nil
 }
 
 func (sb *ServiceBinding) specChanged(old runtime.Object) bool {
@@ -92,11 +94,11 @@ func (sb *ServiceBinding) specChanged(old runtime.Object) bool {
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type
-func (sb *ServiceBinding) ValidateDelete() error {
+func (sb *ServiceBinding) ValidateDelete() (admission.Warnings, error) {
 	servicebindinglog.Info("validate delete", "name", sb.Name)
 
 	// TODO(user): fill in your validation logic upon object deletion.
-	return nil
+	return nil, nil
 }
 
 func (sb *ServiceBinding) validateCredRotatingConfig() error {

api/v1/servicebinding_validating_webhook_test.go

@@ -16,7 +16,7 @@ var _ = Describe("Service Binding Webhook Test", func() {
 	Context("Validator", func() {
 		Context("Validate create", func() {
 			It("should succeed", func() {
-				err := binding.ValidateCreate()
+				_, err := binding.ValidateCreate()
 				Expect(err).ToNot(HaveOccurred())
 			})
 		})
@@ -31,15 +31,15 @@ var _ = Describe("Service Binding Webhook Test", func() {
 				When("Service instance name changed", func() {
 					It("should succeed", func() {
 						newBinding.Spec.ServiceInstanceName = "new-service-instance"
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).ToNot(HaveOccurred())
 					})
 				})
 
 				When("External name changed", func() {
 					It("should succeed", func() {
 						newBinding.Spec.ExternalName = "new-external-instance"
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).ToNot(HaveOccurred())
 					})
 				})
@@ -49,15 +49,15 @@ var _ = Describe("Service Binding Webhook Test", func() {
 						newBinding.Spec.Parameters = &runtime.RawExtension{
 							Raw: []byte("params"),
 						}
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).ToNot(HaveOccurred())
 					})
 				})
 
 				When("ParametersFrom were changed", func() {
 					It("should succeed", func() {
 						newBinding.Spec.ParametersFrom[0].SecretKeyRef.Name = "newName"
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).ToNot(HaveOccurred())
 					})
 				})
@@ -66,7 +66,7 @@ var _ = Describe("Service Binding Webhook Test", func() {
 			When("Metadata changed", func() {
 				It("should succeed", func() {
 					newBinding.Finalizers = append(newBinding.Finalizers, "newFinalizer")
-					err := newBinding.ValidateUpdate(binding)
+					_, err := newBinding.ValidateUpdate(binding)
 					Expect(err).ToNot(HaveOccurred())
 				})
 			})
@@ -78,7 +78,7 @@ var _ = Describe("Service Binding Webhook Test", func() {
 						RotatedBindingTTL: "1s",
 						RotationFrequency: "1s",
 					}
-					err := newBinding.ValidateUpdate(binding)
+					_, err := newBinding.ValidateUpdate(binding)
 					Expect(err).ToNot(HaveOccurred())
 				})
 
@@ -88,14 +88,14 @@ var _ = Describe("Service Binding Webhook Test", func() {
 						RotatedBindingTTL: "1x",
 						RotationFrequency: "1y",
 					}
-					err := newBinding.ValidateUpdate(binding)
+					_, err := newBinding.ValidateUpdate(binding)
 					Expect(err).To(HaveOccurred())
 				})
 
 				It("should fail on update with stale label", func() {
 					newBinding.Spec.ParametersFrom[0].SecretKeyRef.Name = "newName"
 					newBinding.Labels = map[string]string{api.StaleBindingIDLabel: "true"}
-					err := newBinding.ValidateUpdate(binding)
+					_, err := newBinding.ValidateUpdate(binding)
 					Expect(err).To(HaveOccurred())
 				})
 
@@ -104,7 +104,7 @@ var _ = Describe("Service Binding Webhook Test", func() {
 			When("Status changed", func() {
 				It("should succeed", func() {
 					newBinding.Status.BindingID = "12345"
-					err := newBinding.ValidateUpdate(binding)
+					_, err := newBinding.ValidateUpdate(binding)
 					Expect(err).ToNot(HaveOccurred())
 				})
 			})
@@ -120,23 +120,23 @@ var _ = Describe("Service Binding Webhook Test", func() {
 				When("Service instance name changed", func() {
 					It("should fail", func() {
 						newBinding.Spec.ServiceInstanceName = "new-service-instance"
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).To(HaveOccurred())
 					})
 				})
 
 				When("External name changed", func() {
 					It("should fail", func() {
 						newBinding.Spec.ExternalName = "new-external-instance"
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).To(HaveOccurred())
 					})
 				})
 
 				When("secret name changed", func() {
 					It("should fail", func() {
 						newBinding.Spec.SecretName = "newsecret"
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).To(HaveOccurred())
 					})
 				})
@@ -145,7 +145,7 @@ var _ = Describe("Service Binding Webhook Test", func() {
 					It("should fail", func() {
 						secretKey := "secret-key"
 						newBinding.Spec.SecretKey = &secretKey
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).To(HaveOccurred())
 					})
 				})
@@ -154,7 +154,7 @@ var _ = Describe("Service Binding Webhook Test", func() {
 					It("should fail", func() {
 						secretRootKey := "root"
 						newBinding.Spec.SecretRootKey = &secretRootKey
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).To(HaveOccurred())
 					})
 				})
@@ -164,34 +164,34 @@ var _ = Describe("Service Binding Webhook Test", func() {
 						newBinding.Spec.Parameters = &runtime.RawExtension{
 							Raw: []byte("params"),
 						}
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).To(HaveOccurred())
 					})
 				})
 
 				When("ParametersFrom were changed", func() {
 					It("should fail on changed name", func() {
 						newBinding.Spec.ParametersFrom[0].SecretKeyRef.Name = "newName"
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).To(HaveOccurred())
 					})
 
 					It("should fail on changed key", func() {
 						newBinding.Spec.ParametersFrom[0].SecretKeyRef.Key = "newName"
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).To(HaveOccurred())
 					})
 
 					It("should fail on nil array", func() {
 						newBinding.Spec.ParametersFrom = nil
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).To(HaveOccurred())
 					})
 
 					It("should fail on changed array", func() {
 						p := ParametersFromSource{}
 						newBinding.Spec.ParametersFrom[0] = p
-						err := newBinding.ValidateUpdate(binding)
+						_, err := newBinding.ValidateUpdate(binding)
 						Expect(err).To(HaveOccurred())
 					})
 
@@ -201,7 +201,7 @@ var _ = Describe("Service Binding Webhook Test", func() {
 			When("Metadata changed", func() {
 				It("should succeed", func() {
 					newBinding.Finalizers = append(newBinding.Finalizers, "newFinalizer")
-					err := newBinding.ValidateUpdate(binding)
+					_, err := newBinding.ValidateUpdate(binding)
 					Expect(err).ToNot(HaveOccurred())
 				})
 			})
@@ -213,7 +213,7 @@ var _ = Describe("Service Binding Webhook Test", func() {
 						RotatedBindingTTL: "1s",
 						RotationFrequency: "1s",
 					}
-					err := newBinding.ValidateUpdate(binding)
+					_, err := newBinding.ValidateUpdate(binding)
 					Expect(err).ToNot(HaveOccurred())
 				})
 
@@ -223,23 +223,23 @@ var _ = Describe("Service Binding Webhook Test", func() {
 						RotatedBindingTTL: "1x",
 						RotationFrequency: "1y",
 					}
-					err := newBinding.ValidateUpdate(binding)
+					_, err := newBinding.ValidateUpdate(binding)
 					Expect(err).To(HaveOccurred())
 				})
 			})
 
 			When("Status changed", func() {
 				It("should succeed", func() {
 					newBinding.Status.BindingID = "12345"
-					err := newBinding.ValidateUpdate(binding)
+					_, err := newBinding.ValidateUpdate(binding)
 					Expect(err).ToNot(HaveOccurred())
 				})
 			})
 		})
 
 		Context("Validate delete", func() {
 			It("should succeed", func() {
-				err := binding.ValidateDelete()
+				_, err := binding.ValidateDelete()
 				Expect(err).ToNot(HaveOccurred())
 			})
 		})

api/v1/webhooks/servicebinding_mutating_webhook.go

@@ -19,13 +19,13 @@ import (
 var bindinglog = logf.Log.WithName("servicebinding-webhook")
 
 type ServiceBindingDefaulter struct {
-	decoder *admission.Decoder
+	Decoder *admission.Decoder
 }
 
 func (s *ServiceBindingDefaulter) Handle(_ context.Context, req admission.Request) admission.Response {
 	bindinglog.Info("Defaulter webhook for servicebinding")
 	binding := &servicesv1.ServiceBinding{}
-	err := s.decoder.Decode(req, binding)
+	err := s.Decoder.Decode(req, binding)
 	if err != nil {
 		return admission.Errored(http.StatusBadRequest, err)
 	}
@@ -73,8 +73,3 @@ func (s *ServiceBindingDefaulter) Handle(_ context.Context, req admission.Reques
 	}
 	return admission.PatchResponseFromRaw(req.Object.Raw, marshaledInstance)
 }
-
-func (s *ServiceBindingDefaulter) InjectDecoder(d *admission.Decoder) error {
-	s.decoder = d
-	return nil
-}

api/v1/webhooks/serviceinstance_mutating_webhook.go

@@ -19,13 +19,13 @@ import (
 var instancelog = logf.Log.WithName("serviceinstance-webhook")
 
 type ServiceInstanceDefaulter struct {
-	decoder *admission.Decoder
+	Decoder *admission.Decoder
 }
 
 func (s *ServiceInstanceDefaulter) Handle(_ context.Context, req admission.Request) admission.Response {
 	instancelog.Info("Defaulter webhook for serviceinstance")
 	instance := &servicesv1.ServiceInstance{}
-	err := s.decoder.Decode(req, instance)
+	err := s.Decoder.Decode(req, instance)
 	if err != nil {
 		return admission.Errored(http.StatusBadRequest, err)
 	}
@@ -59,7 +59,7 @@ func (s *ServiceInstanceDefaulter) setServiceInstanceUserInfo(req admission.Requ
 		instance.Spec.UserInfo = userInfo
 	} else if req.Operation == v1admission.Update {
 		oldInstance := &servicesv1.ServiceInstance{}
-		err := s.decoder.DecodeRaw(req.OldObject, oldInstance)
+		err := s.Decoder.DecodeRaw(req.OldObject, oldInstance)
 		if err != nil {
 			return err
 		}
@@ -69,8 +69,3 @@ func (s *ServiceInstanceDefaulter) setServiceInstanceUserInfo(req admission.Requ
 	}
 	return nil
 }
-
-func (s *ServiceInstanceDefaulter) InjectDecoder(d *admission.Decoder) error {
-	s.decoder = d
-	return nil
-}

controllers/serviceinstance_controller.go

@@ -631,7 +631,7 @@ func getTags(tags []byte) ([]string, error) {
 
 func getSpecHash(serviceInstance *servicesv1.ServiceInstance) string {
 	spec := serviceInstance.Spec
-	spec.Shared = pointer.BoolPtr(false)
+	spec.Shared = pointer.Bool(false)
 	specBytes, _ := json.Marshal(spec)
 	s := string(specBytes)
 	return generateEncodedMD5Hash(s)

controllers/serviceinstance_controller_test.go

@@ -232,7 +232,14 @@ var _ = Describe("ServiceInstance controller", func() {
 
 					It("provisioning should fail", func() {
 						serviceInstance = createInstance(ctx, instanceSpec)
-						Expect(serviceInstance.Status.Conditions[0].Message).To(ContainSubstring("provided plan id does not match"))
+						Eventually(func() bool {
+							err := k8sClient.Get(ctx, defaultLookupKey, serviceInstance)
+							if err != nil {
+								return false
+							}
+							cond := meta.FindStatusCondition(serviceInstance.Status.Conditions, api.ConditionSucceeded)
+							return cond != nil && strings.Contains(cond.Message, "provided plan id does not match")
+						}, timeout, interval).Should(BeTrue())
 					})
 				})
 			})