Reimplement list kinds to be more generic (#240)

* Reimplement list kinds to generically support typed and untyped lists

* Fix various issues from rebase

* Use iteritems from six

openshift/dynamic/client.py

@@ -7,9 +7,9 @@
 import base64
 import tempfile
 from functools import partial
-from six import PY2, PY3
 from abc import abstractmethod, abstractproperty
 
+import six
 import yaml
 from pprint import pformat
 
@@ -37,6 +37,8 @@
     'ResourceField',
 ]
 
+DISCOVERY_PREFIX = 'apis'
+
 class CacheEncoder(json.JSONEncoder):
 
     def default(self, o):
@@ -55,7 +57,7 @@ def object_hook(self, obj):
             if _type == 'Resource':
                 return Resource(client=client, **obj)
             elif _type == 'ResourceList':
-                return ResourceList(obj['resource'])
+                return ResourceList(client, **obj)
             elif _type == 'ResourceGroup':
                 return ResourceGroup(obj['preferred'], resources=self.object_hook(obj['resources']))
             return obj
@@ -80,12 +82,12 @@ def serialize(resource, response):
     try:
         return ResourceInstance(resource, load_json(response))
     except ValueError:
-        if PY2:
+        if six.PY2:
             return response.data
         return response.data.decode('utf8')
 
 def load_json(response):
-    if PY2:
+    if six.PY2:
         return json.loads(response.data)
     return json.loads(response.data.decode('utf8'))
 
@@ -387,35 +389,96 @@ def __getattr__(self, name):
 class ResourceList(Resource):
     """ Represents a list of API objects """
 
-    def __init__(self, resource):
-        self.resource = resource
-        self.kind = '{}List'.format(resource.kind)
-
-    def get(self, body=None, **kwargs):
+    def __init__(self, client, group='', api_version='v1', base_kind='', kind=None):
+        self.client = client
+        self.group = group
+        self.api_version = api_version
+        self.kind = kind or '{}List'.format(base_kind)
+        self.base_kind = base_kind
+        self.__base_resource = None
+
+    def base_resource(self):
+        if self.__base_resource:
+            return self.__base_resource
+        elif self.base_kind:
+            self.__base_resource = self.client.resources.get(group=self.group, api_version=self.api_version, kind=self.base_kind)
+            return self.__base_resource
+        return None
+
+    def _items_to_resources(self, body):
+        """ Takes a List body and return a dictionary with the following structure:
+            {
+                'api_version': str,
+                'kind': str,
+                'items': [{
+                    'resource': Resource,
+                    'name': str,
+                    'namespace': str,
+                }]
+            }
+        """
         if body is None:
-            return self.resource.get(**kwargs)
+            raise ValueError("You must provide a body when calling methods on a ResourceList")
+
+        api_version = body['apiVersion']
+        kind = body['kind']
+        items = body.get('items')
+        if not items:
+            raise ValueError('The `items` field in the body must be populated when calling methods on a ResourceList')
+
+        if self.kind != kind:
+            raise ValueError('Methods on a {} must be called with a body containing the same kind. Receieved {} instead'.format(self.kind, kind))
+
+        return {
+            'api_version': api_version,
+            'kind': kind,
+            'items': [self._item_to_resource(item) for item in items]
+        }
+
+    def _item_to_resource(self, item):
+        metadata = item.get('metadata', {})
+        resource = self.base_resource()
+        if not resource:
+            api_version = item.get('apiVersion', self.api_version)
+            kind = item.get('kind', self.base_kind)
+            resource = self.client.resources.get(api_version=api_version, kind=kind)
+        return {
+            'resource': resource,
+            'definition': item,
+            'name': metadata.get('name'),
+            'namespace': metadata.get('namespace')
+        }
+
+    def get(self, body, name=None, namespace=None, **kwargs):
+        if name:
+            raise ValueError('Operations on ResourceList objects do not support the `name` argument')
+        resource_list = self._items_to_resources(body)
         response = copy.deepcopy(body)
