Repository: redhat-developer/service-binding-operator
Pull request #64 (2019-07-12 12:35:51)

Support dotted notation in attribute path
Fixes https://jira.coreos.com/browse/ODC-1243

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 **baijum**
You can assign the PR to them by writing `/assign @baijum` 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":["baijum"]} -->
otaviof: /LGTM
baijum: /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/64#issuecomment-511286620" 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/retriever.go b/pkg/controller/servicebindingrequest/retriever.go
index 9dd866bc..cb441433 100644
--- a/pkg/controller/servicebindingrequest/retriever.go
+++ b/pkg/controller/servicebindingrequest/retriever.go
@@ -28,6 +28,23 @@ const (
 	secretPrefix  = "urn:alm:descriptor:servicebindingrequest:env:object:secret"
 )
 
+// getNestedValue retrieve value from dotted key path
+func (r *Retriever) getNestedValue(key string, sectionMap interface{}) (string, error) {
+	if !strings.Contains(key, ".") {
+		value, exists := sectionMap.(map[string]interface{})[key]
+		if !exists {
+			return "", fmt.Errorf("Can't find key '%s'", key)
+		}
+		return fmt.Sprintf("%v", value), nil
+	}
+	attrs := strings.SplitN(key, ".", 2)
+	newSectionMap, exists := sectionMap.(map[string]interface{})[attrs[0]]
+	if !exists {
+		return "", fmt.Errorf("Can't find '%v' section in CR", attrs)
+	}
+	return r.getNestedValue(attrs[1], newSectionMap.(map[string]interface{}))
+}
+
 // getCRKey retrieve key in section from CR object, part of the "plan" instance.
 func (r *Retriever) getCRKey(section string, key string) (string, error) {
 	obj := r.plan.CR.Object
@@ -40,15 +57,7 @@ func (r *Retriever) getCRKey(section string, key string) (string, error) {
 		return "", fmt.Errorf("Can't find '%s' section in CR named '%s'", section, objName)
 	}
 
-	value, exists := sectionMap.(map[string]interface{})[key]
-	if !exists {
-		return "", fmt.Errorf("Can't find key '%s' in section '%s', on object named '%s'",
-			key, section, objName)
-	}
-
-	logger.Info("CR attribute is found!")
-	// making sure we always return a string representation
-	return fmt.Sprintf("%v", value), nil
+	return r.getNestedValue(key, sectionMap)
 }
 
 // read attributes from CR, where place means which top level key name contains the "path" actual


Test patch:
diff --git a/pkg/controller/servicebindingrequest/retriever_test.go b/pkg/controller/servicebindingrequest/retriever_test.go
index f447d03b..0f4c9ce1 100644
--- a/pkg/controller/servicebindingrequest/retriever_test.go
+++ b/pkg/controller/servicebindingrequest/retriever_test.go
@@ -102,3 +102,49 @@ func TestRetriever(t *testing.T) {
 		assert.Nil(t, err)
 	})
 }
+
+func TestRetrieverNestedCRDKey(t *testing.T) {
+	logf.SetLogger(logf.ZapLogger(true))
+	var retriever *Retriever
+
+	ns := "testing"
+	crName := "db-testing"
+
+	crdDescription := mocks.CRDDescriptionMock()
+	cr := mocks.NestedDatabaseCRMock(ns, crName)
+
+	genericCR, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&cr)
+	require.Nil(t, err)
+
+	plan := &Plan{
+		Ns:             ns,
+		Name:           "retriever",
+		CRDDescription: &crdDescription,
+		CR:             &ustrv1.Unstructured{Object: genericCR},
+	}
+
+	dbSecret := mocks.SecretMock(ns, "db-credentials")
+	objs := []runtime.Object{&dbSecret}
+	fakeClient := fake.NewFakeClient(objs...)
+
+	retriever = NewRetriever(context.TODO(), fakeClient, plan)
+	require.NotNil(t, retriever)
+
+	t.Run("Second level", func(t *testing.T) {
+		imageName, err := retriever.getCRKey("spec", "image.name")
+		assert.Nil(t, err)
+		assert.Equal(t, "postgres", imageName)
+	})
+
+	t.Run("Second level error", func(t *testing.T) {
+		_, err := retriever.getCRKey("spec", "image..name")
+		assert.NotNil(t, err)
+	})
+
+	t.Run("Third level", func(t *testing.T) {
+		something, err := retriever.getCRKey("spec", "image.third.something")
+		assert.Nil(t, err)
+		assert.Equal(t, "somevalue", something)
+	})
+
+}
diff --git a/test/mocks/mocks.go b/test/mocks/mocks.go
index f6936de7..2a7ad624 100644
--- a/test/mocks/mocks.go
+++ b/test/mocks/mocks.go
@@ -184,3 +184,51 @@ func DeploymentMock(ns, name string, matchLabels map[string]string) extv1beta1.D
 		},
 	}
 }
+
+//ThirdLevel ...
+type ThirdLevel struct {
+	Something string `json:"something"`
+}
+
+// NestedImage ...
+type NestedImage struct {
+	Name       string     `json:"name"`
+	ThirdLevel ThirdLevel `json:"third"`
+}
+
+// NestedDatabaseSpec ...
+type NestedDatabaseSpec struct {
+	Image NestedImage `json:"image"`
+}
+
+// NestedDatabase ...
+type NestedDatabase struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec NestedDatabaseSpec `json:"spec,omitempty"`
+}
+
+// NestedDatabaseCRMock based on PostgreSQL operator, returning a instantiated object.
+func NestedDatabaseCRMock(ns, name string) NestedDatabase {
+	return NestedDatabase{
+		// usually TypeMeta should not be explicitly defined in mocked objects, however, on using
+		// it via *unstructured.Unstructured it could not find this CR without it.
+		TypeMeta: metav1.TypeMeta{
+			Kind:       CRDKind,
+			APIVersion: fmt.Sprintf("%s/%s", CRDName, CRDVersion),
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: ns,
+			Name:      name,
+		},
+		Spec: NestedDatabaseSpec{
+			Image: NestedImage{
+				Name: "postgres",
+				ThirdLevel: ThirdLevel{
+					Something: "somevalue",
+				},
+			},
+		},
+	}
+}