Repository: redhat-developer/service-binding-operator
Pull request #1043 (2021-10-06 06:02:46)

Add support for CrunchyData Postgres operator
In order to read binding data from the right secret, the value of `path` key of a service binding annotation
is treated as a template:
* multiple json paths can specified and the results of their evaluation are rendered in the final binding value
* the template can contain arbitrary additional strings that are rendered 'as-is' in the final binding value

Hence, it is possible to generate proper secret reference and read binding data from it. In the case of
CrunchyData Postgres Operator, this is the right annotations:

```
"service.binding": "path={.metadata.name}-pguser-{.metadata.name},objectType=Secret",
```

On top of that, `user` is exposed as `username` as well, and `dbname` and `database`, so that
more applications can find all the binding data they need.

openshift-ci[bot]: [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 ask for approval from **pedjak** after the PR has been reviewed.

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":["pedjak"]} -->
openshift-ci[bot]: @pedjak: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
ci/prow/4.7-acceptance | 700a96e2dc2146281899b457f9f36d5004da4cfb | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/1043/pull-ci-redhat-developer-service-binding-operator-master-4.7-acceptance/1445630598474371072) | true | `/test 4.7-acceptance`

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

<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[bot]: @pedjak: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
ci/prow/4.7-acceptance | 700a96e2dc2146281899b457f9f36d5004da4cfb | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/1043/pull-ci-redhat-developer-service-binding-operator-master-4.7-acceptance/1445630598474371072) | true | `/test 4.7-acceptance`
ci/prow/4.9-acceptance | 700a96e2dc2146281899b457f9f36d5004da4cfb | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/1043/pull-ci-redhat-developer-service-binding-operator-master-4.9-acceptance/1445630598558257152) | true | `/test 4.9-acceptance`

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

<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 -->
baijum: /approve
openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/redhat-developer/service-binding-operator/pull/1043#issuecomment-936018627" 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/config/rbac/crunchy_postgres_clusterrole.yaml b/config/rbac/crunchy_postgres_clusterrole.yaml
new file mode 100644
index 00000000..903a6993
--- /dev/null
+++ b/config/rbac/crunchy_postgres_clusterrole.yaml
@@ -0,0 +1,14 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: crunchy-postgres-viewer-role
+  labels:
+    servicebinding.io/controller: "true"
+rules:
+  - apiGroups:
+      - postgres-operator.crunchydata.com
+    resources:
+      - postgresclusters
+    verbs:
+      - get
+      - list
diff --git a/config/rbac/kustomization.yaml b/config/rbac/kustomization.yaml
index cb584674..4a57a18e 100644
--- a/config/rbac/kustomization.yaml
+++ b/config/rbac/kustomization.yaml
@@ -12,6 +12,7 @@ resources:
 - servicebinding_controller_rolebinding.yaml
 # operators supproted out of the box
 - opstree_redis_clusterrole.yaml
+- crunchy_postgres_clusterrole.yaml
 # Comment the following 4 lines if you want to disable
 # the auth proxy (https://github.com/brancz/kube-rbac-proxy)
 # which protects your /metrics endpoint.
diff --git a/controllers/crd/crd_controller.go b/controllers/crd/crd_controller.go
index 85f4b2be..6f0ef142 100644
--- a/controllers/crd/crd_controller.go
+++ b/controllers/crd/crd_controller.go
@@ -42,6 +42,12 @@ var bindingAnnotations = map[schema.GroupVersionKind]map[string]string{
 		"service.binding/host":     "path={.metadata.name}",
 		"service.binding/password": "path={.spec.kubernetesConfig.redisSecret.name},objectType=Secret,sourceKey=password",
 	},
+	schema.GroupVersionKind{Group: "postgres-operator.crunchydata.com", Version: "v1beta1", Kind: "PostgresCluster"}: {
+		"service.binding/type":     "postgresql",
+		"service.binding":          "path={.metadata.name}-pguser-{.metadata.name},objectType=Secret",
+		"service.binding/database": "path={.metadata.name}-pguser-{.metadata.name},objectType=Secret,sourceKey=dbname",
+		"service.binding/username": "path={.metadata.name}-pguser-{.metadata.name},objectType=Secret,sourceKey=user",
+	},
 }
 
 // CrdReconciler reconciles a CustomResourceDefinition resources
