Skip to content

Commit

Permalink
Fix Service.OwnedResources() to return correct result
Browse files Browse the repository at this point in the history
We were bitten by https://github.com/golang/go/wiki/CommonMistakes#using-reference-to-loop-iterator-variable

Fixed the loop to use the new variable.

Signed-off-by: Predrag Knezevic <pknezevi@redhat.com>
  • Loading branch information
pedjak committed Jun 10, 2021
1 parent d4fd4b7 commit 615d956
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 3 deletions.
3 changes: 2 additions & 1 deletion pkg/reconcile/pipeline/context/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ func (s *service) OwnedResources() ([]*unstructured.Unstructured, error) {
}
return nil, err
}
for _, item := range list.Items {
for i := range list.Items {
item := list.Items[i]
for _, ownerRef := range item.GetOwnerReferences() {
if reflect.DeepEqual(ownerRef.UID, uid) {
result = append(result, &item)
Expand Down
120 changes: 118 additions & 2 deletions pkg/reconcile/pipeline/context/service_test.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,27 @@
package context

import (
"errors"
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
"github.com/redhat-developer/service-binding-operator/api/v1alpha1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/dynamic"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/uuid"
"k8s.io/client-go/dynamic/fake"
"k8s.io/client-go/testing"
"strings"
)

var _ = Describe("Service", func() {

var (
client dynamic.Interface
client *fake.FakeDynamicClient
)

DescribeTable("CRD exist", func(version string, gr schema.GroupResource) {
Expand Down Expand Up @@ -43,8 +49,118 @@ var _ = Describe("Service", func() {
Expect(res).To(BeNil())
})

Describe("OwnedResources", func() {
It("should return owned resources", func() {
id := uuid.NewUUID()
id2 := uuid.NewUUID()
ns := "ns1"
u := &unstructured.Unstructured{}
u.SetGroupVersionKind(schema.GroupVersionKind{Group: "foo.bar", Version: "v1", Kind: "Foo"})
u.SetName("foo")
u.SetNamespace(ns)
u.SetUID(id)

var children []interface{}

client = fake.NewSimpleDynamicClient(runtime.NewScheme())

for i := range bindableResourceGVRs {
gvr := bindableResourceGVRs[i]

if gvr.Resource == "configmaps" {
client.PrependReactor("list", gvr.Resource, func(action testing.Action) (handled bool, ret runtime.Object, err error) {
return true, nil, k8serrors.NewNotFound(gvr.GroupResource(), "foo")
})
continue
}
ul := &unstructured.UnstructuredList{}
// compute kind
kind := strings.Title(gvr.Resource)[:len(gvr.Resource)-1]

gvk := gvr.GroupVersion().WithKind(kind)
ou := resource(gvk, "child1", ns, id)

ou2 := resource(gvk, "child2", ns, id2)

children = append(children, ou)
ul.Items = append(ul.Items, *ou, *ou2)

client.PrependReactor("list", gvr.Resource, func(action testing.Action) (handled bool, ret runtime.Object, err error) {
return true, ul, nil
})
}

impl := &service{client: client, resource: u, lookForOwnedResources: true, serviceRef: &v1alpha1.Service{ NamespacedRef: v1alpha1.NamespacedRef{Namespace: &ns}}}

ownedResources, err := impl.OwnedResources()
Expect(err).NotTo(HaveOccurred())
Expect(ownedResources).Should(HaveLen(len(bindableResourceGVRs)-1))
Expect(ownedResources).Should(ConsistOf(children...))
})

DescribeTable("return error if occurs at looking at owned resources", func(failingResourceName string) {
id := uuid.NewUUID()
id2 := uuid.NewUUID()
ns := "ns1"
u := &unstructured.Unstructured{}
u.SetGroupVersionKind(schema.GroupVersionKind{Group: "foo.bar", Version: "v1", Kind: "Foo"})
u.SetName("foo")
u.SetNamespace(ns)
u.SetUID(id)

client = fake.NewSimpleDynamicClient(runtime.NewScheme())
expectedErr := errors.New("foo")

for _, gvr := range bindableResourceGVRs {
ul := &unstructured.UnstructuredList{}
// compute kind
kind := strings.Title(gvr.Resource)[:len(gvr.Resource)-1]
// fix for ConfigMap
if kind == "Configmap" {
kind = "ConfigMap"
}
gvk := gvr.GroupVersion().WithKind(kind)
ou := resource(gvk, "child1", ns, id)

ou2 := resource(gvk, "child2", ns, id2)

ul.Items = append(ul.Items, *ou, *ou2)

resourceName := gvr.Resource
client.PrependReactor("list", resourceName, func(action testing.Action) (handled bool, ret runtime.Object, err error) {
if failingResourceName == resourceName {
return true, nil, expectedErr
}
return true, ul, nil
})
}

impl := &service{client: client, resource: u, lookForOwnedResources: true, serviceRef: &v1alpha1.Service{ NamespacedRef: v1alpha1.NamespacedRef{Namespace: &ns}}}

ownedResources, err := impl.OwnedResources()
Expect(err).Should(Equal(expectedErr))
Expect(ownedResources).Should(BeNil())
},
Entry("fail listing configmap", "configmaps"),
Entry("fail listing services", "services"),
)
})

})

func resource(gvk schema.GroupVersionKind, name string, namespace string, owner types.UID) *unstructured.Unstructured {
u := &unstructured.Unstructured{}
u.SetGroupVersionKind(gvk)
u.SetName(name)
u.SetNamespace(namespace)
u.SetOwnerReferences([]v1.OwnerReference {
{
UID: owner,
},
})
return u
}

func crd(version string, gr schema.GroupResource) *unstructured.Unstructured {
u := &unstructured.Unstructured{}
u.SetGroupVersionKind(schema.GroupVersionKind{Group: "apiextensions.k8s.io", Version: version, Kind: "CustomResourceDefinition"})
Expand Down

0 comments on commit 615d956

Please sign in to comment.