Repository: redhat-developer/service-binding-operator
Pull request #704 (2020-10-01 13:57:20)

Trigger reconcile on updates to services' metadata.Annotations specific to service bindings
Refer https://issues.redhat.com/browse/APPSVC-709

As an extension to https://github.com/redhat-developer/service-binding-operator/pull/597 the new changes can now trigger reconcile on changes to services' metadata.Annotations

Run
Test - `make test-acceptance`


openshift-ci-robot: @Avni-Sharma: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/4.5-acceptance | f13ab22bec07ddf887f028c6f6b6c8611738f44b | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/704/pull-ci-redhat-developer-service-binding-operator-master-4.5-acceptance/1311666523307249664) | `/test 4.5-acceptance`

[Full PR test history](https://prow.ci.openshift.org/pr-history?org=redhat-developer&repo=service-binding-operator&pr=704). [Your PR dashboard](https://prow.ci.openshift.org/pr?query=is:pr+state:open+author:Avni-Sharma).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
openshift-ci-robot: @Avni-Sharma: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/4.5-acceptance | 3f8111378c1613385009313ad144a92750b0220f | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/704/pull-ci-redhat-developer-service-binding-operator-master-4.5-acceptance/1312066591248093184) | `/test 4.5-acceptance`

[Full PR test history](https://prow.ci.openshift.org/pr-history?org=redhat-developer&repo=service-binding-operator&pr=704). [Your PR dashboard](https://prow.ci.openshift.org/pr?query=is:pr+state:open+author:Avni-Sharma).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
openshift-ci-robot: @Avni-Sharma: PR needs rebase.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
openshift-ci-robot: @Avni-Sharma: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/4.5-acceptance | 6d5582ad9401095ccc528943c2008aaf5a115ff6 | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/704/pull-ci-redhat-developer-service-binding-operator-master-4.5-acceptance/1313860409576394752) | `/test 4.5-acceptance`

[Full PR test history](https://prow.ci.openshift.org/pr-history?org=redhat-developer&repo=service-binding-operator&pr=704). [Your PR dashboard](https://prow.ci.openshift.org/pr?query=is:pr+state:open+author:Avni-Sharma).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
pedjak: /cc @pmacik 
pmacik: /retest
openshift-ci-robot: @Avni-Sharma: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/4.5-acceptance | 6d5582ad9401095ccc528943c2008aaf5a115ff6 | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/704/pull-ci-redhat-developer-service-binding-operator-master-4.5-acceptance/1315596254398386176) | `/test 4.5-acceptance`

