bug fix - userInfo cannot be changed error after applying update from…

… kyma ui

api/common/common.go

@@ -3,6 +3,8 @@ package common
 import (
 	"fmt"
 
+	"k8s.io/apimachinery/pkg/api/meta"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -74,11 +76,18 @@ type SAPBTPResource interface {
 	GetParameters() *runtime.RawExtension
 	GetStatus() interface{}
 	SetStatus(status interface{})
-	GetObservedGeneration() int64
-	SetObservedGeneration(int64)
 	DeepClone() SAPBTPResource
 	SetReady(metav1.ConditionStatus)
 	GetReady() metav1.ConditionStatus
 	GetAnnotations() map[string]string
 	SetAnnotations(map[string]string)
 }
+
+func GetObservedGeneration(obj SAPBTPResource) int64 {
+	cond := meta.FindStatusCondition(obj.GetConditions(), ConditionSucceeded)
+	observedGen := int64(0)
+	if cond != nil {
+		observedGen = cond.ObservedGeneration
+	}
+	return observedGen
+}

api/v1/servicebinding_types.go

@@ -123,9 +123,6 @@ type ServiceBindingStatus struct {
 	// Service binding conditions
 	Conditions []metav1.Condition `json:"conditions"`
 
-	// Last generation that was acted on
-	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
-
 	// Indicates whether binding is ready for usage
 	Ready metav1.ConditionStatus `json:"ready,omitempty"`
 
@@ -179,14 +176,6 @@ func (sb *ServiceBinding) SetStatus(status interface{}) {
 	sb.Status = status.(ServiceBindingStatus)
 }
 
-func (sb *ServiceBinding) GetObservedGeneration() int64 {
-	return sb.Status.ObservedGeneration
-}
-
-func (sb *ServiceBinding) SetObservedGeneration(newObserved int64) {
-	sb.Status.ObservedGeneration = newObserved
-}
-
 func (sb *ServiceBinding) DeepClone() common.SAPBTPResource {
 	return sb.DeepCopy()
 }

api/v1/servicebinding_types_test.go

@@ -66,18 +66,6 @@ var _ = Describe("Service Binding Type Test", func() {
 		Expect(binding.GetControllerName()).To(Equal(common.ServiceBindingController))
 	})
 
-	It("should update observed generation", func() {
-		Expect(binding.Status.ObservedGeneration).To(Equal(int64(0)))
-		binding.SetObservedGeneration(2)
-		Expect(binding.GetObservedGeneration()).To(Equal(int64(2)))
-	})
-
-	It("should update observed generation", func() {
-		Expect(binding.Status.ObservedGeneration).To(Equal(int64(0)))
-		binding.SetObservedGeneration(2)
-		Expect(binding.Status.ObservedGeneration).To(Equal(int64(2)))
-	})
-
 	It("should update ready", func() {
 		Expect(binding.Status.Ready).To(Equal(metav1.ConditionStatus("")))
 		binding.SetReady(metav1.ConditionTrue)

api/v1/serviceinstance_types.go

@@ -17,11 +17,16 @@ limitations under the License.
 package v1
 
 import (
+	"crypto/md5"
+	"encoding/hex"
+	"encoding/json"
+
 	"github.com/SAP/sap-btp-service-operator/api/common"
 	"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"
+	"k8s.io/utils/ptr"
 )
 
 // EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
@@ -107,9 +112,6 @@ type ServiceInstanceStatus struct {
 	// Service instance conditions
 	Conditions []metav1.Condition `json:"conditions"`
 
-	// Last generation that was acted on
-	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
-
 	// Indicates whether instance is ready for usage
 	Ready metav1.ConditionStatus `json:"ready,omitempty"`
 
@@ -165,14 +167,6 @@ func (si *ServiceInstance) SetStatus(status interface{}) {
 	si.Status = status.(ServiceInstanceStatus)
 }
 
-func (si *ServiceInstance) GetObservedGeneration() int64 {
-	return si.Status.ObservedGeneration
-}
-
-func (si *ServiceInstance) SetObservedGeneration(newObserved int64) {
-	si.Status.ObservedGeneration = newObserved
-}
-
 func (si *ServiceInstance) DeepClone() common.SAPBTPResource {
 	return si.DeepCopy()
 }
@@ -207,6 +201,15 @@ func init() {
 
 func (si *ServiceInstance) Hub() {}
 
-func (si *ServiceInstance) ShouldBeShared() bool {
+func (si *ServiceInstance) GetShared() bool {
 	return si.Spec.Shared != nil && *si.Spec.Shared
 }
+
+func (si *ServiceInstance) GetSpecHash() string {
+	spec := si.Spec
+	spec.Shared = ptr.To(false)
+	specBytes, _ := json.Marshal(spec)
+	s := string(specBytes)
+	hash := md5.Sum([]byte(s))
+	return hex.EncodeToString(hash[:])
+}

api/v1/serviceinstance_types_test.go

@@ -96,12 +96,6 @@ var _ = Describe("Service Instance Type Test", func() {
 		Expect(instance.GetControllerName()).To(Equal(common.ServiceInstanceController))
 	})
 
-	It("should update observed generation", func() {
-		Expect(instance.Status.ObservedGeneration).To(Equal(int64(0)))
-		instance.SetObservedGeneration(2)
-		Expect(instance.GetObservedGeneration()).To(Equal(int64(2)))
-	})
-
 	It("should update ready", func() {
 		Expect(instance.Status.Ready).To(Equal(metav1.ConditionStatus("")))
 		instance.SetReady(metav1.ConditionTrue)

api/v1/webhooks/serviceinstance_mutating_webhook.go

@@ -44,9 +44,12 @@ func (s *ServiceInstanceDefaulter) Handle(_ context.Context, req admission.Reque
 			return admission.Errored(http.StatusInternalServerError, err)
 		}
 
-		if !reflect.DeepEqual(instance.Spec.UserInfo, oldInstance.Spec.UserInfo) {
+		if instance.Spec.UserInfo == nil {
+			instance.Spec.UserInfo = oldInstance.Spec.UserInfo
+		} else if !reflect.DeepEqual(instance.Spec.UserInfo, oldInstance.Spec.UserInfo) {
 			return admission.Errored(http.StatusBadRequest, fmt.Errorf("modifying spec.userInfo is not allowed"))
-		} else if !reflect.DeepEqual(oldInstance.Spec, instance.Spec) { //UserInfo is updated only when spec is changed
+		}
+		if !reflect.DeepEqual(oldInstance.Spec, instance.Spec) { //UserInfo is updated only when spec is changed
 			instance.Spec.UserInfo = &v1.UserInfo{
 				Username: req.UserInfo.Username,
 				UID:      req.UserInfo.UID,

controllers/servicebinding_controller.go

@@ -91,8 +91,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 	serviceBinding = serviceBinding.DeepCopy()
-	log.Info(fmt.Sprintf("Current generation is %v and observed is %v", serviceBinding.Generation, serviceBinding.GetObservedGeneration()))
-	serviceBinding.SetObservedGeneration(serviceBinding.Generation)
+	log.Info(fmt.Sprintf("Current generation is %v and observed is %v", serviceBinding.Generation, common.GetObservedGeneration(serviceBinding)))
 
 	if len(serviceBinding.GetConditions()) == 0 {
 		if err := utils.InitConditions(ctx, r.Client, serviceBinding); err != nil {
@@ -182,7 +181,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 
 	if utils.IsInProgress(serviceInstance) {
 		log.Info(fmt.Sprintf("Service instance with k8s name %s is not ready for binding yet", serviceInstance.Name))
-		utils.SetInProgressConditions(ctx, smClientTypes.CREATE, fmt.Sprintf("creation in progress, waiting for service instance '%s' to be ready", serviceBinding.Spec.ServiceInstanceName), serviceBinding)
+		utils.SetInProgressConditions(ctx, smClientTypes.CREATE, fmt.Sprintf("creation in progress, waiting for service instance '%s' to be ready", serviceBinding.Spec.ServiceInstanceName), serviceBinding, false)
 
 		return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, utils.UpdateStatus(ctx, r.Client, serviceBinding)
 	}
@@ -242,7 +241,7 @@ func (r *ServiceBindingReconciler) updateSecret(ctx context.Context, serviceBind
 				return err
 			}
 			log.Info("Updating binding", "bindingID", smBinding.ID)
-			utils.SetSuccessConditions(smClientTypes.UPDATE, serviceBinding)
+			utils.SetSuccessConditions(smClientTypes.UPDATE, serviceBinding, false)
 		}
 	}
 	return nil
@@ -291,7 +290,7 @@ func (r *ServiceBindingReconciler) createBinding(ctx context.Context, smClient s
 		log.Info("Create smBinding request is async")
 		serviceBinding.Status.OperationURL = operationURL
 		serviceBinding.Status.OperationType = smClientTypes.CREATE
-		utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "", serviceBinding)
+		utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "", serviceBinding, false)
 		if err := utils.UpdateStatus(ctx, r.Client, serviceBinding); err != nil {
 			log.Error(err, "unable to update ServiceBinding status")
 			return ctrl.Result{}, err
@@ -313,7 +312,7 @@ func (r *ServiceBindingReconciler) createBinding(ctx context.Context, smClient s
 	serviceBinding.Status.BindingID = smBinding.ID
 	serviceBinding.Status.SubaccountID = subaccountID
 	serviceBinding.Status.Ready = metav1.ConditionTrue
-	utils.SetSuccessConditions(smClientTypes.CREATE, serviceBinding)
+	utils.SetSuccessConditions(smClientTypes.CREATE, serviceBinding, false)
 	log.Info("Updating binding", "bindingID", smBinding.ID)
 
 	return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding)
@@ -336,7 +335,7 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *v
 			if smBinding != nil {
 				log.Info("binding exists in SM continue with deletion")
 				serviceBinding.Status.BindingID = smBinding.ID
-				utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "delete after recovery", serviceBinding)
+				utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "delete after recovery", serviceBinding, false)
 				return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding)
 			}
 
@@ -367,7 +366,7 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *v
 			log.Info("Deleting binding async")
 			serviceBinding.Status.OperationURL = operationURL
 			serviceBinding.Status.OperationType = smClientTypes.DELETE
-			utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "", serviceBinding)
+			utils.SetInProgressConditions(ctx, smClientTypes.DELETE, "", serviceBinding, false)
 			if err := utils.UpdateStatus(ctx, r.Client, serviceBinding); err != nil {
 				return ctrl.Result{}, err
 			}
@@ -392,7 +391,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *v1.
 	status, statusErr := smClient.Status(serviceBinding.Status.OperationURL, nil)
 	if statusErr != nil {
 		log.Info(fmt.Sprintf("failed to fetch operation, got error from SM: %s", statusErr.Error()), "operationURL", serviceBinding.Status.OperationURL)
-		utils.SetInProgressConditions(ctx, serviceBinding.Status.OperationType, string(smClientTypes.INPROGRESS), serviceBinding)
+		utils.SetInProgressConditions(ctx, serviceBinding.Status.OperationType, string(smClientTypes.INPROGRESS), serviceBinding, false)
 		freshStatus := v1.ServiceBindingStatus{
 			Conditions: serviceBinding.GetConditions(),
 		}
@@ -414,7 +413,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *v1.
 		fallthrough
 	case smClientTypes.PENDING:
 		if len(status.Description) != 0 {
-			utils.SetInProgressConditions(ctx, status.Type, status.Description, serviceBinding)
+			utils.SetInProgressConditions(ctx, status.Type, status.Description, serviceBinding, true)
 			if err := utils.UpdateStatus(ctx, r.Client, serviceBinding); err != nil {
 				log.Error(err, "unable to update ServiceBinding polling description")
 				return ctrl.Result{}, err
@@ -423,7 +422,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *v1.
 		return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, nil
 	case smClientTypes.FAILED:
 		// non transient error - should not retry
-		utils.SetFailureConditions(status.Type, status.Description, serviceBinding)
+		utils.SetFailureConditions(status.Type, status.Description, serviceBinding, true)
 		if serviceBinding.Status.OperationType == smClientTypes.DELETE {
 			serviceBinding.Status.OperationURL = ""
 			serviceBinding.Status.OperationType = ""
@@ -438,7 +437,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *v1.
 			return ctrl.Result{}, fmt.Errorf(errMsg)
 		}
 	case smClientTypes.SUCCEEDED:
-		utils.SetSuccessConditions(status.Type, serviceBinding)
+		utils.SetSuccessConditions(status.Type, serviceBinding, true)
 		switch serviceBinding.Status.OperationType {
 		case smClientTypes.CREATE:
 			smBinding, err := smClient.GetBindingByID(serviceBinding.Status.BindingID, nil)
@@ -453,7 +452,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *v1.
 			if err := r.storeBindingSecret(ctx, serviceBinding, smBinding); err != nil {
 				return r.handleSecretError(ctx, smClientTypes.CREATE, err, serviceBinding)
 			}
-			utils.SetSuccessConditions(status.Type, serviceBinding)
+			utils.SetSuccessConditions(status.Type, serviceBinding, false)
 		case smClientTypes.DELETE:
 			return r.deleteSecretAndRemoveFinalizer(ctx, serviceBinding)
 		}
@@ -495,10 +494,6 @@ func (r *ServiceBindingReconciler) getBindingForRecovery(ctx context.Context, sm
 func (r *ServiceBindingReconciler) maintain(ctx context.Context, binding *v1.ServiceBinding) (ctrl.Result, error) {
 	log := utils.GetLogger(ctx)
 	shouldUpdateStatus := false
-	if binding.Generation != binding.Status.ObservedGeneration {
-		binding.SetObservedGeneration(binding.Generation)
-		shouldUpdateStatus = true
-	}
 	if !utils.IsFailed(binding) {
 		secret, err := r.getSecret(ctx, binding.Namespace, binding.Spec.SecretName)
 		if err != nil {
@@ -507,7 +502,7 @@ func (r *ServiceBindingReconciler) maintain(ctx context.Context, binding *v1.Ser
 				log.Info(fmt.Sprintf("secret not found recovering binding %s", binding.Name))
 				binding.Status.BindingID = ""
 				binding.Status.Ready = metav1.ConditionFalse
-				utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "recreating deleted secret", binding)
+				utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "recreating deleted secret", binding, false)
 				shouldUpdateStatus = true
 				r.Recorder.Event(binding, corev1.EventTypeWarning, "SecretDeleted", "SecretDeleted")
 			} else {
@@ -565,7 +560,6 @@ func (r *ServiceBindingReconciler) setOwner(ctx context.Context, serviceInstance
 }
 
 func (r *ServiceBindingReconciler) resyncBindingStatus(ctx context.Context, k8sBinding *v1.ServiceBinding, smBinding *smClientTypes.ServiceBinding) {
-	k8sBinding.Status.ObservedGeneration = k8sBinding.Generation
 	k8sBinding.Status.BindingID = smBinding.ID
 	k8sBinding.Status.InstanceID = smBinding.ServiceInstanceID
 	k8sBinding.Status.OperationURL = ""
@@ -587,11 +581,11 @@ func (r *ServiceBindingReconciler) resyncBindingStatus(ctx context.Context, k8sB
 	case smClientTypes.INPROGRESS:
 		k8sBinding.Status.OperationURL = sm.BuildOperationURL(smBinding.LastOperation.ID, smBinding.ID, smClientTypes.ServiceBindingsURL)
 		k8sBinding.Status.OperationType = smBinding.LastOperation.Type
-		utils.SetInProgressConditions(ctx, smBinding.LastOperation.Type, smBinding.LastOperation.Description, k8sBinding)
+		utils.SetInProgressConditions(ctx, smBinding.LastOperation.Type, smBinding.LastOperation.Description, k8sBinding, false)
 	case smClientTypes.SUCCEEDED:
-		utils.SetSuccessConditions(operationType, k8sBinding)
+		utils.SetSuccessConditions(operationType, k8sBinding, false)
 	case smClientTypes.FAILED:
-		utils.SetFailureConditions(operationType, description, k8sBinding)
+		utils.SetFailureConditions(operationType, description, k8sBinding, false)
 	}
 }
 
@@ -621,6 +615,11 @@ func (r *ServiceBindingReconciler) storeBindingSecret(ctx context.Context, k8sBi
 	}
 	secret.Labels[common.ManagedByBTPOperatorLabel] = "true"
 
+	if secret.Annotations == nil {
+		secret.Annotations = map[string]string{}
+	}
+	secret.Annotations["binding"] = k8sBinding.Name
+
 	return r.createOrUpdateBindingSecret(ctx, k8sBinding, secret)
 }
 
@@ -988,7 +987,7 @@ func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, bindin
 
 	binding.Status.BindingID = ""
 	binding.Status.Ready = metav1.ConditionFalse
-	utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "rotating binding credentials", binding)
+	utils.SetInProgressConditions(ctx, smClientTypes.CREATE, "rotating binding credentials", binding, false)
 	utils.SetCredRotationInProgressConditions(common.CredRotating, "", binding)
 	return utils.UpdateStatus(ctx, r.Client, binding)
 }