Repository: redhat-developer/service-binding-operator
Pull request #565 (2020-07-16 06:19:09)

Split BindingReady into CollectionReady and InjectionReady
### Motivation

To fix  #381 

### Changes

Splits the `BindingReady` type into more specific types:
- CollectionReady which would be related to the collection and persistence of intermediate manifests (secrets, config maps, etc)
- InjectionReady which would be related to change the applications' manifests to use those intermediate manifests:

### How to test

1. Follow the steps given in this [example](https://github.com/redhat-developer/service-binding-operator/tree/master/examples/nodejs_postgresql) to:
- install DB operator, Service Binding Operator
- Import an application
- Create a DB instance 

2. Create the following `ServiceBindingRequest`:

```
apiVersion: apps.openshift.io/v1alpha1
kind: ServiceBindingRequest
metadata:
  name: binding-request
  namespace: service-binding-demo
spec:
 backingServiceSelector:
    group: postgresql.baiju.dev
    version: v1alpha1
    kind: Database
    resourceRef: db-demo
```

3. Use the following command to check if the status has been updated:
`oc get ServiceBindingRequest binding-request -o yaml`

The output should have `status.conditions.type` as 
- `CollectionReady` with `status: "True"`
-  `InjectionReady` with `status: "False"`

```
status:
  conditions:
  - lastHeartbeatTime: 2020-04-23T06:25:16Z
    lastTransitionTime: 2020-04-23T06:25:16Z
    message: application ResourceRef or MatchLabel not found
    reason: EmptyApplicationSelector
    status: "True"
    type: CollectionReady
  - lastHeartbeatTime: 2020-04-23T06:25:16Z
    lastTransitionTime: 2020-04-23T06:25:16Z
    message: application ResourceRef or MatchLabel not found
    reason: EmptyApplicationSelector
    status: "False"
    type: InjectionReady
```

## Demo

https://youtu.be/NI6Fd5l65rw

openshift-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **dhritishikhar**
You can assign the PR to them by writing `/assign @dhritishikhar` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=redhat-developer%2Fservice-binding-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/redhat-developer/service-binding-operator/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["dhritishikhar"]} -->
baijum: /lgtm
/approve
openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/redhat-developer/service-binding-operator/pull/565#issuecomment-659212601" title="Approved">baijum</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=redhat-developer%2Fservice-binding-operator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/redhat-developer/service-binding-operator/blob/master/OWNERS)~~ [baijum]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/pkg/controller/servicebindingrequest/binder.go b/pkg/controller/servicebindingrequest/binder.go
index fc551069..97eccc61 100644
--- a/pkg/controller/servicebindingrequest/binder.go
+++ b/pkg/controller/servicebindingrequest/binder.go
@@ -2,11 +2,11 @@ package servicebindingrequest
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"strings"
 	"time"
 
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -62,9 +62,6 @@ func (f extraFieldsModifierFunc) ModifyExtraFields(u *unstructured.Unstructured)
 	return f(u)
 }
 
-var emptyApplicationSelectorErr = errors.New("application ResourceRef or MatchLabel not found")
-var applicationNotFound = errors.New("Application is already deleted")
-
 // search objects based in Kind/APIVersion, which contain the labels defined in ApplicationSelector.
 func (b *binder) search() (*unstructured.UnstructuredList, error) {
 	// If Application name is present
@@ -73,7 +70,7 @@ func (b *binder) search() (*unstructured.UnstructuredList, error) {
 	} else if b.sbr.Spec.ApplicationSelector.LabelSelector != nil {
 		return b.getApplicationByLabelSelector()
 	} else {
-		return nil, emptyApplicationSelectorErr
+		return nil, errEmptyApplicationSelector
 	}
 }
 
@@ -87,11 +84,13 @@ func (b *binder) getApplicationByName() (*unstructured.UnstructuredList, error)
 	object, err := b.dynClient.Resource(gvr).Namespace(ns).
 		Get(b.sbr.Spec.ApplicationSelector.ResourceRef, metav1.GetOptions{})
 	if err != nil {
+		if errors.IsNotFound(err) {
+			return nil, errApplicationNotFound
+		}
 		return nil, err
 	}
 