-        namespace = kwargs.pop('namespace', None)
+
         response['items'] = [
-            self.resource.get(name=item['metadata']['name'], namespace=item['metadata'].get('namespace', namespace), **kwargs)
-            for item in body['items']
+            item['resource'].get(name=item['name'], namespace=item['namespace'] or namespace, **kwargs)
+            for item in resource_list['items']
         ]
         return ResourceInstance(self, response)
 
-    def delete(self, body=None, *args, **kwargs):
+    def delete(self, body, name=None, namespace=None, **kwargs):
+        if name:
+            raise ValueError('Operations on ResourceList objects do not support the `name` argument')
+        resource_list = self._items_to_resources(body)
         response = copy.deepcopy(body)
-        namespace = kwargs.pop('namespace', None)
+
         response['items'] = [
-            self.resource.delete(name=item['metadata']['name'], namespace=item['metadata'].get('namespace', namespace), **kwargs)
-            for item in body['items']
+            item['resource'].delete(name=item['name'], namespace=item['namespace'] or namespace, **kwargs)
+            for item in resource_list['items']
         ]
         return ResourceInstance(self, response)
 
-    def verb_mapper(self, verb, body=None, **kwargs):
+    def verb_mapper(self, verb, body, **kwargs):
+        resource_list = self._items_to_resources(body)
         response = copy.deepcopy(body)
         response['items'] = [
-            getattr(self.resource, verb)(body=item, **kwargs)
-            for item in body['items']
+            getattr(item['resource'], verb)(body=item['definition'], **kwargs)
+            for item in resource_list['items']
         ]
         return ResourceInstance(self, response)
 
@@ -428,15 +491,20 @@ def replace(self, *args, **kwargs):
     def patch(self, *args, **kwargs):
         return self.verb_mapper('patch', *args, **kwargs)
 
-    def __getattr__(self, name):
-        return getattr(self.resource, name)
-
     def to_dict(self):
         return {
             '_type': 'ResourceList',
-            'resource': self.resource.to_dict(),
+            'group': self.group,
+            'api_version': self.api_version,
+            'kind': self.kind,
+            'base_kind': self.base_kind
         }
 
