fix binding validating webhook (#516)

Author: kerenlahav

api/v1/servicebinding_validating_webhook.go

@@ -79,8 +79,15 @@ func (sb *ServiceBinding) ValidateUpdate(_ context.Context, oldObj, newObj runti
 		}
 	}
 
-	specChanged := newBinding.specChanged(oldBinding, newBinding)
-	if specChanged && (newBinding.Status.BindingID != "" || isStale) {
+	if newBinding.Spec.UserInfo == nil {
+		newBinding.Spec.UserInfo = oldBinding.Spec.UserInfo
+	} else if !reflect.DeepEqual(newBinding.Spec.UserInfo, oldBinding.Spec.UserInfo) {
+		return nil, fmt.Errorf("modifying spec.userInfo is not allowed")
+	}
+
+	isSpecChanged := newBinding.specChanged(oldBinding)
+	if isSpecChanged && (newBinding.Status.BindingID != "" || isStale) {
+
 		return nil, fmt.Errorf("updating service bindings is not supported")
 	}
 	return nil, nil
@@ -93,9 +100,9 @@ func (sb *ServiceBinding) validateRotationLabels(old *ServiceBinding) bool {
 	return sb.ObjectMeta.Labels[common.StaleBindingRotationOfLabel] == old.ObjectMeta.Labels[common.StaleBindingRotationOfLabel]
 }
 
-func (sb *ServiceBinding) specChanged(oldBinding *ServiceBinding, newBinding *ServiceBinding) bool {
+func (sb *ServiceBinding) specChanged(oldBinding *ServiceBinding) bool {
 	oldSpec := oldBinding.Spec.DeepCopy()
-	newSpec := newBinding.Spec.DeepCopy()
+	newSpec := sb.Spec.DeepCopy()
 
 	//allow changing cred rotation config
 	oldSpec.CredRotationPolicy = nil

api/v1/servicebinding_validating_webhook_test.go

@@ -5,6 +5,7 @@ import (
 	"github.com/lithammer/dedent"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	v1 "k8s.io/api/authentication/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
@@ -86,6 +87,26 @@ var _ = Describe("Service Binding Webhook Test", func() {
 						Expect(err).ToNot(HaveOccurred())
 					})
 				})
+
+				When("UserInfo changed", func() {
+					It("should fail", func() {
+						newBinding.Spec.UserInfo = &v1.UserInfo{
+							Username: "username",
+						}
+						_, err := newBinding.ValidateUpdate(nil, binding, newBinding)
+						Expect(err).To(HaveOccurred())
+						Expect(err.Error()).To(ContainSubstring("modifying spec.userInfo is not allowed"))
+					})
+					It("should succeed if new binding user info is empty", func() {
+						newBinding.Spec.UserInfo = nil
+						_, err := newBinding.ValidateUpdate(nil, binding, newBinding)
+						Expect(err).ToNot(HaveOccurred())
+					})
+					It("should succeed if user info not changed", func() {
+						_, err := newBinding.ValidateUpdate(nil, binding, newBinding)
+						Expect(err).ToNot(HaveOccurred())
+					})
+				})
 			})
 
 			When("Metadata changed", func() {

controllers/servicebinding_controller_test.go

@@ -915,7 +915,7 @@ stringData:
 				}
 				err := k8sClient.Update(ctx, createdBinding)
 				Expect(err).To(HaveOccurred())
-				Expect(err.Error()).To(ContainSubstring("updating service bindings is not supported"))
+				Expect(err.Error()).To(ContainSubstring("modifying spec.userInfo is not allowed"))
 			})
 		})
 	})