[Full PR test history](https://prow.ci.openshift.org/pr-history?org=redhat-developer&repo=service-binding-operator&pr=704). [Your PR dashboard](https://prow.ci.openshift.org/pr?query=is:pr+state:open+author:Avni-Sharma).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
pmacik: /lgtm
pmacik: /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/704#issuecomment-707632946" title="Approved">pmacik</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)~~ [pmacik]

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/servicebinding/sbrcontroller.go b/pkg/controller/servicebinding/sbrcontroller.go
index 752d7891..0e4d80e5 100644
--- a/pkg/controller/servicebinding/sbrcontroller.go
+++ b/pkg/controller/servicebinding/sbrcontroller.go
@@ -2,6 +2,7 @@ package servicebinding
 
 import (
 	"os"
+	"reflect"
 	"strings"
 
 	olmv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
@@ -20,6 +21,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	"github.com/redhat-developer/service-binding-operator/pkg/apis/operators/v1alpha1"
+	"github.com/redhat-developer/service-binding-operator/pkg/controller/servicebinding/binding"
 	"github.com/redhat-developer/service-binding-operator/pkg/log"
 )
 
@@ -106,6 +108,16 @@ func isOfKind(obj runtime.Object, kind string) bool {
 	return strings.EqualFold(obj.GetObjectKind().GroupVersionKind().Kind, kind)
 }
 
+// check whether annotations specify bindings
+func isAnnotationValid(ann map[string]string) bool {
+	for k := range ann {
+		if strings.HasPrefix(k, binding.AnnotationPrefix) {
+			return true
+		}
+	}
+	return false
+}
+
 // updateEvent returns a predicate handler function.
 func updateFunc(logger *log.Log) func(updateEvent event.UpdateEvent) bool {
 	return func(e event.UpdateEvent) bool {
@@ -141,8 +153,13 @@ func updateFunc(logger *log.Log) func(updateEvent event.UpdateEvent) bool {
 		} else {
 			statusAreEqual = statusComparison.Success
 		}
-
-		shouldReconcile := !specsAreEqual || !statusAreEqual
+		annotationsAreEqual := true
+		annOld := e.MetaOld.GetAnnotations()
+		annNew := e.MetaNew.GetAnnotations()
+		if isAnnotationValid(annOld) && isAnnotationValid(annNew) {
+			annotationsAreEqual = reflect.DeepEqual(annOld, annNew)
+		}
+		shouldReconcile := !specsAreEqual || !statusAreEqual || !annotationsAreEqual
 
 		logger.Debug("Resource update event received",
 			"GVK", e.ObjectNew.GetObjectKind().GroupVersionKind(),


Test patch:
diff --git a/pkg/controller/servicebinding/sbrcontroller_test.go b/pkg/controller/servicebinding/sbrcontroller_test.go
index 89a8c1db..c14a82d1 100644
--- a/pkg/controller/servicebinding/sbrcontroller_test.go
+++ b/pkg/controller/servicebinding/sbrcontroller_test.go
@@ -146,6 +146,63 @@ func TestSBRControllerBuildGVKPredicate(t *testing.T) {
 				AvailableReplicas: 3,
 			},
 		}
+		deploymentD := &appsv1.Deployment{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Deployment",
+				APIVersion: "apps/v1",
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{
+					"service": "binding",
+				},
+			},
+			Spec: appsv1.DeploymentSpec{
+				Selector: &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"new-app": "demo",
+					},
+				},
+			},
+			Status: appsv1.DeploymentStatus{
+				AvailableReplicas: 3,
+			},
+		}
+		deploymentE := &appsv1.Deployment{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Deployment",
+				APIVersion: "apps/v1",
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{
+					"service.binding/db.host": "backend",
+				},
+			},
+			Spec: appsv1.DeploymentSpec{
+				Selector: &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"new-app": "demo",
+					},
+				},
+			},
+		}
+		deploymentF := &appsv1.Deployment{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Deployment",
+				APIVersion: "apps/v1",
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{
+					"service.binding/db.port": "8080",
+				},
+			},
+			Spec: appsv1.DeploymentSpec{
+				Selector: &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"new-app": "demo",
+					},
+				},
+			},
+		}
 		secretA := &corev1.Secret{
 			TypeMeta: metav1.TypeMeta{
 				Kind:       "Secret",
@@ -248,6 +305,22 @@ func TestSBRControllerBuildGVKPredicate(t *testing.T) {
 				aMeta:  deploymentB.ObjectMeta,
 				bMeta:  deploymentC.ObjectMeta,
 			},
+			{
+				name:   "Predicate evaluation true: supported update as the deployment metadata annotations required for service binding is changed",
+				wanted: true,
+				a:      deploymentE,
+				b:      deploymentF,
+				aMeta:  deploymentE.ObjectMeta,
+				bMeta:  deploymentF.ObjectMeta,
+			},
+			{
+				name:   "Predicate evaluation false: non supported update as the deployment metadata annotations not required for service binding is changed",
+				wanted: false,
+				a:      deploymentC,
+				b:      deploymentD,
+				aMeta:  deploymentC.ObjectMeta,
+				bMeta:  deploymentD.ObjectMeta,
+			},
 		}
 		for _, tt := range tests {
 			t.Run(tt.name, func(t *testing.T) {
diff --git a/test/acceptance/features/bindAppToService.feature b/test/acceptance/features/bindAppToService.feature
index b58bc1d4..49b3d0a6 100644
--- a/test/acceptance/features/bindAppToService.feature
+++ b/test/acceptance/features/bindAppToService.feature
@@ -804,3 +804,105 @@ Feature: Bind an application to a service
             """
         Then Error message "spec: Required value" is thrown
         And Service Binding "binding-request-remove-spec" is not updated
+
+
+   Scenario: Backend Service metadata annotations update for service bindings gets propagated to the binding secret
+        Given OLM Operator "backend" is running
+        * The Custom Resource is present
+            """
+            apiVersion: "stable.example.com/v1"
+            kind: Backend
+            metadata:
+                name: backend-demo-2
+                annotations:
+                    service.binding/host: path={.spec.host}
+            spec:
+                host: example.com
+            status:
+                ready:true
+            """
+        * Service Binding is applied
+            """
+            apiVersion: operators.coreos.com/v1alpha1
+            kind: ServiceBinding
+            metadata:
+                name: binding-request-backend-ann-sb
+            spec:
+                services:
+                -   group: stable.example.com
+                    version: v1
+                    kind: Backend
+                    name: backend-demo-2
+            """
+        And Secret "binding-request-backend-ann-sb" contains "BACKEND_HOST" key with value "example.com"
+        And Secret "binding-request-backend-ann-sb" does not contain "BACKEND_READY"
+        # Backend metadata.annotations for service binding is updated
+        When The Custom Resource is present
+            """
+            apiVersion: "stable.example.com/v1"
+            kind: Backend
+            metadata:
+                name: backend-demo-2
+                annotations:
+                    service.binding/host: path={.spec.host}
+                    service.binding/ready: path={.status.ready}
+            spec:
+                host: example.com
+            status:
+                ready: true
+            """
+        Then jq ".status.conditions[] | select(.type=="CollectionReady").status" of Service Binding "binding-request-backend-ann-sb" should be changed to "True"
+        And jq ".status.conditions[] | select(.type=="InjectionReady").status" of Service Binding "binding-request-backend-ann-sb" should be changed to "False"
+        And Secret "binding-request-backend-ann-sb" contains "BACKEND_READY" key with value "true"
+        And Secret "binding-request-backend-ann-sb" contains "BACKEND_HOST" key with value "example.com"
+        And jq ".status.conditions[] | select(.type=="InjectionReady").reason" of Service Binding "binding-request-backend-ann-sb" should be changed to "EmptyApplication"
+
+
+   @negative
+   Scenario: Backend Service metadata annotations update not specific to service bindings does not get propagated to the binding secret
+        Given OLM Operator "backend" is running
+        * The Custom Resource is present
+            """
+            apiVersion: "stable.example.com/v1"
+            kind: Backend
+            metadata:
+                name: backend-demo-3
+                annotations:
+                    host : "demo.com"
+            spec:
+                host: example.com
+            status:
+                ready:true
+            """
+        * Service Binding is applied
+            """
+            apiVersion: operators.coreos.com/v1alpha1
+            kind: ServiceBinding
+            metadata:
+                name: binding-request-backend-ann
+            spec:
+                services:
+                -   group: stable.example.com
+                    version: v1
+                    kind: Backend
+                    name: backend-demo-3
+            """
+        And jq ".status.conditions[] | select(.type=="CollectionReady").status" of Service Binding "binding-request-backend-ann" should be changed to "True"
+        And jq ".status.conditions[] | select(.type=="InjectionReady").status" of Service Binding "binding-request-backend-ann" should be changed to "False"
+        And Secret "binding-request-backend-ann" is empty
+        And jq ".status.conditions[] | select(.type=="InjectionReady").reason" of Service Binding "binding-request-backend-ann" should be changed to "EmptyApplication"
+        # Backend metadata.annotations not pertaining to service binding is updated
+        When The Custom Resource is present
+            """
+            apiVersion: "stable.example.com/v1"
+            kind: Backend
+            metadata:
+                name: backend-demo-3
+                annotations:
+                    host : "example.common"
+            spec:
+                host: example.com
+            status:
+                ready: true
+            """
+        Then Secret "binding-request-backend-ann" is empty
diff --git a/test/acceptance/features/steps/steps.py b/test/acceptance/features/steps/steps.py
index 4619c98b..132c31ad 100644
--- a/test/acceptance/features/steps/steps.py
+++ b/test/acceptance/features/steps/steps.py
@@ -219,6 +219,7 @@ def then_app_is_connected_to_db(context, db_name):
 
 
 # STEP
+@given(u'jsonpath "{json_path}" of Service Binding "{sbr_name}" should be changed to "{json_value_regex}"')
 @then(u'jsonpath "{json_path}" of Service Binding "{sbr_name}" should be changed to "{json_value_regex}"')
 def then_sbo_jsonpath_is(context, json_path, sbr_name, json_value_regex):
     openshift = Openshift()
@@ -436,3 +437,24 @@ def validate_persistent_sb(context, sb_name):
         assert True
     else:
         assert False, "Service Binding got updated"
+
+
+@given(u'Secret "{secret_name}" does not contain "{key}"')
+def check_secret_key(context, secret_name, key):
+    openshift = Openshift()
+    json_path = f'{{.data.{key}}}'
+    polling2.poll(lambda: openshift.get_resource_info_by_jsonpath("secrets", secret_name, context.namespace.name,
+                                                                  json_path) == "",
+                  step=5, timeout=120, ignore_exceptions=(binascii.Error,))
+
+
+@step(u'Secret "{secret_name}" is empty')
+def validate_secret_empty(context, secret_name):
+    openshift = Openshift()
+    try:
+        polling2.poll(lambda: json.loads(
+            openshift.get_resource_info_by_jq("secrets", secret_name, context.namespace.name, ".data",
+                                              wait=False)) == "null", step=5, timeout=20,
+                      ignore_exceptions=(json.JSONDecodeError,))
+    except polling2.TimeoutException:
+        pass