+    def __getattr__(self, name):
+        if self.base_resource():
+            return getattr(self.base_resource(), name)
+        return None
+
 
 class Subresource(Resource):
     """ Represents a subresource of an API resource. This generally includes operations
@@ -514,7 +582,7 @@ class Discoverer(object):
     def __init__(self, client, cache_file):
         self.client = client
         default_cache_id = self.client.configuration.host
-        if PY3:
+        if six.PY3:
             default_cache_id = default_cache_id.encode('utf-8')
         default_cachefile_name = 'osrcp-{0}.json'.format(base64.b64encode(default_cache_id).decode('utf-8'))
         self.__cache_file = cache_file or os.path.join(tempfile.gettempdir(), default_cachefile_name)
@@ -569,16 +637,17 @@ def default_groups(self, request_resources=False):
                     if request_resources else ResourceGroup(True))
                 }}
 
+            groups[DISCOVERY_PREFIX] = {'': {
+                'v1': ResourceGroup(True, resources = {"List": ResourceList(self.client)})
+            }}
         return groups
 
     def parse_api_groups(self, request_resources=False):
         """ Discovers all API groups present in the cluster """
         if not self._cache.get('resources'):
-            prefix = 'apis'
-            groups_response = load_json(self.client.request('GET', '/{}'.format(prefix)))['groups']
+            groups_response = load_json(self.client.request('GET', '/{}'.format(DISCOVERY_PREFIX)))['groups']
 
             groups = self.default_groups(request_resources=request_resources)
-            groups[prefix] = {}
 
             for group in groups_response:
                 new_group = {}
@@ -587,9 +656,9 @@ def parse_api_groups(self, request_resources=False):
                     preferred = version_raw == group['preferredVersion']
                     resources = {}
                     if request_resources:
-                        resources = self.get_resources_for_api_version(prefix, group['name'], version, preferred)
+                        resources = self.get_resources_for_api_version(DISCOVERY_PREFIX, group['name'], version, preferred)
                     new_group[version] = ResourceGroup(preferred, resources=resources)
-                groups[prefix][group['name']] = new_group
+                groups[DISCOVERY_PREFIX][group['name']] = new_group
             self._cache['resources'] = groups
         return self._cache['resources']
 
@@ -631,8 +700,10 @@ def get_resources_for_api_version(self, prefix, group, version, preferred):
                 client=self.client,
                 preferred=preferred,
                 subresources=subresources.get(resource['name']),
-                **resource)
-            resources['{}List'.format(resource['kind'])] = ResourceList(resources[resource['kind']])
+                **resource
+            )
+            resource_list = ResourceList(self.client, group=group, api_version=version, base_kind=resource['kind'])
+            resources[resource_list.kind] = resource_list
         return resources
 
     def get(self, **kwargs):
@@ -738,7 +809,7 @@ def __iter__(self):
                             prefix, group, version, rg.preferred)
                         self._cache['resources'][prefix][group][version] = rg
                         self.__update_cache = True
-                    for resource in rg.resources:
+                    for _, resource in six.iteritems(rg.resources):
                         yield resource
         self.__maybe_write_cache()
 
@@ -916,7 +987,7 @@ def main():
         item = {}
         item[key] = {k: v for k, v in resource.__dict__.items() if k not in ('client', 'subresources', 'resource')}
         if isinstance(resource, ResourceList):
-            item[key]["resource"] = '{}.{}'.format(resource.resource.group_version, resource.resource.kind)
+            item[key]["resource"] = '{}.{}'.format(resource.group_version, resource.kind)
         else:
             item[key]['subresources'] = {}
             for name, value in resource.subresources.items():

test/functional/dynamic/conftest.py

@@ -171,6 +171,11 @@ def client(clusterrole, namespace, kubeconfig, port, admin_client):
 def perform_update_action_in_namespace(context, client, action, namespace, definition, update):
 
     def set_resource_version(r, resource):
+        if isinstance(resource, ResourceList):
+            for i, item in enumerate(r['items']):
+                base_resource = resource._item_to_resource(item)['resource']
+                r['items'][i] = set_resource_version(item, base_resource)
+            return r
         try:
             r['metadata']['resourceVersion'] = resource.get(r['metadata']['name'], namespace=namespace).metadata.resourceVersion
         except exceptions.NotFoundError:
@@ -183,10 +188,7 @@ def set_resource_version(r, resource):
     try:
         if action == 'replace':
             replace = load_definition(update)
-            if isinstance(resource, ResourceList):
-                replace['items'] = [set_resource_version(item, resource.resource) for item in replace['items']]
-            else:
-                replace = set_resource_version(replace, resource)
+            replace = set_resource_version(replace, resource)
             context['instance'] = resource.replace(body=replace, namespace=namespace)
         elif action == 'patch':
             patch = load_definition(update)
@@ -247,29 +249,29 @@ def resource_should_match_update(admin_client, namespace, update, definition):
 @then(parsers.parse('The content of <filename> does not exist in <namespace>'))
 def resource_not_in_namespace(admin_client, namespace, definition):
 
-    def assert_resource_does_not_exist(resource, resource_definition, namespace):
+    def assert_resource_does_not_exist(r, resource, namespace):
+        if isinstance(resource, ResourceList):
+            for item in r['items']:
+                resource = resource._item_to_resource(item)['resource']
+                return assert_resource_does_not_exist(item, resource, namespace)
         with pytest.raises(exceptions.NotFoundError):
-            resource.get(resource_definition['metadata']['name'], namespace)
+            resource.get(r['metadata']['name'], namespace)
 
     resource = admin_client.resources.get(api_version=definition['apiVersion'], kind=definition['kind'])
-    if isinstance(resource, ResourceList):
-        for item in definition['items']:
-            assert_resource_does_not_exist(resource.resource, item, namespace)
-    else:
-        assert_resource_does_not_exist(resource, definition, namespace)
+    assert_resource_does_not_exist(definition, resource, namespace)
 
 
 @then(parsers.parse('The content of <filename> exists in <namespace>'))
 def resource_in_namespace(admin_client, namespace, definition):
 
-    def assert_resource_exists(resource, resource_definition, namespace):
-        instance = resource.get(resource_definition['metadata']['name'], namespace)
-        assert instance.metadata.name == resource_definition['metadata']['name']
+    def assert_resource_exists(r, resource, namespace):
+        if isinstance(resource, ResourceList):
+            for item in r['items']:
+                base_resource = resource._item_to_resource(item)['resource']
+                return assert_resource_exists(item, base_resource, namespace)
+        instance = resource.get(r['metadata']['name'], namespace)
+        assert instance.metadata.name == r['metadata']['name']
         assert instance.metadata.namespace == namespace
 
     resource = admin_client.resources.get(api_version=definition['apiVersion'], kind=definition['kind'])
-    if isinstance(resource, ResourceList):
-        for item in definition['items']:
-            assert_resource_exists(resource.resource, item, namespace)
-    else:
-        assert_resource_exists(resource, definition, namespace)
+    assert_resource_exists(definition, resource, namespace)

test/functional/dynamic/create.feature

@@ -1,9 +1,10 @@
 Feature: Create
 
     Examples:
-    | filename                         | namespace |
-    | definitions/v1_Pod_test.yaml     | test      |
-    | definitions/v1_PodList_test.yaml | test2     |
+    | filename                         | namespace    |
+    | definitions/v1_Pod_test.yaml     | test-create  |
+    | definitions/v1_PodList_test.yaml | test-create2 |
+    | definitions/v1_List_test.yaml    | test-create  |
 
 
     Scenario Outline: Create a resource for the first time

test/functional/dynamic/definitions/v1_List_test.yaml

@@ -0,0 +1,16 @@
+---
+apiVersion: v1
+kind: List
+items:
+- kind: ConfigMap
+  apiVersion: v1
+  metadata:
+    name: test-config1
+  data:
+    index: "1"
+- kind: ConfigMap
+  apiVersion: v1
+  metadata:
+    name: test-config2
+  data:
+    index: "2"

test/functional/dynamic/definitions/v1_List_test_replace.yaml

@@ -0,0 +1,16 @@
+---
+apiVersion: v1
+kind: List
+items:
+- kind: ConfigMap
+  apiVersion: v1
+  metadata:
+    name: test-config1
+  data:
+    overwritten: "true"
+- kind: ConfigMap
+  apiVersion: v1
+  metadata:
+    name: test-config2
+  data:
+    overwritten: "true"

test/functional/dynamic/delete.feature

@@ -1,9 +1,10 @@
 Feature: Delete
 
     Examples:
-    | filename                         | namespace |
-    | definitions/v1_Pod_test.yaml     | test      |
-    | definitions/v1_PodList_test.yaml | test2     |
+    | filename                         | namespace    |
+    | definitions/v1_Pod_test.yaml     | test-delete  |
+    | definitions/v1_PodList_test.yaml | test-delete2 |
+    | definitions/v1_List_test.yaml    | test-delete  |
 
     Scenario Outline: Delete a resource that exists
         Given I have edit permissions in <namespace>

test/functional/dynamic/patch.feature

@@ -1,9 +1,10 @@
 Feature: Patch
 
     Examples:
-    | filename                         | namespace       | update                                 |
-    | definitions/v1_Pod_test.yaml     | test-patch      | definitions/v1_Pod_test_patch.yaml     |
-    | definitions/v1_PodList_test.yaml | test-patch2     | definitions/v1_PodList_test_patch.yaml |
+    | filename                         | namespace   | update                                 |
+    | definitions/v1_Pod_test.yaml     | test-patch  | definitions/v1_Pod_test_patch.yaml     |
+    | definitions/v1_PodList_test.yaml | test-patch2 | definitions/v1_PodList_test_patch.yaml |
+    | definitions/v1_List_test.yaml    | test-patch  | definitions/v1_List_test_replace.yaml  |
 
     Scenario Outline: Patch a resource that does not exist
         Given I have edit permissions in <namespace>