Add detail=[True, False] query string to API list endpoints

We currently support /[nodes, ports, portgroups, chassis]/detail as an
API endpoint for getting a detailed list of each object. This does not
fit the RESTful and resourceful API design principles of <resource
type>/<resource id> and makes it hard to consume the API from frameworks
that expect that structure. We can't remove the old endpoint, so that is
safeguarded by the restricted node names list. This patch adds a
?detail=[True, False] query string to the list API endpoints to allow
those consuming the API to use the expected URL form.

Change-Id: I694919b4a4eaa3419318bbee1cde79de15e19afa
Story: #1662921
Task: #10176

6 files changed, 102 insertions, 28 deletions

diff --git a/ironic/api/controllers/v1/chassis.py b/ironic/api/controllers/v1/chassis.py
index 467584494..4c9f89f46 100644
--- a/ironic/api/controllers/v1/chassis.py
+++ b/ironic/api/controllers/v1/chassis.py
@@ -174,7 +174,7 @@ class ChassisController(rest.RestController):
     invalid_sort_key_list = ['extra']
 
     def _get_chassis_collection(self, marker, limit, sort_key, sort_dir,
-                                resource_url=None, fields=None):
+                                resource_url=None, fields=None, detail=None):
         limit = api_utils.validate_limit(limit)
         sort_dir = api_utils.validate_sort_dir(sort_dir)
         marker_obj = None
@@ -190,17 +190,22 @@ class ChassisController(rest.RestController):
         chassis = objects.Chassis.list(pecan.request.context, limit,
                                        marker_obj, sort_key=sort_key,
                                        sort_dir=sort_dir)
+        parameters = {}
+        if detail is not None:
+            parameters['detail'] = detail
+
         return ChassisCollection.convert_with_links(chassis, limit,
                                                     url=resource_url,
                                                     fields=fields,
                                                     sort_key=sort_key,
-                                                    sort_dir=sort_dir)
+                                                    sort_dir=sort_dir,
+                                                    **parameters)
 
     @METRICS.timer('ChassisController.get_all')
     @expose.expose(ChassisCollection, types.uuid, int,
-                   wtypes.text, wtypes.text, types.listtype)
+                   wtypes.text, wtypes.text, types.listtype, types.boolean)
     def get_all(self, marker=None, limit=None, sort_key='id', sort_dir='asc',
-                fields=None):
+                fields=None, detail=None):
         """Retrieve a list of chassis.
 
         :param marker: pagination marker for large data sets.
@@ -217,10 +222,12 @@ class ChassisController(rest.RestController):
         policy.authorize('baremetal:chassis:get', cdict, cdict)
 
         api_utils.check_allow_specify_fields(fields)
-        if fields is None:
-            fields = _DEFAULT_RETURN_FIELDS
+
+        fields = api_utils.get_request_return_fields(fields, detail,
+                                                     _DEFAULT_RETURN_FIELDS)
+
         return self._get_chassis_collection(marker, limit, sort_key, sort_dir,
-                                            fields=fields)
+                                            fields=fields, detail=detail)
 
     @METRICS.timer('ChassisController.detail')
     @expose.expose(ChassisCollection, types.uuid, int,
diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py
index bcbdbb40f..597c8ce4e 100644
--- a/ironic/api/controllers/v1/node.py
+++ b/ironic/api/controllers/v1/node.py
@@ -1524,7 +1524,7 @@ class NodesController(rest.RestController):
                               maintenance, provision_state, marker, limit,
                               sort_key, sort_dir, driver=None,
                               resource_class=None, resource_url=None,
-                              fields=None, fault=None):
+                              fields=None, fault=None, detail=None):
         if self.from_chassis and not chassis_uuid:
             raise exception.MissingParameterValue(
                 _("Chassis id not specified."))
@@ -1583,6 +1583,9 @@ class NodesController(rest.RestController):
             if maintenance:
                 parameters['maintenance'] = maintenance
 
+        if detail is not None:
+            parameters['detail'] = detail
+
         return NodeCollection.convert_with_links(nodes, limit,
                                                  url=resource_url,
                                                  fields=fields,
@@ -1676,11 +1679,11 @@ class NodesController(rest.RestController):
     @expose.expose(NodeCollection, types.uuid, types.uuid, types.boolean,
                    types.boolean, wtypes.text, types.uuid, int, wtypes.text,
                    wtypes.text, wtypes.text, types.listtype, wtypes.text,
-                   wtypes.text)
+                   wtypes.text, types.boolean)
     def get_all(self, chassis_uuid=None, instance_uuid=None, associated=None,
                 maintenance=None, provision_state=None, marker=None,
                 limit=None, sort_key='id', sort_dir='asc', driver=None,
-                fields=None, resource_class=None, fault=None):
+                fields=None, resource_class=None, fault=None, detail=None):
         """Retrieve a list of nodes.
 
         :param chassis_uuid: Optional UUID of a chassis, to get only nodes for
@@ -1720,15 +1723,18 @@ class NodesController(rest.RestController):
         api_utils.check_allow_specify_driver(driver)
         api_utils.check_allow_specify_resource_class(resource_class)
         api_utils.check_allow_filter_by_fault(fault)
