diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py index bd2e4e91b3..76ec83d81b 100644 --- a/cinder/api/v2/volumes.py +++ b/cinder/api/v2/volumes.py @@ -179,6 +179,7 @@ def _get_volumes(self, req, is_detail): limit = params.pop('limit', None) sort_key = params.pop('sort_key', 'created_at') sort_dir = params.pop('sort_dir', 'desc') + params.pop('offset', None) filters = params remove_invalid_options(context, diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py index 78a9046c25..d176f92f5a 100644 --- a/cinder/tests/api/v2/test_volumes.py +++ b/cinder/tests/api/v2/test_volumes.py @@ -419,13 +419,13 @@ def test_volume_index_limit(self): def test_volume_index_limit_negative(self): req = fakes.HTTPRequest.blank('/v2/volumes?limit=-1') - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.Invalid, self.controller.index, req) def test_volume_index_limit_non_int(self): req = fakes.HTTPRequest.blank('/v2/volumes?limit=a') - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.Invalid, self.controller.index, req) @@ -436,6 +436,31 @@ def test_volume_index_limit_marker(self): self.assertEquals(len(volumes), 1) self.assertEquals(volumes[0]['id'], '1') + def test_volume_index_limit_offset(self): + def stub_volume_get_all_by_project(context, project_id, marker, limit, + sort_key, sort_dir): + return [ + stubs.stub_volume(1, display_name='vol1'), + stubs.stub_volume(2, display_name='vol2'), + ] + self.stubs.Set(db, 'volume_get_all_by_project', + stub_volume_get_all_by_project) + req = fakes.HTTPRequest.blank('/v2/volumes?limit=2&offset=1') + res_dict = self.controller.index(req) + volumes = res_dict['volumes'] + self.assertEquals(len(volumes), 1) + self.assertEquals(volumes[0]['id'], 2) + + req = fakes.HTTPRequest.blank('/v2/volumes?limit=-1&offset=1') + self.assertRaises(exception.InvalidInput, + self.controller.index, + req) + + req = fakes.HTTPRequest.blank('/v2/volumes?limit=a&offset=1') + self.assertRaises(exception.InvalidInput, + self.controller.index, + req) + def test_volume_detail_with_marker(self): def stub_volume_get_all_by_project(context, project_id, marker, limit, sort_key, sort_dir): @@ -460,13 +485,13 @@ def test_volume_detail_limit(self): def test_volume_detail_limit_negative(self): req = fakes.HTTPRequest.blank('/v2/volumes/detail?limit=-1') - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.Invalid, self.controller.index, req) def test_volume_detail_limit_non_int(self): req = fakes.HTTPRequest.blank('/v2/volumes/detail?limit=a') - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.Invalid, self.controller.index, req) @@ -477,6 +502,31 @@ def test_volume_detail_limit_marker(self): self.assertEquals(len(volumes), 1) self.assertEquals(volumes[0]['id'], '1') + def test_volume_detail_limit_offset(self): + def stub_volume_get_all_by_project(context, project_id, marker, limit, + sort_key, sort_dir): + return [ + stubs.stub_volume(1, display_name='vol1'), + stubs.stub_volume(2, display_name='vol2'), + ] + self.stubs.Set(db, 'volume_get_all_by_project', + stub_volume_get_all_by_project) + req = fakes.HTTPRequest.blank('/v2/volumes/detail?limit=2&offset=1') + res_dict = self.controller.index(req) + volumes = res_dict['volumes'] + self.assertEquals(len(volumes), 1) + self.assertEquals(volumes[0]['id'], 2) + + req = fakes.HTTPRequest.blank('/v2/volumes/detail?limit=-1&offset=1') + self.assertRaises(exception.InvalidInput, + self.controller.index, + req) + + req = fakes.HTTPRequest.blank('/v2/volumes/detail?limit=a&offset=1') + self.assertRaises(exception.InvalidInput, + self.controller.index, + req) + def test_volume_list_by_name(self): def stub_volume_get_all_by_project(context, project_id, marker, limit, sort_key, sort_dir): diff --git a/cinder/volume/api.py b/cinder/volume/api.py index a6fb584b18..f57b6b8604 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -330,6 +330,16 @@ def get_all(self, context, marker=None, limit=None, sort_key='created_at', sort_dir='desc', filters={}): check_policy(context, 'get_all') + try: + if limit is not None: + limit = int(limit) + if limit < 0: + msg = _('limit param must be positive') + raise exception.InvalidInput(reason=msg) + except ValueError: + msg = _('limit param must be an integer') + raise exception.InvalidInput(reason=msg) + if (context.is_admin and 'all_tenants' in filters): # Need to remove all_tenants to pass the filtering below. del filters['all_tenants']