diff options
author | Will Szumski <will@stackhpc.com> | 2018-09-10 15:08:19 +0100 |
---|---|---|
committer | Julia Kreger <juliaashleykreger@gmail.com> | 2019-10-22 15:55:23 +0000 |
commit | 7c9131bc6d80392fbe750b03a482e0f9d70fb043 (patch) | |
tree | 141a1f22a69c80b310824a6dc1ba982d5a9e5e88 | |
parent | f4575b9faf340d9c69700c7b05324564158aa9a7 (diff) | |
download | ironic-7c9131bc6d80392fbe750b03a482e0f9d70fb043.tar.gz |
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
(cherry picked from commit 02a3e77a3e06fcdf492fd36540d812843e1ec839)
-rw-r--r-- | ironic/api/controllers/v1/chassis.py | 45 | ||||
-rw-r--r-- | ironic/api/controllers/v1/node.py | 111 | ||||
-rw-r--r-- | ironic/api/controllers/v1/port.py | 63 | ||||
-rw-r--r-- | ironic/api/controllers/v1/portgroup.py | 52 | ||||
-rw-r--r-- | ironic/api/controllers/v1/volume_connector.py | 48 | ||||
-rw-r--r-- | ironic/api/controllers/v1/volume_target.py | 46 | ||||
-rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_chassis.py | 17 | ||||
-rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_node.py | 17 | ||||
-rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_port.py | 20 | ||||
-rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_portgroup.py | 20 | ||||
-rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_volume_connector.py | 19 | ||||
-rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_volume_target.py | 17 | ||||
-rw-r--r-- | releasenotes/notes/fix-pagination-marker-with-custom-field-query-65ca29001a03e036.yaml | 8 |
13 files changed, 360 insertions, 123 deletions
diff --git a/ironic/api/controllers/v1/chassis.py b/ironic/api/controllers/v1/chassis.py index 4c9f89f46..8f665a340 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 1808a4f94..9f512028f 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 3fc907b7c..b72f0ddbf 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) + @classmethod + def convert_with_links(cls, rpc_port, fields=None, sanitize=True): + port = Port(**rpc_port.as_dict()) - # never expose the node_id attribute - port.node_id = wtypes.Unset + port._validate_fields(fields) - # never expose the portgroup_id attribute - port.portgroup_id = wtypes.Unset + url = pecan.request.public_url port.links = [link.Link.make_link('self', url, - 'ports', port_uuid), + 'ports', port.uuid), link.Link.make_link('bookmark', url, - 'ports', port_uuid, + 'ports', port.uuid, bookmark=True) ] + + if not sanitize: + return port + + port.sanitize(fields=fields) + return port - @classmethod - def convert_with_links(cls, rpc_port, fields=None): - port = Port(**rpc_port.as_dict()) + def _validate_fields(self, fields=None): + if fields is not None: + api_utils.check_for_invalid_fields(fields, self.as_dict()) + + 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 + """ + hide_fields_in_newer_versions(self) if fields is not None: - api_utils.check_for_invalid_fields(fields, port.as_dict()) + self.unset_fields_except(fields) - hide_fields_in_newer_versions(port) + # never expose the node_id attribute + self.node_id = wtypes.Unset - return cls._convert_with_links(port, pecan.request.public_url, - fields=fields) + # 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 b48dd4b69..0091c4c03 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 b3d342199..ede708f0a 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 2b901afe6..7c97a41d6 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 d41d6e8b0..4718f3076 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 269ceefa6..9cce45dc3 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 69206637b..67467b932 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 38cd447d5..8591873b1 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 b970abe2d..e1ed61bbe 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 3fcca729b..abffd3799 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 000000000..15222d869 --- /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 <https://storyboard.openstack.org/#!/story/2003192>`_. + |