diff --git a/pkg/binding/definition.go b/pkg/binding/definition.go
index 986aa4ff..ba4679a2 100644
--- a/pkg/binding/definition.go
+++ b/pkg/binding/definition.go
@@ -166,7 +166,7 @@ func (d *mapFromDataFieldDefinition) Apply(u *unstructured.Unstructured) (Value,
 	if len(res) != 1 {
 		return nil, fmt.Errorf("only one value should be returned for %v but we got %v", d.path, res)
 	}
-	resourceName := res[0].Elem().String()
+	resourceName := fmt.Sprintf("%v", res[0].Interface())
 
 	var otherObj *unstructured.Unstructured
 	if d.objectType == secretObjectType {
diff --git a/pkg/binding/jsonpath.go b/pkg/binding/jsonpath.go
index cadc5f38..e48d0c8d 100644
--- a/pkg/binding/jsonpath.go
+++ b/pkg/binding/jsonpath.go
@@ -1,10 +1,9 @@
 package binding
 
 import (
-	"fmt"
-	"reflect"
-
 	"k8s.io/client-go/util/jsonpath"
+	"reflect"
+	"strings"
 )
 
 // getValuesByJSONPath returns values from the given map matching the provided JSONPath
@@ -23,7 +22,13 @@ func getValuesByJSONPath(obj map[string]interface{}, path string) ([]reflect.Val
 		return nil, err
 	}
 	if len(result) > 1 {
-		return nil, fmt.Errorf("more than one item found in the result: %v", result)
+		w := strings.Builder{}
+		for i := range result {
+			if err := j.PrintResults(&w, result[i]); err != nil {
+				return nil, err
+			}
+		}
+		return []reflect.Value{reflect.ValueOf(w.String())}, nil
 	}
 	return result[0], nil
 }
diff --git a/pkg/binding/model.go b/pkg/binding/model.go
index c9bad704..3d60dd79 100644
--- a/pkg/binding/model.go
+++ b/pkg/binding/model.go
@@ -65,7 +65,7 @@ func newModel(annotationValue string) (*model, error) {
 		}
 		return nil, fmt.Errorf("path not found: %q", annotationValue)
 	}
-	if !strings.HasPrefix(path, "{") || !strings.HasSuffix(path, "}") {
+	if n := strings.Count(path, "{"); n == 0 || n != strings.Count(path, "}") {
 		return nil, fmt.Errorf("path has invalid syntax: %q", path)
 	}
 


Test patch:
diff --git a/pkg/binding/definition_test.go b/pkg/binding/definition_test.go
index 901541c9..3ab7defb 100644
--- a/pkg/binding/definition_test.go
+++ b/pkg/binding/definition_test.go
@@ -42,6 +42,22 @@ func TestStringDefinition(t *testing.T) {
 				"foo": "bar",
 			},
 		},
