Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
baijum committed Jun 8, 2021
1 parent ee27a52 commit af6e990
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 57 deletions.
5 changes: 5 additions & 0 deletions api/v1alpha1/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ func (builder *conditionsBuilder) ApplicationNotFound() *conditionsBuilder {
return builder
}

func (builder *conditionsBuilder) InvalidApplicationReference() *conditionsBuilder {
builder.reason = InvalidApplicationReferenceReason
return builder
}

func (builder *conditionsBuilder) Msg(msg string) *conditionsBuilder {
builder.message = msg
return builder
Expand Down
2 changes: 2 additions & 0 deletions api/v1alpha1/servicebinding_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ const (
EmptyApplicationReason = "EmptyApplication"
// ApplicationNotFoundReason is used when the application is not found.
ApplicationNotFoundReason = "ApplicationNotFound"
// InvalidApplicationReferenceReason is used when the application referecne is wrong
InvalidApplicationReferenceReason = "InvalidApplicationReference"
// ServiceNotFoundReason is used when the service is not found.
ServiceNotFoundReason = "ServiceNotFound"

Expand Down
15 changes: 7 additions & 8 deletions pkg/reconcile/pipeline/context/impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,35 +127,34 @@ func (i *impl) Applications() ([]pipeline.Application, error) {
return nil, errs.New("Both label selector and name for application should not be used")
}
gvr, err := i.typeLookup.ResourceForReferable(ref)
if err != nil {
return nil, err
}
if i.serviceBinding.Spec.Application.Name != "" {
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
}
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 {
emptyResult := make([]pipeline.Application, 0)
return emptyResult, err
}

if len(objList.Items) == 0 {
return nil, nil
return emptyResult, nil
}

for _, j := range objList.Items {
i.applications = append(i.applications, &application{gvr: gvr, persistedResource: &j, bindingPath: i.serviceBinding.Spec.Application.BindingPath})
for index, _ := range objList.Items {
i.applications = append(i.applications, &application{gvr: gvr, persistedResource: &(objList.Items[index]), bindingPath: i.serviceBinding.Spec.Application.BindingPath})
}

}
Expand Down
65 changes: 18 additions & 47 deletions pkg/reconcile/pipeline/context/impl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,51 +94,6 @@ 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 1 if application is 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)

u := &unstructured.Unstructured{}
u.SetName("app1")
u.SetNamespace(sb.Namespace)
u.SetGroupVersionKind(schema.GroupVersionKind{Group: "app", Version: "v1", Kind: "Foo"})
u.SetLabels(map[string]string{"env": "prod"})
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(1))
Expect(applications[0].Resource()).To(Equal(u))
Expect(applications[0].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 2 if 2 applications are specified through label seclector", func(bindingPath *v1alpha1.BindingPath, expectedContainerPath string) {
ls := &metav1.LabelSelector{
MatchLabels: map[string]string{"env": "prod"},
Expand Down Expand Up @@ -185,6 +140,10 @@ var _ = Describe("Context", func() {
applications, err := ctx.Applications()
Expect(err).NotTo(HaveOccurred())
Expect(applications).To(HaveLen(2))
Expect(applications[0].Resource()).To(Equal(u1))
Expect(applications[0].ContainersPath()).To(Equal(expectedContainerPath))
Expect(applications[1].Resource()).To(Equal(u2))
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"),
Expand Down Expand Up @@ -256,12 +215,20 @@ var _ = Describe("Context", func() {
Application: &ref,
},
}
client := fake.NewSimpleDynamicClient(runtime.NewScheme())

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}

_, err := ctx.Applications()
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal("Both label selector and name for application should not be used"))
})
It("should return error if application list returns error", func() {
ls := &metav1.LabelSelector{
Expand Down Expand Up @@ -289,16 +256,19 @@ var _ = Describe("Context", func() {

gvr := &schema.GroupVersionResource{Group: "app", Version: "v1", Resource: "foos"}
typeLookup.EXPECT().ResourceForReferable(&ref).Return(gvr, nil)

client := fake.NewSimpleDynamicClient(runtime.NewScheme())
client.PrependReactor("list", "foos",
func(action testing.Action) (handled bool, ret runtime.Object, err error) {
return true, &v1.DeploymentList{}, e.New("Error creating secret")
return true, &v1.DeploymentList{}, e.New("Error listing foo")
})

ctx := &impl{client: client, serviceBinding: &sb, typeLookup: typeLookup}

_, err := ctx.Applications()
Expect(err).To(HaveOccurred())
// FIXME: This is not the expected error message
Expect(err.Error()).To(ContainSubstring("no kind is registered for the type v1.DeploymentList in scheme"))
})
It("should return error if application is not found", func() {
ref := v1alpha1.Application{
Expand Down Expand Up @@ -330,6 +300,7 @@ var _ = Describe("Context", func() {
Expect(err).To(HaveOccurred())
Expect(errors.IsNotFound(err)).To(BeTrue())
})

})

Describe("Services", func() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Feature: Bind an application with label selector to a service
Given Namespace [TEST_NAMESPACE] is used
* Service Binding Operator is running

Scenario: Bind an application with label selector to backend service
Scenario: Bind successfully an application owning a given label to a service
Given Generic test application "gen-app-a-s-c" is running
* CustomResourceDefinition backends.stable.example.com is available
* The Secret is present
Expand Down Expand Up @@ -57,7 +57,7 @@ Feature: Bind an application with label selector to a service
And The application env var "BACKEND_USERNAME" has value "AzureDiamond"
And The application env var "BACKEND_PASSWORD" has value "hunter2"

Scenario: Attempt to bind an application with label selector and name to a service
Scenario: Unsuccessful attempt to bind an application with label selector and name to a service
Given Generic test application "gen-app-a-s-e" is running
* CustomResourceDefinition backends.stable.example.com is available
* The Secret is present
Expand Down

0 comments on commit af6e990

Please sign in to comment.