Repository: redhat-developer/service-binding-operator
Pull request #1037 (2021-09-29 08:50:31)

Add optional `.spec.name` to ServiceBinding to `binding.operators.coreos.com/v1alpha1 API`
Until now, the binding name was set through `.metadata.name` of `ServiceBinding` resource,
and that name was used in constructing binding mount point, i.e. `<SERVICE_BINDING_ROOT>/<binding name>`

Since `.metadata.name` value must be unique within all `ServiceBinding` resources in a given namespace,
the introduction of `.spec.name` allows us to overwrite `.metadata.name` and have more predictable
mount points inside containers.

This work aligns `binding.operators.coreos.com/v1alpha1 API` even further to the spec API:

https://github.com/servicebinding/spec#resource-type-schema-1

openshift-ci[bot]: [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 ask for approval from **pedjak** after the PR has been reviewed.

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":["pedjak"]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/1037?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redhat-developer) Report
> Merging [#1037](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/1037?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redhat-developer) (e60ecff) into [master](https://codecov.io/gh/redhat-developer/service-binding-operator/commit/d4799a6482404e386daef21b305265bd15561b5c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redhat-developer) (d4799a6) will **increase** coverage by `0.11%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/1037/graphs/tree.svg?width=650&height=150&src=pr&token=CICBeVWlrC&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redhat-developer)](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/1037?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redhat-developer)

```diff
@@            Coverage Diff             @@
##           master    #1037      +/-   ##
==========================================
+ Coverage   57.77%   57.88%   +0.11%     
==========================================
  Files          28       28              
  Lines        1570     1572       +2     
==========================================
+ Hits          907      910       +3     
+ Misses        545      544       -1     
  Partials      118      118              
```


| [Impacted Files](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/1037?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redhat-developer) | Coverage Δ | |
|---|---|---|
| [apis/binding/v1alpha1/servicebinding\_types.go](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/1037/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redhat-developer#diff-YXBpcy9iaW5kaW5nL3YxYWxwaGExL3NlcnZpY2ViaW5kaW5nX3R5cGVzLmdv) | `35.29% <ø> (ø)` | |
| [pkg/reconcile/pipeline/context/impl.go](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/1037/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=redhat-developer#diff-cGtnL3JlY29uY2lsZS9waXBlbGluZS9jb250ZXh0L2ltcGwuZ28=) | `65.88% <100.00%> (+0.79%)` | :arrow_up: |

------

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

pmacik: /lgtm
/approve
openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/redhat-developer/service-binding-operator/pull/1037#issuecomment-930101286" title="Approved">pmacik</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)~~ [pmacik]

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/apis/binding/v1alpha1/servicebinding_types.go b/apis/binding/v1alpha1/servicebinding_types.go
index 9df862b2..7bf93c02 100644
--- a/apis/binding/v1alpha1/servicebinding_types.go
+++ b/apis/binding/v1alpha1/servicebinding_types.go
@@ -30,6 +30,11 @@ var templates = map[string]string{
 
 // ServiceBindingSpec defines the desired state of ServiceBinding.
 type ServiceBindingSpec struct {
+	// Name is the name of the service as projected into the workload container.  Defaults to .metadata.name.
+	// +kubebuilder:validation:Pattern=`^[a-z0-9\-\.]*$`
+	// +kubebuilder:validation:MaxLength=253
+	// +optional
+	Name string `json:"name,omitempty"`
 	// NamingStrategy defines custom string template for preparing binding
 	// names.  It can be set to pre-defined strategies: `none`,
 	// `lowercase`, or `uppercase`.  Otherwise, it is treated as a custom
diff --git a/config/crd/bases/binding.operators.coreos.com_servicebindings.yaml b/config/crd/bases/binding.operators.coreos.com_servicebindings.yaml
index ac232eda..f4a0187d 100644
--- a/config/crd/bases/binding.operators.coreos.com_servicebindings.yaml
+++ b/config/crd/bases/binding.operators.coreos.com_servicebindings.yaml
@@ -161,6 +161,12 @@ spec:
                   - value
                   type: object
                 type: array
+              name:
+                description: Name is the name of the service as projected into the
+                  workload container.  Defaults to .metadata.name.
+                maxLength: 253
+                pattern: ^[a-z0-9\-\.]*$
+                type: string
               namingStrategy:
                 description: 'NamingStrategy defines custom string template for preparing
                   binding names.  It can be set to pre-defined strategies: `none`,
diff --git a/pkg/reconcile/pipeline/context/impl.go b/pkg/reconcile/pipeline/context/impl.go
index 73843621..58b9f124 100644
--- a/pkg/reconcile/pipeline/context/impl.go
+++ b/pkg/reconcile/pipeline/context/impl.go
@@ -128,6 +128,9 @@ var Provider = func(client dynamic.Interface, subjectAccessReviewClient clientau
 }
 
 func (i *bindingImpl) BindingName() string {
+	if i.serviceBinding.Spec.Name != "" {
+		return i.serviceBinding.Spec.Name
+	}
 	return i.bindingMeta.GetName()
 }
 


Test patch:
diff --git a/pkg/reconcile/pipeline/context/impl_test.go b/pkg/reconcile/pipeline/context/impl_test.go
index 8ba805e0..ec399d7d 100644
--- a/pkg/reconcile/pipeline/context/impl_test.go
+++ b/pkg/reconcile/pipeline/context/impl_test.go
@@ -5,13 +5,12 @@ import (
 	"encoding/base64"
 	e "errors"
 	"fmt"
-	"github.com/redhat-developer/service-binding-operator/apis"
-	bindingapi "github.com/redhat-developer/service-binding-operator/apis/binding/v1alpha1"
-
 	"github.com/golang/mock/gomock"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/ginkgo/extensions/table"
 	. "github.com/onsi/gomega"
+	"github.com/redhat-developer/service-binding-operator/apis"
+	bindingapi "github.com/redhat-developer/service-binding-operator/apis/binding/v1alpha1"
 	"github.com/redhat-developer/service-binding-operator/pkg/converter"
 	"github.com/redhat-developer/service-binding-operator/pkg/reconcile/pipeline"
 	"github.com/redhat-developer/service-binding-operator/pkg/reconcile/pipeline/context/mocks"
@@ -424,6 +423,31 @@ var _ = Describe("Context", func() {
 			Expect(err).To(HaveOccurred())
 		})
 	})
+	Describe("Binding Name", func() {
+		var testProvider = Provider(nil, nil, nil)
+		It("should be equal on .spec.name if specified", func() {
+			ctx, _ := testProvider.Get(&bindingapi.ServiceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "sb1",
+				},
+				Spec: bindingapi.ServiceBindingSpec{
+					Name: "sb2",
+				},
+			})
+
+			Expect(ctx.BindingName()).To(Equal("sb2"))
+		})
+
+		It("should be equal on .name if .spec.name not specified", func() {
+			ctx, _ := testProvider.Get(&bindingapi.ServiceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "sb1",
+				},
+			})
+
+			Expect(ctx.BindingName()).To(Equal("sb1"))
+		})
+	})
 
 	Describe("Binding Secret Name", func() {
 		var testProvider = Provider(nil, nil, nil)
diff --git a/test/acceptance/features/bindAppToProvisionedService.feature b/test/acceptance/features/bindAppToProvisionedService.feature
index c0b59997..543cbb9e 100644
--- a/test/acceptance/features/bindAppToProvisionedService.feature
+++ b/test/acceptance/features/bindAppToProvisionedService.feature
@@ -345,6 +345,40 @@ Feature: Bind application to provisioned service
             db
             """
 
+  Scenario: Bind application to provisioned service and inject binding into folder specified by .spec.name
+    Given Generic test application is running
+    When Service Binding is applied
+          """
+          apiVersion: binding.operators.coreos.com/v1alpha1
+          kind: ServiceBinding
+          metadata:
+              name: $scenario_id
+          spec:
+              name: foo
+              services:
+                - group: stable.example.com
+                  version: v1
+                  kind: ProvisionedBackend
+                  name: provisioned-service-2
+              application:
+                name: $scenario_id
+                group: apps
+                version: v1
+                kind: Deployment
+          """
+    Then Service Binding is ready
+    And Content of file "/bindings/foo/username" in application pod is
+            """
+            foo
+            """
+    And Content of file "/bindings/foo/password" in application pod is
+            """
+            bar
+            """
+    And Content of file "/bindings/foo/type" in application pod is
+            """
+            db
+            """
 
 
   @spec