Merge "Ensure pagination marker is always set"

This commit is contained in:
Zuul 2018-10-26 13:54:14 +00:00 committed by Gerrit Code Review
commit 0ea86814ad
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

@ -1103,59 +1103,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
@ -1165,39 +1181,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):
@ -1268,9 +1294,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

@ -770,6 +770,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>`_.