Allow resolved vars in details list calls

Currently the details=all flag allows us to only
get local variables. This changes that to be resolved
variables such that it matches the rest of the variables
calls.

It also adds details calls to /clouds and /projects
endpoints where it was missing.

Closes Bug: 1667767

Change-Id: Ia2de9110aa118c3fe7cd9374ceed2ce74f13c74e

craton/api/v1/base.py

@@ -7,7 +7,6 @@ import decorator
 
 import flask
 import flask_restful as restful
-from oslo_serialization import jsonutils
 
 from craton.api.v1.validators import ensure_project_exists
 from craton.api.v1.validators import request_validate
@@ -31,19 +30,6 @@ class Resource(restful.Resource):
         return resp
 
 
-def get_resource_with_vars(obj):
-    r_obj = []
-    for resource in obj:
-        r = jsonutils.to_primitive(resource, convert_instances=True)
-        r['variables'] = jsonutils.to_primitive(resource.variables)
-        r_obj.append(r)
-
-    if r_obj:
-        return r_obj
-
-    return obj
-
-
 @decorator.decorator
 def http_codes(f, *args, **kwargs):
     try:

craton/api/v1/resources/inventory/cells.py

@@ -23,7 +23,8 @@ class Cells(base.Resource):
             context, request_args, pagination_params,
         )
         if details:
-            cells_obj = base.get_resource_with_vars(cells_obj)
+            cells_obj = [utils.get_resource_with_vars(request_args, cell)
+                         for cell in cells_obj]
 
         links = base.links_from(link_params)
         response_body = {'cells': cells_obj, 'links': links}
@@ -53,9 +54,7 @@ class CellById(base.Resource):
     @base.http_codes
     def get(self, context, id, request_args):
         cell_obj = dbapi.cells_get_by_id(context, id)
-        cell_obj = utils.format_variables(request_args, cell_obj)
+        cell = utils.get_resource_with_vars(request_args, cell_obj)
-        cell = jsonutils.to_primitive(cell_obj)
-        cell['variables'] = jsonutils.to_primitive(cell_obj.vars)
         return cell, 200, None
 
     def put(self, context, id, request_data):

craton/api/v1/resources/inventory/clouds.py

@@ -3,6 +3,7 @@ from oslo_log import log
 
 from craton.api import v1
 from craton.api.v1 import base
+from craton.api.v1.resources import utils
 from craton import db as dbapi
 from craton import util
 
