summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorWill Szumski <will@stackhpc.com>2018-09-10 15:08:19 +0100
committerWill Szumski <will@stackhpc.com>2018-09-11 20:32:34 +0100
commit02a3e77a3e06fcdf492fd36540d812843e1ec839 (patch)
tree5280b57f5f089cb4852f1c3efe507cd2b3581937
parent36e87dc5b472d79470b783fbba9ce396e3cbb96e (diff)
downloadironic-02a3e77a3e06fcdf492fd36540d812843e1ec839.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
-rw-r--r--ironic/api/controllers/v1/chassis.py45
-rw-r--r--ironic/api/controllers/v1/node.py111
-rw-r--r--ironic/api/controllers/v1/port.py63
-rw-r--r--ironic/api/controllers/v1/portgroup.py52
-rw-r--r--ironic/api/controllers/v1/volume_connector.py48
-rw-r--r--ironic/api/controllers/v1/volume_target.py46
-rw-r--r--ironic/tests/unit/api/controllers/v1/test_chassis.py17
-rw-r--r--ironic/tests/unit/api/controllers/v1/test_node.py17
-rw-r--r--ironic/tests/unit/api/controllers/v1/test_port.py20
-rw-r--r--ironic/tests/unit/api/controllers/v1/test_portgroup.py20
-rw-r--r--ironic/tests/unit/api/controllers/v1/test_volume_connector.py19
-rw-r--r--ironic/tests/unit/api/controllers/v1/test_volume_target.py17
-rw-r--r--releasenotes/notes/fix-pagination-marker-with-custom-field-query-65ca29001a03e036.yaml8
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 ccdada419..40c64ad81 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 eeee5b6ba..3a7013f16 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>`_.
+