Repository: redhat-developer/service-binding-operator
Pull request #591 (2020-07-30 09:42:16)

[acceptance-tests] Fix python linting - dead code detection
### Motivation

Currently the dead code detection part of python linters `make lint-python-code` do not find the `*.py` files and is useless.

### Changes

This PR fixes the dead code detection script (`hack/check-python/detect-dead-code.sh`) to include all the `*.py` files

### Testing

`make lint-python-code`

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 **sbose78**
You can assign the PR to them by writing `/assign @sbose78` 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":["sbose78"]} -->
openshift-ci-robot: @pmacik: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/unit | 56f873d2985fae098d8975b1a8aca6eff19571c5 | [link](https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/591/pull-ci-redhat-developer-service-binding-operator-master-unit/1288771892773130240) | `/test unit`

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

<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 -->
Avni-Sharma: /test unit

Patch:
diff --git a/examples/java_postgresql_customvar/README.md b/examples/java_postgresql_customvar/README.md
index 43f3d4b6..aadb27cc 100644
--- a/examples/java_postgresql_customvar/README.md
+++ b/examples/java_postgresql_customvar/README.md
@@ -202,11 +202,11 @@ spec:
     id: postgresDB
   customEnvVar:
     - name: JDBC_URL
-      value: 'jdbc:postgresql://{{ postgresDB.status.dbConnectionIP }}:{{ .postgresDB.status.dbConnectionPort }}/{{ postgresDB.status.dbName }}'
+      value: 'jdbc:postgresql://{{ .postgresDB.status.dbConnectionIP }}:{{ .postgresDB.status.dbConnectionPort }}/{{ .postgresDB.status.dbName }}'
     - name: DB_USER
-      value: '{{ postgresDB.status.dbConfigMap.username }}'
+      value: '{{ .postgresDB.status.dbCredentials.user }}'
     - name: DB_PASSWORD
-      value: '{{ postgresDB.status.dbConfigMap.password }}'
+      value: '{{ .postgresDB.status.dbCredentials.password }}'
 EOS
 ```
 
diff --git a/examples/java_postgresql_customvar/service-binding-request.yaml b/examples/java_postgresql_customvar/service-binding-request.yaml
index 27457bd3..8344dfaf 100644
--- a/examples/java_postgresql_customvar/service-binding-request.yaml
+++ b/examples/java_postgresql_customvar/service-binding-request.yaml
@@ -18,8 +18,8 @@ spec:
     id: postgresDB
   customEnvVar:
     - name: JDBC_URL
-      value: 'jdbc:postgresql://{{ postgresDB.status.dbConnectionIP }}:{{ .postgresDB.status.dbConnectionPort }}/{{ postgresDB.status.dbName }}'
+      value: 'jdbc:postgresql://{{ .postgresDB.status.dbConnectionIP }}:{{ .postgresDB.status.dbConnectionPort }}/{{ .postgresDB.status.dbName }}'
     - name: DB_USER
-      value: '{{ postgresDB.status.dbConfigMap.username }}'
+      value: '{{ .postgresDB.status.dbCredentials.user }}'
     - name: DB_PASSWORD
-      value: '{{ postgresDB.status.dbConfigMap.password }}'
+      value: '{{ .postgresDB.status.dbCredentials.password }}'
diff --git a/examples/knative_postgresql_customvar/README.md b/examples/knative_postgresql_customvar/README.md
index 7e0253a4..cbf2e1a6 100644
--- a/examples/knative_postgresql_customvar/README.md
+++ b/examples/knative_postgresql_customvar/README.md
@@ -205,11 +205,12 @@ spec:
     resourceRef: db-demo
   customEnvVar:
     - name: JDBC_URL
-      value: 'jdbc:postgresql://{{ postgresDB.status.dbConnectionIP }}:{{ .postgresDB.status.dbConnectionPort }}/{{ postgresDB.status.dbName }}'
+      value: 'jdbc:postgresql://{{ .postgresDB.status.dbConnectionIP }}:{{ .postgresDB.status.dbConnectionPort }}/{{ .postgresDB.status.dbName }}'
     - name: DB_USER
-      value: '{{ postgresDB.status.dbConfigMap.username }}'
+      value: '{{ .postgresDB.status.dbCredentials.user }}'
     - name: DB_PASSWORD
-      value: '{{ postgresDB.status.dbConfigMap.password }}'
+      value: '{{ .postgresDB.status.dbCredentials.password }}'
+
 EOS
 ```
 
