Repository: redhat-developer/service-binding-operator
Pull request #669 (2020-09-08 15:41:12)

[acceptnace-tests] Enable running specific features or scenarios by tag expression
### Motivation

Currently the only hard-coded tag expression is `~@disabled`, but there is no way to pass any additional tags or expressions without actual editing the `Makefile`.

### Changes

This PR introduces a way to pass additional tag expression to enable running specific acceptance test scenarios/features per tags.

### Testing

`TEST_ACCEPTANCE_TAGS=@negative make test-acceptance` to run just the negative scenarios.

or

1) Add e.g. `@wip` tag to a single scenario in any `*.feature` file:
```
Feature: ...

   @wip
   Scenario: ...
   ...
```
2) Run `TEST_ACCEPTANCE_TAGS=@wip make test-acceptnace` to run only the tagged scenario.


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 **avni-sharma**
You can assign the PR to them by writing `/assign @avni-sharma` 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":["avni-sharma"]} -->
openshift-ci-robot: Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with `/test all`

Patch:
diff --git a/Makefile b/Makefile
index d72faed9..24264a04 100644
--- a/Makefile
+++ b/Makefile
@@ -162,6 +162,14 @@ TEST_ACCEPTANCE_START_SBO ?= local
 TEST_ACCEPTANCE_OUTPUT_DIR ?= $(OUTPUT_DIR)/acceptance-tests
 TEST_ACCEPTANCE_ARTIFACTS ?= /tmp/artifacts
 
+TEST_ACCEPTANCE_TAGS ?=
+
+ifdef TEST_ACCEPTANCE_TAGS
+TEST_ACCEPTANCE_TAGS_ARG := --tags="~@disabled" --tags="$(TEST_ACCEPTANCE_TAGS)"
+else
+TEST_ACCEPTANCE_TAGS_ARG := --tags="~@disabled"
+endif
+
 ## -- Static code analysis (lint) targets --
 
 .PHONY: lint
@@ -321,7 +329,7 @@ test-acceptance: test-acceptance-setup
 	$(Q)TEST_ACCEPTANCE_START_SBO=$(TEST_ACCEPTANCE_START_SBO) \
 		TEST_ACCEPTANCE_SBO_STARTED=$(TEST_ACCEPTANCE_SBO_STARTED) \
 		TEST_NAMESPACE=$(TEST_NAMESPACE) \
-		$(PYTHON_VENV_DIR)/bin/behave --junit --junit-directory $(TEST_ACCEPTANCE_OUTPUT_DIR) $(V_FLAG) --no-capture --no-capture-stderr --tags="~@disabled" test/acceptance/features
+		$(PYTHON_VENV_DIR)/bin/behave --junit --junit-directory $(TEST_ACCEPTANCE_OUTPUT_DIR) $(V_FLAG) --no-capture --no-capture-stderr $(TEST_ACCEPTANCE_TAGS_ARG) test/acceptance/features
 	$(Q)kill $(TEST_ACCEPTANCE_SBO_STARTED)
 
 .PHONY: test-acceptance-artifacts
diff --git a/pkg/controller/servicebindingrequest/sbrcontroller.go b/pkg/controller/servicebindingrequest/sbrcontroller.go
index a833c465..0dc323e7 100644
--- a/pkg/controller/servicebindingrequest/sbrcontroller.go
+++ b/pkg/controller/servicebindingrequest/sbrcontroller.go
@@ -113,18 +113,28 @@ func updateFunc(logger *log.Log) func(updateEvent event.UpdateEvent) bool {
 		isConfigMap := isOfKind(e.ObjectNew, "ConfigMap")
 
 		if isSecret || isConfigMap {
-			dataFieldsAreEqual, err := compareObjectFields(e.ObjectNew, e.ObjectOld, "data")
+			dataFieldsAreEqual, err := compareObjectFields(e.ObjectOld, e.ObjectNew, "data")
 			if err != nil {
 				logger.Error(err, "error comparing object fields: %s", err.Error())
 				return false
 			}
-
-			logger.Debug("Predicate evaluated", "dataFieldsAreEqual", dataFieldsAreEqual)
+			logger.Debug("Predicate evaluated for Secret/ConfigMap", "dataFieldsAreEqual", dataFieldsAreEqual)
 			return !dataFieldsAreEqual
 		}
+		specsAreEqual, err := compareObjectFields(e.ObjectOld, e.ObjectNew, "spec")
+		if err != nil {
+			logger.Error(err, "error comparing object's spec fields: %s", err.Error())
+			return false
+		}
+		statusAreEqual, err := compareObjectFields(e.ObjectOld, e.ObjectNew, "status")
+		if err != nil {
+			logger.Error(err, "error comparing object's status fields: %s", err.Error())
+			return false
+		}
+		shouldReconcile := !specsAreEqual || !statusAreEqual
+		logger.Debug("Resource update event received", "GVK", e.ObjectNew.GetObjectKind(), "spec changed", !specsAreEqual, "status changed", !statusAreEqual, "should reconcile", shouldReconcile)
 
-		// ignore updates to CR status in which case metadata.Generation does not change
-		return e.MetaOld.GetGeneration() != e.MetaNew.GetGeneration()
+		return shouldReconcile
 	}
 }
 


