Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix support for label selectors #971

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 31 additions & 7 deletions pkg/reconcile/pipeline/context/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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{}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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})
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The persistedResource value (address of the item) is taken using the index.
Ref. https://play.golang.org/p/ePs7st6f91g

}
}
i.applications = append(i.applications, &application{gvr: gvr, persistedResource: u, bindingPath: i.serviceBinding.Spec.Application.BindingPath})
} else {
i.applications = make([]*application, 0)
}
Expand Down
141 changes: 141 additions & 0 deletions pkg/reconcile/pipeline/context/impl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
e "errors"
"fmt"

"github.com/golang/mock/gomock"
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
Expand All @@ -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() {
Expand Down Expand Up @@ -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))
pedjak marked this conversation as resolved.
Show resolved Hide resolved

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,
},
}
pedjak marked this conversation as resolved.
Show resolved Hide resolved

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())
Copy link
Contributor

@pedjak pedjak Jun 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but which error has occurred? we need to assert that as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. af6e990

Expect(err.Error()).To(Equal(expectedError))
})
It("should return error if application is not found", func() {
ref := v1alpha1.Application{
Ref: v1alpha1.Ref{
Expand Down
59 changes: 59 additions & 0 deletions test/acceptance/features/bindMultipleAppsToSingleService.feature
Original file line number Diff line number Diff line change
@@ -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
30 changes: 30 additions & 0 deletions test/acceptance/features/steps/generic_testapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"')
Expand Down Expand Up @@ -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)
5 changes: 5 additions & 0 deletions test/acceptance/features/steps/openshift.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"