diff --git a/examples/knative_postgresql_customvar/service-binding-request.yaml b/examples/knative_postgresql_customvar/service-binding-request.yaml
index 6b27e6f6..17c7f38e 100644
--- a/examples/knative_postgresql_customvar/service-binding-request.yaml
+++ b/examples/knative_postgresql_customvar/service-binding-request.yaml
@@ -18,8 +18,8 @@ spec:
     id: postgresDB
   customEnvVar:
     - name: JDBC_URL
-      value: 'jdbc:postgresql://{{ postgresDB.status.dbConnectionIP }}:{{ .postgresDB.status.dbConnectionPort }}/{{ postgresDB.status.dbName }}'
+      value: 'jdbc:postgresql://{{ .postgresDB.status.dbConnectionIP }}:{{ .postgresDB.status.dbConnectionPort }}/{{ .postgresDB.status.dbName }}'
     - name: DB_USER
-      value: '{{ postgresDB.status.dbConfigMap.username }}'
+      value: '{{ .postgresDB.status.dbCredentials.user }}'
     - name: DB_PASSWORD
-      value: '{{ postgresDB.status.dbConfigMap.password }}'
+      value: '{{ .postgresDB.status.dbCredentials.password }}'
diff --git a/hack/check-python/detect-dead-code.sh b/hack/check-python/detect-dead-code.sh
index 2e409a7c..a9fe9d9f 100755
--- a/hack/check-python/detect-dead-code.sh
+++ b/hack/check-python/detect-dead-code.sh
@@ -36,7 +36,7 @@ echo
 # checks for the whole directories
 for directory in $directories
 do
-    files=$(find "$directory" -prune -o -name '*.py' -print)
+    files=$(find "$directory" -path "$PYTHON_VENV_DIR" -prune -o -name '*.py' -print)
 
     check_files "$files"
 done
