Repository: redhat-developer/service-binding-operator
Pull request #273 (2019-12-09 08:32:07)

Use corev1.EnvVar instead of CustomEnvMap

### Motivation

Fixes https://jira.coreos.com/browse/APPSVC-325

### Changes

The change should be backward compatible

### Testing

Existing tests should run


For further more details refer the CONTRIBUTING.md

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"]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/273?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 `100%`.

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

```diff
@@            Coverage Diff            @@
##             master     #273   +/-   ##
=========================================
  Coverage          ?   58.71%           
=========================================
  Files             ?       16           
  Lines             ?     1279           
  Branches          ?        0           
=========================================
  Hits              ?      751           
  Misses            ?      393           
  Partials          ?      135
```


| [Impacted Files](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/273?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...troller/servicebindingrequest/custom\_env\_parser.go](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/273/diff?src=pr&el=tree#diff-cGtnL2NvbnRyb2xsZXIvc2VydmljZWJpbmRpbmdyZXF1ZXN0L2N1c3RvbV9lbnZfcGFyc2VyLmdv) | `75% <100%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/273?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/273?src=pr&el=footer). Last update [b0c9641...566205b](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/273?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

isutton: > `make build` on an clean tree results in the following:
> 
> ```
> GOARCH=amd64 GOOS=linux go build  -o ./out/operator cmd/manager/main.go
> verifying github.com/operator-framework/operator-sdk@v0.12.0: checksum mismatch
>         downloaded: h1:aD4qPbSAbZgRj1jFdFLq/dBI4P4aKX8d4rJowyQtTYM=
>         go.sum:     h1:9eAD1L8e6pPCpFCAacBUVf2eloDkRuVm29GTCOktLqQ=
> 
> SECURITY ERROR
> This download does NOT match an earlier download recorded in go.sum.
> The bits may have been replaced on the origin server, or an attacker may
> have intercepted the download attempt.
> 
> For more information, see 'go help module-auth'.
> make: *** [Makefile:283: out/operator] Error 1
> ```
> 
> Removing the offending line and executing `make build` update `go.sum` with the correct checksum, so either something is missing from your side or we need to verify the reason we have different checksums.

Please disregard this comment, I found an issue in my environment that didn't make it clear at the time I wrote this, although the inner comment about GO111MODULE and the path being used in the generated file still applies though.


Avni-Sharma: /lgtm
akashshinde: /lgtm

Patch:
diff --git a/build/README.md b/build/README.md
new file mode 100644
index 00000000..643031b7
--- /dev/null
+++ b/build/README.md
@@ -0,0 +1,1 @@
+The current directory(build/) is used by `operator-sdk up local` for running the operator locally.
\ No newline at end of file
diff --git a/pkg/apis/apps/v1alpha1/servicebindingrequest_types.go b/pkg/apis/apps/v1alpha1/servicebindingrequest_types.go
index a0021ff4..c6c11643 100644
--- a/pkg/apis/apps/v1alpha1/servicebindingrequest_types.go
+++ b/pkg/apis/apps/v1alpha1/servicebindingrequest_types.go
@@ -1,6 +1,7 @@
 package v1alpha1
 
 import (
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -22,7 +23,7 @@ type ServiceBindingRequestSpec struct {
 
 	// Custom env variables
 	// +optional
-	CustomEnvVar []CustomEnvMap `json:"customEnvVar"`
+	CustomEnvVar []corev1.EnvVar `json:"customEnvVar"`
 
 	// BackingServiceSelector is used to identify the backing service operator.
 	BackingServiceSelector BackingServiceSelector `json:"backingServiceSelector"`
@@ -37,16 +38,6 @@ type ServiceBindingRequestSpec struct {
 	DetectBindingResources bool `json:"detectBindingResources"`
 }
 
-// CustomEnvMap is a set of Name and Value of an environment variable
-// +k8s:openapi-gen=true
-// +optional
-type CustomEnvMap struct {
-	// key of custom env variable
-	Name string `json:"name"`
-	// value template, currently supports expected is go template
-	Value string `json:"value"`
-}
-
 // ServiceBindingRequestStatus defines the observed state of ServiceBindingRequest
 // +k8s:openapi-gen=true
 type ServiceBindingRequestStatus struct {
diff --git a/pkg/apis/apps/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/apps/v1alpha1/zz_generated.deepcopy.go
index 535d041b..8086e9d2 100644
--- a/pkg/apis/apps/v1alpha1/zz_generated.deepcopy.go
+++ b/pkg/apis/apps/v1alpha1/zz_generated.deepcopy.go
@@ -5,6 +5,7 @@
 package v1alpha1
 
 import (
+	v1 "k8s.io/api/core/v1"
 	runtime "k8s.io/apimachinery/pkg/runtime"
 )
 
@@ -47,22 +48,6 @@ func (in *BackingServiceSelector) DeepCopy() *BackingServiceSelector {
 	return out
 }
 
-// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
-func (in *CustomEnvMap) DeepCopyInto(out *CustomEnvMap) {
-	*out = *in
-	return
-}
-
-// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CustomEnvMap.
-func (in *CustomEnvMap) DeepCopy() *CustomEnvMap {
-	if in == nil {
-		return nil
-	}
-	out := new(CustomEnvMap)
-	in.DeepCopyInto(out)
-	return out
-}
-
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ServiceBindingRequest) DeepCopyInto(out *ServiceBindingRequest) {
 	*out = *in
@@ -95,7 +80,7 @@ func (in *ServiceBindingRequest) DeepCopyObject() runtime.Object {
 func (in *ServiceBindingRequestList) DeepCopyInto(out *ServiceBindingRequestList) {
 	*out = *in
 	out.TypeMeta = in.TypeMeta
-	out.ListMeta = in.ListMeta
+	in.ListMeta.DeepCopyInto(&out.ListMeta)
 	if in.Items != nil {
 		in, out := &in.Items, &out.Items
 		*out = make([]ServiceBindingRequest, len(*in))
@@ -129,8 +114,10 @@ func (in *ServiceBindingRequestSpec) DeepCopyInto(out *ServiceBindingRequestSpec
 	*out = *in
 	if in.CustomEnvVar != nil {
 		in, out := &in.CustomEnvVar, &out.CustomEnvVar
-		*out = make([]CustomEnvMap, len(*in))
-		copy(*out, *in)
+		*out = make([]v1.EnvVar, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
 	}
 	out.BackingServiceSelector = in.BackingServiceSelector
 	in.ApplicationSelector.DeepCopyInto(&out.ApplicationSelector)
diff --git a/pkg/apis/apps/v1alpha1/zz_generated.openapi.go b/pkg/apis/apps/v1alpha1/zz_generated.openapi.go
index c21649a3..19d74f42 100644
--- a/pkg/apis/apps/v1alpha1/zz_generated.openapi.go
+++ b/pkg/apis/apps/v1alpha1/zz_generated.openapi.go
@@ -11,12 +11,11 @@ import (
 
 func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition {
 	return map[string]common.OpenAPIDefinition{
-		"./pkg/apis/apps/v1alpha1.ApplicationSelector":         schema_pkg_apis_apps_v1alpha1_ApplicationSelector(ref),
-		"./pkg/apis/apps/v1alpha1.BackingServiceSelector":      schema_pkg_apis_apps_v1alpha1_BackingServiceSelector(ref),
-		"./pkg/apis/apps/v1alpha1.CustomEnvMap":                schema_pkg_apis_apps_v1alpha1_CustomEnvMap(ref),
-		"./pkg/apis/apps/v1alpha1.ServiceBindingRequest":       schema_pkg_apis_apps_v1alpha1_ServiceBindingRequest(ref),
-		"./pkg/apis/apps/v1alpha1.ServiceBindingRequestSpec":   schema_pkg_apis_apps_v1alpha1_ServiceBindingRequestSpec(ref),
-		"./pkg/apis/apps/v1alpha1.ServiceBindingRequestStatus": schema_pkg_apis_apps_v1alpha1_ServiceBindingRequestStatus(ref),
+		"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1.ApplicationSelector":         schema_pkg_apis_apps_v1alpha1_ApplicationSelector(ref),
+		"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1.BackingServiceSelector":      schema_pkg_apis_apps_v1alpha1_BackingServiceSelector(ref),
+		"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1.ServiceBindingRequest":       schema_pkg_apis_apps_v1alpha1_ServiceBindingRequest(ref),
+		"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1.ServiceBindingRequestSpec":   schema_pkg_apis_apps_v1alpha1_ServiceBindingRequestSpec(ref),
+		"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1.ServiceBindingRequestStatus": schema_pkg_apis_apps_v1alpha1_ServiceBindingRequestStatus(ref),
 	}
 }
 
@@ -25,11 +24,13 @@ func schema_pkg_apis_apps_v1alpha1_ApplicationSelector(ref common.ReferenceCallb
 		Schema: spec.Schema{
 			SchemaProps: spec.SchemaProps{
 				Description: "ApplicationSelector defines the selector based on labels and GVR",
+				Type:        []string{"object"},
 				Properties: map[string]spec.Schema{
 					"matchLabels": {
 						SchemaProps: spec.SchemaProps{
 							Type: []string{"object"},
 							AdditionalProperties: &spec.SchemaOrBool{
+								Allows: true,
 								Schema: &spec.Schema{
 									SchemaProps: spec.SchemaProps{
 										Type:   []string{"string"},
@@ -67,7 +68,6 @@ func schema_pkg_apis_apps_v1alpha1_ApplicationSelector(ref common.ReferenceCallb
 				Required: []string{"matchLabels", "version", "resource", "resourceRef"},
 			},
 		},
-		Dependencies: []string{},
 	}
 }
 
@@ -76,6 +76,7 @@ func schema_pkg_apis_apps_v1alpha1_BackingServiceSelector(ref common.ReferenceCa
 		Schema: spec.Schema{
 			SchemaProps: spec.SchemaProps{
 				Description: "BackingServiceSelector defines the selector based on resource name, version, and resource kind",
+				Type:        []string{"object"},
 				Properties: map[string]spec.Schema{
 					"group": {
 						SchemaProps: spec.SchemaProps{
@@ -105,35 +106,6 @@ func schema_pkg_apis_apps_v1alpha1_BackingServiceSelector(ref common.ReferenceCa
 				Required: []string{"group", "version", "kind", "resourceRef"},
 			},
 		},
-		Dependencies: []string{},
-	}
-}
-
-func schema_pkg_apis_apps_v1alpha1_CustomEnvMap(ref common.ReferenceCallback) common.OpenAPIDefinition {
-	return common.OpenAPIDefinition{
-		Schema: spec.Schema{
-			SchemaProps: spec.SchemaProps{
-				Description: "CustomEnvMap is a set of Name and Value of an environment variable",
-				Properties: map[string]spec.Schema{
-					"name": {
-						SchemaProps: spec.SchemaProps{
-							Description: "key of custom env variable",
-							Type:        []string{"string"},
-							Format:      "",
-						},
-					},
-					"value": {
-						SchemaProps: spec.SchemaProps{
-							Description: "value template, currently supports expected is go template",
-							Type:        []string{"string"},
-							Format:      "",
-						},
-					},
-				},
-				Required: []string{"name", "value"},
-			},
-		},
-		Dependencies: []string{},
 	}
 }
 
@@ -142,6 +114,7 @@ func schema_pkg_apis_apps_v1alpha1_ServiceBindingRequest(ref common.ReferenceCal
 		Schema: spec.Schema{
 			SchemaProps: spec.SchemaProps{
 				Description: "ServiceBindingRequest is the Schema for the servicebindings API",
+				Type:        []string{"object"},
 				Properties: map[string]spec.Schema{
 					"kind": {
 						SchemaProps: spec.SchemaProps{
@@ -164,19 +137,19 @@ func schema_pkg_apis_apps_v1alpha1_ServiceBindingRequest(ref common.ReferenceCal
 					},
 					"spec": {
 						SchemaProps: spec.SchemaProps{
-							Ref: ref("./pkg/apis/apps/v1alpha1.ServiceBindingRequestSpec"),
+							Ref: ref("github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1.ServiceBindingRequestSpec"),
 						},
 					},
 					"status": {
 						SchemaProps: spec.SchemaProps{
-							Ref: ref("./pkg/apis/apps/v1alpha1.ServiceBindingRequestStatus"),
+							Ref: ref("github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1.ServiceBindingRequestStatus"),
 						},
 					},
 				},
 			},
 		},
 		Dependencies: []string{
-			"./pkg/apis/apps/v1alpha1.ServiceBindingRequestSpec", "./pkg/apis/apps/v1alpha1.ServiceBindingRequestStatus", "k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta"},
+			"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1.ServiceBindingRequestSpec", "github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1.ServiceBindingRequestStatus", "k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta"},
 	}
 }
 
@@ -185,6 +158,7 @@ func schema_pkg_apis_apps_v1alpha1_ServiceBindingRequestSpec(ref common.Referenc
 		Schema: spec.Schema{
 			SchemaProps: spec.SchemaProps{
 				Description: "ServiceBindingRequestSpec defines the desired state of ServiceBindingRequest",
+				Type:        []string{"object"},
 				Properties: map[string]spec.Schema{
 					"mountPathPrefix": {
 						SchemaProps: spec.SchemaProps{
@@ -207,7 +181,7 @@ func schema_pkg_apis_apps_v1alpha1_ServiceBindingRequestSpec(ref common.Referenc
 							Items: &spec.SchemaOrArray{
 								Schema: &spec.Schema{
 									SchemaProps: spec.SchemaProps{
-										Ref: ref("./pkg/apis/apps/v1alpha1.CustomEnvMap"),
+										Ref: ref("k8s.io/api/core/v1.EnvVar"),
 									},
 								},
 							},
@@ -216,13 +190,13 @@ func schema_pkg_apis_apps_v1alpha1_ServiceBindingRequestSpec(ref common.Referenc
 					"backingServiceSelector": {
 						SchemaProps: spec.SchemaProps{
 							Description: "BackingServiceSelector is used to identify the backing service operator.",
-							Ref:         ref("./pkg/apis/apps/v1alpha1.BackingServiceSelector"),
+							Ref:         ref("github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1.BackingServiceSelector"),
 						},
 					},
 					"applicationSelector": {
 						SchemaProps: spec.SchemaProps{
 							Description: "ApplicationSelector is used to identify the application connecting to the backing service operator.",
-							Ref:         ref("./pkg/apis/apps/v1alpha1.ApplicationSelector"),
+							Ref:         ref("github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1.ApplicationSelector"),
 						},
 					},
 					"detectBindingResources": {
@@ -237,7 +211,7 @@ func schema_pkg_apis_apps_v1alpha1_ServiceBindingRequestSpec(ref common.Referenc
 			},
 		},
 		Dependencies: []string{
-			"./pkg/apis/apps/v1alpha1.ApplicationSelector", "./pkg/apis/apps/v1alpha1.BackingServiceSelector", "./pkg/apis/apps/v1alpha1.CustomEnvMap"},
+			"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1.ApplicationSelector", "github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1.BackingServiceSelector", "k8s.io/api/core/v1.EnvVar"},
 	}
 }
 
@@ -246,6 +220,7 @@ func schema_pkg_apis_apps_v1alpha1_ServiceBindingRequestStatus(ref common.Refere
 		Schema: spec.Schema{
 			SchemaProps: spec.SchemaProps{
 				Description: "ServiceBindingRequestStatus defines the observed state of ServiceBindingRequest",
+				Type:        []string{"object"},
 				Properties: map[string]spec.Schema{
 					"bindingStatus": {
 						SchemaProps: spec.SchemaProps{
@@ -278,6 +253,5 @@ func schema_pkg_apis_apps_v1alpha1_ServiceBindingRequestStatus(ref common.Refere
 				},
 			},
 		},
-		Dependencies: []string{},
 	}
 }
diff --git a/pkg/controller/servicebindingrequest/custom_env_parser.go b/pkg/controller/servicebindingrequest/custom_env_parser.go
index 0083c2b7..6f6b5b31 100644
--- a/pkg/controller/servicebindingrequest/custom_env_parser.go
+++ b/pkg/controller/servicebindingrequest/custom_env_parser.go
@@ -4,17 +4,17 @@ import (
 	"bytes"
 	"text/template"
 
-	"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1"
+	corev1 "k8s.io/api/core/v1"
 )
 
-// CustomEnvParser is responsible to interpolate a given CustomEnvMap containing templates.
+// CustomEnvParser is responsible to interpolate a given EnvVar containing templates.
 type CustomEnvParser struct {
-	EnvMap []v1alpha1.CustomEnvMap
+	EnvMap []corev1.EnvVar
 	Cache  map[string]interface{}
 }
 
 // NewCustomEnvParser returns a new CustomEnvParser.
-func NewCustomEnvParser(envMap []v1alpha1.CustomEnvMap, cache map[string]interface{}) *CustomEnvParser {
+func NewCustomEnvParser(envMap []corev1.EnvVar, cache map[string]interface{}) *CustomEnvParser {
 	return &CustomEnvParser{
 		EnvMap: envMap,
 		Cache:  cache,


Test patch:
diff --git a/pkg/controller/servicebindingrequest/custom_env_parser_test.go b/pkg/controller/servicebindingrequest/custom_env_parser_test.go
index 50f64079..522269b4 100644
--- a/pkg/controller/servicebindingrequest/custom_env_parser_test.go
+++ b/pkg/controller/servicebindingrequest/custom_env_parser_test.go
@@ -1,9 +1,10 @@
 package servicebindingrequest
 
 import (
-	"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1"
-	"github.com/stretchr/testify/require"
 	"testing"
+
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
 )
 
 func TestCustomEnvPath_Parse(t *testing.T) {
@@ -19,12 +20,12 @@ func TestCustomEnvPath_Parse(t *testing.T) {
 		},
 	}
 
-	envMap := []v1alpha1.CustomEnvMap{
-		v1alpha1.CustomEnvMap{
+	envMap := []corev1.EnvVar{
+		corev1.EnvVar{
 			Name:  "JDBC_CONNECTION_STRING",
 			Value: `{{ .spec.dbName }}:{{ .status.creds.user }}@{{ .status.creds.pass }}`,
 		},
-		v1alpha1.CustomEnvMap{
+		corev1.EnvVar{
 			Name:  "ANOTHER_STRING",
 			Value: `{{ .spec.dbName }}_{{ .status.creds.user }}`,
 		},
diff --git a/pkg/controller/servicebindingrequest/retriever_test.go b/pkg/controller/servicebindingrequest/retriever_test.go
index f11f6076..4e06058c 100644
--- a/pkg/controller/servicebindingrequest/retriever_test.go
+++ b/pkg/controller/servicebindingrequest/retriever_test.go
@@ -4,9 +4,9 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 
-	"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1"
 	"github.com/redhat-developer/service-binding-operator/test/mocks"
 )
 
@@ -227,7 +227,7 @@ func TestCustomEnvParser(t *testing.T) {
 
 		t.Logf("\nCache %+v", retriever.cache)
 
-		envMap := []v1alpha1.CustomEnvMap{
+		envMap := []corev1.EnvVar{
 			{
 				Name:  "JDBC_CONNECTION_STRING",
 				Value: `{{ .spec.imageName }}@{{ .status.dbCredentials.password }}`,
diff --git a/test/mocks/mocks.go b/test/mocks/mocks.go
index 06252498..c9b4b616 100644
--- a/test/mocks/mocks.go
+++ b/test/mocks/mocks.go
@@ -385,7 +385,7 @@ func ServiceBindingRequestMock(
 		},
 		Spec: v1alpha1.ServiceBindingRequestSpec{
 			MountPathPrefix: "/var/redhat",
-			CustomEnvVar: []v1alpha1.CustomEnvMap{
+			CustomEnvVar: []corev1.EnvVar{
 				{
 					Name:  "IMAGE_PATH",
 					Value: "spec.imagePath",