-        if fields is None:
-            fields = _DEFAULT_RETURN_FIELDS
+
+        fields = api_utils.get_request_return_fields(fields, detail,
+                                                     _DEFAULT_RETURN_FIELDS)
+
         return self._get_nodes_collection(chassis_uuid, instance_uuid,
                                           associated, maintenance,
                                           provision_state, marker,
                                           limit, sort_key, sort_dir,
                                           driver=driver,
                                           resource_class=resource_class,
-                                          fields=fields, fault=fault)
+                                          fields=fields, fault=fault,
+                                          detail=detail)
 
     @METRICS.timer('NodesController.detail')
     @expose.expose(NodeCollection, types.uuid, types.uuid, types.boolean,
diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py
index 9cdab746d..3fc907b7c 100644
--- a/ironic/api/controllers/v1/port.py
+++ b/ironic/api/controllers/v1/port.py
@@ -314,7 +314,7 @@ class PortsController(rest.RestController):
 
     def _get_ports_collection(self, node_ident, address, portgroup_ident,
                               marker, limit, sort_key, sort_dir,
-                              resource_url=None, fields=None):
+                              resource_url=None, fields=None, detail=None):
 
         limit = api_utils.validate_limit(limit)
         sort_dir = api_utils.validate_sort_dir(sort_dir)
@@ -362,12 +362,17 @@ class PortsController(rest.RestController):
             ports = objects.Port.list(pecan.request.context, limit,
                                       marker_obj, sort_key=sort_key,
                                       sort_dir=sort_dir)
+        parameters = {}
+
+        if detail is not None:
+            parameters['detail'] = detail
 
         return PortCollection.convert_with_links(ports, limit,
                                                  url=resource_url,
                                                  fields=fields,
                                                  sort_key=sort_key,
-                                                 sort_dir=sort_dir)
+                                                 sort_dir=sort_dir,
+                                                 **parameters)
 
     def _get_ports_by_address(self, address):
         """Retrieve a port by its address.
@@ -407,10 +412,11 @@ class PortsController(rest.RestController):
     @METRICS.timer('PortsController.get_all')
     @expose.expose(PortCollection, types.uuid_or_name, types.uuid,
                    types.macaddress, types.uuid, int, wtypes.text,
-                   wtypes.text, types.listtype, types.uuid_or_name)
+                   wtypes.text, types.listtype, types.uuid_or_name,
+                   types.boolean)
     def get_all(self, node=None, node_uuid=None, address=None, marker=None,
                 limit=None, sort_key='id', sort_dir='asc', fields=None,
-                portgroup=None):
+                portgroup=None, detail=None):
         """Retrieve a list of ports.
 
         Note that the 'node_uuid' interface is deprecated in favour
@@ -441,11 +447,12 @@ class PortsController(rest.RestController):
         api_utils.check_allow_specify_fields(fields)
         self._check_allowed_port_fields(fields)
         self._check_allowed_port_fields([sort_key])
+
         if portgroup and not api_utils.allow_portgroups_subcontrollers():
             raise exception.NotAcceptable()
 
-        if fields is None:
-            fields = _DEFAULT_RETURN_FIELDS
+        fields = api_utils.get_request_return_fields(fields, detail,
+                                                     _DEFAULT_RETURN_FIELDS)
 
         if not node_uuid and node:
             # We're invoking this interface using positional notation, or
@@ -457,7 +464,8 @@ class PortsController(rest.RestController):
 
         return self._get_ports_collection(node_uuid or node, address,
                                           portgroup, marker, limit, sort_key,
-                                          sort_dir, fields=fields)
+                                          sort_dir, fields=fields,
+                                          detail=detail)
 
     @METRICS.timer('PortsController.detail')
     @expose.expose(PortCollection, types.uuid_or_name, types.uuid,
diff --git a/ironic/api/controllers/v1/portgroup.py b/ironic/api/controllers/v1/portgroup.py
index 1fca2666a..b48dd4b69 100644
--- a/ironic/api/controllers/v1/portgroup.py
+++ b/ironic/api/controllers/v1/portgroup.py
@@ -262,7 +262,8 @@ class PortgroupsController(pecan.rest.RestController):
 
     def _get_portgroups_collection(self, node_ident, address,
                                    marker, limit, sort_key, sort_dir,
-                                   resource_url=None, fields=None):
+                                   resource_url=None, fields=None,
+                                   detail=None):
         """Return portgroups collection.
 
         :param node_ident: UUID or name of a node.
@@ -305,12 +306,16 @@ class PortgroupsController(pecan.rest.RestController):
             portgroups = objects.Portgroup.list(pecan.request.context, limit,
                                                 marker_obj, sort_key=sort_key,
                                                 sort_dir=sort_dir)
+        parameters = {}
+        if detail is not None:
+            parameters['detail'] = detail
 
         return PortgroupCollection.convert_with_links(portgroups, limit,
                                                       url=resource_url,
                                                       fields=fields,
                                                       sort_key=sort_key,
-                                                      sort_dir=sort_dir)
+                                                      sort_dir=sort_dir,
+                                                      **parameters)
 
     def _get_portgroups_by_address(self, address):
         """Retrieve a portgroup by its address.