@@ -20,12 +21,16 @@ class Clouds(base.Resource):
         """
         cloud_id = request_args.get("id")
         cloud_name = request_args.get("name")
+        details = request_args.get("details")
 
         if not (cloud_id or cloud_name):
             # Get all clouds for this project
             clouds_obj, link_params = dbapi.clouds_get_all(
                 context, request_args, pagination_params,
             )
+            if details:
+                clouds_obj = [utils.get_resource_with_vars(request_args, c)
+                              for c in clouds_obj]
         else:
             if cloud_name:
                 cloud_obj = dbapi.clouds_get_by_name(context, cloud_name)

craton/api/v1/resources/inventory/devices.py

@@ -17,6 +17,7 @@ class Devices(base.Resource):
     @base.pagination_context
     def get(self, context, request_args, pagination_params):
         """Get all devices, with optional filtering."""
+        details = request_args.get("details")
         device_objs, link_params = dbapi.devices_get_all(
             context, request_args, pagination_params,
         )
@@ -24,7 +25,12 @@ class Devices(base.Resource):
 
         devices = {"hosts": [], "network-devices": []}
         for device_obj in device_objs:
-            device = jsonutils.to_primitive(device_obj)
+            if details:
+                device = utils.get_resource_with_vars(request_args,
+                                                      device_obj)
+            else:
+                device = jsonutils.to_primitive(device_obj)
+
             utils.add_up_link(context, device)
 
             if isinstance(device_obj, models.Host):

craton/api/v1/resources/inventory/hosts.py

@@ -22,7 +22,8 @@ class Hosts(base.Resource):
             context, request_args, pagination_params,
         )
         if details:
-            hosts_obj = base.get_resource_with_vars(hosts_obj)
+            hosts_obj = [utils.get_resource_with_vars(request_args, h)
+                         for h in hosts_obj]
 
         links = base.links_from(link_params)
         response_body = jsonutils.to_primitive(
@@ -61,9 +62,7 @@ class HostById(base.Resource):
     def get(self, context, id, request_args):
         """Get host by given id"""
         host_obj = dbapi.hosts_get_by_id(context, id)
-        host_obj = utils.format_variables(request_args, host_obj)
+        host = utils.get_resource_with_vars(request_args, host_obj)
-        host = jsonutils.to_primitive(host_obj)
-        host['variables'] = jsonutils.to_primitive(host_obj.vars)
 
         utils.add_up_link(context, host)
 

craton/api/v1/resources/inventory/networks.py

@@ -23,7 +23,8 @@ class Networks(base.Resource):
             context, request_args, pagination_params,
         )
         if details:
-            networks_obj = base.get_resource_with_vars(networks_obj)
+            networks_obj = [utils.get_resource_with_vars(request_args, n)
+                            for n in networks_obj]
 
         links = base.links_from(link_params)
         response_body = {'networks': networks_obj, 'links': links}
@@ -78,7 +79,8 @@ class NetworkDevices(base.Resource):
             context, request_args, pagination_params,
         )
         if details:
-            devices_obj = base.get_resource_with_vars(devices_obj)
+            devices_obj = [utils.get_resource_with_vars(request_args, d)
+                           for d in devices_obj]
 
         links = base.links_from(link_params)
         response_body = jsonutils.to_primitive(

craton/api/v1/resources/inventory/regions.py

@@ -29,7 +29,8 @@ class Regions(base.Resource):
                 context, request_args, pagination_params,
             )
             if details:
-                regions_obj = base.get_resource_with_vars(regions_obj)
+                regions_obj = [utils.get_resource_with_vars(request_args, r)
+                               for r in regions_obj]
         else:
             if region_name:
                 region_obj = dbapi.regions_get_by_name(context, region_name)
@@ -69,9 +70,7 @@ class RegionsById(base.Resource):
     @base.http_codes
     def get(self, context, id, request_args):
         region_obj = dbapi.regions_get_by_id(context, id)
-        region_obj = utils.format_variables(request_args, region_obj)
+        region = utils.get_resource_with_vars(request_args, region_obj)
-        region = jsonutils.to_primitive(region_obj)
-        region['variables'] = jsonutils.to_primitive(region_obj.vars)
         return region, 200, None
 
     def put(self, context, id, request_data):

craton/api/v1/resources/projects.py

@@ -3,6 +3,7 @@ from oslo_log import log
 
 from craton.api import v1
 from craton.api.v1 import base
+from craton.api.v1.resources import utils
 from craton import db as dbapi
 
 
@@ -16,6 +17,7 @@ class Projects(base.Resource):
     def get(self, context, request_args, pagination_params):
         """Get all projects. Requires super admin privileges."""
         project_name = request_args["name"]
+        details = request_args.get("details")
 
         if project_name:
             projects_obj, link_params = dbapi.projects_get_by_name(
@@ -25,6 +27,10 @@ class Projects(base.Resource):
             projects_obj, link_params = dbapi.projects_get_all(
                 context, request_args, pagination_params,
             )
+            if details:
+                projects_obj = [utils.get_resource_with_vars(request_args, p)
+                                for p in projects_obj]
+
         links = base.links_from(link_params)
         response_body = {'projects': projects_obj, 'links': links}
         return jsonutils.to_primitive(response_body), 200, None

craton/api/v1/resources/utils.py

@@ -1,4 +1,5 @@
 from flask import url_for
+from oslo_serialization import jsonutils
 
 from craton import db as dbapi
 
@@ -6,7 +7,7 @@ from craton import db as dbapi
 def format_variables(args, obj):
     """Update resource response with requested type of variables."""
     if args:
-        resolved_values = args["resolved-values"]
+        resolved_values = args.get("resolved-values", None)
     else:
         resolved_values = None
 
@@ -17,6 +18,14 @@ def format_variables(args, obj):
     return obj
 
 
+def get_resource_with_vars(args, obj):
+    """Get resource in json primitive with variables."""
+    obj = format_variables(args, obj)
+    res = jsonutils.to_primitive(obj)
+    res['variables'] = jsonutils.to_primitive(obj.vars)
+    return res
+
+
 def get_device_type(context, device_id):
     device = dbapi.resource_get_by_id(context, "devices", device_id)
     return device.type

craton/api/v1/schemas.py

@@ -833,6 +833,14 @@ validators = {
                     "default": False,
                     "type": "boolean",
                 },
+                "resolved-values": {
+                    "default": True,
+                    "type": "boolean",
+                },
+                "details": {
+                    "default": False,
+                    "type": "boolean",
+                },
             }),
         },
     },
@@ -916,6 +924,10 @@ validators = {
                     "type": "integer",
                     "description": "ID of the region to get",
                 },
+                "resolved-values": {
+                    "default": True,
+                    "type": "boolean",
+                },
             }),
         },
     },
@@ -938,6 +950,10 @@ validators = {
                     "type": "integer",
                     "description": "ID of the cloud to get",
                 },
+                "details": {
+                    "default": False,
+                    "type": "boolean",
+                },
             }),
         },
     },
@@ -991,6 +1007,10 @@ validators = {
                     "type": "integer",
                     "description": "ID of host to get",
                 },
+                "resolved-values": {
+                    "default": True,
+                    "type": "boolean",
+                },
             }),
         },
     },
@@ -1051,6 +1071,10 @@ validators = {
                     "type": "string",
                     "description": "name of the cell to get",
                 },
+                "resolved-values": {
+                    "default": True,
+                    "type": "boolean",
+                },
             }),
         },
     },
@@ -1111,6 +1135,10 @@ validators = {
                     "type": "string",
                     "description": "variable filters to get a project",
                 },
+                "details": {
+                    "default": False,
+                    "type": "boolean",
+                },
             }, marker_type="string"),
         },
     },
@@ -1187,6 +1215,10 @@ validators = {
                     "type": "string",
                     "description": "cell id of the device to get",
                 },
+                "resolved-values": {
+                    "default": True,
+                    "type": "boolean",
+                },
             }),
         },
     },
@@ -1377,6 +1409,14 @@ validators = {
                     "type": "string",
                     "description": "cell idof the network to get",
                 },
+                "resolved-values": {
+                    "default": True,
+                    "type": "boolean",
+                },
+                "details": {
+                    "default": False,
+                    "type": "boolean",
+                },
             }),
         },
     },

craton/tests/functional/__init__.py

@@ -363,10 +363,11 @@ class DeviceTestBase(TestCase):
     def create_cloud(self, name='cloud-1'):
         return super(DeviceTestBase, self).create_cloud(name=name)
 
-    def create_region(self, name='region-1', cloud=None):
+    def create_region(self, name='region-1', cloud=None, variables=None):
         return super(DeviceTestBase, self).create_region(
             name=name,
-            cloud=cloud if cloud else self.cloud
+            cloud=cloud if cloud else self.cloud,
+            variables=variables,
         )
 
     def create_network_device(self, name, device_type, ip_address, region=None,

craton/tests/functional/test_cell_calls.py

@@ -89,6 +89,24 @@ class APIV1CellTest(APIV1ResourceWithVariablesTestCase):
                "('updated_at' was unexpected)"]
         self.assertEqual(cell.json()['errors'], msg)
 
+    def test_cells_get_all_with_details(self):
+        self.create_cell('cell1', variables={'a': 'b'})
+        self.create_cell('cell2', variables={'c': 'd'})
+        url = self.url + '/v1/cells?details=all'
+        resp = self.get(url)
+        cells = resp.json()['cells']
+        self.assertEqual(2, len(cells))
+        for cell in cells:
+            self.assertTrue('variables' in cell)
+
+        for cell in cells:
+            if cell['name'] == 'cell1':
+                expected = {'a': 'b', "region": "one"}
+                self.assertEqual(expected, cell['variables'])
+            if cell['name'] == 'cell2':
+                expected = {'c': 'd', "region": "one"}
+                self.assertEqual(expected, cell['variables'])
+
     def test_cells_get_all_for_region(self):
         # Create a cell first
         self.create_cell('cell-1')

craton/tests/functional/test_cloud_calls.py

@@ -85,6 +85,23 @@ class APIV1CloudTest(TestCase):
         self.assertEqual(200, resp.status_code)
         self.assertEqual(2, len(resp.json()))
 
+    def test_clouds_get_all_with_details_filter(self):
+        c1 = self.create_cloud("ORD1", variables={'a': 'b'})
+        c2 = self.create_cloud("ORD2", variables={'c': 'd'})
+        url = self.url + '/v1/clouds?details=all'
+        resp = self.get(url)
+        self.assertEqual(200, resp.status_code)
+        clouds = resp.json()['clouds']
+        self.assertEqual(2, len(clouds))
+        for cloud in clouds:
+            self.assertTrue('variables' in cloud)
+
+        for cloud in clouds:
+            if cloud['name'] == 'ORD1':
+                self.assertEqual(c1['variables'], {'a': 'b'})
+            if cloud['name'] == 'ORD2':
+                self.assertEqual(c2['variables'], {'c': 'd'})
+
     def test_clouds_get_all_with_name_filter(self):
         self.create_cloud("ORD1")
         self.create_cloud("ORD2")

craton/tests/functional/test_host_calls.py

@@ -96,6 +96,23 @@ class APIV1HostTest(DeviceTestBase, APIV1ResourceWithVariablesTestCase):
                "('updated_at' was unexpected)"]
         self.assertEqual(host.json()['errors'], msg)
 
+    def test_get_all_hosts_with_details(self):
+        region_vars = {'x': 'y'}
+        region = self.create_region(name='region1', variables=region_vars)
+        variables = {"a": "b"}
+        self.create_host('host1', 'server', '192.168.1.1', region=region,
+                         **variables)
+        self.create_host('host2', 'server', '192.168.1.2', region=region,
+                         **variables)
+        url = self.url + '/v1/hosts?details=all'
+        resp = self.get(url)
+        self.assertEqual(200, resp.status_code)
+        hosts = resp.json()['hosts']
+        self.assertEqual(2, len(hosts))
+        for host in hosts:
+            self.assertTrue('variables' in host)
+            self.assertEqual({'a': 'b', 'x': 'y'}, host['variables'])
+
     def test_host_get_by_ip_filter(self):
         self.create_host('host1', 'server', '192.168.1.1')
         self.create_host('host2', 'server', '192.168.1.2')

craton/tests/functional/test_network_calls.py

@@ -104,3 +104,29 @@ class APIV1NetworkSchemaTest(TestCase):
         msg = ["Additional properties are not allowed ('updated_at' was "
                "unexpected)"]
         self.assertEqual(network.json()['errors'], msg)
+
+    def test_network_get_all_with_details(self):
+        payload = {
+            'cloud_id': self.cloud['id'],
+            'region_id': self.region['id'],
+            'name': 'a',
+            'cidr': self.cidr,
+            'netmask': self.netmask,
+            'gateway': self.gateway,
+            'variables': {'a': 'b'},
+        }
+        resp = self.post(self.networks_url, data=payload)
+        self.assertEqual(201, resp.status_code)
+
+        payload['name'] = 'b'
+        resp = self.post(self.networks_url, data=payload)
+        self.assertEqual(201, resp.status_code)
+
+        url = self.networks_url + '?details=all'
+        resp = self.get(url)
+        self.assertEqual(200, resp.status_code)
+        networks = resp.json()['networks']
+
+        for network in networks:
+            self.assertTrue('variables' in network)
+            self.assertEqual({'a': 'b'}, network['variables'])

craton/tests/functional/test_region_calls.py

@@ -118,6 +118,24 @@ class APIV1RegionTest(RegionTests):
         self.assertEqual(200, resp.status_code)
         self.assertEqual(2, len(resp.json()))
 
+    def test_regions_get_all_with_details(self):
+        self.create_region('ORD1', variables={'a': 'b'})
+        self.create_region('ORD2', variables={'c': 'd'})
+        url = self.url + '/v1/regions?details=all'
+        resp = self.get(url)
+        self.assertEqual(200, resp.status_code)
+        regions = resp.json()['regions']
+        self.assertEqual(2, len(regions))
+        for region in regions:
+            self.assertTrue('variables' in region)
+        for region in regions:
+            if region['name'] == 'ORD1':
+                self.assertEqual({'a': 'b', 'version': 'x'},
+                                 region['variables'])
+            if region['name'] == 'ORD2':
+                self.assertEqual({'c': 'd', 'version': 'x'},
+                                 region['variables'])
+
     def test_regions_get_all_with_name_filter(self):
         self.create_region("ORD1")
         self.create_region("ORD2")

craton/tests/unit/fake_resources.py

@@ -153,6 +153,7 @@ class Networks(object):
         self.gateway = gateway
         self.netmask = netmask
         self.variables = variables
+        self.resolved = copy.copy(variables)
         self.labels = labels
         self.cloud_id = cloud_id
         self.region_id = region_id

craton/tests/unit/test_api.py

@@ -252,7 +252,8 @@ class APIV1CellsTest(APIV1Test):
         resp = self.get('v1/cells')
         self.assertEqual(len(resp.json), len(fake_resources.CELL_LIST))
         mock_cells.assert_called_once_with(
-            mock.ANY, {}, {'limit': 30, 'marker': None},
+            mock.ANY, {'resolved-values': True},
+            {'limit': 30, 'marker': None},
         )
 
     @mock.patch.object(dbapi, 'cells_get_all')
@@ -932,7 +933,8 @@ class APIV1HostsTest(APIV1Test):
         resp = self.get('/v1/hosts')
         self.assertEqual(len(resp.json['hosts']), 3)
         fake_hosts.assert_called_once_with(
-            mock.ANY, {}, {'limit': 30, 'marker': None},
+            mock.ANY, {'resolved-values': True},
+            {'limit': 30, 'marker': None},
         )
 
     @mock.patch.object(dbapi, 'hosts_get_all')
@@ -949,6 +951,7 @@ class APIV1HostsTest(APIV1Test):
         ip_address = '10.10.0.1'
         filters = {
             'region_id': 1, 'ip_address': ip_address,
+            'resolved-values': True,
         }
         path_query = '/v1/hosts?region_id={}&ip_address={}'.format(
             region_id, ip_address
@@ -1312,7 +1315,8 @@ class APIV1NetworksTest(APIV1Test):
         resp = self.get('/v1/networks')
         self.assertEqual(len(resp.json['networks']), 3)
         fake_networks.assert_called_once_with(
-            mock.ANY, {}, {'limit': 30, 'marker': None},
+            mock.ANY, {'resolved-values': True, 'details': False},
+            {'limit': 30, 'marker': None},
         )
 
     @mock.patch.object(dbapi, 'networks_get_all')
@@ -1511,7 +1515,8 @@ class APIV1NetworkDevicesTest(APIV1Test):
     def test_get_network_devices_by_ip_address_filter(self, fake_devices):
         region_id = '1'
         ip_address = '10.10.0.1'
-        filters = {'region_id': region_id, 'ip_address': ip_address}
+        filters = {'region_id': region_id, 'ip_address': ip_address,
+                   'resolved-values': True}
         path_query = '/v1/network-devices?region_id={}&ip_address={}'.format(
             region_id, ip_address
         )
@@ -1538,7 +1543,8 @@ class APIV1NetworkDevicesTest(APIV1Test):
         resp = self.get('/v1/network-devices')
         self.assertEqual(len(resp.json), 2)
         fake_devices.assert_called_once_with(
-            mock.ANY, {}, {'limit': 30, 'marker': None},
+            mock.ANY, {'resolved-values': True},
+            {'limit': 30, 'marker': None},
         )
 
     @mock.patch.object(dbapi, 'network_devices_get_all')