From 02a3e77a3e06fcdf492fd36540d812843e1ec839 Mon Sep 17 00:00:00 2001 From: Will Szumski Date: Mon, 10 Sep 2018 15:08:19 +0100 Subject: [PATCH] Ensure pagination marker is always set Previously the pagination marker was not being set for list queries where `uuid` was not in the list of requested fields. The affected endpoints were: port, portgroup, volume_target, volume_connector, node and chassis. The next marker contains the UUID of the last element that was returned to the client. This information was being removed from elements in the collection. This meant that we could no longer return a UUID for this marker. An extra parameter, `sanitize` was added to `convert_with_links`. This allows us to delay stripping this field from the elements until after the next marker has been set. Story: 2003192 Task: 23346 Change-Id: Ied45f698c3431c5113bae46c1a4f1de1cdaa2d74 --- ironic/api/controllers/v1/chassis.py | 45 ++++--- ironic/api/controllers/v1/node.py | 111 +++++++++++------- ironic/api/controllers/v1/port.py | 77 +++++++----- ironic/api/controllers/v1/portgroup.py | 52 +++++--- ironic/api/controllers/v1/volume_connector.py | 48 +++++--- ironic/api/controllers/v1/volume_target.py | 46 +++++--- .../unit/api/controllers/v1/test_chassis.py | 17 +++ .../unit/api/controllers/v1/test_node.py | 17 +++ .../unit/api/controllers/v1/test_port.py | 20 ++++ .../unit/api/controllers/v1/test_portgroup.py | 20 ++++ .../controllers/v1/test_volume_connector.py | 19 +++ .../api/controllers/v1/test_volume_target.py | 17 +++ ...h-custom-field-query-65ca29001a03e036.yaml | 8 ++ 13 files changed, 367 insertions(+), 130 deletions(-) create mode 100644 releasenotes/notes/fix-pagination-marker-with-custom-field-query-65ca29001a03e036.yaml diff --git a/ironic/api/controllers/v1/chassis.py b/ironic/api/controllers/v1/chassis.py index 4c9f89f467..8f665a340a 100644 --- a/ironic/api/controllers/v1/chassis.py +++ b/ironic/api/controllers/v1/chassis.py @@ -76,42 +76,54 @@ class Chassis(base.APIBase): @staticmethod def _convert_with_links(chassis, url, fields=None): - # NOTE(lucasagomes): Since we are able to return a specified set of - # fields the "uuid" can be unset, so we need to save it in another - # variable to use when building the links - chassis_uuid = chassis.uuid - if fields is not None: - chassis.unset_fields_except(fields) - else: + if fields is None: chassis.nodes = [link.Link.make_link('self', url, 'chassis', - chassis_uuid + "/nodes"), + chassis.uuid + "/nodes"), link.Link.make_link('bookmark', url, 'chassis', - chassis_uuid + "/nodes", + chassis.uuid + "/nodes", bookmark=True) ] chassis.links = [link.Link.make_link('self', url, - 'chassis', chassis_uuid), + 'chassis', chassis.uuid), link.Link.make_link('bookmark', url, - 'chassis', chassis_uuid, + 'chassis', chassis.uuid, bookmark=True) ] return chassis @classmethod - def convert_with_links(cls, rpc_chassis, fields=None): + def convert_with_links(cls, rpc_chassis, fields=None, sanitize=True): chassis = Chassis(**rpc_chassis.as_dict()) if fields is not None: api_utils.check_for_invalid_fields(fields, chassis.as_dict()) - return cls._convert_with_links(chassis, pecan.request.public_url, - fields) + chassis = cls._convert_with_links(chassis, pecan.request.public_url, + fields) + + if not sanitize: + return chassis + + chassis.sanitize(fields) + return chassis + + def sanitize(self, fields=None): + """Removes sensitive and unrequested data. + + Will only keep the fields specified in the ``fields`` parameter. + + :param fields: + list of fields to preserve, or ``None`` to preserve them all + :type fields: list of str + """ + if fields is not None: + self.unset_fields_except(fields) @classmethod def sample(cls, expand=True): @@ -141,10 +153,13 @@ class ChassisCollection(collection.Collection): @staticmethod def convert_with_links(chassis, limit, url=None, fields=None, **kwargs): collection = ChassisCollection() - collection.chassis = [Chassis.convert_with_links(ch, fields=fields) + collection.chassis = [Chassis.convert_with_links(ch, fields=fields, + sanitize=False) for ch in chassis] url = url or None collection.next = collection.get_next(limit, url=url, **kwargs) + for item in collection.chassis: + item.sanitize(fields) return collection @classmethod diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 1808a4f942..9f512028fb 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1126,59 +1126,75 @@ class Node(base.APIBase): @staticmethod def _convert_with_links(node, url, fields=None, show_states_links=True, show_portgroups=True, show_volume=True): - # NOTE(lucasagomes): Since we are able to return a specified set of - # fields the "uuid" can be unset, so we need to save it in another - # variable to use when building the links - node_uuid = node.uuid - if fields is not None: - node.unset_fields_except(fields) - else: + if fields is None: node.ports = [link.Link.make_link('self', url, 'nodes', - node_uuid + "/ports"), + node.uuid + "/ports"), link.Link.make_link('bookmark', url, 'nodes', - node_uuid + "/ports", + node.uuid + "/ports", bookmark=True) ] if show_states_links: node.states = [link.Link.make_link('self', url, 'nodes', - node_uuid + "/states"), + node.uuid + "/states"), link.Link.make_link('bookmark', url, 'nodes', - node_uuid + "/states", + node.uuid + "/states", bookmark=True)] if show_portgroups: node.portgroups = [ link.Link.make_link('self', url, 'nodes', - node_uuid + "/portgroups"), + node.uuid + "/portgroups"), link.Link.make_link('bookmark', url, 'nodes', - node_uuid + "/portgroups", + node.uuid + "/portgroups", bookmark=True)] if show_volume: node.volume = [ link.Link.make_link('self', url, 'nodes', - node_uuid + "/volume"), + node.uuid + "/volume"), link.Link.make_link('bookmark', url, 'nodes', - node_uuid + "/volume", + node.uuid + "/volume", bookmark=True)] - # NOTE(lucasagomes): The numeric ID should not be exposed to - # the user, it's internal only. - node.chassis_id = wtypes.Unset - node.links = [link.Link.make_link('self', url, 'nodes', - node_uuid), + node.uuid), link.Link.make_link('bookmark', url, 'nodes', - node_uuid, bookmark=True) + node.uuid, bookmark=True) ] return node @classmethod - def convert_with_links(cls, rpc_node, fields=None): + def convert_with_links(cls, rpc_node, fields=None, sanitize=True): node = Node(**rpc_node.as_dict()) if fields is not None: api_utils.check_for_invalid_fields(fields, node.as_dict()) + show_states_links = ( + api_utils.allow_links_node_states_and_driver_properties()) + show_portgroups = api_utils.allow_portgroups_subcontrollers() + show_volume = api_utils.allow_volume() + + node = cls._convert_with_links(node, pecan.request.public_url, + fields=fields, + show_states_links=show_states_links, + show_portgroups=show_portgroups, + show_volume=show_volume) + if not sanitize: + return node + + node.sanitize(fields) + + return node + + def sanitize(self, fields): + """Removes sensitive and unrequested data. + + Will only keep the fields specified in the ``fields`` parameter. + + :param fields: + list of fields to preserve, or ``None`` to preserve them all + :type fields: list of str + """ cdict = pecan.request.context.to_policy_values() # NOTE(deva): the 'show_password' policy setting name exists for legacy # purposes and can not be changed. Changing it will cause @@ -1188,39 +1204,49 @@ class Node(base.APIBase): show_instance_secrets = policy.check("show_instance_secrets", cdict, cdict) - if not show_driver_secrets and node.driver_info != wtypes.Unset: - node.driver_info = strutils.mask_dict_password( - node.driver_info, "******") + if not show_driver_secrets and self.driver_info != wtypes.Unset: + self.driver_info = strutils.mask_dict_password( + self.driver_info, "******") # NOTE(derekh): mask ssh keys for the ssh power driver. # As this driver is deprecated masking here (opposed to strutils) # is simpler, and easier to backport. This can be removed along # with support for the ssh power driver. - if node.driver_info.get('ssh_key_contents'): - node.driver_info['ssh_key_contents'] = "******" + if self.driver_info.get('ssh_key_contents'): + self.driver_info['ssh_key_contents'] = "******" - if not show_instance_secrets and node.instance_info != wtypes.Unset: - node.instance_info = strutils.mask_dict_password( - node.instance_info, "******") + if not show_instance_secrets and self.instance_info != wtypes.Unset: + self.instance_info = strutils.mask_dict_password( + self.instance_info, "******") # NOTE(deva): agent driver may store a swift temp_url on the # instance_info, which shouldn't be exposed to non-admin users. # Now that ironic supports additional policies, we need to hide # it here, based on this policy. # Related to bug #1613903 - if node.instance_info.get('image_url'): - node.instance_info['image_url'] = "******" + if self.instance_info.get('image_url'): + self.instance_info['image_url'] = "******" + + update_state_in_older_versions(self) + hide_fields_in_newer_versions(self) + + if fields is not None: + self.unset_fields_except(fields) + + # NOTE(lucasagomes): The numeric ID should not be exposed to + # the user, it's internal only. + self.chassis_id = wtypes.Unset - update_state_in_older_versions(node) - hide_fields_in_newer_versions(node) show_states_links = ( api_utils.allow_links_node_states_and_driver_properties()) show_portgroups = api_utils.allow_portgroups_subcontrollers() show_volume = api_utils.allow_volume() - return cls._convert_with_links(node, pecan.request.public_url, - fields=fields, - show_states_links=show_states_links, - show_portgroups=show_portgroups, - show_volume=show_volume) + + if not show_volume: + self.volume = wtypes.Unset + if not show_portgroups: + self.portgroups = wtypes.Unset + if not show_states_links: + self.states = wtypes.Unset @classmethod def sample(cls, expand=True): @@ -1290,9 +1316,14 @@ class NodeCollection(collection.Collection): @staticmethod def convert_with_links(nodes, limit, url=None, fields=None, **kwargs): collection = NodeCollection() - collection.nodes = [Node.convert_with_links(n, fields=fields) + collection.nodes = [Node.convert_with_links(n, fields=fields, + sanitize=False) for n in nodes] collection.next = collection.get_next(limit, url=url, **kwargs) + + for node in collection.nodes: + node.sanitize(fields) + return collection @classmethod diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index 3fc907b7c3..b72f0ddbf5 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -186,40 +186,51 @@ class Port(base.APIBase): setattr(self, 'portgroup_uuid', kwargs.get('portgroup_id', wtypes.Unset)) - @staticmethod - def _convert_with_links(port, url, fields=None): - # NOTE(lucasagomes): Since we are able to return a specified set of - # fields the "uuid" can be unset, so we need to save it in another - # variable to use when building the links - port_uuid = port.uuid - if fields is not None: - port.unset_fields_except(fields) - - # never expose the node_id attribute - port.node_id = wtypes.Unset - - # never expose the portgroup_id attribute - port.portgroup_id = wtypes.Unset - - port.links = [link.Link.make_link('self', url, - 'ports', port_uuid), - link.Link.make_link('bookmark', url, - 'ports', port_uuid, - bookmark=True) - ] - return port - @classmethod - def convert_with_links(cls, rpc_port, fields=None): + def convert_with_links(cls, rpc_port, fields=None, sanitize=True): port = Port(**rpc_port.as_dict()) + port._validate_fields(fields) + + url = pecan.request.public_url + + port.links = [link.Link.make_link('self', url, + 'ports', port.uuid), + link.Link.make_link('bookmark', url, + 'ports', port.uuid, + bookmark=True) + ] + + if not sanitize: + return port + + port.sanitize(fields=fields) + + return port + + def _validate_fields(self, fields=None): if fields is not None: - api_utils.check_for_invalid_fields(fields, port.as_dict()) + api_utils.check_for_invalid_fields(fields, self.as_dict()) - hide_fields_in_newer_versions(port) + def sanitize(self, fields=None): + """Removes sensitive and unrequested data. - return cls._convert_with_links(port, pecan.request.public_url, - fields=fields) + Will only keep the fields specified in the ``fields`` parameter. + + :param fields: + list of fields to preserve, or ``None`` to preserve them all + :type fields: list of str + """ + hide_fields_in_newer_versions(self) + + if fields is not None: + self.unset_fields_except(fields) + + # never expose the node_id attribute + self.node_id = wtypes.Unset + + # never expose the portgroup_id attribute + self.portgroup_id = wtypes.Unset @classmethod def sample(cls, expand=True): @@ -268,7 +279,8 @@ class PortCollection(collection.Collection): collection.ports = [] for rpc_port in rpc_ports: try: - port = Port.convert_with_links(rpc_port, fields=fields) + port = Port.convert_with_links(rpc_port, fields=fields, + sanitize=False) except exception.NodeNotFound: # NOTE(dtantsur): node was deleted after we fetched the port # list, meaning that the port was also deleted. Skip it. @@ -282,11 +294,16 @@ class PortCollection(collection.Collection): LOG.debug('Removing port group UUID from port %s as the port ' 'group was deleted', rpc_port.uuid) rpc_port.portgroup_id = None - port = Port.convert_with_links(rpc_port, fields=fields) + port = Port.convert_with_links(rpc_port, fields=fields, + sanitize=False) collection.ports.append(port) collection.next = collection.get_next(limit, url=url, **kwargs) + + for item in collection.ports: + item.sanitize(fields=fields) + return collection @classmethod diff --git a/ironic/api/controllers/v1/portgroup.py b/ironic/api/controllers/v1/portgroup.py index b48dd4b693..0091c4c031 100644 --- a/ironic/api/controllers/v1/portgroup.py +++ b/ironic/api/controllers/v1/portgroup.py @@ -131,41 +131,58 @@ class Portgroup(base.APIBase): @staticmethod def _convert_with_links(portgroup, url, fields=None): """Add links to the portgroup.""" - # NOTE(lucasagomes): Since we are able to return a specified set of - # fields the "uuid" can be unset, so we need to save it in another - # variable to use when building the links - portgroup_uuid = portgroup.uuid - if fields is not None: - portgroup.unset_fields_except(fields) - else: + if fields is None: portgroup.ports = [ link.Link.make_link('self', url, 'portgroups', - portgroup_uuid + "/ports"), + portgroup.uuid + "/ports"), link.Link.make_link('bookmark', url, 'portgroups', - portgroup_uuid + "/ports", bookmark=True) + portgroup.uuid + "/ports", bookmark=True) ] # never expose the node_id attribute portgroup.node_id = wtypes.Unset portgroup.links = [link.Link.make_link('self', url, - 'portgroups', portgroup_uuid), + 'portgroups', portgroup.uuid), link.Link.make_link('bookmark', url, - 'portgroups', portgroup_uuid, + 'portgroups', portgroup.uuid, bookmark=True) ] return portgroup @classmethod - def convert_with_links(cls, rpc_portgroup, fields=None): + def convert_with_links(cls, rpc_portgroup, fields=None, sanitize=True): """Add links to the portgroup.""" portgroup = Portgroup(**rpc_portgroup.as_dict()) if fields is not None: api_utils.check_for_invalid_fields(fields, portgroup.as_dict()) - return cls._convert_with_links(portgroup, pecan.request.host_url, - fields=fields) + portgroup = cls._convert_with_links(portgroup, pecan.request.host_url, + fields=fields) + + if not sanitize: + return portgroup + + portgroup.sanitize(fields) + + return portgroup + + def sanitize(self, fields=None): + """Removes sensitive and unrequested data. + + Will only keep the fields specified in the ``fields`` parameter. + + :param fields: + list of fields to preserve, or ``None`` to preserve them all + :type fields: list of str + """ + + if fields is not None: + self.unset_fields_except(fields) + + # never expose the node_id attribute + self.node_id = wtypes.Unset @classmethod def sample(cls, expand=True): @@ -212,9 +229,14 @@ class PortgroupCollection(collection.Collection): def convert_with_links(rpc_portgroups, limit, url=None, fields=None, **kwargs): collection = PortgroupCollection() - collection.portgroups = [Portgroup.convert_with_links(p, fields=fields) + collection.portgroups = [Portgroup.convert_with_links(p, fields=fields, + sanitize=False) for p in rpc_portgroups] collection.next = collection.get_next(limit, url=url, **kwargs) + + for item in collection.portgroups: + item.sanitize(fields=fields) + return collection @classmethod diff --git a/ironic/api/controllers/v1/volume_connector.py b/ironic/api/controllers/v1/volume_connector.py index b3d3421990..ede708f0aa 100644 --- a/ironic/api/controllers/v1/volume_connector.py +++ b/ironic/api/controllers/v1/volume_connector.py @@ -117,35 +117,50 @@ class VolumeConnector(base.APIBase): wtypes.Unset) @staticmethod - def _convert_with_links(connector, url, fields=None): - # NOTE(lucasagomes): Since we are able to return a specified set of - # fields the "uuid" can be unset, so we need to save it in another - # variable to use when building the links - connector_uuid = connector.uuid - if fields is not None: - connector.unset_fields_except(fields) + def _convert_with_links(connector, url): - # never expose the node_id attribute - connector.node_id = wtypes.Unset connector.links = [link.Link.make_link('self', url, 'volume/connectors', - connector_uuid), + connector.uuid), link.Link.make_link('bookmark', url, 'volume/connectors', - connector_uuid, + connector.uuid, bookmark=True) ] return connector @classmethod - def convert_with_links(cls, rpc_connector, fields=None): + def convert_with_links(cls, rpc_connector, fields=None, sanitize=True): connector = VolumeConnector(**rpc_connector.as_dict()) if fields is not None: api_utils.check_for_invalid_fields(fields, connector.as_dict()) - return cls._convert_with_links(connector, pecan.request.public_url, - fields=fields) + connector = cls._convert_with_links(connector, + pecan.request.public_url) + + if not sanitize: + return connector + + connector.sanitize(fields) + + return connector + + def sanitize(self, fields=None): + """Removes sensitive and unrequested data. + + Will only keep the fields specified in the ``fields`` parameter. + + :param fields: + list of fields to preserve, or ``None`` to preserve them all + :type fields: list of str + """ + + if fields is not None: + self.unset_fields_except(fields) + + # never expose the node_id attribute + self.node_id = wtypes.Unset @classmethod def sample(cls, expand=True): @@ -181,11 +196,14 @@ class VolumeConnectorCollection(collection.Collection): detail=None, **kwargs): collection = VolumeConnectorCollection() collection.connectors = [ - VolumeConnector.convert_with_links(p, fields=fields) + VolumeConnector.convert_with_links(p, fields=fields, + sanitize=False) for p in rpc_connectors] if detail: kwargs['detail'] = detail collection.next = collection.get_next(limit, url=url, **kwargs) + for connector in collection.connectors: + connector.sanitize(fields) return collection @classmethod diff --git a/ironic/api/controllers/v1/volume_target.py b/ironic/api/controllers/v1/volume_target.py index 2b901afe6f..7c97a41d69 100644 --- a/ironic/api/controllers/v1/volume_target.py +++ b/ironic/api/controllers/v1/volume_target.py @@ -124,35 +124,49 @@ class VolumeTarget(base.APIBase): wtypes.Unset) @staticmethod - def _convert_with_links(target, url, fields=None): - # NOTE(lucasagomes): Since we are able to return a specified set of - # fields the "uuid" can be unset, so we need to save it in another - # variable to use when building the links - target_uuid = target.uuid - if fields is not None: - target.unset_fields_except(fields) + def _convert_with_links(target, url): - # never expose the node_id attribute - target.node_id = wtypes.Unset target.links = [link.Link.make_link('self', url, 'volume/targets', - target_uuid), + target.uuid), link.Link.make_link('bookmark', url, 'volume/targets', - target_uuid, + target.uuid, bookmark=True) ] return target @classmethod - def convert_with_links(cls, rpc_target, fields=None): + def convert_with_links(cls, rpc_target, fields=None, sanitize=True): target = VolumeTarget(**rpc_target.as_dict()) if fields is not None: api_utils.check_for_invalid_fields(fields, target.as_dict()) - return cls._convert_with_links(target, pecan.request.public_url, - fields=fields) + target = cls._convert_with_links(target, pecan.request.public_url) + + if not sanitize: + return target + + target.sanitize(fields) + + return target + + def sanitize(self, fields=None): + """Removes sensitive and unrequested data. + + Will only keep the fields specified in the ``fields`` parameter. + + :param fields: + list of fields to preserve, or ``None`` to preserve them all + :type fields: list of str + """ + + if fields is not None: + self.unset_fields_except(fields) + + # never expose the node_id attribute + self.node_id = wtypes.Unset @classmethod def sample(cls, expand=True): @@ -199,11 +213,13 @@ class VolumeTargetCollection(collection.Collection): detail=None, **kwargs): collection = VolumeTargetCollection() collection.targets = [ - VolumeTarget.convert_with_links(p, fields=fields) + VolumeTarget.convert_with_links(p, fields=fields, sanitize=False) for p in rpc_targets] if detail: kwargs['detail'] = detail collection.next = collection.get_next(limit, url=url, **kwargs) + for target in collection.targets: + target.sanitize(fields) return collection @classmethod diff --git a/ironic/tests/unit/api/controllers/v1/test_chassis.py b/ironic/tests/unit/api/controllers/v1/test_chassis.py index d41d6e8b07..4718f30763 100644 --- a/ironic/tests/unit/api/controllers/v1/test_chassis.py +++ b/ironic/tests/unit/api/controllers/v1/test_chassis.py @@ -230,6 +230,23 @@ class TestListChassis(test_api_base.BaseApiTest): next_marker = data['chassis'][-1]['uuid'] self.assertIn(next_marker, data['next']) + def test_get_collection_pagination_no_uuid(self): + fields = 'extra' + limit = 2 + chassis_list = [] + for id_ in range(3): + chassis = obj_utils.create_test_chassis( + self.context, + uuid=uuidutils.generate_uuid()) + chassis_list.append(chassis) + + data = self.get_json( + '/chassis?fields=%s&limit=%s' % (fields, limit), + headers={api_base.Version.string: str(api_v1.max_version())}) + + self.assertEqual(limit, len(data['chassis'])) + self.assertIn('marker=%s' % chassis_list[limit - 1].uuid, data['next']) + def test_sort_key(self): ch_list = [] for id_ in range(3): diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index ccdada419b..40c64ad815 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -736,6 +736,23 @@ class TestListNodes(test_api_base.BaseApiTest): next_marker = data['nodes'][-1]['uuid'] self.assertIn(next_marker, data['next']) + def test_get_collection_pagination_no_uuid(self): + fields = 'name' + limit = 2 + nodes = [] + for id_ in range(3): + node = obj_utils.create_test_node( + self.context, + uuid=uuidutils.generate_uuid()) + nodes.append(node) + + data = self.get_json( + '/nodes?fields=%s&limit=%s' % (fields, limit), + headers={api_base.Version.string: str(api_v1.max_version())}) + + self.assertEqual(limit, len(data['nodes'])) + self.assertIn('marker=%s' % nodes[limit - 1].uuid, data['next']) + def test_collection_links_instance_uuid_param(self): cfg.CONF.set_override('max_limit', 1, 'api') nodes = [] diff --git a/ironic/tests/unit/api/controllers/v1/test_port.py b/ironic/tests/unit/api/controllers/v1/test_port.py index eeee5b6ba7..3a7013f168 100644 --- a/ironic/tests/unit/api/controllers/v1/test_port.py +++ b/ironic/tests/unit/api/controllers/v1/test_port.py @@ -368,6 +368,26 @@ class TestListPorts(test_api_base.BaseApiTest): # We always append "links" self.assertItemsEqual(['uuid', 'extra', 'links'], port) + def test_get_collection_next_marker_no_uuid(self): + fields = 'address' + limit = 2 + ports = [] + for i in range(3): + port = obj_utils.create_test_port( + self.context, + node_id=self.node.id, + uuid=uuidutils.generate_uuid(), + address='52:54:00:cf:2d:3%s' % i + ) + ports.append(port) + + data = self.get_json( + '/ports?fields=%s&limit=%s' % (fields, limit), + headers={api_base.Version.string: str(api_v1.max_version())}) + + self.assertEqual(limit, len(data['ports'])) + self.assertIn('marker=%s' % ports[limit - 1].uuid, data['next']) + def test_get_custom_fields_invalid_fields(self): port = obj_utils.create_test_port(self.context, node_id=self.node.id) fields = 'uuid,spongebob' diff --git a/ironic/tests/unit/api/controllers/v1/test_portgroup.py b/ironic/tests/unit/api/controllers/v1/test_portgroup.py index 38cd447d55..8591873b16 100644 --- a/ironic/tests/unit/api/controllers/v1/test_portgroup.py +++ b/ironic/tests/unit/api/controllers/v1/test_portgroup.py @@ -327,6 +327,26 @@ class TestListPortgroups(test_api_base.BaseApiTest): next_marker = data['portgroups'][-1]['uuid'] self.assertIn(next_marker, data['next']) + def test_get_collection_pagination_no_uuid(self): + fields = 'address' + limit = 2 + portgroups = [] + for id_ in range(3): + portgroup = obj_utils.create_test_portgroup( + self.context, + node_id=self.node.id, + uuid=uuidutils.generate_uuid(), + name='portgroup%s' % id_, + address='52:54:00:cf:2d:3%s' % id_) + portgroups.append(portgroup) + + data = self.get_json( + '/portgroups?fields=%s&limit=%s' % (fields, limit), + headers=self.headers) + + self.assertEqual(limit, len(data['portgroups'])) + self.assertIn('marker=%s' % portgroups[limit - 1].uuid, data['next']) + def test_ports_subresource(self): pg = obj_utils.create_test_portgroup(self.context, uuid=uuidutils.generate_uuid(), diff --git a/ironic/tests/unit/api/controllers/v1/test_volume_connector.py b/ironic/tests/unit/api/controllers/v1/test_volume_connector.py index b970abe2dd..e1ed61bbed 100644 --- a/ironic/tests/unit/api/controllers/v1/test_volume_connector.py +++ b/ironic/tests/unit/api/controllers/v1/test_volume_connector.py @@ -273,6 +273,25 @@ class TestListVolumeConnectors(test_api_base.BaseApiTest): next_marker = data['connectors'][-1]['uuid'] self.assertIn(next_marker, data['next']) + def test_get_collection_pagination_no_uuid(self): + fields = 'connector_id' + limit = 2 + connectors = [] + for id_ in range(3): + volume_connector = obj_utils.create_test_volume_connector( + self.context, + node_id=self.node.id, + connector_id='test-connector_id-%s' % id_, + uuid=uuidutils.generate_uuid()) + connectors.append(volume_connector) + + data = self.get_json( + '/volume/connectors?fields=%s&limit=%s' % (fields, limit), + headers=self.headers) + + self.assertEqual(limit, len(data['connectors'])) + self.assertIn('marker=%s' % connectors[limit - 1].uuid, data['next']) + def test_collection_links_detail(self): connectors = [] for id_ in range(5): diff --git a/ironic/tests/unit/api/controllers/v1/test_volume_target.py b/ironic/tests/unit/api/controllers/v1/test_volume_target.py index 3fcca729bc..abffd3799b 100644 --- a/ironic/tests/unit/api/controllers/v1/test_volume_target.py +++ b/ironic/tests/unit/api/controllers/v1/test_volume_target.py @@ -258,6 +258,23 @@ class TestListVolumeTargets(test_api_base.BaseApiTest): self.assertIn(next_marker, data['next']) self.assertIn('volume/targets', data['next']) + def test_get_collection_pagination_no_uuid(self): + fields = 'boot_index' + limit = 2 + targets = [] + for id_ in range(3): + target = obj_utils.create_test_volume_target( + self.context, node_id=self.node.id, + uuid=uuidutils.generate_uuid(), boot_index=id_) + targets.append(target) + + data = self.get_json( + '/volume/targets?fields=%s&limit=%s' % (fields, limit), + headers=self.headers) + + self.assertEqual(limit, len(data['targets'])) + self.assertIn('marker=%s' % targets[limit - 1].uuid, data['next']) + def test_collection_links_detail(self): targets = [] for id_ in range(5): diff --git a/releasenotes/notes/fix-pagination-marker-with-custom-field-query-65ca29001a03e036.yaml b/releasenotes/notes/fix-pagination-marker-with-custom-field-query-65ca29001a03e036.yaml new file mode 100644 index 0000000000..15222d8696 --- /dev/null +++ b/releasenotes/notes/fix-pagination-marker-with-custom-field-query-65ca29001a03e036.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixes an issue where the pagination marker was not being set if ``uuid`` was not + in the list of requested fields when executing a list query. The affected API endpoints + were: port, portgroup, volume_target, volume_connector, node and chassis. + `See story 2003192 for more details `_. +