diff --git a/nova/api/openstack/compute/schemas/volumes.py b/nova/api/openstack/compute/schemas/volumes.py index b08ea5f8e5..36bc1704b8 100644 --- a/nova/api/openstack/compute/schemas/volumes.py +++ b/nova/api/openstack/compute/schemas/volumes.py @@ -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 diff --git a/nova/api/openstack/compute/volumes.py b/nova/api/openstack/compute/volumes.py index 190110ecb9..a1ada86922 100644 --- a/nova/api/openstack/compute/volumes.py +++ b/nova/api/openstack/compute/volumes.py @@ -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.""" diff --git a/nova/tests/unit/api/openstack/compute/test_snapshots.py b/nova/tests/unit/api/openstack/compute/test_snapshots.py index 9b87d5f891..077af38b14 100644 --- a/nova/tests/unit/api/openstack/compute/test_snapshots.py +++ b/nova/tests/unit/api/openstack/compute/test_snapshots.py @@ -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): diff --git a/nova/tests/unit/api/openstack/compute/test_volumes.py b/nova/tests/unit/api/openstack/compute/test_volumes.py index 84c69d11dd..f9ac90f0a8 100644 --- a/nova/tests/unit/api/openstack/compute/test_volumes.py +++ b/nova/tests/unit/api/openstack/compute/test_volumes.py @@ -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