Test patch:
diff --git a/pkg/controller/servicebindingrequest/sbrcontroller_test.go b/pkg/controller/servicebindingrequest/sbrcontroller_test.go
index d49110f2..baf3fc25 100644
--- a/pkg/controller/servicebindingrequest/sbrcontroller_test.go
+++ b/pkg/controller/servicebindingrequest/sbrcontroller_test.go
@@ -94,25 +94,77 @@ func TestSBRControllerBuildGVKPredicate(t *testing.T) {
 
 	// update verifies whether only the accepted manifests trigger the reconciliation process
 	t.Run("update", func(t *testing.T) {
-		deploymentA := &appsv1.Deployment{}
-		configMapA := &corev1.ConfigMap{
-			ObjectMeta: metav1.ObjectMeta{
-				Generation: 1,
+		deploymentA := &appsv1.Deployment{
+			Spec: appsv1.DeploymentSpec{
+				Selector: &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"app": "demo",
+					},
+				},
+			},
+			Status: appsv1.DeploymentStatus{
+				AvailableReplicas: 2,
 			},
 		}
-		configMapB := &corev1.ConfigMap{
-			ObjectMeta: metav1.ObjectMeta{
-				Generation: 2,
+		deploymentB := &appsv1.Deployment{
+			Spec: appsv1.DeploymentSpec{
+				Selector: &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"app": "demo",
+					},
+				},
+			},
+			Status: appsv1.DeploymentStatus{
+				AvailableReplicas: 3,
+			},
+		}
+		deploymentC := &appsv1.Deployment{
+			Spec: appsv1.DeploymentSpec{
+				Selector: &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"new-app": "demo",
+					},
+				},
+			},
+			Status: appsv1.DeploymentStatus{
+				AvailableReplicas: 3,
 			},
 		}
 		secretA := &corev1.Secret{
-			ObjectMeta: metav1.ObjectMeta{
-				Generation: 1,
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Secret",
+				APIVersion: "v1",
+			},
+			Data: map[string][]byte{
+				"user": []byte("username"),
 			},
 		}
+
 		secretB := &corev1.Secret{
-			ObjectMeta: metav1.ObjectMeta{
-				Generation: 2,
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Secret",
+				APIVersion: "v1",
+			},
+			Data: map[string][]byte{
+				"password": []byte("password"),
+			},
+		}
+		configMapA := &corev1.ConfigMap{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "ConfigMap",
+				APIVersion: "v1",
+			},
+			Data: map[string]string{
+				"CP": "ConnectionPort",
+			},
+		}
+		configMapB := &corev1.ConfigMap{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "ConfigMap",
+				APIVersion: "v1",
+			},
+			Data: map[string]string{
+				"Host": "Hostname",
 			},
 		}
 