@@ -330,9 +335,11 @@ class PortgroupsController(pecan.rest.RestController):
 
     @METRICS.timer('PortgroupsController.get_all')
     @expose.expose(PortgroupCollection, types.uuid_or_name, types.macaddress,
-                   types.uuid, int, wtypes.text, wtypes.text, types.listtype)
+                   types.uuid, int, wtypes.text, wtypes.text, types.listtype,
+                   types.boolean)
     def get_all(self, node=None, address=None, marker=None,
-                limit=None, sort_key='id', sort_dir='asc', fields=None):
+                limit=None, sort_key='id', sort_dir='asc', fields=None,
+                detail=None):
         """Retrieve a list of portgroups.
 
         :param node: UUID or name of a node, to get only portgroups for that
@@ -358,13 +365,14 @@ class PortgroupsController(pecan.rest.RestController):
         api_utils.check_allowed_portgroup_fields(fields)
         api_utils.check_allowed_portgroup_fields([sort_key])
 
-        if fields is None:
-            fields = _DEFAULT_RETURN_FIELDS
+        fields = api_utils.get_request_return_fields(fields, detail,
+                                                     _DEFAULT_RETURN_FIELDS)
 
         return self._get_portgroups_collection(node, address,
                                                marker, limit,
                                                sort_key, sort_dir,
-                                               fields=fields)
+                                               fields=fields,
+                                               detail=detail)
 
     @METRICS.timer('PortgroupsController.detail')
     @expose.expose(PortgroupCollection, types.uuid_or_name, types.macaddress,
diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py
index c740f5d4c..070c72b64 100644
--- a/ironic/api/controllers/v1/utils.py
+++ b/ironic/api/controllers/v1/utils.py
@@ -850,3 +850,46 @@ def handle_patch_port_like_extra_vif(rpc_object, api_object, patch):
         int_info = rpc_object.internal_info.get('tenant_vif_port_id')
         if (int_info and int_info == rpc_object.extra.get('vif_port_id')):
             api_object.internal_info.pop('tenant_vif_port_id')
+
+
+def allow_detail_query():
+    """Check if passing a detail=True query string is allowed.
+
+    Version 1.43 allows a user to pass the detail query string to
+    list the resource with all the fields.
+    """
+    return (pecan.request.version.minor >=
+            versions.MINOR_43_ENABLE_DETAIL_QUERY)
+
+
+def get_request_return_fields(fields, detail, default_fields):
+    """Calculate fields to return from an API request
+
+    The fields query and detail=True query can not be passed into a request at
+    the same time. To use the detail query we need to be on a version of the
+    API greater than 1.43. This function raises an InvalidParameterValue
+    exception if either of these conditions are not met.
+
+    If these checks pass then this function will return either the fields
+    passed in or the default fields provided.
+
+    :param fields: The fields query passed into the API request.
+    :param detail: The detail query passed into the API request.
+    :param default_fields: The default fields to return if fields=None and
+        detail=None.
+    :raises: InvalidParameterValue if there is an invalid combination of query
+        strings or API version.
+    :returns: 'fields' passed in value or 'default_fields'
+    """
+
+    if detail is not None and not allow_detail_query():
+        raise exception.InvalidParameterValue(
+            "Invalid query parameter ?detail=%s received." % detail)
+
+    if fields is not None and detail:
+        raise exception.InvalidParameterValue(
+            "Can not specify ?detail=True and fields in the same request.")
+
+    if fields is None and not detail:
+        return default_fields
+    return fields
diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py
index 0a2812f19..f1f61b0cb 100644
--- a/ironic/api/controllers/v1/versions.py
+++ b/ironic/api/controllers/v1/versions.py
@@ -80,6 +80,7 @@ BASE_VERSION = 1
 #        Add bios_interface to the node object.
 # v1.41: Add inspection abort support.
 # v1.42: Expose fault field to node.
+# v1.43: Add detail=True flag to all API endpoints
 
 MINOR_0_JUNO = 0
 MINOR_1_INITIAL_VERSION = 1
@@ -124,6 +125,7 @@ MINOR_39_INSPECT_WAIT = 39
 MINOR_40_BIOS_INTERFACE = 40
 MINOR_41_INSPECTION_ABORT = 41
 MINOR_42_FAULT = 42
+MINOR_43_ENABLE_DETAIL_QUERY = 43
 
 # When adding another version, update:
 # - MINOR_MAX_VERSION
@@ -131,7 +133,7 @@ MINOR_42_FAULT = 42
 #   explanation of what changed in the new version
 # - common/release_mappings.py, RELEASE_MAPPING['master']['api']
 
-MINOR_MAX_VERSION = MINOR_42_FAULT
+MINOR_MAX_VERSION = MINOR_43_ENABLE_DETAIL_QUERY
 
 # String representations of the minor and maximum versions
 _MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION)