openstack
/
nova
Code Issues Proposed changes

Implement query param schema for volume, snapshot API 

volume & snapshot API accept query param to filter volumes,
snapshots.
This commit adds json schema to validating the valid
query parameters.

There is no change in API behaviour and additionalProperty
is kept True for backward compatibility.

Partially implements blueprint json-schema-validation-for-query-param

Change-Id: I9e91ddabb2e62440f04ab3aac3b7e96a9fdd59dc
This commit is contained in:
ghanshyam 2017-11-19 16:15:39 +03:00
parent 57728836f2
commit 0d1743a83d
4 changed files with 232 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
nova/api/openstack/compute/schemas/volumes.py
View File

@ -90,3 +90,20 @@ create_volume_attachment_v249['properties']['volumeAttachment'][
update_volume_attachment = copy.deepcopy(create_volume_attachment)
del update_volume_attachment['properties']['volumeAttachment'][
'properties']['device']
index_query = {
'type': 'object',
'properties': {
'limit': parameter_types.multi_params(
parameter_types.non_negative_integer),
'offset': parameter_types.multi_params(
parameter_types.non_negative_integer)
},
# NOTE(gmann): This is kept True to keep backward compatibility.
# As of now Schema validation stripped out the additional parameters and
# does not raise 400. In the future, we may block the additional parameters
# by bump in Microversion.
'additionalProperties': True
}
detail_query = index_query

5
nova/api/openstack/compute/volumes.py
View File

@ -132,12 +132,14 @@ class VolumeController(wsgi.Controller):
raise exc.HTTPNotFound(explanation=e.format_message())
@wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION)
@validation.query_schema(volumes_schema.index_query)
@extensions.expected_errors(())
def index(self, req):
"""Returns a summary list of volumes."""
return self._items(req, entity_maker=_translate_volume_summary_view)
@wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION)
@validation.query_schema(volumes_schema.detail_query)
@extensions.expected_errors(())
def detail(self, req):
"""Returns a detailed list of volumes."""
@ -262,6 +264,7 @@ class VolumeAttachmentController(wsgi.Controller):
super(VolumeAttachmentController, self).__init__()
@extensions.expected_errors(404)
@validation.query_schema(volumes_schema.index_query)
def index(self, req, server_id):
"""Returns the list of volume attachments for a given instance."""
context = req.environ['nova.context']
@ -501,12 +504,14 @@ class SnapshotController(wsgi.Controller):
raise exc.HTTPNotFound(explanation=e.format_message())
@wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION)
@validation.query_schema(volumes_schema.index_query)
@extensions.expected_errors(())
def index(self, req):
"""Returns a summary list of snapshots."""
return self._items(req, entity_maker=_translate_snapshot_summary_view)
@wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION)
@validation.query_schema(volumes_schema.detail_query)
@extensions.expected_errors(())
def detail(self, req):
"""Returns a detailed list of snapshots."""

80
nova/tests/unit/api/openstack/compute/test_snapshots.py
View File

@ -139,6 +139,86 @@ class SnapshotApiTestV21(test.NoDBTestCase):
resp_snapshots = resp_dict['snapshots']
self.assertEqual(1, len(resp_snapshots))
def _test_list_with_invalid_filter(self, url):
prefix = '/os-snapshots'
req = fakes.HTTPRequest.blank(prefix + url)
controller_list = self.controller.index
if 'detail' in url:
controller_list = self.controller.detail
self.assertRaises(exception.ValidationError,
controller_list, req)
def test_list_with_invalid_non_int_limit(self):
self._test_list_with_invalid_filter('?limit=-9')
def test_list_with_invalid_string_limit(self):
self._test_list_with_invalid_filter('?limit=abc')
def test_list_duplicate_query_with_invalid_string_limit(self):
self._test_list_with_invalid_filter(
'?limit=1&limit=abc')
def test_detail_list_with_invalid_non_int_limit(self):
self._test_list_with_invalid_filter('/detail?limit=-9')
def test_detail_list_with_invalid_string_limit(self):
self._test_list_with_invalid_filter('/detail?limit=abc')
def test_detail_list_duplicate_query_with_invalid_string_limit(self):
self._test_list_with_invalid_filter(
'/detail?limit=1&limit=abc')
def test_list_with_invalid_non_int_offset(self):
self._test_list_with_invalid_filter('?offset=-9')
def test_list_with_invalid_string_offset(self):
self._test_list_with_invalid_filter('?offset=abc')
def test_list_duplicate_query_with_invalid_string_offset(self):
self._test_list_with_invalid_filter(
'?offset=1&offset=abc')
def test_detail_list_with_invalid_non_int_offset(self):
self._test_list_with_invalid_filter('/detail?offset=-9')
def test_detail_list_with_invalid_string_offset(self):
self._test_list_with_invalid_filter('/detail?offset=abc')
def test_detail_list_duplicate_query_with_invalid_string_offset(self):
self._test_list_with_invalid_filter(
'/detail?offset=1&offset=abc')
def _test_list_duplicate_query_parameters_validation(self, url):
params = {
'limit': 1,
'offset': 1
}
controller_list = self.controller.index
if 'detail' in url:
controller_list = self.controller.detail
for param, value in params.items():
req = fakes.HTTPRequest.blank(
url + '?%s=%s&%s=%s' %
(param, value, param, value))
controller_list(req)
def test_list_duplicate_query_parameters_validation(self):
self._test_list_duplicate_query_parameters_validation('/os-snapshots')
def test_detail_list_duplicate_query_parameters_validation(self):
self._test_list_duplicate_query_parameters_validation(
'/os-snapshots/detail')
def test_list_with_additional_filter(self):
req = fakes.HTTPRequest.blank(
'/os-snapshots?limit=1&offset=1&additional=something')
self.controller.index(req)
def test_detail_list_with_additional_filter(self):
req = fakes.HTTPRequest.blank(
'/os-snapshots/detail?limit=1&offset=1&additional=something')
self.controller.detail(req)
class TestSnapshotAPIDeprecation(test.NoDBTestCase):