@@ -121,55 +173,53 @@ func TestSBRControllerBuildGVKPredicate(t *testing.T) {
 			wanted bool
 			a      runtime.Object
 			b      runtime.Object
-			metaA  metav1.Object
-			metaB  metav1.Object
 		}{
 			{
-				name:   "non supported update",
+				name:   "Predicate evaluation false: non supported update as deployment spec is same",
 				wanted: false,
 				a:      deploymentA,
 				b:      deploymentA,
-				metaA:  deploymentA.GetObjectMeta(),
-				metaB:  deploymentA.GetObjectMeta(),
 			},
 			{
-				name:   "supported update no changes",
+				name:   "Predicate evaluation false: non supported update as there is no change in the ConfigMap",
 				wanted: false,
 				a:      configMapA,
 				b:      configMapA,
-				metaA:  configMapA.GetObjectMeta(),
-				metaB:  configMapA.GetObjectMeta(),
 			},
 			{
-				name:   "supported update no changes",
+				name:   "Predicate evaluation false: non supported update there is no change in the Secret",
 				wanted: false,
 				a:      secretA,
 				b:      secretA,
-				metaA:  secretA.GetObjectMeta(),
-				metaB:  secretA.GetObjectMeta(),
 			},
 			{
-				name:   "supported update generation changed",
+				name:   "Predicate evaluation true: supported update as there is an update in the ConfigMap",
 				wanted: true,
 				a:      configMapA,
 				b:      configMapB,
-				metaA:  configMapA.GetObjectMeta(),
-				metaB:  configMapB.GetObjectMeta(),
 			},
 			{
-				name:   "supported update generation changed",
+				name:   "Predicate evaluation true: supported update as there is an update in the Secret",
 				wanted: true,
 				a:      secretA,
 				b:      secretB,
-				metaA:  secretA.GetObjectMeta(),
-				metaB:  secretB.GetObjectMeta(),
+			},
+			{
+				name:   "Predicate evaluation true: supported update as the deployment status changed",
+				wanted: true,
+				a:      deploymentA,
+				b:      deploymentB,
+			},
+			{
+				name:   "Predicate evaluation true: supported update as the deployment spec changed",
+				wanted: true,
+				a:      deploymentB,
+				b:      deploymentC,
 			},
 		}
 		for _, tt := range tests {
 			t.Run(tt.name, func(t *testing.T) {
 				e := event.UpdateEvent{
-					MetaOld:   tt.metaA,
-					MetaNew:   tt.metaB,
 					ObjectOld: tt.a,
 					ObjectNew: tt.b,
 				}
diff --git a/test/acceptance/features/bindAppToService.feature b/test/acceptance/features/bindAppToService.feature
index eeef5d82..74754ee9 100644
--- a/test/acceptance/features/bindAppToService.feature
+++ b/test/acceptance/features/bindAppToService.feature
@@ -222,3 +222,55 @@ Feature: Bind an application to a service
         And jq ".status.conditions[] | select(.type=="InjectionReady").status" of Service Binding Request "binding-request-empty-app" should be changed to "False"
         And jq ".status.conditions[] | select(.type=="InjectionReady").reason" of Service Binding Request "binding-request-empty-app" should be changed to "EmptyApplicationSelector"
 
+
+    Scenario: Backend Service status update gets propagated to the binding secret
+        Given OLM Operator "backend" is running
+        * Backend CR "backend-demo" is applied
+            """
+            apiVersion: "stable.example.com/v1"
+            kind: Backend
+            metadata:
+                name: backend-demo
+                annotations:
+                    servicebindingoperator.redhat.io/status.ready: 'binding:env:attribute'
+                    servicebindingoperator.redhat.io/spec.host: 'binding:env:attribute'
+            spec:
+                host: example.common
+            """
+        * Service Binding request is applied
+            """
+            apiVersion: apps.openshift.io/v1alpha1
+            kind: ServiceBindingRequest
+            metadata:
+                name: binding-request-backend
+            spec:
+                backingServiceSelector:
+                    group: stable.example.com
+                    version: v1
+                    kind: Backend
+                    resourceRef: backend-demo
+                    id: SBR
+                customEnvVar:
+                  - name: CustomReady
+                    value: '{{ .SBR.status.ready }}'
+                  - name: CustomHost
+                    value: '{{ .SBR.spec.host }}'
+            """
+        Then jq ".status.conditions[] | select(.type=="CollectionReady").status" of Service Binding Request "binding-request-backend" should be changed to "True"
+        And jq ".status.conditions[] | select(.type=="InjectionReady").status" of Service Binding Request "binding-request-backend" should be changed to "False"
+        Then Secret "binding-request-backend" contains "CustomReady" key with value "<no value>"
+        When Backend status in "backend-demo" is updated
+            """
+            apiVersion: "stable.example.com/v1"
+            kind: Backend
+            metadata:
+                name: backend-demo
+                annotations:
+                    servicebindingoperator.redhat.io/status.ready: 'binding:env:attribute'
+                    servicebindingoperator.redhat.io/spec.host: 'binding:env:attribute'
+            spec:
+                host: example.common
+            status:
+                ready: true
+            """
+        Then Secret "binding-request-backend" contains "CustomReady" key with value "true"
\ No newline at end of file
diff --git a/test/acceptance/features/steps/openshift.py b/test/acceptance/features/steps/openshift.py
index 5e04f546..e8a7ec61 100644
--- a/test/acceptance/features/steps/openshift.py
+++ b/test/acceptance/features/steps/openshift.py
@@ -334,3 +334,8 @@ spec:
             return lst
         print('Resource list is empty under namespace - {}'.format(namespace))
         return None
+
+    def oc_apply_yaml_file(self, yaml):
+        (output, exit_code) = self.cmd.run("oc apply -f " + yaml)
+        assert exit_code == 0, "Applying yaml file failed as the exit code is not 0"
+        return output
diff --git a/test/acceptance/features/steps/steps.py b/test/acceptance/features/steps/steps.py
index e4d09e7a..f1835803 100644
--- a/test/acceptance/features/steps/steps.py
+++ b/test/acceptance/features/steps/steps.py
@@ -4,6 +4,7 @@
 # ----------------------------------------------------------------------------
 import os
 import re
+import base64
 
 from behave import given, then, when
 from pyshould import should, should_not
@@ -284,6 +285,53 @@ def then_envFrom_contains_intermediate_secret_name(context, intermediate_secret_
         intermediate_secret_name, wait=True, timeout=120) is not None, f"There is no deployment with intermediate secret {intermediate_secret_name}"
 
 
+# STEP
+@given(u'OLM Operator "{backend_service}" is running')
+def operator_manifest_installed(context, backend_service):
+    openshift = Openshift()
+    _ = openshift.oc_apply_yaml_file(os.path.join(os.getcwd(), "test/acceptance/resources/", backend_service + ".operator.manifest.yaml"))
+
+
+# STEP
+@given(u'Backend CR "{cr_name}" is applied')
+def create_backend_cr(context, cr_name):
+    create_cr(context, cr_name)
+
+
+# STEP
+@given(u'Service Binding request is applied')
+def create_sbr(context):
+    sbr_is_applied(context)
+
+
+# STEP
+@when(u'Backend status in "{backend_name}" is updated')
+def backend_status_updated(context, backend_name):
+    create_cr(context, backend_name)
+
+
+# STEP
+@then(u'Secret "{secret_name}" contains "{secret_key}" key with value "{secret_value}"')
+def check_secret_key_value(context, secret_name, secret_key, secret_value):
+    openshift = Openshift()
+    json_path = f'{{.data.{secret_key}}}'
+    output = openshift.get_resource_info_by_jsonpath("secrets", secret_name, context.namespace.name, json_path)
+    timeout = 180
+    interval = 5
+    attempts = timeout/interval
+    while True:
+        actual_secret_value = base64.b64decode(output).decode('ascii')
+        if (secret_value == actual_secret_value) or attempts <= 0:
+            break
+        else:
+            attempts -= 1
+            time.sleep(interval)
+            output = openshift.get_resource_info_by_jsonpath("secrets", secret_name, context.namespace.name, json_path)
+    result = base64.decodebytes(bytes(output, 'utf-8'))
+    result | should.be_equal_to(bytes(secret_value, 'utf-8'))
+
+
+# STEP
 @given(u'The CRD "{crd_name}" is present')
 def create_crd(context, crd_name):
     openshift = Openshift()
@@ -299,8 +347,8 @@ def create_cr(context, cr_name):
     openshift = Openshift()
     yaml = context.text
     output = openshift.oc_apply(yaml)
-    result = re.search(rf'.*{cr_name}.*(created|unchanged)', output)
-    result | should_not.be_none.desc("CRD {cr_name} Created")
+    result = re.search(rf'.*{cr_name}.*(created|unchanged|configured)', output)
+    result | should_not.be_none.desc("CR {cr_name} Created/Updated")
 
 
 @then(u'Secret "{secret_ref}" has been injected in to CR "{cr_name}" of kind "{crd_name}" at path "{json_path}"')
diff --git a/test/acceptance/resources/backend.operator.manifest.yaml b/test/acceptance/resources/backend.operator.manifest.yaml
new file mode 100644
index 00000000..c90c1595
--- /dev/null
+++ b/test/acceptance/resources/backend.operator.manifest.yaml
@@ -0,0 +1,67 @@
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  name: backends.stable.example.com
+spec:
+  group: stable.example.com
+  versions:
+    - name: v1
+      served: true
+      storage: true
+  scope: Namespaced
+  names:
+    plural: backends
+    singular: backend
+    kind: Backend
+    shortNames:
+      - bk
+---
+apiVersion: operators.coreos.com/v1alpha1
+kind: ClusterServiceVersion
+metadata:
+  annotations:
+    capabilities: Basic Install
+  name: backend-operator.v0.1.0
+spec:
+  apiservicedefinitions: {}
+  customresourcedefinitions:
+    owned:
+    - description: Backend is the Schema for the backend API
+      kind: Backend
+      name: backends.stable.example.com
+      version: v1
+  displayName: Backend Operator
+  install:
+    spec:
+      deployments:
+      - name: backend-operator
+        spec:
+          replicas: 1
+          selector:
+            matchLabels:
+              name: backend-operator
+          strategy: {}
+          template:
+            metadata:
+              labels:
+                name: backend-operator
+            spec:
+              containers:
+              - command:
+                - backend-operator
+                env:
+                - name: WATCH_NAMESPACE
+                  valueFrom:
+                    fieldRef:
+                      fieldPath: metadata.annotations['olm.targetNamespaces']
+                - name: POD_NAME
+                  valueFrom:
+                    fieldRef:
+                      fieldPath: metadata.name
+                - name: OPERATOR_NAME
+                  value: backend-operator
+                image: REPLACE_IMAGE
+                imagePullPolicy: Always
+                name: backend-operator
+                resources: {}
+    strategy: deployment
\ No newline at end of file