-	objList := &unstructured.UnstructuredList{Items: []unstructured.Unstructured{*object}}
-	return objList, nil
+	return &unstructured.UnstructuredList{Items: []unstructured.Unstructured{*object}}, nil
 }
 
 func (b *binder) getApplicationByLabelSelector() (*unstructured.UnstructuredList, error) {
@@ -105,7 +104,17 @@ func (b *binder) getApplicationByLabelSelector() (*unstructured.UnstructuredList
 	opts := metav1.ListOptions{
 		LabelSelector: labels.Set(matchLabels).String(),
 	}
-	return b.dynClient.Resource(gvr).Namespace(ns).List(opts)
+
+	objList, err := b.dynClient.Resource(gvr).Namespace(ns).List(opts)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(objList.Items) == 0 {
+		return nil, errApplicationNotFound
+	}
+
+	return objList, nil
 }
 
 // extractSpecVolumes based on volume path, extract it unstructured. It can return error on trying
@@ -570,9 +579,6 @@ func (b *binder) remove(objs *unstructured.UnstructuredList) error {
 func (b *binder) unbind() error {
 	objs, err := b.search()
 	if err != nil {
-		if errors.Is(err, applicationNotFound) {
-			return nil
-		}
 		return err
 	}
 	return b.remove(objs)
@@ -583,9 +589,6 @@ func (b *binder) unbind() error {
 func (b *binder) bind() ([]*unstructured.Unstructured, error) {
 	objs, err := b.search()
 	if err != nil {
-		if errors.Is(err, applicationNotFound) {
-			return nil, nil
-		}
 		return nil, err
 	}
 	return b.update(objs)
diff --git a/pkg/controller/servicebindingrequest/reconciler.go b/pkg/controller/servicebindingrequest/reconciler.go
index b2d8600e..aaab75b8 100644
--- a/pkg/controller/servicebindingrequest/reconciler.go
+++ b/pkg/controller/servicebindingrequest/reconciler.go
@@ -19,11 +19,19 @@ import (
 )
 
 const (
-	// BindingReady indicates that the binding succeeded
-	BindingReady conditionsv1.ConditionType = "Ready"
+	// CollectionReady indicates readiness for collection and persistance of intermediate manifests
+	CollectionReady conditionsv1.ConditionType = "CollectionReady"
+	// InjectionReady indicates readiness to change application manifests to use those intermediate manifests
+	// If status is true, it indicates that the binding succeeded
+	InjectionReady conditionsv1.ConditionType = "InjectionReady"
 	// EmptyServiceSelectorsReason is used when the ServiceBindingRequest has empty
 	// backingServiceSelectors.
 	EmptyServiceSelectorsReason = "EmptyServiceSelectors"
+	// EmptyApplicationSelectorReason is used when the ServiceBindingRequest has empty
+	// applicationSelector.
+	EmptyApplicationSelectorReason = "EmptyApplicationSelector"
+	// ApplicationNotFoundReason is used when the application is not found.
+	ApplicationNotFoundReason = "ApplicationNotFound"
 )
 
 // Reconciler reconciles a ServiceBindingRequest object
@@ -108,7 +116,7 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
 	// fetch and validate namespaced ServiceBindingRequest instance
 	sbr, err := r.getServiceBindingRequest(request.NamespacedName)
 	if err != nil {
-		if errors.Is(err, applicationNotFound) {
+		if errors.Is(err, errApplicationNotFound) {
 			logger.Info("SBR deleted after application deletion")
 			return done()
 		}
@@ -131,13 +139,22 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
 
 	selectors := extractServiceSelectors(sbr)
 	if len(selectors) == 0 {
-		conditionsv1.SetStatusCondition(&sbr.Status.Conditions, conditionsv1.Condition{
-			Type:    BindingReady,
-			Status:  corev1.ConditionFalse,
-			Reason:  EmptyServiceSelectorsReason,
-			Message: "The spec.backingServiceSelectors field is empty.",
-		})
-		_, updateErr := updateServiceBindingRequestStatus(r.dynClient, sbr)
+		_, updateErr := updateServiceBindingRequestStatus(
+			r.dynClient,
+			sbr,
+			conditionsv1.Condition{
+				Type:    CollectionReady,
+				Status:  corev1.ConditionFalse,
+				Reason:  EmptyServiceSelectorsReason,
+				Message: errEmptyBackingServiceSelectors.Error(),
+			},
+			conditionsv1.Condition{
+				Type:    InjectionReady,
+				Status:  corev1.ConditionFalse,
+				Reason:  EmptyServiceSelectorsReason,
+				Message: errEmptyBackingServiceSelectors.Error(),
+			},
+		)
 		if updateErr == nil {
 			return done()
 		}
diff --git a/pkg/controller/servicebindingrequest/service.go b/pkg/controller/servicebindingrequest/service.go
index 49f867ec..a07f0b8e 100644
--- a/pkg/controller/servicebindingrequest/service.go
+++ b/pkg/controller/servicebindingrequest/service.go
@@ -24,6 +24,11 @@ var (
 	// errEmptyBackingServiceSelectors is returned when no backing service selectors have been
 	// informed in the Service Binding Request.
 	errEmptyBackingServiceSelectors = errors.New("backing service selectors are empty")
+	// errEmptyApplicationSelector is returned when no application selectors have been
+	// informed in the Service Binding Request.
+	errEmptyApplicationSelector = errors.New("application selectors are empty")
+	// errApplicationNotFound is returned when no application is found
+	errApplicationNotFound = errors.New("application not found")
 )
 
 func findService(
diff --git a/pkg/controller/servicebindingrequest/servicebinder.go b/pkg/controller/servicebindingrequest/servicebinder.go
index 03e3aa32..3f48fa8c 100644
--- a/pkg/controller/servicebindingrequest/servicebinder.go
+++ b/pkg/controller/servicebindingrequest/servicebinder.go
@@ -2,6 +2,7 @@ package servicebindingrequest
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1"
@@ -153,7 +154,7 @@ func (b *serviceBinder) unbind() (reconcile.Result, error) {
 	}
 
 	logger.Debug("Removing resource finalizers...")
-	b.sbr.SetFinalizers(removeStringSlice(b.sbr.GetFinalizers(), finalizer))
+	removeFinalizer(b.sbr)
 	if _, err := b.updateServiceBindingRequest(b.sbr); err != nil {
 		return noRequeue(err)
 	}
@@ -166,7 +167,11 @@ func (b *serviceBinder) unbind() (reconcile.Result, error) {
 func updateServiceBindingRequestStatus(
 	dynClient dynamic.Interface,
 	sbr *v1alpha1.ServiceBindingRequest,
+	conditions ...conditionsv1.Condition,
 ) (*v1alpha1.ServiceBindingRequest, error) {
+	for _, v := range conditions {
+		conditionsv1.SetStatusCondition(&sbr.Status.Conditions, v)
+	}
 	u, err := converter.ToUnstructured(sbr)
 	if err != nil {
 		return nil, err
@@ -222,7 +227,7 @@ func (b *serviceBinder) onError(
 		b.setApplicationObjects(sbrStatus, objs)
 	}
 	conditionsv1.SetStatusCondition(&sbrStatus.Conditions, conditionsv1.Condition{
-		Type:    BindingReady,
+		Type:    InjectionReady,
 		Status:  corev1.ConditionFalse,
 		Reason:  bindingFail,
 		Message: b.message(err),
@@ -236,6 +241,62 @@ func (b *serviceBinder) onError(
 	return requeueOnNotFound(err, requeueAfter)
 }
 
+// isApplicationSelectorEmpty returns true if applicationSelector is not declared in
+// the Service Binding Request.
+func isApplicationSelectorEmpty(
+	application v1alpha1.ApplicationSelector,
+) bool {
+	var emptyApplication v1alpha1.ApplicationSelector
+	return application == emptyApplication ||
+		application.ResourceRef == "" &&
+			application.LabelSelector.MatchLabels == nil
+}
+
+func addFinalizer(sbr *v1alpha1.ServiceBindingRequest) {
+	sbr.SetFinalizers(append(removeStringSlice(sbr.GetFinalizers(), finalizer), finalizer))
+}
+
+func removeFinalizer(sbr *v1alpha1.ServiceBindingRequest) {
+	sbr.SetFinalizers(removeStringSlice(sbr.GetFinalizers(), finalizer))
+}
+
+// handleApplicationError handles scenarios when:
+// 1. applicationSelector not declared in the Service Binding Request
+// 2. application not found
+func (b *serviceBinder) handleApplicationError(reason string, applicationError error, sbrStatus *v1alpha1.ServiceBindingRequestStatus) (reconcile.Result, error) {
+	conditionsv1.SetStatusCondition(&sbrStatus.Conditions, conditionsv1.Condition{
+		Type:    InjectionReady,
+		Status:  corev1.ConditionFalse,
+		Reason:  reason,
+		Message: applicationError.Error(),
+	})
+
+	// updating status of request instance
+	sbr, err := b.updateStatusServiceBindingRequest(b.sbr, sbrStatus)
+	if err != nil {
+		return requeueError(err)
+	}
+
+	// appending finalizer, should be later removed upon resource deletion
+	addFinalizer(b.sbr)
+	if _, err = b.updateServiceBindingRequest(sbr); err != nil {
+		return requeueError(err)
+	}
+
+	b.logger.Info(applicationError.Error())
+
+	if errors.Is(applicationError, errApplicationNotFound) {
+		removeFinalizer(b.sbr)
+		if _, err = b.updateServiceBindingRequest(sbr); err != nil {
+			return requeueError(err)
+		}
+		return requeue(applicationError, requeueAfter)
+
+	}
+
+	return done()
+}
+
 // bind configures binding between the Service Binding Request and its related objects.
 func (b *serviceBinder) bind() (reconcile.Result, error) {
 	sbrStatus := b.sbr.Status.DeepCopy()
@@ -249,10 +310,21 @@ func (b *serviceBinder) bind() (reconcile.Result, error) {
 	}
 	sbrStatus.Secret = secretObj.GetName()
 
+	conditionsv1.SetStatusCondition(&sbrStatus.Conditions, conditionsv1.Condition{
+		Type:   CollectionReady,
+		Status: corev1.ConditionTrue,
+	})
+
+	if isApplicationSelectorEmpty(b.sbr.Spec.ApplicationSelector) {
+		return b.handleApplicationError(EmptyApplicationSelectorReason, errEmptyApplicationSelector, sbrStatus)
+	}
 	updatedObjects, err := b.binder.bind()
 	if err != nil {
 		b.logger.Error(err, "On binding application.")
-		return b.onError(err, b.sbr, sbrStatus, updatedObjects)
+		if errors.Is(err, errApplicationNotFound) {
+			return b.handleApplicationError(ApplicationNotFoundReason, errApplicationNotFound, sbrStatus)
+		}
+		return b.onError(err, b.sbr, sbrStatus, nil)
 	}
 	b.setApplicationObjects(sbrStatus, updatedObjects)
 
@@ -264,7 +336,7 @@ func (b *serviceBinder) bind() (reconcile.Result, error) {
 	}
 
 	conditionsv1.SetStatusCondition(&sbrStatus.Conditions, conditionsv1.Condition{
-		Type:   BindingReady,
+		Type:   InjectionReady,
 		Status: corev1.ConditionTrue,
 	})
 
@@ -275,9 +347,10 @@ func (b *serviceBinder) bind() (reconcile.Result, error) {
 	}
 
 	// appending finalizer, should be later removed upon resource deletion
-	sbr.SetFinalizers(append(removeStringSlice(b.sbr.GetFinalizers(), finalizer), finalizer))
+	addFinalizer(sbr)
+
 	if _, err = b.updateServiceBindingRequest(sbr); err != nil {
-		return noRequeue(err)
+		return requeueError(err)
 	}
 
 	b.logger.Info("All done!")


Test patch:
diff --git a/pkg/controller/servicebindingrequest/reconciler_test.go b/pkg/controller/servicebindingrequest/reconciler_test.go
index 5206e31a..0509a9c9 100644
--- a/pkg/controller/servicebindingrequest/reconciler_test.go
+++ b/pkg/controller/servicebindingrequest/reconciler_test.go
@@ -50,6 +50,32 @@ func reconcileRequest() reconcile.Request {
 	}
 }
 
+func requireConditionPresentAndTrue(t *testing.T, condition conditionsv1.ConditionType, sbrConditions []conditionsv1.Condition) {
+	require.True(t,
+		conditionsv1.IsStatusConditionPresentAndEqual(
+			sbrConditions,
+			condition,
+			corev1.ConditionTrue,
+		),
+		"%+v should exist and be true; existing conditions: %+v",
+		condition,
+		sbrConditions,
+	)
+}
+
+func requireConditionPresentAndFalse(t *testing.T, condition conditionsv1.ConditionType, sbrConditions []conditionsv1.Condition) {
+	require.True(t,
+		conditionsv1.IsStatusConditionPresentAndEqual(
+			sbrConditions,
+			condition,
+			corev1.ConditionFalse,
+		),
+		"%+v should exist and be false; existing conditions: %+v",
+		condition,
+		sbrConditions,
+	)
+}
+
 type fakeResourceWatcher struct {
 	record map[schema.GroupVersionKind]bool
 	mapper meta.RESTMapper
@@ -105,8 +131,9 @@ func TestApplicationSelectorByName(t *testing.T) {
 		sbrOutput, err := r.getServiceBindingRequest(namespacedName)
 		require.NoError(t, err)
 
-		require.Equal(t, BindingReady, sbrOutput.Status.Conditions[0].Type)
-		require.Equal(t, corev1.ConditionTrue, sbrOutput.Status.Conditions[0].Status)
+		requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
+		requireConditionPresentAndTrue(t, InjectionReady, sbrOutput.Status.Conditions)
+
 		require.Equal(t, 1, len(sbrOutput.Status.Applications))
 		expectedStatus := v1alpha1.BoundApplication{
 			GroupVersionKind: v1.GroupVersionKind{
@@ -167,8 +194,9 @@ func TestReconcilerReconcileUsingSecret(t *testing.T) {
 		sbrOutput, err := r.getServiceBindingRequest(namespacedName)
 		require.NoError(t, err)
 
-		require.Equal(t, BindingReady, sbrOutput.Status.Conditions[0].Type)
-		require.Equal(t, corev1.ConditionTrue, sbrOutput.Status.Conditions[0].Status)
+		requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
+		requireConditionPresentAndTrue(t, InjectionReady, sbrOutput.Status.Conditions)
+
 		require.Equal(t, reconcilerName, sbrOutput.Status.Secret)
 
 		require.Equal(t, 1, len(sbrOutput.Status.Applications))
@@ -229,7 +257,7 @@ func TestReconcilerReconcileUsingVolumes(t *testing.T) {
 	})
 }
 
-func TestReconcilerGenericBinding(t *testing.T) {
+func TestApplicationNotFound(t *testing.T) {
 	backingServiceResourceRef := "backingService1"
 	matchLabels := map[string]string{
 		"connects-to": "database",
@@ -249,22 +277,15 @@ func TestReconcilerGenericBinding(t *testing.T) {
 
 	// Reconcile without deployment
 	res, err := r.Reconcile(reconcileRequest())
-	require.NoError(t, err)
-	require.False(t, res.Requeue)
+	require.EqualError(t, err, errApplicationNotFound.Error())
+	require.True(t, res.Requeue)
 
 	namespacedName := types.NamespacedName{Namespace: reconcilerNs, Name: reconcilerName}
 	sbrOutput, err := r.getServiceBindingRequest(namespacedName)
 	require.NoError(t, err)
 
-	require.True(t,
-		conditionsv1.IsStatusConditionPresentAndEqual(
-			sbrOutput.Status.Conditions,
-			BindingReady,
-			corev1.ConditionTrue,
-		),
-		"Ready condition should exist and true; existing conditions: %+v",
-		sbrOutput.Status.Conditions,
-	)
+	requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
+	requireConditionPresentAndFalse(t, InjectionReady, sbrOutput.Status.Conditions)
 	require.Len(t, sbrOutput.Status.Applications, 0)
 
 	// Reconcile with deployment
@@ -286,8 +307,9 @@ func TestReconcilerGenericBinding(t *testing.T) {
 	sbrOutput2, err := r.getServiceBindingRequest(namespacedName)
 	require.NoError(t, err)
 
-	require.Equal(t, BindingReady, sbrOutput2.Status.Conditions[0].Type)
-	require.Equal(t, corev1.ConditionTrue, sbrOutput2.Status.Conditions[0].Status)
+	requireConditionPresentAndTrue(t, CollectionReady, sbrOutput2.Status.Conditions)
+	requireConditionPresentAndTrue(t, InjectionReady, sbrOutput2.Status.Conditions)
+
 	require.Equal(t, reconcilerName, sbrOutput2.Status.Secret)
 	require.Equal(t, 1, len(sbrOutput2.Status.Applications))
 
@@ -313,8 +335,17 @@ func TestReconcilerGenericBinding(t *testing.T) {
 
 	sbrOutput3, err := r.getServiceBindingRequest(namespacedName)
 	require.NoError(t, err)
-	require.Equal(t, BindingReady, sbrOutput3.Status.Conditions[0].Type)
-	require.Equal(t, corev1.ConditionTrue, sbrOutput3.Status.Conditions[0].Status)
+
+	u, err = fakeDynClient.Resource(deploymentsGVR).Get(reconcilerName, metav1.GetOptions{})
+	require.NoError(t, err)
+
+	d = appsv1.Deployment{}
+	err = runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, &d)
+	require.NoError(t, err)
+
+	requireConditionPresentAndTrue(t, CollectionReady, sbrOutput3.Status.Conditions)
+	requireConditionPresentAndTrue(t, InjectionReady, sbrOutput3.Status.Conditions)
+
 	require.Equal(t, reconcilerName, sbrOutput3.Status.Secret)
 	require.Equal(t, s.Data["password"], []byte("abc123"))
 	require.Equal(t, 1, len(sbrOutput3.Status.Applications))
@@ -365,8 +396,9 @@ func TestReconcilerReconcileWithConflictingAppSelc(t *testing.T) {
 			},
 		}
 
-		require.Equal(t, BindingReady, sbrOutput.Status.Conditions[0].Type)
-		require.Equal(t, corev1.ConditionTrue, sbrOutput.Status.Conditions[0].Status)
+		requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
+		requireConditionPresentAndTrue(t, InjectionReady, sbrOutput.Status.Conditions)
+
 		require.Equal(t, reconcilerName, sbrOutput.Status.Secret)
 		require.Len(t, sbrOutput.Status.Applications, 1)
 		require.True(t, reflect.DeepEqual(expectedStatus, sbrOutput.Status.Applications[0]))
@@ -393,11 +425,8 @@ func TestEmptyApplicationSelector(t *testing.T) {
 	sbrOutput, err := r.getServiceBindingRequest(namespacedName)
 	require.NoError(t, err)
 
-	require.Equal(t, BindingReady, sbrOutput.Status.Conditions[0].Type)
-	// Currently the Conditions[0].Status would be true as application's absence won't cause error
-	// TODO New steps to conditions to be introduced - InjectionReady, CollectionReady
-	require.Equal(t, corev1.ConditionTrue, sbrOutput.Status.Conditions[0].Status)
-	require.Equal(t, 0, len(sbrOutput.Status.Applications))
+	requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
+	requireConditionPresentAndFalse(t, InjectionReady, sbrOutput.Status.Conditions)
 }
 
 func TestBindTwoSbrsWithSingleApplication(t *testing.T) {
@@ -452,8 +481,8 @@ func TestBindTwoSbrsWithSingleApplication(t *testing.T) {
 			},
 		}
 
-		require.Equal(t, BindingReady, sbrOutput.Status.Conditions[0].Type)
-		require.Equal(t, corev1.ConditionTrue, sbrOutput.Status.Conditions[0].Status)
+		requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
+		requireConditionPresentAndTrue(t, InjectionReady, sbrOutput.Status.Conditions)
 		require.Equal(t, sbrName1, sbrOutput.Status.Secret)
 		require.Len(t, sbrOutput.Status.Applications, 1)
 		require.True(t, reflect.DeepEqual(expectedStatus, sbrOutput.Status.Applications[0]))
@@ -489,8 +518,8 @@ func TestBindTwoSbrsWithSingleApplication(t *testing.T) {
 			},
 		}
 
-		require.Equal(t, BindingReady, sbrOutput.Status.Conditions[0].Type)
-		require.Equal(t, corev1.ConditionTrue, sbrOutput.Status.Conditions[0].Status)
+		requireConditionPresentAndTrue(t, CollectionReady, sbrOutput.Status.Conditions)
+		requireConditionPresentAndTrue(t, InjectionReady, sbrOutput.Status.Conditions)
 		require.Equal(t, sbrName2, sbrOutput.Status.Secret)
 		require.Len(t, sbrOutput.Status.Applications, 1)
 		require.True(t, reflect.DeepEqual(expectedStatus, sbrOutput.Status.Applications[0]))
diff --git a/pkg/controller/servicebindingrequest/servicebinder_test.go b/pkg/controller/servicebindingrequest/servicebinder_test.go
index 64fe9ba1..444d084a 100644
--- a/pkg/controller/servicebindingrequest/servicebinder_test.go
+++ b/pkg/controller/servicebindingrequest/servicebinder_test.go
@@ -105,31 +105,14 @@ func TestServiceBinder_Bind(t *testing.T) {
 			if len(args.wantConditions) > 0 {
 				// proceed to find whether conditions match wanted conditions
 				for _, c := range args.wantConditions {
-					for _, cond := range sb.sbr.Status.Conditions {
-						expected := conditionsv1.Condition{}
-						got := conditionsv1.Condition{}
-						if len(c.Type) > 0 {
-							expected.Type = c.Type
-							got.Type = cond.Type
-						}
-						if len(c.Status) > 0 {
-							expected.Status = c.Status
-							got.Status = cond.Status
-						}
-						if len(c.Reason) > 0 {
-							expected.Reason = c.Reason
-							got.Reason = cond.Reason
-						}
-						if len(c.Message) > 0 {
-							expected.Message = c.Message
-							got.Message = cond.Message
-						}
-
-						require.Equal(t, expected, got)
+					if c.Status == corev1.ConditionTrue {
+						requireConditionPresentAndTrue(t, c.Type, sb.sbr.Status.Conditions)
+					}
+					if c.Status == corev1.ConditionFalse {
+						requireConditionPresentAndFalse(t, c.Type, sb.sbr.Status.Conditions)
 					}
 				}
 			}
-
 			// regardless of the result, verify the actions expected by the reconciliation
 			// process have been issued if user has specified wanted actions
 			if len(args.wantActions) > 0 {
@@ -409,7 +392,11 @@ func TestServiceBinder_Bind(t *testing.T) {
 		},
 		wantConditions: []wantedCondition{
 			{
-				Type:   BindingReady,
+				Type:   CollectionReady,
+				Status: corev1.ConditionTrue,
+			},
+			{
+				Type:   InjectionReady,
 				Status: corev1.ConditionTrue,
 			},
 		},
@@ -448,7 +435,11 @@ func TestServiceBinder_Bind(t *testing.T) {
 		},
 		wantConditions: []wantedCondition{
 			{
-				Type:   BindingReady,
+				Type:   CollectionReady,
+				Status: corev1.ConditionTrue,
+			},
+			{
+				Type:   InjectionReady,
 				Status: corev1.ConditionTrue,
 			},
 		},
@@ -488,7 +479,11 @@ func TestServiceBinder_Bind(t *testing.T) {
 		},
 		wantConditions: []wantedCondition{
 			{
-				Type:   BindingReady,
+				Type:   CollectionReady,
+				Status: corev1.ConditionTrue,
+			},
+			{
+				Type:   InjectionReady,
 				Status: corev1.ConditionTrue,
 			},
 		},
@@ -506,13 +501,16 @@ func TestServiceBinder_Bind(t *testing.T) {
 			},
 			restMapper: testutils.BuildTestRESTMapper(),
 		},
-		wantErr: emptyApplicationSelectorErr,
 		wantConditions: []wantedCondition{
 			{
-				Type:    BindingReady,
+				Type:   CollectionReady,
+				Status: corev1.ConditionTrue,
+			},
+			{
+				Type:    InjectionReady,
 				Status:  corev1.ConditionFalse,
-				Reason:  bindingFail,
-				Message: emptyApplicationSelectorErr.Error(),
+				Reason:  EmptyApplicationSelectorReason,
+				Message: errEmptyApplicationSelector.Error(),
 			},
 		},
 	}))
@@ -543,7 +541,11 @@ func TestServiceBinder_Bind(t *testing.T) {
 		},
 		wantConditions: []wantedCondition{
 			{
-				Type:   BindingReady,
+				Type:   CollectionReady,
+				Status: corev1.ConditionTrue,
+			},
+			{
+				Type:   InjectionReady,
 				Status: corev1.ConditionTrue,
 			},
 		},
diff --git a/test/e2e/servicebindingrequest_test.go b/test/e2e/servicebindingrequest_test.go
index fb9ff155..8656a71a 100644
--- a/test/e2e/servicebindingrequest_test.go
+++ b/test/e2e/servicebindingrequest_test.go
@@ -24,6 +24,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 
+	conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1"
 	"github.com/redhat-developer/service-binding-operator/pkg/apis"
 	"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1"
 	"github.com/redhat-developer/service-binding-operator/pkg/controller/servicebindingrequest"
@@ -190,6 +191,7 @@ func assertDeploymentEnvFrom(
 // assertSBRStatus will determine if SBR is on "success" state
 func assertSBRStatus(
 	ctx context.Context,
+	t *testing.T,
 	f *framework.Framework,
 	namespacedName types.NamespacedName,
 ) error {
@@ -198,16 +200,25 @@ func assertSBRStatus(
 		return err
 	}
 
-	for i, condition := range sbr.Status.Conditions {
-		if condition.Type != servicebindingrequest.BindingReady && condition.Status != corev1.ConditionTrue {
-			return fmt.Errorf(
-				"Condition.Type and Condition.Status is '%s' and '%s' instead of '%s' and '%s'",
-				sbr.Status.Conditions[i].Type,
-				sbr.Status.Conditions[i].Status,
-				servicebindingrequest.BindingReady,
-				corev1.ConditionTrue)
-		}
-	}
+	require.True(t,
+		conditionsv1.IsStatusConditionPresentAndEqual(
+			sbr.Status.Conditions,
+			servicebindingrequest.CollectionReady,
+			corev1.ConditionTrue,
+		),
+		"CollectionReady condition should exist and true; existing conditions: %+v",
+		sbr.Status.Conditions,
+	)
+
+	require.True(t,
+		conditionsv1.IsStatusConditionPresentAndEqual(
+			sbr.Status.Conditions,
+			servicebindingrequest.InjectionReady,
+			corev1.ConditionTrue,
+		),
+		"InjectionReady condition should exist and true; existing conditions: %+v",
+		sbr.Status.Conditions,
+	)
 	return nil
 }
 
@@ -497,7 +508,7 @@ func serviceBindingRequestTest(
 	t.Log("Inspecting SBR status...")
 	require.Eventually(t, func() bool {
 		t.Logf("Inspecting SBR: '%s'", sbrNamespacedName)
-		err := assertSBRStatus(todoCtx, f, sbrNamespacedName)
+		err := assertSBRStatus(todoCtx, t, f, sbrNamespacedName)
 		if err != nil {
 			t.Logf("Error on inspecting SBR: '%#v'", err)
 			return false