130
nova/tests/unit/api/openstack/compute/test_volumes.py
View File

@ -338,6 +338,84 @@ class VolumeApiTestV21(test.NoDBTestCase):
self.assertIn('Volume 456 could not be found.',
encodeutils.safe_decode(resp.body))
def _test_list_with_invalid_filter(self, url):
prefix = '/os-volumes'
req = fakes.HTTPRequest.blank(prefix + url)
self.assertRaises(exception.ValidationError,
volumes_v21.VolumeController().index,
req)
def test_list_with_invalid_non_int_limit(self):
self._test_list_with_invalid_filter('?limit=-9')
def test_list_with_invalid_string_limit(self):
self._test_list_with_invalid_filter('?limit=abc')
def test_list_duplicate_query_with_invalid_string_limit(self):
self._test_list_with_invalid_filter(
'?limit=1&limit=abc')
def test_detail_list_with_invalid_non_int_limit(self):
self._test_list_with_invalid_filter('/detail?limit=-9')
def test_detail_list_with_invalid_string_limit(self):
self._test_list_with_invalid_filter('/detail?limit=abc')
def test_detail_list_duplicate_query_with_invalid_string_limit(self):
self._test_list_with_invalid_filter(
'/detail?limit=1&limit=abc')
def test_list_with_invalid_non_int_offset(self):
self._test_list_with_invalid_filter('?offset=-9')
def test_list_with_invalid_string_offset(self):
self._test_list_with_invalid_filter('?offset=abc')
def test_list_duplicate_query_with_invalid_string_offset(self):
self._test_list_with_invalid_filter(
'?offset=1&offset=abc')
def test_detail_list_with_invalid_non_int_offset(self):
self._test_list_with_invalid_filter('/detail?offset=-9')
def test_detail_list_with_invalid_string_offset(self):
self._test_list_with_invalid_filter('/detail?offset=abc')
def test_detail_list_duplicate_query_with_invalid_string_offset(self):
self._test_list_with_invalid_filter(
'/detail?offset=1&offset=abc')
def _test_list_duplicate_query_parameters_validation(self, url):
params = {
'limit': 1,
'offset': 1
}
for param, value in params.items():
req = fakes.HTTPRequest.blank(
self.url_prefix + url + '?%s=%s&%s=%s' %
(param, value, param, value))
resp = req.get_response(self.app)
self.assertEqual(200, resp.status_int)
def test_list_duplicate_query_parameters_validation(self):
self._test_list_duplicate_query_parameters_validation('/os-volumes')
def test_detail_list_duplicate_query_parameters_validation(self):
self._test_list_duplicate_query_parameters_validation(
'/os-volumes/detail')
def test_list_with_additional_filter(self):
req = fakes.HTTPRequest.blank(self.url_prefix +
'/os-volumes?limit=1&offset=1&additional=something')
resp = req.get_response(self.app)
self.assertEqual(200, resp.status_int)
def test_detail_list_with_additional_filter(self):
req = fakes.HTTPRequest.blank(self.url_prefix +
'/os-volumes/detail?limit=1&offset=1&additional=something')
resp = req.get_response(self.app)
self.assertEqual(200, resp.status_int)
class VolumeAttachTestsV21(test.NoDBTestCase):
validation_error = exception.ValidationError
@ -700,6 +778,58 @@ class VolumeAttachTestsV21(test.NoDBTestCase):
self.attachments,
fake_func=fake_swap_volume_for_bdm_not_found)
def _test_list_with_invalid_filter(self, url):
prefix = '/servers/id/os-volume_attachments'
req = fakes.HTTPRequest.blank(prefix + url)
self.assertRaises(exception.ValidationError,
self.attachments.index,
req,
FAKE_UUID)
def test_list_with_invalid_non_int_limit(self):
self._test_list_with_invalid_filter('?limit=-9')
def test_list_with_invalid_string_limit(self):
self._test_list_with_invalid_filter('?limit=abc')
def test_list_duplicate_query_with_invalid_string_limit(self):
self._test_list_with_invalid_filter(
'?limit=1&limit=abc')
def test_list_with_invalid_non_int_offset(self):
self._test_list_with_invalid_filter('?offset=-9')
def test_list_with_invalid_string_offset(self):
self._test_list_with_invalid_filter('?offset=abc')
def test_list_duplicate_query_with_invalid_string_offset(self):
self._test_list_with_invalid_filter(
'?offset=1&offset=abc')
@mock.patch.object(objects.BlockDeviceMappingList,
'get_by_instance_uuid')
def test_list_duplicate_query_parameters_validation(self, mock_get):
fake_bdms = objects.BlockDeviceMappingList()
mock_get.return_value = fake_bdms
params = {
'limit': 1,
'offset': 1
}
for param, value in params.items():
req = fakes.HTTPRequest.blank(
'/servers/id/os-volume_attachments' + '?%s=%s&%s=%s' %
(param, value, param, value))
self.attachments.index(req, FAKE_UUID)
@mock.patch.object(objects.BlockDeviceMappingList,
'get_by_instance_uuid')
def test_list_with_additional_filter(self, mock_get):
fake_bdms = objects.BlockDeviceMappingList()
mock_get.return_value = fake_bdms
req = fakes.HTTPRequest.blank(
'/servers/id/os-volume_attachments?limit=1&additional=something')
self.attachments.index(req, FAKE_UUID)
class VolumeAttachTestsV249(test.NoDBTestCase):
validation_error = exception.ValidationError