From 14fa1986f7ff173a8a509eb54d5099fb54b8eda3 Mon Sep 17 00:00:00 2001 From: Adrien Dorsaz Date: Wed, 24 Mar 2021 22:06:22 +0100 Subject: [PATCH] ResourceContains has to iterate over all elements 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. --- pkg/kubernetes/util.go | 15 +++-- pkg/kubernetes/util_test.go | 117 ++++++++++++++++++++++++++++++------ 2 files changed, 110 insertions(+), 22 deletions(-) diff --git a/pkg/kubernetes/util.go b/pkg/kubernetes/util.go index 04fbbf6..6a8e11e 100644 --- a/pkg/kubernetes/util.go +++ b/pkg/kubernetes/util.go @@ -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" ) @@ -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 { @@ -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 +} diff --git a/pkg/kubernetes/util_test.go b/pkg/kubernetes/util_test.go index 9652c8b..d6327ed 100644 --- a/pkg/kubernetes/util_test.go +++ b/pkg/kubernetes/util_test.go @@ -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": "bar", + "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)) } }