Skip to content

Commit

Permalink
Bug fix - create binding when instance update failed should be allowed (
Browse files Browse the repository at this point in the history 
#492)
  • Loading branch information
@kerenlahav
kerenlahav authored Dec 19, 2024
1 parent 9148d9a commit e9d69cc
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 143 deletions.
4 changes: 0 additions & 4 deletions config/crd/bases/services.cloud.sap.com_servicebindings.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,6 @@ spec:
description: Indicates when binding secret was rotated
format: date-time
type: string
observedGeneration:
description: Last generation that was acted on
format: int64
type: integer
operationType:
description: The operation type (CREATE/UPDATE/DELETE) for ongoing
operation
Expand Down
4 changes: 0 additions & 4 deletions config/crd/bases/services.cloud.sap.com_serviceinstances.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,10 +255,6 @@ spec:
description: The generated ID of the instance, will be automatically
filled once the instance is created
type: string
observedGeneration:
description: Last generation that was acted on
format: int64
type: integer
operationType:
description: The operation type (CREATE/UPDATE/DELETE) for ongoing
operation
Expand Down
200 changes: 86 additions & 114 deletions controllers/servicebinding_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,67 +123,52 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
return r.delete(ctx, serviceBinding, serviceInstance)
}

if len(serviceBinding.Status.OperationURL) > 0 {
// ongoing operation - poll status from SM
return r.poll(ctx, serviceBinding, serviceInstance)
}

if !controllerutil.ContainsFinalizer(serviceBinding, common.FinalizerName) {
controllerutil.AddFinalizer(serviceBinding, common.FinalizerName)
if controllerutil.AddFinalizer(serviceBinding, common.FinalizerName) {
log.Info(fmt.Sprintf("added finalizer '%s' to service binding", common.FinalizerName))
if err := r.Client.Update(ctx, serviceBinding); err != nil {
return ctrl.Result{}, err
}
}
condition := meta.FindStatusCondition(serviceBinding.Status.Conditions, common.ConditionReady)

if serviceBinding.Status.BindingID != "" && condition != nil && condition.ObservedGeneration != serviceBinding.Generation {
err := r.updateSecret(ctx, serviceBinding, serviceInstance, log)
if err != nil {
return r.handleSecretError(ctx, smClientTypes.UPDATE, err, serviceBinding)
}
err = utils.UpdateStatus(ctx, r.Client, serviceBinding)
if err != nil {
return r.handleSecretError(ctx, smClientTypes.UPDATE, err, serviceBinding)
}
if len(serviceBinding.Status.OperationURL) > 0 {
// ongoing operation - poll status from SM
return r.poll(ctx, serviceBinding, serviceInstance)
}

isBindingReady := condition != nil && condition.Status == metav1.ConditionTrue
if isBindingReady {
if isStaleServiceBinding(serviceBinding) {
return r.handleStaleServiceBinding(ctx, serviceBinding)
}
if utils.IsMarkedForDeletion(serviceInstance.ObjectMeta) {
log.Info(fmt.Sprintf("service instance name: %s namespace: %s is marked for deletion, unable to create binding", serviceInstance.Name, serviceInstance.Namespace))
utils.SetBlockedCondition(ctx, "instance is in deletion process", serviceBinding)
return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding)
}

if initCredRotationIfRequired(serviceBinding) {
return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding)
}
if !serviceInstanceReady(serviceInstance) {
log.Info(fmt.Sprintf("service instance name: %s namespace: %s is not ready, unable to create binding", serviceInstance.Name, serviceInstance.Namespace))
utils.SetBlockedCondition(ctx, "service instance is not ready", serviceBinding)
return ctrl.Result{Requeue: true}, utils.UpdateStatus(ctx, r.Client, serviceBinding)
}

// should rotate creds
if meta.IsStatusConditionTrue(serviceBinding.Status.Conditions, common.ConditionCredRotationInProgress) {
log.Info("rotating credentials")
if err := r.rotateCredentials(ctx, serviceBinding, serviceInstance); err != nil {
return ctrl.Result{}, err
}
}

if isBindingReady {
log.Info("Binding in final state")
return r.maintain(ctx, serviceBinding)
}

if serviceNotUsable(serviceInstance) {
instanceErr := fmt.Errorf("service instance '%s' is not usable", serviceBinding.Spec.ServiceInstanceName)
utils.SetBlockedCondition(ctx, instanceErr.Error(), serviceBinding)
if err := utils.UpdateStatus(ctx, r.Client, serviceBinding); err != nil {
return ctrl.Result{}, err
// is binding ready
if meta.IsStatusConditionTrue(serviceBinding.Status.Conditions, common.ConditionReady) {
if isStaleServiceBinding(serviceBinding) {
log.Info("binding is stale, handling")
return r.handleStaleServiceBinding(ctx, serviceBinding)
}
return ctrl.Result{}, instanceErr
}

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, false)
if initCredRotationIfRequired(serviceBinding) {
log.Info("cred rotation required, updating status")
return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding)
}

return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, utils.UpdateStatus(ctx, r.Client, serviceBinding)
log.Info("binding in final state, maintaining secret")
return r.maintain(ctx, serviceBinding, serviceInstance)
}

