From 233d7d5727443ecb0ccd0526a02ea0d0695d8805 Mon Sep 17 00:00:00 2001 From: Sam Betts Date: Thu, 9 Feb 2017 12:26:18 +0000 Subject: [PATCH] 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 / 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 --- api-ref/source/baremetal-api-v1-chassis.inc | 5 ++ api-ref/source/baremetal-api-v1-nodes.inc | 10 ++- .../source/baremetal-api-v1-portgroups.inc | 5 ++ api-ref/source/baremetal-api-v1-ports.inc | 5 ++ .../contributor/webapi-version-history.rst | 6 ++ ironic/api/controllers/v1/chassis.py | 21 ++++-- ironic/api/controllers/v1/node.py | 18 +++-- ironic/api/controllers/v1/port.py | 22 ++++-- ironic/api/controllers/v1/portgroup.py | 22 ++++-- ironic/api/controllers/v1/utils.py | 43 ++++++++++++ ironic/api/controllers/v1/versions.py | 4 +- ironic/common/release_mappings.py | 2 +- .../unit/api/controllers/v1/test_chassis.py | 43 ++++++++++++ .../unit/api/controllers/v1/test_node.py | 66 ++++++++++++++++++ .../unit/api/controllers/v1/test_port.py | 68 +++++++++++++++++++ .../unit/api/controllers/v1/test_portgroup.py | 49 +++++++++++++ ...etail_true_api_query-cb6944847830cd1a.yaml | 11 +++ 17 files changed, 370 insertions(+), 30 deletions(-) create mode 100644 releasenotes/notes/add_detail_true_api_query-cb6944847830cd1a.yaml diff --git a/api-ref/source/baremetal-api-v1-chassis.inc b/api-ref/source/baremetal-api-v1-chassis.inc index 39563dcac0..f52b9d5b7a 100644 --- a/api-ref/source/baremetal-api-v1-chassis.inc +++ b/api-ref/source/baremetal-api-v1-chassis.inc @@ -203,6 +203,10 @@ List chassis Lists all chassis. +.. versionadded:: 1.43 + Added the ``detail`` boolean request parameter. When specified ``True`` this + causes the response to include complete details about each chassis. + Normal response codes: 200 .. TODO: add error codes @@ -217,6 +221,7 @@ Request - sort_dir: sort_dir - sort_key: sort_key - fields: fields + - detail: detail Response Parameters ------------------- diff --git a/api-ref/source/baremetal-api-v1-nodes.inc b/api-ref/source/baremetal-api-v1-nodes.inc index e85d5f279e..6a6eaf1d14 100644 --- a/api-ref/source/baremetal-api-v1-nodes.inc +++ b/api-ref/source/baremetal-api-v1-nodes.inc @@ -132,7 +132,7 @@ and any defaults added for non-specified fields. Most fields default to "null" or "". The list and example below are representative of the response as of API -microversion 1.42. +microversion 1.43. .. rest_parameters:: parameters.yaml @@ -215,6 +215,10 @@ provision state, and maintenance setting for each Node. .. versionadded:: 1.42 Introduced the ``fault`` field. +.. versionadded:: 1.43 + Added the ``detail`` boolean request parameter. When specified ``True`` this + causes the response to include complete details about each node. + Normal response codes: 200 Error codes: 400,403,406 @@ -236,6 +240,7 @@ Request - marker: marker - sort_dir: sort_dir - sort_key: sort_key + - detail: detail Response -------- @@ -261,6 +266,9 @@ List Nodes Detailed .. rest_method:: GET /v1/nodes/detail +.. deprecated:: + Use ?detail=True query string instead. + Return a list of bare metal Nodes with complete details. Some filtering is possible by passing in flags with the request. diff --git a/api-ref/source/baremetal-api-v1-portgroups.inc b/api-ref/source/baremetal-api-v1-portgroups.inc index 6149ff054f..b1fc4dea2a 100644 --- a/api-ref/source/baremetal-api-v1-portgroups.inc +++ b/api-ref/source/baremetal-api-v1-portgroups.inc @@ -28,6 +28,10 @@ some parameters with the request. By default, this query will return the UUID, name and address for each Portgroup. +.. versionadded:: 1.43 + Added the ``detail`` boolean request parameter. When specified ``True`` this + causes the response to include complete details about each portgroup. + Normal response code: 200 Error codes: 400,401,403,404 @@ -44,6 +48,7 @@ Request - marker: marker - sort_dir: sort_dir - sort_key: sort_key + - detail: detail Response -------- diff --git a/api-ref/source/baremetal-api-v1-ports.inc b/api-ref/source/baremetal-api-v1-ports.inc index 88e08a53a4..db58297dde 100644 --- a/api-ref/source/baremetal-api-v1-ports.inc +++ b/api-ref/source/baremetal-api-v1-ports.inc @@ -42,6 +42,10 @@ By default, this query will return the uuid and address for each Port. .. versionadded:: 1.34 Added the ``physical_network`` field. +.. versionadded:: 1.43 + Added the ``detail`` boolean request parameter. When specified ``True`` this + causes the response to include complete details about each port. + Normal response code: 200 Request @@ -58,6 +62,7 @@ Request - marker: marker - sort_dir: sort_dir - sort_key: sort_key + - detail: detail Response -------- diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index a23d1dad57..b5b68fd8ed 100644 --- a/doc/source/contributor/webapi-version-history.rst +++ b/doc/source/contributor/webapi-version-history.rst @@ -2,6 +2,12 @@ REST API Version History ======================== +1.43 (Rocky, master) +--------------------- + +Added ``?detail=`` boolean query to the API list endpoints to provide a more +RESTful alternative to the existing ``/nodes/detail`` and similar endpoints. + 1.42 (Rocky, master) -------------------- diff --git a/ironic/api/controllers/v1/chassis.py b/ironic/api/controllers/v1/chassis.py index 467584494c..4c9f89f467 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 bcbdbb40f0..597c8ce4e0 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 9cdab746dc..3fc907b7c3 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 1fca2666a1..b48dd4b693 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 c740f5d4c0..070c72b64a 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 0a2812f19b..f1f61b0cbc 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) diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 0b928817e0..f616a7b71c 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -100,7 +100,7 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.42', + 'api': '1.43', 'rpc': '1.44', 'objects': { 'Node': ['1.25'], diff --git a/ironic/tests/unit/api/controllers/v1/test_chassis.py b/ironic/tests/unit/api/controllers/v1/test_chassis.py index 94e278c575..d41d6e8b07 100644 --- a/ironic/tests/unit/api/controllers/v1/test_chassis.py +++ b/ironic/tests/unit/api/controllers/v1/test_chassis.py @@ -123,6 +123,49 @@ class TestListChassis(test_api_base.BaseApiTest): self.assertIn('extra', data['chassis'][0]) self.assertIn('nodes', data['chassis'][0]) + def test_detail_query(self): + chassis = obj_utils.create_test_chassis(self.context) + data = self.get_json( + '/chassis?detail=True', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertEqual(chassis.uuid, data['chassis'][0]["uuid"]) + self.assertIn('extra', data['chassis'][0]) + self.assertIn('nodes', data['chassis'][0]) + + def test_detail_query_false(self): + obj_utils.create_test_chassis(self.context) + data1 = self.get_json( + '/chassis', + headers={api_base.Version.string: str(api_v1.max_version())}) + data2 = self.get_json( + '/chassis?detail=False', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertEqual(data1['chassis'], data2['chassis']) + + def test_detail_using_query_and_fields(self): + obj_utils.create_test_chassis(self.context) + response = self.get_json( + '/chassis?detail=True&fields=description', + headers={api_base.Version.string: str(api_v1.max_version())}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + + def test_detail_using_query_false_and_fields(self): + obj_utils.create_test_chassis(self.context) + data = self.get_json( + '/chassis?detail=False&fields=description', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertIn('description', data['chassis'][0]) + self.assertNotIn('uuid', data['chassis'][0]) + + def test_detail_using_query_old_version(self): + obj_utils.create_test_chassis(self.context) + response = self.get_json( + '/chassis?detail=True', + headers={api_base.Version.string: str(api_v1.min_version())}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + def test_detail_against_single(self): chassis = obj_utils.create_test_chassis(self.context) response = self.get_json('/chassis/%s/detail' % chassis['uuid'], diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 6f1a3e6db5..50f8b03410 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -430,6 +430,72 @@ class TestListNodes(test_api_base.BaseApiTest): # never expose the chassis_id self.assertNotIn('chassis_id', data['nodes'][0]) + def test_detail_using_query(self): + node = obj_utils.create_test_node(self.context, + chassis_id=self.chassis.id) + data = self.get_json( + '/nodes?detail=True', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertEqual(node.uuid, data['nodes'][0]["uuid"]) + self.assertIn('name', data['nodes'][0]) + self.assertIn('driver', data['nodes'][0]) + self.assertIn('driver_info', data['nodes'][0]) + self.assertIn('extra', data['nodes'][0]) + self.assertIn('properties', data['nodes'][0]) + self.assertIn('chassis_uuid', data['nodes'][0]) + self.assertIn('reservation', data['nodes'][0]) + self.assertIn('maintenance', data['nodes'][0]) + self.assertIn('console_enabled', data['nodes'][0]) + self.assertIn('target_power_state', data['nodes'][0]) + self.assertIn('target_provision_state', data['nodes'][0]) + self.assertIn('provision_updated_at', data['nodes'][0]) + self.assertIn('inspection_finished_at', data['nodes'][0]) + self.assertIn('inspection_started_at', data['nodes'][0]) + self.assertIn('raid_config', data['nodes'][0]) + self.assertIn('target_raid_config', data['nodes'][0]) + self.assertIn('network_interface', data['nodes'][0]) + self.assertIn('resource_class', data['nodes'][0]) + for field in api_utils.V31_FIELDS: + self.assertIn(field, data['nodes'][0]) + # never expose the chassis_id + self.assertNotIn('chassis_id', data['nodes'][0]) + + def test_detail_query_false(self): + obj_utils.create_test_node(self.context) + data1 = self.get_json( + '/nodes', + headers={api_base.Version.string: str(api_v1.max_version())}) + data2 = self.get_json( + '/nodes?detail=False', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertEqual(data1['nodes'], data2['nodes']) + + def test_detail_using_query_false_and_fields(self): + obj_utils.create_test_node(self.context) + data = self.get_json( + '/nodes?detail=False&fields=name', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertIn('name', data['nodes'][0]) + self.assertNotIn('uuid', data['nodes'][0]) + + def test_detail_using_query_and_fields(self): + obj_utils.create_test_node(self.context, + chassis_id=self.chassis.id) + response = self.get_json( + '/nodes?detail=True&fields=name', + headers={api_base.Version.string: str(api_v1.max_version())}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + + def test_detail_using_query_old_version(self): + obj_utils.create_test_node(self.context, + chassis_id=self.chassis.id) + response = self.get_json( + '/nodes?detail=True', + headers={api_base.Version.string: str(api_v1.min_version())}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + def test_detail_against_single(self): node = obj_utils.create_test_node(self.context) response = self.get_json('/nodes/%s/detail' % node.uuid, diff --git a/ironic/tests/unit/api/controllers/v1/test_port.py b/ironic/tests/unit/api/controllers/v1/test_port.py index 372a30bc77..eeee5b6ba7 100644 --- a/ironic/tests/unit/api/controllers/v1/test_port.py +++ b/ironic/tests/unit/api/controllers/v1/test_port.py @@ -442,6 +442,74 @@ class TestListPorts(test_api_base.BaseApiTest): self.assertNotIn('node_id', data['ports'][0]) self.assertNotIn('portgroup_id', data['ports'][0]) + def test_detail_query(self): + llc = {'switch_info': 'switch', 'switch_id': 'aa:bb:cc:dd:ee:ff', + 'port_id': 'Gig0/1'} + portgroup = obj_utils.create_test_portgroup(self.context, + node_id=self.node.id) + port = obj_utils.create_test_port(self.context, node_id=self.node.id, + portgroup_id=portgroup.id, + pxe_enabled=False, + local_link_connection=llc, + physical_network='physnet1') + data = self.get_json( + '/ports?detail=True', + headers={api_base.Version.string: str(api_v1.max_version())} + ) + self.assertEqual(port.uuid, data['ports'][0]["uuid"]) + self.assertIn('extra', data['ports'][0]) + self.assertIn('internal_info', data['ports'][0]) + self.assertIn('node_uuid', data['ports'][0]) + self.assertIn('pxe_enabled', data['ports'][0]) + self.assertIn('local_link_connection', data['ports'][0]) + self.assertIn('portgroup_uuid', data['ports'][0]) + self.assertIn('physical_network', data['ports'][0]) + # never expose the node_id and portgroup_id + self.assertNotIn('node_id', data['ports'][0]) + self.assertNotIn('portgroup_id', data['ports'][0]) + + def test_detail_query_false(self): + obj_utils.create_test_port(self.context, node_id=self.node.id, + pxe_enabled=False, + physical_network='physnet1') + data1 = self.get_json( + '/ports', + headers={api_base.Version.string: str(api_v1.max_version())}) + data2 = self.get_json( + '/ports?detail=False', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertEqual(data1['ports'], data2['ports']) + + def test_detail_using_query_false_and_fields(self): + obj_utils.create_test_port(self.context, node_id=self.node.id, + pxe_enabled=False, + physical_network='physnet1') + data = self.get_json( + '/ports?detail=False&fields=internal_info', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertIn('internal_info', data['ports'][0]) + self.assertNotIn('uuid', data['ports'][0]) + + def test_detail_using_query_and_fields(self): + obj_utils.create_test_port(self.context, node_id=self.node.id, + pxe_enabled=False, + physical_network='physnet1') + response = self.get_json( + '/ports?detail=True&fields=name', + headers={api_base.Version.string: str(api_v1.max_version())}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + + def test_detail_using_query_old_version(self): + obj_utils.create_test_port(self.context, node_id=self.node.id, + pxe_enabled=False, + physical_network='physnet1') + response = self.get_json( + '/ports?detail=True', + headers={api_base.Version.string: str(api_v1.min_version())}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + def test_detail_against_single(self): port = obj_utils.create_test_port(self.context, node_id=self.node.id) response = self.get_json('/ports/%s/detail' % port.uuid, diff --git a/ironic/tests/unit/api/controllers/v1/test_portgroup.py b/ironic/tests/unit/api/controllers/v1/test_portgroup.py index 006d433d73..38cd447d55 100644 --- a/ironic/tests/unit/api/controllers/v1/test_portgroup.py +++ b/ironic/tests/unit/api/controllers/v1/test_portgroup.py @@ -201,6 +201,55 @@ class TestListPortgroups(test_api_base.BaseApiTest): # never expose the node_id self.assertNotIn('node_id', data['portgroups'][0]) + def test_detail_query(self): + portgroup = obj_utils.create_test_portgroup(self.context, + node_id=self.node.id) + data = self.get_json('/portgroups?detail=True', headers=self.headers) + self.assertEqual(portgroup.uuid, data['portgroups'][0]["uuid"]) + self.assertIn('extra', data['portgroups'][0]) + self.assertIn('node_uuid', data['portgroups'][0]) + self.assertIn('standalone_ports_supported', data['portgroups'][0]) + # never expose the node_id + self.assertNotIn('node_id', data['portgroups'][0]) + + def test_detail_query_false(self): + obj_utils.create_test_portgroup(self.context, + node_id=self.node.id) + data1 = self.get_json( + '/portgroups', + headers={api_base.Version.string: str(api_v1.max_version())}) + data2 = self.get_json( + '/portgroups?detail=False', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertEqual(data1['portgroups'], data2['portgroups']) + + def test_detail_using_query_false_and_fields(self): + obj_utils.create_test_portgroup(self.context, + node_id=self.node.id) + data = self.get_json( + '/portgroups?detail=False&fields=internal_info', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertIn('internal_info', data['portgroups'][0]) + self.assertNotIn('uuid', data['portgroups'][0]) + + def test_detail_using_query_and_fields(self): + obj_utils.create_test_portgroup(self.context, + node_id=self.node.id) + response = self.get_json( + '/portgroups?detail=True&fields=name', + headers={api_base.Version.string: str(api_v1.max_version())}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + + def test_detail_using_query_old_version(self): + obj_utils.create_test_portgroup(self.context, + node_id=self.node.id) + response = self.get_json( + '/portgroups?detail=True', + headers={api_base.Version.string: '1.42'}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + def test_detail_invalid_api_version(self): response = self.get_json( '/portgroups/detail', diff --git a/releasenotes/notes/add_detail_true_api_query-cb6944847830cd1a.yaml b/releasenotes/notes/add_detail_true_api_query-cb6944847830cd1a.yaml new file mode 100644 index 0000000000..d3c8eb3b99 --- /dev/null +++ b/releasenotes/notes/add_detail_true_api_query-cb6944847830cd1a.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + Add ``?detail=`` boolean query to the API list endpoints to provide a more + RESTful alternative to the existing ``/nodes/detail`` and similar endpoints. The + default is False. Now these API requests are possible: + + * ``/nodes?detail=True`` + * ``/ports?detail=True`` + * ``/chassis?detail=True`` + * ``/portgroups?detail=True``