Repository: redhat-developer/service-binding-operator
Pull request #178 (2019-09-20 00:11:32)

Ensure lastbound env key is not added multiple times
Ensure the following is added only once, but updated on subsequent reconciles.

```
 spec:
      containers:
        - resources: {}
          terminationMessagePath: /dev/termination-log
          name: app2
          env:
            - name: lastbound
              value: '2019-09-19T19:57:06-04:00'
```

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"]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/178?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@5023ee4`). [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/178/graphs/tree.svg?width=650&token=CICBeVWlrC&height=150&src=pr)](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/178?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##             master     #178   +/-   ##
=========================================
  Coverage          ?   63.22%           
=========================================
  Files             ?       13           
  Lines             ?      862           
  Branches          ?        0           
=========================================
  Hits              ?      545           
  Misses            ?      241           
  Partials          ?       76
```


| [Impacted Files](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/178?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/controller/servicebindingrequest/binder.go](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/178/diff?src=pr&el=tree#diff-cGtnL2NvbnRyb2xsZXIvc2VydmljZWJpbmRpbmdyZXF1ZXN0L2JpbmRlci5nbw==) | `68.75% <100%> (ø)` | |

------

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

DhritiShikhar: /lgtm

Patch:
diff --git a/pkg/controller/servicebindingrequest/binder.go b/pkg/controller/servicebindingrequest/binder.go
index c3d5df59..98ef2142 100644
--- a/pkg/controller/servicebindingrequest/binder.go
+++ b/pkg/controller/servicebindingrequest/binder.go
@@ -191,10 +191,24 @@ func (b *Binder) updateContainers(
 }
 
 func (b *Binder) appendEnvVar(envList []corev1.EnvVar, envParam string, envValue string) []corev1.EnvVar {
-	return append(envList, corev1.EnvVar{
-		Name:  envParam,
-		Value: envValue,
-	})
+	var updatedEnvList []corev1.EnvVar
+
+	alreadyPresent := false
+	for _, env := range envList {
+		if env.Name == envParam {
+			env.Value = envValue
+			alreadyPresent = true
+		}
+		updatedEnvList = append(updatedEnvList, env)
+	}
+
+	if !alreadyPresent {
+		updatedEnvList = append(updatedEnvList, corev1.EnvVar{
+			Name:  envParam,
+			Value: envValue,
+		})
+	}
+	return updatedEnvList
 }
 
 // appendEnvFrom based on secret name and list of EnvFromSource instances, making sure secret is


Test patch:
diff --git a/pkg/controller/servicebindingrequest/binder_test.go b/pkg/controller/servicebindingrequest/binder_test.go
index da7d2905..c594a4e2 100644
--- a/pkg/controller/servicebindingrequest/binder_test.go
+++ b/pkg/controller/servicebindingrequest/binder_test.go
@@ -91,6 +91,25 @@ func TestBinderNew(t *testing.T) {
 	})
 }
 
+func TestAppendEnvVar(t *testing.T) {
+	envName := "lastbound"
+	envList := []corev1.EnvVar{
+		corev1.EnvVar{
+			Name:  envName,
+			Value: "lastboundvalue",
+		},
+	}
+
+	binder := &Binder{}
+	updatedEnvVarList := binder.appendEnvVar(envList, envName, "someothervalue")
+
+	// validate that no new key is added.
+	// the existing key should be overwritten with the new value.
+
+	require.Len(t, updatedEnvVarList, 1)
+	require.Equal(t, updatedEnvVarList[0].Value, "someothervalue")
+}
+
 func TestBinderApplicationName(t *testing.T) {
 	ns := "binder"
 	name := "service-binding-request"