//set owner instance only for original bindings (not rotated)
Expand All @@ -199,6 +184,7 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque

if serviceBinding.Status.BindingID == "" {
if err := r.validateSecretNameIsAvailable(ctx, serviceBinding); err != nil {
log.Error(err, "secret validation failed")
utils.SetBlockedCondition(ctx, err.Error(), serviceBinding)
return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, serviceBinding)
}
Expand All @@ -224,29 +210,6 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
return ctrl.Result{}, nil
}

func (r *ServiceBindingReconciler) updateSecret(ctx context.Context, serviceBinding *v1.ServiceBinding, serviceInstance *v1.ServiceInstance, log logr.Logger) error {
log.Info("Updating secret according to the new template")
smClient, err := r.GetSMClient(ctx, serviceInstance)
if err != nil {
return err
}
smBinding, err := smClient.GetBindingByID(serviceBinding.Status.BindingID, nil)
if err != nil {
log.Error(err, "failed to get binding for update secret")
return err
}
if smBinding != nil {
if smBinding.Credentials != nil {
if err = r.storeBindingSecret(ctx, serviceBinding, smBinding); err != nil {
return err
}
log.Info("Updating binding", "bindingID", smBinding.ID)
utils.SetSuccessConditions(smClientTypes.UPDATE, serviceBinding, false)
}
}
return nil
}

func (r *ServiceBindingReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&v1.ServiceBinding{}).
Expand Down Expand Up @@ -458,6 +421,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *v1.
}
}

log.Info(fmt.Sprintf("finished polling operation %s '%s'", serviceBinding.Status.OperationType, serviceBinding.Status.OperationURL))
serviceBinding.Status.OperationURL = ""
serviceBinding.Status.OperationType = ""

Expand Down Expand Up @@ -491,43 +455,51 @@ func (r *ServiceBindingReconciler) getBindingForRecovery(ctx context.Context, sm
return nil, nil
}

