Repository: redhat-developer/service-binding-operator
Pull request #275 (2019-12-09 12:08:01)

separate Secret and configmap switch case
While reading secret the data is encoded. This PR makes a separate switch case for reading the secret data.

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"]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/275?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@b0c9641`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `80%`.

[![Impacted file tree graph](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/275/graphs/tree.svg?width=650&token=CICBeVWlrC&height=150&src=pr)](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/275?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##             master     #275   +/-   ##
=========================================
  Coverage          ?   58.85%           
=========================================
  Files             ?       16           
  Lines             ?     1288           
  Branches          ?        0           
=========================================
  Hits              ?      758           
  Misses            ?      394           
  Partials          ?      136
```


| [Impacted Files](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/275?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ler/servicebindingrequest/bind\_custom\_resources.go](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/275/diff?src=pr&el=tree#diff-cGtnL2NvbnRyb2xsZXIvc2VydmljZWJpbmRpbmdyZXF1ZXN0L2JpbmRfY3VzdG9tX3Jlc291cmNlcy5nbw==) | `80.39% <80%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/275?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/275?src=pr&el=footer). Last update [b0c9641...548cbd4](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/275?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

sbose78: /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/e2e | 319ec27b4903a06dd3ebb648b042144f75791fda | [link](https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/275/pull-ci-redhat-developer-service-binding-operator-master-e2e/1012) | `/test e2e`

[Full PR test history](https://prow.svc.ci.openshift.org/pr-history?org=redhat-developer&repo=service-binding-operator&pr=275). [Your PR dashboard](https://prow.svc.ci.openshift.org/pr?query=is:pr+state:open+author:Avni-Sharma). Please help us cut down on flakes by [linking to](https://github.com/kubernetes/community/blob/master/contributors/devel/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/redhat-developer/service-binding-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<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 -->
isutton: /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/275#issuecomment-572131815" title="Approved">isutton</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)~~ [isutton]

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/bind_custom_resources.go b/pkg/controller/servicebindingrequest/bind_custom_resources.go
index 19469e99..5520202a 100644
--- a/pkg/controller/servicebindingrequest/bind_custom_resources.go
+++ b/pkg/controller/servicebindingrequest/bind_custom_resources.go
@@ -1,6 +1,8 @@
 package servicebindingrequest
 
 import (
+	"encoding/base64"
+
 	"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -69,8 +71,8 @@ func (b DetectBindableResources) GetBindableVariables() (map[string]interface{},
 	}
 	for _, resource := range ownedResources {
 		switch resource.GetKind() {
-		// In case ConfigMap and Secret we would read data field
-		case "ConfigMap", "Secret":
+		// In case of ConfigMap we would read data field
+		case "ConfigMap":
 			for _, v := range path[resource.GetKind()] {
 				d, exist, err := unstructured.NestedMap(resource.Object, v...)
 				if err != nil {
@@ -83,6 +85,21 @@ func (b DetectBindableResources) GetBindableVariables() (map[string]interface{},
 					}
 				}
 			}
+		// In case of Secret we would decode and read data field
+		case "Secret":
+			for _, v := range path[resource.GetKind()] {
+				d, exist, err := unstructured.NestedMap(resource.Object, v...)
+				if err != nil {
+					// skipping on error
+					continue
+				}
+				if exist {
+					for k, val := range d {
+						value, _ := base64.StdEncoding.DecodeString(val.(string))
+						b.data[k] = value
+					}
+				}
+			}
 
 			// In case of Route and Service we would extract information from respective path
 		case "Route", "Service":


Test patch:
diff --git a/pkg/controller/servicebindingrequest/bind_custom_resources_test.go b/pkg/controller/servicebindingrequest/bind_custom_resources_test.go
index e650f292..58209430 100644
--- a/pkg/controller/servicebindingrequest/bind_custom_resources_test.go
+++ b/pkg/controller/servicebindingrequest/bind_custom_resources_test.go
@@ -1,6 +1,8 @@
 package servicebindingrequest
 
 import (
+	"testing"
+
 	v12 "github.com/openshift/api/route/v1"
 	pgv1alpha1 "github.com/operator-backing-service-samples/postgresql-operator/pkg/apis/postgresql/v1alpha1"
 	"github.com/redhat-developer/service-binding-operator/test/mocks"
@@ -9,13 +11,13 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"testing"
 )
 
-func TestBindNonBindableResources_GetOwnedResources(t *testing.T) {
+var trueBool = true
+
+func TestBindNonBindableResources_ConfigMap_GetOwnedResources(t *testing.T) {
 	f := mocks.NewFake(t, "test")
 	cr := mocks.DatabaseCRMock("test", "test")
-	trueBool := true
 	reference := v1.OwnerReference{
 		APIVersion:         cr.APIVersion,
 		Kind:               cr.Kind,
@@ -63,12 +65,76 @@ func TestBindNonBindableResources_GetOwnedResources(t *testing.T) {
 	t.Run("Should return configmap as owned resource", func(t *testing.T) {
 		resources, err := b.GetOwnedResources()
 		require.NoError(t, err)
-		require.Equal(t, 2, len(resources), "Should return 1 owned resource")
+		require.Equal(t, 2, len(resources), "Should return 2 owned resource")
+	})
+
+	t.Run("Should return all variables exist in the configmap data section and route", func(t *testing.T) {
+		data, err := b.GetBindableVariables()
+		str1 := b.data["user"]
+		require.NoError(t, err)
+		require.Equal(t, 3, len(data), "")
+		require.Equal(t, str1, "user", "The intermediate data values are equal ")
+	})
+}
+
+func TestBindNonBindableResources_Secret_GetOwnedResources(t *testing.T) {
+	f := mocks.NewFake(t, "test")
+	cr := mocks.DatabaseCRMock("test", "test")
+	reference := v1.OwnerReference{
+		APIVersion:         cr.APIVersion,
+		Kind:               cr.Kind,
+		Name:               cr.Name,
+		UID:                cr.UID,
+		Controller:         &trueBool,
+		BlockOwnerDeletion: &trueBool,
+	}
+	secret := mocks.SecretMock("test", "test_database")
+	us := &unstructured.Unstructured{}
+	uc, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&secret)
+	require.NoError(t, err)
+	us.Object = uc
+	us.SetOwnerReferences([]v1.OwnerReference{reference})
+	route, err := runtime.DefaultUnstructuredConverter.ToUnstructured(mocks.RouteCRMock("test", "test"))
+	require.NoError(t, err)
+	usRoute := &unstructured.Unstructured{Object: route}
+	usRoute.SetOwnerReferences([]v1.OwnerReference{reference})
+	f.S.AddKnownTypes(pgv1alpha1.SchemeGroupVersion, &pgv1alpha1.Database{})
+	f.S.AddKnownTypes(v12.SchemeGroupVersion, &v12.Route{})
+	f.AddMockResource(cr)
+	f.AddMockResource(us)
+	f.AddMockResource(&unstructured.Unstructured{Object: route})
+
+	unstructuredCr, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(cr)
+	u := &unstructured.Unstructured{Object: unstructuredCr}
+	b := NewDetectBindableResources(
+		nil,
+		u,
+		[]schema.GroupVersionResource{
+			{
+				Group:    "",
+				Version:  "v1",
+				Resource: "secrets",
+			},
+			{
+				Group:    "",
+				Version:  "v1",
+				Resource: "routes",
+			},
+		},
+		f.FakeDynClient(),
+	)
+
+	t.Run("Should return secret as owned resource", func(t *testing.T) {
+		resources, err := b.GetOwnedResources()
+		require.NoError(t, err)
+		require.Equal(t, 2, len(resources), "Should return 2 owned resource")
 	})
 
-	t.Run("Should return all variables exist in the configmap data section", func(t *testing.T) {
+	t.Run("Should return all variables exist in the secret data section and route", func(t *testing.T) {
 		data, err := b.GetBindableVariables()
+		str2 := (b.data["user"]).([]byte)
 		require.NoError(t, err)
 		require.Equal(t, 3, len(data), "")
+		require.Equal(t, str2, []byte("user"), "The intermediate data values are equal ")
 	})
 }