Skip to content

Commit

Permalink
ResourceContains has to iterate over all elements
Browse files Browse the repository at this point in the history
The ResourceContains method was checking only if first resource were
containing the requested value.

For example, if first element did not have the value but the second did,
ResourceContains returned false, because it did not check the second element.

The loop is moved outside to easily unit test ResourceContains: otherwise it
would require to mock kubernetes client and its methods.

As the new unit test is very similar to the one for ObjectContains, we removed
the later and extended metadata field in mock object values to cover all
ObjectContains case too.
  • Loading branch information
Trim committed Apr 7, 2021
1 parent 7f0a587 commit a6dccbf
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 22 deletions.
15 changes: 11 additions & 4 deletions pkg/kubernetes/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"k8s.io/client-go/dynamic"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
)

Expand Down Expand Up @@ -35,11 +36,8 @@ func (k *kubernetesImpl) ResourceContains(namespace, value string, resource sche
if err != nil {
return false, err
}
for _, item := range objectlist.Items {
return ObjectContains(item.Object, value), nil
}

return false, nil
return UnstructuredListContains(objectlist, value), nil
}

func (k *kubernetesImpl) initClient() error {
Expand Down Expand Up @@ -84,3 +82,12 @@ func ObjectContains(genericObject interface{}, value string) bool {
return false
}
}

func UnstructuredListContains(unstructuredList *unstructured.UnstructuredList, value string) bool {
for _, item := range unstructuredList.Items {
if ObjectContains(item.Object, value) {
return true
}
}
return false
}
117 changes: 99 additions & 18 deletions pkg/kubernetes/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,44 +4,125 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

type ObjectContainsTestCase struct {
object interface{}
value string
expected bool
type UnstructuredListContainsTestCase struct {
objectlist *unstructured.UnstructuredList
value string
expected bool
}

func Test_ObjectContains(t *testing.T) {
testcases := []ObjectContainsTestCase{
func Test_UnstructuredListContains(t *testing.T) {
testcases := []UnstructuredListContainsTestCase{
// Successful lookup string in map[string]interface{}
{
object: map[string]interface{}{"int": 20, "bool": true, "string": "foo"},
objectlist: &unstructured.UnstructuredList{
Object: map[string]interface{}{"kind": "List", "apiVersion": "v1"},
Items: []unstructured.Unstructured{
{Object: map[string]interface{}{
"kind": "Pod",
"apiVersion": "v1",
"metadata": map[string]interface{}{
"int": 20,
"bool": true,
"[]interface": []interface{}{map[string]interface{}{
"serviceAccount": "foo",
"name": "bar",
}},
"name": "foo",
},
}},
},
},
value: "foo",
expected: true,
},
// Successful lookup string in []interface{}
{
object: map[string]interface{}{"int": 20, "bool": true, "string": "foo"},
objectlist: &unstructured.UnstructuredList{
Object: map[string]interface{}{"kind": "List", "apiVersion": "v1"},
Items: []unstructured.Unstructured{
{Object: map[string]interface{}{
"kind": "Pod",
"apiVersion": "v1",
"metadata": map[string]interface{}{
"int": 20,
"bool": true,
"[]interface": []interface{}{map[string]interface{}{
"serviceAccount": "foo",
"name": "bar",
}},
"name": "foo",
},
}},
},
},
value: "bar",
expected: true,
},
// Lookup for non-existing value
{
objectlist: &unstructured.UnstructuredList{
Object: map[string]interface{}{"kind": "List", "apiVersion": "v1"},
Items: []unstructured.Unstructured{
{Object: map[string]interface{}{
"kind": "Pod",
"apiVersion": "v1",
"metadata": map[string]interface{}{
"int": 20,
"bool": true,
"[]interface": []interface{}{map[string]interface{}{
"serviceAccount": "foo",
"name": "foo",
}},
"name": "foo",
},
}},
},
},
value: "bar",
expected: false,
},
// Successful lookup for value in second element
{
object: map[string]interface{}{
"apiVersion": "v1",
"kind": "Pod",
"metadata": map[string]interface{}{"name": "seiso", "namespace": "seiso-test"},
"spec": map[string]interface{}{
"containers": []interface{}{map[string]interface{}{
"serviceAccount": "default",
"image": "docker.io/appuio/oc:0b81a958f590ed7ed8be6ec0a2a87816228a482c",
objectlist: &unstructured.UnstructuredList{
Object: map[string]interface{}{"kind": "List", "apiVersion": "v1"},
Items: []unstructured.Unstructured{
{Object: map[string]interface{}{
"kind": "Pod",
"apiVersion": "v1",
"metadata": map[string]interface{}{
"int": 20,
"bool": true,
"[]interface": []interface{}{map[string]interface{}{
"serviceAccount": "foo",
"name": "foo",
}},
"name": "foo",
},
}},
{Object: map[string]interface{}{
"kind": "Pod",
"apiVersion": "v1",
"metadata": map[string]interface{}{
"int": 20,
"bool": true,
"[]interface": []interface{}{map[string]interface{}{
"serviceAccount": "foo",
"name": "foo",
}},
"name": "bar",
},
}},
},
},
value: "oc:0b81a958f590ed7ed8be6ec0a2a87816228a482c",
value: "bar",
expected: true,
},
}

for _, testcase := range testcases {
assert.Equal(t, testcase.expected, ObjectContains(testcase.object, testcase.value))
assert.Equal(t, testcase.expected, UnstructuredListContains(testcase.objectlist, testcase.value))
}
}

0 comments on commit a6dccbf

Please sign in to comment.