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
This commit is contained in:
Will Szumski 2018-09-10 15:08:19 +01:00
parent 36e87dc5b4
commit 02a3e77a3e
13 changed files with 367 additions and 130 deletions

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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):

View File

@ -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 = []

View File

@ -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'

View File

@ -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(),

View File

@ -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):

View File

@ -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):

View File

@ -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>`_.