diff --git a/pkg/controller/servicebindingrequest/annotations/attribute.go b/pkg/controller/servicebindingrequest/annotations/attribute.go
index 2d1a27ee..40a63612 100644
--- a/pkg/controller/servicebindingrequest/annotations/attribute.go
+++ b/pkg/controller/servicebindingrequest/annotations/attribute.go
@@ -47,6 +47,10 @@ func newAttributeHandler(
 	if len(bi.ResourceReferencePath) > 0 {
 		outputPath = bi.ResourceReferencePath
 	}
+	inputPath := bi.SourcePath
+	if inputPath == "" {
+		inputPath = bi.ResourceReferencePath
+	}
 
 	// the current implementation removes "status." and "spec." from fields exported through
 	// annotations.
@@ -57,7 +61,7 @@ func newAttributeHandler(
 	}
 
 	return &attributeHandler{
-		inputPath:  bi.SourcePath,
+		inputPath:  inputPath,
 		outputPath: outputPath,
 		resource:   resource,
 	}
diff --git a/pkg/controller/servicebindingrequest/annotations/bindinginfo.go b/pkg/controller/servicebindingrequest/annotations/bindinginfo.go
index 0617dfc8..970db4e2 100644
--- a/pkg/controller/servicebindingrequest/annotations/bindinginfo.go
+++ b/pkg/controller/servicebindingrequest/annotations/bindinginfo.go
@@ -38,21 +38,24 @@ func NewBindingInfo(name string, value string) (*bindingInfo, error) {
 	if len(cleanName) == 0 {
 		return nil, ErrEmptyAnnotationName
 	}
-
 	parts := strings.SplitN(cleanName, "-", 2)
 
 	resourceReferencePath := parts[0]
-	sourcePath := parts[0]
-
+	// sourcePath := parts[0]
+	sourcePath := ""
+	descriptor := ""
 	// the annotation is a reference to another manifest
 	if len(parts) == 2 {
 		sourcePath = parts[1]
+		descriptor = strings.Join([]string{value, sourcePath}, ":")
+	} else {
+		descriptor = strings.Join([]string{value, resourceReferencePath}, ":")
 	}
 
 	return &bindingInfo{
 		ResourceReferencePath: resourceReferencePath,
 		SourcePath:            sourcePath,
-		Descriptor:            strings.Join([]string{value, sourcePath}, ":"),
+		Descriptor:            descriptor,
 		Value:                 value,
 	}, nil
 }
diff --git a/pkg/controller/servicebindingrequest/annotations/resource.go b/pkg/controller/servicebindingrequest/annotations/resource.go
index ec309570..3438a6c6 100644
--- a/pkg/controller/servicebindingrequest/annotations/resource.go
+++ b/pkg/controller/servicebindingrequest/annotations/resource.go
@@ -85,7 +85,7 @@ func discoverBindingType(val string) (bindingType, error) {
 // the resulting slice.
 func getInputPathFields(bi *bindingInfo, inputPathPrefix *string) []string {
 	inputPathFields := []string{}
-	if bi.ResourceReferencePath != bi.SourcePath {
+	if bi.SourcePath != "" {
 		inputPathFields = append(inputPathFields, bi.SourcePath)
 	}
 	if inputPathPrefix != nil && len(*inputPathPrefix) > 0 {
@@ -105,7 +105,6 @@ func (h *resourceHandler) Handle() (result, error) {
 	if err != nil {
 		return result{}, err
 	}
-
 	inputPathFields := getInputPathFields(h.bindingInfo, h.inputPathRoot)
 	val, ok, err := unstructured.NestedFieldCopy(resource.Object, inputPathFields...)
 	if !ok {
@@ -121,8 +120,6 @@ func (h *resourceHandler) Handle() (result, error) {
 		return result{}, err
 	}
 
-	outputPathParts := []string{strings.ToLower(gvk.Kind)}
-
 	if mapVal, ok := val.(map[string]interface{}); ok {
 		tmpVal := make(map[string]interface{})
 		for k, v := range mapVal {
@@ -138,7 +135,6 @@ func (h *resourceHandler) Handle() (result, error) {
 		if err != nil {
 			return result{}, err
 		}
-		outputPathParts = append(outputPathParts, nested.NewPath(h.bindingInfo.SourcePath).GetParts()...)
 	}
 
 	typ, err := discoverBindingType(h.bindingInfo.Value)
@@ -147,18 +143,27 @@ func (h *resourceHandler) Handle() (result, error) {
 	}
 
 	// prefix the output path with the kind of the resource.
-	outputPath := strings.Join(outputPathParts, ".")
+	outputPath := ""
+	rawDataPath := ""
 
-	rawDataPath := strings.Join([]string{
-		h.bindingInfo.ResourceReferencePath,
-		h.bindingInfo.SourcePath,
-	}, ".")
+	if h.bindingInfo.SourcePath == "" {
+		outputPath = strings.ToLower(gvk.Kind)
+		rawDataPath = h.bindingInfo.ResourceReferencePath
+	} else {
+		outputPath = strings.Join([]string{
+			strings.ToLower(gvk.Kind),
+			h.bindingInfo.SourcePath,
+		}, ".")
+		rawDataPath = strings.Join([]string{
+			h.bindingInfo.ResourceReferencePath,
+			h.bindingInfo.SourcePath,
+		}, ".")
+	}
 
 	return result{
-		Data: nested.ComposeValue(val, nested.NewPath(outputPath)),
-		Type: typ,
-		Path: outputPath,
-
+		Data:    nested.ComposeValue(val, nested.NewPath(outputPath)),
+		Type:    typ,
+		Path:    outputPath,
 		RawData: nested.ComposeValue(val, nested.NewPath(rawDataPath)),
 	}, nil
 }
@@ -189,10 +194,6 @@ func NewResourceHandler(
 		return nil, invalidArgumentErr("bi")
 	}
 
-	if len(bi.SourcePath) == 0 {
-		return nil, invalidArgumentErr("bi.Path")
-	}
-
 	if len(bi.ResourceReferencePath) == 0 {
 		return nil, invalidArgumentErr("bi.ResourceReferencePath")
 	}
diff --git a/pkg/controller/servicebindingrequest/nested/path.go b/pkg/controller/servicebindingrequest/nested/path.go
index ba9d2501..3bdb7ca2 100644
--- a/pkg/controller/servicebindingrequest/nested/path.go
+++ b/pkg/controller/servicebindingrequest/nested/path.go
@@ -129,6 +129,9 @@ func (p path) clean() path {
 
 // NewPath creates a new path with the given string in the format 'a.b.c'.
 func NewPath(s string) path {
+	if s == "" {
+		return path{}
+	}
 	parts := strings.Split(s, ".")
 	return newPathWithParts(parts)
 }


Test patch:
diff --git a/pkg/controller/servicebindingrequest/annotations/bindinginfo_test.go b/pkg/controller/servicebindingrequest/annotations/bindinginfo_test.go
index 98dbe1ee..05f88213 100644
--- a/pkg/controller/servicebindingrequest/annotations/bindinginfo_test.go
+++ b/pkg/controller/servicebindingrequest/annotations/bindinginfo_test.go
@@ -39,7 +39,7 @@ func TestNewBindingInfo(t *testing.T) {
 			want: &bindingInfo{
 				Descriptor:            "binding:status.connectionString",
 				ResourceReferencePath: "status.connectionString",
-				SourcePath:            "status.connectionString",
+				SourcePath:            "",
 				Value:                 "binding",
 			},
 			name:    "{path} annotation",
diff --git a/pkg/controller/servicebindingrequest/annotations/configmap_test.go b/pkg/controller/servicebindingrequest/annotations/configmap_test.go
index d717f805..34b82f33 100644
--- a/pkg/controller/servicebindingrequest/annotations/configmap_test.go
+++ b/pkg/controller/servicebindingrequest/annotations/configmap_test.go
@@ -16,11 +16,12 @@ import (
 
 func TestConfigMapHandler(t *testing.T) {
 	type args struct {
-		name      string
-		value     string
-		service   map[string]interface{}
-		resources []runtime.Object
-		expected  map[string]interface{}
+		name            string
+		value           string
+		service         map[string]interface{}
+		resources       []runtime.Object
+		expectedData    map[string]interface{}
+		expectedRawData map[string]interface{}
 	}
 
 	assertHandler := func(args args) func(*testing.T) {
@@ -45,7 +46,8 @@ func TestConfigMapHandler(t *testing.T) {
 			got, err := handler.Handle()
 			require.NoError(t, err)
 			require.NotNil(t, got)
-			require.Equal(t, args.expected, got.Data)
+			require.Equal(t, args.expectedData, got.Data)
+			require.Equal(t, args.expectedRawData, got.RawData)
 		}
 	}
 
@@ -75,11 +77,18 @@ func TestConfigMapHandler(t *testing.T) {
 				},
 			},
 		},
-		expected: map[string]interface{}{
+		expectedData: map[string]interface{}{
 			"configmap": map[string]interface{}{
 				"password": "hunter2",
 			},
 		},
+		expectedRawData: map[string]interface{}{
+			"status": map[string]interface{}{
+				"dbCredentials": map[string]interface{}{
+					"password": "hunter2",
+				},
+			},
+		},
 	}))
 
 	t.Run("configmap/map", assertHandler(args{
@@ -108,11 +117,19 @@ func TestConfigMapHandler(t *testing.T) {
 				},
 			},
 		},
-		expected: map[string]interface{}{
+		expectedData: map[string]interface{}{
 			"configmap": map[string]interface{}{
 				"username": "AzureDiamond",
 				"password": "hunter2",
 			},
 		},
+		expectedRawData: map[string]interface{}{
+			"status": map[string]interface{}{
+				"dbCredentials": map[string]interface{}{
+					"username": "AzureDiamond",
+					"password": "hunter2",
+				},
+			},
+		},
 	}))
 }
diff --git a/pkg/controller/servicebindingrequest/annotations/secret_test.go b/pkg/controller/servicebindingrequest/annotations/secret_test.go
index f8b5d596..e6b0d2bd 100644
--- a/pkg/controller/servicebindingrequest/annotations/secret_test.go
+++ b/pkg/controller/servicebindingrequest/annotations/secret_test.go
@@ -16,11 +16,12 @@ import (
 
 func TestSecretHandler(t *testing.T) {
 	type args struct {
-		name      string
-		value     string
-		service   map[string]interface{}
-		resources []runtime.Object
-		expected  map[string]interface{}
+		name            string
+		value           string
+		service         map[string]interface{}
+		resources       []runtime.Object
+		expectedData    map[string]interface{}
+		expectedRawData map[string]interface{}
 	}
 
 	assertHandler := func(args args) func(*testing.T) {
@@ -45,7 +46,8 @@ func TestSecretHandler(t *testing.T) {
 			got, err := handler.Handle()
 			require.NoError(t, err)
 			require.NotNil(t, got)
-			require.Equal(t, args.expected, got.Data)
+			require.Equal(t, args.expectedData, got.Data)
+			require.Equal(t, args.expectedRawData, got.RawData)
 		}
 	}
 
@@ -74,11 +76,18 @@ func TestSecretHandler(t *testing.T) {
 				},
 			},
 		},
-		expected: map[string]interface{}{
+		expectedData: map[string]interface{}{
 			"secret": map[string]interface{}{
 				"password": "hunter2",
 			},
 		},
+		expectedRawData: map[string]interface{}{
+			"status": map[string]interface{}{
+				"dbCredentials": map[string]interface{}{
+					"password": "hunter2",
+				},
+			},
+		},
 	}))
 
 	t.Run("secret/map", assertHandler(args{
@@ -107,11 +116,19 @@ func TestSecretHandler(t *testing.T) {
 				},
 			},
 		},
-		expected: map[string]interface{}{
+		expectedData: map[string]interface{}{
 			"secret": map[string]interface{}{
 				"username": "AzureDiamond",
 				"password": "hunter2",
 			},
 		},
+		expectedRawData: map[string]interface{}{
+			"status": map[string]interface{}{
+				"dbCredentials": map[string]interface{}{
+					"username": "AzureDiamond",
+					"password": "hunter2",
+				},
+			},
+		},
 	}))
 }