Fix support for label selectors

- Fix `.spec.application.name` field becoming mandatory
- If more than one application is selected through label selectors, all
  will be considered for binding

Fixes #965

Signed-off-by: Baiju Muthukadan <baiju.m.mail@gmail.com>

pkg/reconcile/pipeline/context/impl.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"crypto/sha1"
 	"encoding/hex"
+	"sort"
+
 	"github.com/redhat-developer/service-binding-operator/api/v1alpha1"
 	"github.com/redhat-developer/service-binding-operator/pkg/client/kubernetes"
 	"github.com/redhat-developer/service-binding-operator/pkg/converter"
@@ -13,9 +15,9 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
-	"sort"
 )
 
 var _ pipeline.Context = &impl{}
@@ -92,7 +94,7 @@ func (i *impl) Mappings() map[string]string {
 func (i *impl) Services() ([]pipeline.Service, error) {
 	if i.services == nil {
 		serviceRefs := i.serviceBinding.Spec.Services
-		for idx := 0; idx<len(serviceRefs); idx++ {
+		for idx := 0; idx < len(serviceRefs); idx++ {
 			serviceRef := serviceRefs[idx]
 			gvr, err := i.typeLookup.ResourceForReferable(&serviceRef)
 			if err != nil {
@@ -109,7 +111,7 @@ func (i *impl) Services() ([]pipeline.Service, error) {
 		}
 	}
 	services := make([]pipeline.Service, len(i.services))
-	for idx := 0; idx<len(i.services); idx++ {
+	for idx := 0; idx < len(i.services); idx++ {
 		services[idx] = i.services[idx]
 	}
 	return services, nil
@@ -122,11 +124,33 @@ func (i *impl) Applications() ([]pipeline.Application, error) {
 			if err != nil {
 				return nil, err
 			}
-			u, err := i.client.Resource(*gvr).Namespace(i.serviceBinding.Namespace).Get(context.Background(), ref.Name, metav1.GetOptions{})
-			if err != nil {
-				return nil, err
+			if i.serviceBinding.Spec.Application.Name != "" {
+				u, err := i.client.Resource(*gvr).Namespace(i.serviceBinding.Namespace).Get(context.Background(), ref.Name, metav1.GetOptions{})
+				if err != nil {
+					return nil, err
+				}
+				i.applications = append(i.applications, &application{gvr: gvr, persistedResource: u, bindingPath: i.serviceBinding.Spec.Application.BindingPath})
+			}
+			if i.serviceBinding.Spec.Application.LabelSelector != nil && i.serviceBinding.Spec.Application.LabelSelector.MatchLabels != nil {
+				matchLabels := i.serviceBinding.Spec.Application.LabelSelector.MatchLabels
+				opts := metav1.ListOptions{
+					LabelSelector: labels.Set(matchLabels).String(),
+				}
+
+				emptyResult := make([]pipeline.Application, 0)
+				objList, err := i.client.Resource(*gvr).Namespace(i.serviceBinding.Namespace).List(context.Background(), opts)
+				if err != nil {
+					return emptyResult, err
+				}
+
+				if len(objList.Items) == 0 {
+					return emptyResult, nil
+				}
+
+				for index := range objList.Items {
+					i.applications = append(i.applications, &application{gvr: gvr, persistedResource: &(objList.Items[index]), bindingPath: i.serviceBinding.Spec.Application.BindingPath})
+				}
 			}
-			i.applications = append(i.applications, &application{gvr: gvr, persistedResource: u, bindingPath: i.serviceBinding.Spec.Application.BindingPath})
 		} else {
 			i.applications = make([]*application, 0)
 		}

pkg/reconcile/pipeline/context/impl_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	e "errors"
 	"fmt"
+
 	"github.com/golang/mock/gomock"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/ginkgo/extensions/table"
@@ -21,6 +22,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/dynamic/fake"
+	"k8s.io/client-go/testing"
 )
 
 var _ = Describe("Context", func() {
@@ -91,7 +93,146 @@ var _ = Describe("Context", func() {
 			Entry("no binding path specified", nil, defaultContainerPath),
 			Entry("binding path specified", &v1alpha1.BindingPath{ContainersPath: "foo.bar"}, "foo.bar"),
 		)
+		DescribeTable("should return slice of size 2 if 2 applications are specified through label seclector", func(bindingPath *v1alpha1.BindingPath, expectedContainerPath string) {
+			ls := &metav1.LabelSelector{
+				MatchLabels: map[string]string{"env": "prod"},
+			}
+
+			ref := v1alpha1.Application{
+				Ref: v1alpha1.Ref{
+					Group:   "app",
+					Version: "v1",
+					Kind:    "Foo",
+				},
+				LabelSelector: ls,
+				BindingPath:   bindingPath,
+			}
+
+			sb := v1alpha1.ServiceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "sb1",
+					Namespace: "ns1",
+				},
+				Spec: v1alpha1.ServiceBindingSpec{
+					Application: &ref,
+				},
+			}
+			gvr := &schema.GroupVersionResource{Group: "app", Version: "v1", Resource: "foos"}
+			typeLookup.EXPECT().ResourceForReferable(&ref).Return(gvr, nil)
+
+			u1 := &unstructured.Unstructured{}
+			u1.SetName("app1")
+			u1.SetNamespace(sb.Namespace)
+			u1.SetGroupVersionKind(schema.GroupVersionKind{Group: "app", Version: "v1", Kind: "Foo"})
+			u1.SetLabels(map[string]string{"env": "prod"})
 
+			u2 := &unstructured.Unstructured{}
+			u2.SetName("app2")
+			u2.SetNamespace(sb.Namespace)
+			u2.SetGroupVersionKind(schema.GroupVersionKind{Group: "app", Version: "v1", Kind: "Foo"})
+			u2.SetLabels(map[string]string{"env": "prod"})
+
+			client := fake.NewSimpleDynamicClient(runtime.NewScheme(), u1, u2)
+
+			ctx := &impl{client: client, serviceBinding: &sb, typeLookup: typeLookup}
+
+			applications, err := ctx.Applications()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(applications).To(HaveLen(2))
+
+			Expect(applications[0].Resource().GetName()).NotTo(Equal(applications[1].Resource().GetName()))
+			Expect(applications[0].Resource()).Should(BeElementOf(u1, u2))
+			Expect(applications[1].Resource()).Should(BeElementOf(u1, u2))
+			Expect(applications[0].ContainersPath()).To(Equal(expectedContainerPath))
+			Expect(applications[1].ContainersPath()).To(Equal(expectedContainerPath))
+		},
+			Entry("no binding path specified", nil, defaultContainerPath),
+			Entry("binding path specified", &v1alpha1.BindingPath{ContainersPath: "foo.bar"}, "foo.bar"),
+		)
+		DescribeTable("should return slice of size 0 if no application is matching through label seclector", func(bindingPath *v1alpha1.BindingPath, expectedContainerPath string) {
+			ls := &metav1.LabelSelector{
+				MatchLabels: map[string]string{"env": "prod"},
+			}
+
+			ref := v1alpha1.Application{
+				Ref: v1alpha1.Ref{
+					Group:   "app",
+					Version: "v1",
+					Kind:    "Foo",
+				},
+				LabelSelector: ls,
+				BindingPath:   bindingPath,
+			}
+
+			sb := v1alpha1.ServiceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "sb1",
+					Namespace: "ns1",
+				},
+				Spec: v1alpha1.ServiceBindingSpec{
+					Application: &ref,
+				},
+			}
+			gvr := &schema.GroupVersionResource{Group: "app", Version: "v1", Resource: "foos"}
+			typeLookup.EXPECT().ResourceForReferable(&ref).Return(gvr, nil)
+
+			u := &unstructured.Unstructured{}
+			u.SetName("app")
+			u.SetNamespace(sb.Namespace)
+			u.SetGroupVersionKind(schema.GroupVersionKind{Group: "app", Version: "v1", Kind: "Foo"})
+			u.SetLabels(map[string]string{"env": "stage"})
+			client := fake.NewSimpleDynamicClient(runtime.NewScheme(), u)
+
+			ctx := &impl{client: client, serviceBinding: &sb, typeLookup: typeLookup}
+
+			applications, err := ctx.Applications()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(applications).To(HaveLen(0))
+		},
+			Entry("no binding path specified", nil, defaultContainerPath),
+			Entry("binding path specified", &v1alpha1.BindingPath{ContainersPath: "foo.bar"}, "foo.bar"),
+		)
+
+		It("should return error if application list returns error", func() {
+			ls := &metav1.LabelSelector{
+				MatchLabels: map[string]string{"env": "prod"},
+			}
+
+			ref := v1alpha1.Application{
+				Ref: v1alpha1.Ref{
+					Group:   "app",
+					Version: "v1",
+					Kind:    "Foo",
+				},
+				LabelSelector: ls,
+			}
+
+			sb := v1alpha1.ServiceBinding{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "sb1",
+					Namespace: "ns1",
+				},
+				Spec: v1alpha1.ServiceBindingSpec{
+					Application: &ref,
+				},
+			}
+
+			gvr := &schema.GroupVersionResource{Group: "app", Version: "v1", Resource: "foos"}
+			typeLookup.EXPECT().ResourceForReferable(&ref).Return(gvr, nil)
+
+			client := fake.NewSimpleDynamicClient(runtime.NewScheme())
+			expectedError := "Error listing foo"
+			client.PrependReactor("list", "foos",
+				func(action testing.Action) (handled bool, ret runtime.Object, err error) {
+					return true, nil, e.New(expectedError)
+				})
+
+			ctx := &impl{client: client, serviceBinding: &sb, typeLookup: typeLookup}
+
+			_, err := ctx.Applications()
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(Equal(expectedError))
+		})
 		It("should return error if application is not found", func() {
 			ref := v1alpha1.Application{
 				Ref: v1alpha1.Ref{

test/acceptance/features/bindMultipleAppsToSingleService.feature

@@ -0,0 +1,59 @@
+Feature: Bind multiple applications to a single service
+
+    As a user of Service Binding Operator
+    I want to bind multiple applications to a single service that depends on
+
+    Background:
+        Given Namespace [TEST_NAMESPACE] is used
+        * Service Binding Operator is running
+
+    Scenario: Successfully bind two applications to a single service
+        Given Test applications "gen-app-a-s-f-1" and "gen-app-a-s-f-2" is running
+        * The common label "app-custom=test" is set for both apps
+        * CustomResourceDefinition backends.stable.example.com is available
+        * The Secret is present
+            """
+            apiVersion: v1
+            kind: Secret
+            metadata:
+                name: backend-secret
+            stringData:
+                username: AzureDiamond
+                password: hunter2
+            """
+        And The Custom Resource is present
+            """
+            apiVersion: stable.example.com/v1
+            kind: Backend
+            metadata:
+                name: service-a-s-f
+                annotations:
+                    service.binding: path={.status.data.dbCredentials},objectType=Secret,elementType=map
+            status:
+                data:
+                    dbCredentials: backend-secret
+            """
+        When Service Binding is applied
+            """
+            apiVersion: binding.operators.coreos.com/v1alpha1
+            kind: ServiceBinding
+            metadata:
+                name: service-binding-a-s-f
+            spec:
+                bindAsFiles: false
+                services:
+                  - group: stable.example.com
+                    version: v1
+                    kind: Backend
+                    name: service-a-s-f
+                application:
+                    labelSelector:
+                      matchLabels:
+                        app-custom: test
+                    group: apps
+                    version: v1
+                    resource: deployments
+            """
+        Then Service Binding "service-binding-a-s-f" is ready
+        And The application env var "BACKEND_USERNAME" has value "AzureDiamond" in both apps
+        And The application env var "BACKEND_PASSWORD" has value "hunter2" in both apps

test/acceptance/features/steps/generic_testapp.py

@@ -38,6 +38,9 @@ def assert_file_not_exist(self, file_path):
         polling2.poll(lambda: requests.get(url=f"http://{self.route_url}{file_path}"),
                       check_success=lambda r: r.status_code == 404, step=5, timeout=400, ignore_exceptions=(requests.exceptions.ConnectionError,))
 
+    def set_label(self, label):
+        self.openshift.set_label(self.name, label, self.namespace)
+
 
 @step(u'Generic test application "{application_name}" is running')
 @step(u'Generic test application "{application_name}" is running with binding root as "{bindingRoot}"')
@@ -74,3 +77,30 @@ def check_file_value(context, file_path):
 @step(u'File "{file_path}" is unavailable in application pod')
 def check_file_unavailable(context, file_path):
     context.application.assert_file_not_exist(file_path)
+
+
+@step(u'Test applications "{first_app_name}" and "{second_app_name}" is running')
+def are_two_apps_running(context, first_app_name, second_app_name, bindingRoot=None):
+    application1 = GenericTestApp(first_app_name, context.namespace.name)
+    if not application1.is_running():
+        print("application1 is not running, trying to import it")
+        application1.install(bindingRoot=bindingRoot)
+    context.application1 = application1
+
+    application2 = GenericTestApp(second_app_name, context.namespace.name)
+    if not application2.is_running():
+        print("application2 is not running, trying to import it")
+        application2.install(bindingRoot=bindingRoot)
+    context.application2 = application2
+
+
+@step(u'The common label "{label}" is set for both apps')
+def set_common_label(context, label):
+    context.application1.set_label(f"{label}")
+    context.application2.set_label(f"{label}")
+
+
+@step(u'The application env var "{name}" has value "{value}" in both apps')
+def check_env_var_value_in_both_apps(context, name, value):
+    polling2.poll(lambda: context.application1.get_env_var_value(name) == value, step=5, timeout=400)
+    polling2.poll(lambda: context.application2.get_env_var_value(name) == value, step=5, timeout=400)

test/acceptance/features/steps/openshift.py

@@ -478,3 +478,8 @@ def new_app(self, name, image_name, namespace, bindingRoot=None):
             else:
                 (output, exit_code) = self.cmd.run(cmd)
         assert exit_code == 0, f"Non-zero exit code ({exit_code}) returned when attempting to create a new app using following command line {cmd}\n: {output}"
+
+    def set_label(self, name, label, namespace):
+        cmd = f"{ctx.cli} label deployments {name} '{label}' -n {namespace}"
+        (output, exit_code) = self.cmd.run(cmd)
+        assert exit_code == 0, f"Non-zero exit code ({exit_code}) returned when attempting set label: {cmd}\n: {output}"