+		{
+			description: "value with prefix",
+			outputName:  "foo",
+			path:        "foo-{.status.dbCredentials.username}",
+			expectedValue: map[string]interface{}{
+				"foo": "foo-AzureDiamond",
+			},
+		},
+		{
+			description: "concatenate two json paths",
+			outputName:  "foo",
+			path:        "{.status.dbCredentials.username}-{.status.dbCredentials.password}",
+			expectedValue: map[string]interface{}{
+				"foo": "AzureDiamond-foo",
+			},
+		},
 	}
 
 	u := &unstructured.Unstructured{
@@ -49,6 +65,7 @@ func TestStringDefinition(t *testing.T) {
 			"status": map[string]interface{}{
 				"dbCredentials": map[string]interface{}{
 					"username": "AzureDiamond",
+					"password": "foo",
 				},
 			},
 		},
@@ -234,6 +251,37 @@ func TestMapFromSecretDataField(t *testing.T) {
 	require.Equal(t, v, val.Get())
 }
 
+func TestMapFromConstructedSecretName(t *testing.T) {
+	f := mocks.NewFake(t, "test-namespace")
+	f.AddMockedUnstructuredSecret("foo-dbCredentials-secret")
+	d := &mapFromDataFieldDefinition{
+		secretConfigMapReader: &secretConfigMapReader{
+			secretReader:    secretsReader(f.FakeDynClient()),
+			configMapReader: configMapsReader(f.FakeDynClient()),
+		},
+		objectType: secretObjectType,
+		definition: definition{
+			path: "foo-{.status.dbCredentials}",
+		},
+	}
+	val, err := d.Apply(&unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"namespace": "test-namespace",
+			},
+			"status": map[string]interface{}{
+				"dbCredentials": "dbCredentials-secret",
+			},
+		},
+	})
+	require.NoError(t, err)
+	v := map[string]string{
+		"username": "user",
+		"password": "password",
+	}
+	require.Equal(t, v, val.Get())
+}
+
 func TestMapFromConfigMapDataField(t *testing.T) {
 	f := mocks.NewFake(t, "test-namespace")
 	f.AddMockedUnstructuredConfigMap("dbCredentials-configMap")
diff --git a/pkg/binding/jsonpath_test.go b/pkg/binding/jsonpath_test.go
index 959fae0f..f7cec8c2 100644
--- a/pkg/binding/jsonpath_test.go
+++ b/pkg/binding/jsonpath_test.go
@@ -59,31 +59,35 @@ func TestGetValueByJSONPath(t *testing.T) {
 		WantErr bool
 	}{
 		{
-			Path: ".spec.serviceName",
+			Path: "{.spec.serviceName}",
 			Want: "db1-svc",
 		},
 		{
-			Path: ".spec.template.spec.containers[0].name",
+			Path: "{.spec.template.spec.containers[0].name}",
 			Want: "db1",
 		},
 		{
-			Path: ".spec.template.spec.containers[0].env[2].value",
+			Path: "{.spec.template.spec.containers[0].env[2].value}",
 			Want: "mydb",
 		},
 		{
-			Path: ".spec.template.spec.containers[?(@.name==\"db1\")].env[?(@.name==\"POSTGRESQL_USER\")].value",
+			Path: "{.spec.template.spec.containers[?(@.name==\"db1\")].env[?(@.name==\"POSTGRESQL_USER\")].value}",
 			Want: "user1",
 		},
 		{
-			Path: ".spec.template.metadata.labels",
+			Path: "{.spec.template.metadata.labels}",
 			Want: map[string]interface{}{
 				"app": "db1",
 			},
 		},
 		{
-			Path:    ".foo",
+			Path:    "{.foo}",
 			WantErr: true,
 		},
+		{
+			Path: "foo-{.spec.serviceName}",
+			Want: "foo-db1-svc",
+		},
 	}
 
 	var u unstructured.Unstructured
@@ -94,7 +98,7 @@ func TestGetValueByJSONPath(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.Path, func(t *testing.T) {
-			result, err := getValuesByJSONPath(u.Object, "{"+test.Path+"}")
+			result, err := getValuesByJSONPath(u.Object, test.Path)
 			if (err != nil) != test.WantErr {
 				t.Errorf("Expecting err %v, got %v\n", test.WantErr, err)
 			}
diff --git a/test/acceptance/features/steps/crunchypostgresoperator.py b/test/acceptance/features/steps/crunchypostgresoperator.py
new file mode 100644
index 00000000..06869913
--- /dev/null
+++ b/test/acceptance/features/steps/crunchypostgresoperator.py
@@ -0,0 +1,24 @@
+from olm import Operator
+from environment import ctx
+from behave import given
+
+
+class CrunchyPostgresOperator(Operator):
+
+    def __init__(self, name="pgo"):
+        self.name = name
+        if ctx.cli == "oc":
+            self.operator_catalog_source_name = "community-operators"
+        else:
+            self.operator_catalog_source_name = "operatorhubio-catalog"
+        self.operator_catalog_channel = "v5"
+        self.package_name = "postgresql"
+
+
+@given(u'Crunchy Data Postgres operator is running')
+def install(_context):
+    operator = CrunchyPostgresOperator()
+    if not operator.is_running():
+        operator.install_operator_subscription()
+        operator.is_running(wait=True)
+    print("Crunchy Data Postgres operator is running")
diff --git a/test/acceptance/features/steps/generic_testapp.py b/test/acceptance/features/steps/generic_testapp.py
index 456dfd6c..becec22b 100644
--- a/test/acceptance/features/steps/generic_testapp.py
+++ b/test/acceptance/features/steps/generic_testapp.py
@@ -79,11 +79,17 @@ def check_env_var_existence(context, name):
 
 @step(u'Content of file "{file_path}" in application pod is')
 def check_file_value(context, file_path):
-    value = context.text.strip()
+    value = Template(context.text.strip()).substitute(NAMESPACE=context.namespace.name)
     resource = Template(file_path).substitute(scenario_id=scenario_id(context))
     polling2.poll(lambda: context.application.get_file_value(resource) == value, step=5, timeout=400)
 
 
+@step(u'File "{file_path}" exists in application pod')
+def check_file_exists(context, file_path):
+    resource = Template(file_path).substitute(scenario_id=scenario_id(context))
+    polling2.poll(lambda: context.application.get_file_value(resource) != "", step=5, timeout=400)
+
+
 @step(u'File "{file_path}" is unavailable in application pod')
 def check_file_unavailable(context, file_path):
     context.application.assert_file_not_exist(file_path)
diff --git a/test/acceptance/features/supportExistingOperatorBackedServices.feature b/test/acceptance/features/supportExistingOperatorBackedServices.feature
index cd8199e0..68ddaae7 100644
--- a/test/acceptance/features/supportExistingOperatorBackedServices.feature
+++ b/test/acceptance/features/supportExistingOperatorBackedServices.feature
@@ -86,4 +86,75 @@ Feature: Support a number of existing operator-backed services out of the box
     And Content of file "/bindings/$scenario_id/password" in application pod is
            """
            redisSecret!
-           """
\ No newline at end of file
+           """
+
+  Scenario: Bind test application to Postgres provisioned by Crunchy Data Postgres operator
+    Given Crunchy Data Postgres operator is running
+    * Generic test application is running
+    * The Custom Resource is present
+          """
+          apiVersion: postgres-operator.crunchydata.com/v1beta1
+          kind: PostgresCluster
+          metadata:
+            name: hippo
+          spec:
+            image: registry.developers.crunchydata.com/crunchydata/crunchy-postgres:centos8-13.4-1
+            postgresVersion: 13
+            instances:
+              - name: instance1
+                dataVolumeClaimSpec:
+                  accessModes:
+                  - "ReadWriteOnce"
+                  resources:
+                    requests:
+                      storage: 1Gi
+            backups:
+              pgbackrest:
+                image: registry.developers.crunchydata.com/crunchydata/crunchy-pgbackrest:centos8-2.35-0
+                repos:
+                - name: repo1
+                  volume:
+                    volumeClaimSpec:
+                      accessModes:
+                      - "ReadWriteOnce"
+                      resources:
+                        requests:
+                          storage: 1Gi
+          """
+    When Service Binding is applied
+          """
+          apiVersion: binding.operators.coreos.com/v1alpha1
+          kind: ServiceBinding
+          metadata:
+              name: $scenario_id
+          spec:
+              services:
+              - group: postgres-operator.crunchydata.com
+                version: v1beta1
+                kind: PostgresCluster
+                name: hippo
+              application:
+                name: $scenario_id
+                group: apps
+                version: v1
+                resource: deployments
+          """
+    Then Service Binding is ready
+    And Kind PostgresCluster with apiVersion postgres-operator.crunchydata.com/v1beta1 is listed in bindable kinds
+    And Content of file "/bindings/$scenario_id/type" in application pod is
+           """
+           postgresql
+           """
+    And Content of file "/bindings/$scenario_id/host" in application pod is
+           """
+           hippo-primary.$NAMESPACE.svc
+           """
+    And Content of file "/bindings/$scenario_id/database" in application pod is
+           """
+           hippo
+           """
+    And Content of file "/bindings/$scenario_id/username" in application pod is
+           """
+           hippo
+           """
+    And File "/bindings/$scenario_id/password" exists in application pod