func (r *ServiceBindingReconciler) maintain(ctx context.Context, binding *v1.ServiceBinding) (ctrl.Result, error) {
func (r *ServiceBindingReconciler) maintain(ctx context.Context, binding *v1.ServiceBinding, instance *v1.ServiceInstance) (ctrl.Result, error) {
log := utils.GetLogger(ctx)
shouldUpdateStatus := false
if !utils.IsFailed(binding) {
secret, err := r.getSecret(ctx, binding.Namespace, binding.Spec.SecretName)
if err != nil {
// secret was deleted
if apierrors.IsNotFound(err) && !utils.IsMarkedForDeletion(binding.ObjectMeta) {
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, false)
shouldUpdateStatus = true
r.Recorder.Event(binding, corev1.EventTypeWarning, "SecretDeleted", "SecretDeleted")
} else {
return ctrl.Result{}, err
}
} else { // secret exists, validate it has the required labels
if secret.Labels == nil {
secret.Labels = map[string]string{}
}
if secret.Labels[common.ManagedByBTPOperatorLabel] != "true" {
secret.Labels[common.ManagedByBTPOperatorLabel] = "true"
if err = r.Client.Update(ctx, secret); err != nil {
log.Error(err, "failed to update secret labels")
return ctrl.Result{}, err
}
}
if err := r.maintainSecret(ctx, binding, instance); err != nil {
log.Error(err, "failed to maintain secret")
return r.handleSecretError(ctx, smClientTypes.UPDATE, err, binding)
}

log.Info("maintain finished successfully")
return ctrl.Result{}, nil
}

func (r *ServiceBindingReconciler) maintainSecret(ctx context.Context, serviceBinding *v1.ServiceBinding, serviceInstance *v1.ServiceInstance) error {
log := utils.GetLogger(ctx)
if common.GetObservedGeneration(serviceBinding) == serviceBinding.Generation {
log.Info("observed generation is up to date, checking if secret exists")
if _, err := r.getSecret(ctx, serviceBinding.Namespace, serviceBinding.Spec.SecretName); err == nil {
log.Info("secret exists, no need to maintain secret")
return nil
}

log.Info("binding's secret was not found")
r.Recorder.Event(serviceBinding, corev1.EventTypeWarning, "SecretDeleted", "SecretDeleted")
}

if shouldUpdateStatus {
log.Info(fmt.Sprintf("maintanance required for binding %s", binding.Name))
return ctrl.Result{}, utils.UpdateStatus(ctx, r.Client, binding)
log.Info("maintaining binding's secret")
smClient, err := r.GetSMClient(ctx, serviceInstance)
if err != nil {
return err
}
smBinding, err := smClient.GetBindingByID(serviceBinding.Status.BindingID, nil)
if err != nil {
log.Error(err, "failed to get binding for update secret")
return err
}
if smBinding != nil {
if smBinding.Credentials != nil {
if err = r.storeBindingSecret(ctx, serviceBinding, smBinding); err != nil {
return err
}
log.Info("Updating binding", "bindingID", smBinding.ID)
utils.SetSuccessConditions(smClientTypes.UPDATE, serviceBinding, false)
}
}

return ctrl.Result{}, nil
return utils.UpdateStatus(ctx, r.Client, serviceBinding)
}

func (r *ServiceBindingReconciler) getServiceInstanceForBinding(ctx context.Context, binding *v1.ServiceBinding) (*v1.ServiceInstance, error) {
Expand Down Expand Up @@ -917,17 +889,10 @@ func (r *ServiceBindingReconciler) addInstanceInfo(ctx context.Context, binding
}

func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, binding *v1.ServiceBinding, serviceInstance *v1.ServiceInstance) error {
suffix := "-" + utils.RandStringRunes(6)
log := utils.GetLogger(ctx)
if binding.Annotations != nil {
if _, ok := binding.Annotations[common.ForceRotateAnnotation]; ok {
log.Info("Credentials rotation - deleting force rotate annotation")
delete(binding.Annotations, common.ForceRotateAnnotation)
if err := r.Client.Update(ctx, binding); err != nil {
log.Info("Credentials rotation - failed to delete force rotate annotation")
return err
}
}
if err := r.removeForceRotateAnnotationIfNeeded(ctx, binding, log); err != nil {
log.Info("Credentials rotation - failed to delete force rotate annotation")
return err
}

credInProgressCondition := meta.FindStatusCondition(binding.GetConditions(), common.ConditionCredRotationInProgress)
Expand Down Expand Up @@ -957,12 +922,14 @@ func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, bindin
}

if len(bindings.Items) == 0 {
// create the backup binding
smClient, err := r.GetSMClient(ctx, serviceInstance)
if err != nil {
return err
}

// rename current binding
suffix := "-" + utils.RandStringRunes(6)
log.Info("Credentials rotation - renaming binding to old in SM", "current", binding.Spec.ExternalName)
if _, errRenaming := smClient.RenameBinding(binding.Status.BindingID, binding.Spec.ExternalName+suffix, binding.Name+suffix); errRenaming != nil {
log.Error(errRenaming, "Credentials rotation - failed renaming binding to old in SM", "binding", binding.Spec.ExternalName)
Expand Down Expand Up @@ -992,6 +959,17 @@ func (r *ServiceBindingReconciler) rotateCredentials(ctx context.Context, bindin
return utils.UpdateStatus(ctx, r.Client, binding)
}

func (r *ServiceBindingReconciler) removeForceRotateAnnotationIfNeeded(ctx context.Context, binding *v1.ServiceBinding, log logr.Logger) error {
if binding.Annotations != nil {
if _, ok := binding.Annotations[common.ForceRotateAnnotation]; ok {
log.Info("Credentials rotation - deleting force rotate annotation")
delete(binding.Annotations, common.ForceRotateAnnotation)
return r.Client.Update(ctx, binding)
}
}
return nil
}

func (r *ServiceBindingReconciler) stopRotation(ctx context.Context, binding *v1.ServiceBinding) error {
conditions := binding.GetConditions()
meta.RemoveStatusCondition(&conditions, common.ConditionCredRotationInProgress)
Expand Down Expand Up @@ -1089,7 +1067,7 @@ func isStaleServiceBinding(binding *v1.ServiceBinding) bool {
}

func initCredRotationIfRequired(binding *v1.ServiceBinding) bool {
if utils.IsFailed(binding) || !credRotationEnabled(binding) || meta.IsStatusConditionTrue(binding.Status.Conditions, common.ConditionCredRotationInProgress) {
if utils.IsFailed(binding) || !credRotationEnabled(binding) {
return false
}
_, forceRotate := binding.Annotations[common.ForceRotateAnnotation]
Expand Down Expand Up @@ -1154,14 +1132,8 @@ func bindingAlreadyOwnedByInstance(instance *v1.ServiceInstance, binding *v1.Ser
return false
}

func serviceNotUsable(instance *v1.ServiceInstance) bool {
if utils.IsMarkedForDeletion(instance.ObjectMeta) {
return true
}
if len(instance.Status.Conditions) != 0 {
return instance.Status.Conditions[0].Reason == utils.GetConditionReason(smClientTypes.CREATE, smClientTypes.FAILED)
}
return false
func serviceInstanceReady(instance *v1.ServiceInstance) bool {
return instance.Status.Ready == metav1.ConditionTrue
}

func getInstanceNameForSecretCredentials(instance *v1.ServiceInstance) []byte {
Expand Down
Loading

0 comments on commit e9d69cc

Please sign in to comment.