Pop out 'offset' and 'limit' before use for filter
In previous code of _items() from api/v{1,2}/snapshots.py, and also the _items)_ from api/v1/volume.py.we didn't pop out the 'offset' and 'limit' fields from HTTP get params before we use such params for filter.This is the root cause for bug #1205956 For non-admin user, since 'offset' and 'limit' is not in the allowed_search_options, so the volumes.remove_invalid_options will help to filter them out. As a result, it walks around this bug. But for admin user,the volumes.remove_invalid_options will not try to filter the search_options.So for admin user, the 'limit' will appear in search_options, then obviously get no result. fixed bug #1205956 Change-Id: Ib1a66c9d104ac52d6eae18be7f06d02985d4c2fd
This commit is contained in:
parent
6f762cf926
commit
08af981ffc
|
@ -139,8 +139,12 @@ class SnapshotsController(wsgi.Controller):
|
|||
"""Returns a list of snapshots, transformed through entity_maker."""
|
||||
context = req.environ['cinder.context']
|
||||
|
||||
search_opts = {}
|
||||
search_opts.update(req.GET)
|
||||
#pop out limit and offset , they are not search_opts
|
||||
search_opts = req.GET.copy()
|
||||
search_opts.pop('limit', None)
|
||||
search_opts.pop('offset', None)
|
||||
|
||||
#filter out invalid option
|
||||
allowed_search_options = ('status', 'volume_id', 'display_name')
|
||||
volumes.remove_invalid_options(context, search_opts,
|
||||
allowed_search_options)
|
||||
|
|
|
@ -251,8 +251,10 @@ class VolumeController(wsgi.Controller):
|
|||
def _items(self, req, entity_maker):
|
||||
"""Returns a list of volumes, transformed through entity_maker."""
|
||||
|
||||
search_opts = {}
|
||||
search_opts.update(req.GET)
|
||||
#pop out limit and offset , they are not search_opts
|
||||
search_opts = req.GET.copy()
|
||||
search_opts.pop('limit', None)
|
||||
search_opts.pop('offset', None)
|
||||
|
||||
if 'metadata' in search_opts:
|
||||
search_opts['metadata'] = ast.literal_eval(search_opts['metadata'])
|
||||
|
|
|
@ -139,8 +139,12 @@ class SnapshotsController(wsgi.Controller):
|
|||
"""Returns a list of snapshots, transformed through entity_maker."""
|
||||
context = req.environ['cinder.context']
|
||||
|
||||
search_opts = {}
|
||||
search_opts.update(req.GET)
|
||||
#pop out limit and offset , they are not search_opts
|
||||
search_opts = req.GET.copy()
|
||||
search_opts.pop('limit', None)
|
||||
search_opts.pop('offset', None)
|
||||
|
||||
#filter out invalid option
|
||||
allowed_search_options = ('status', 'volume_id', 'name')
|
||||
volumes.remove_invalid_options(context, search_opts,
|
||||
allowed_search_options)
|
||||
|
|
|
@ -314,6 +314,32 @@ class SnapshotApiTest(test.TestCase):
|
|||
self.assertTrue('snapshots' in res)
|
||||
self.assertEqual(1, len(res['snapshots']))
|
||||
|
||||
def test_list_snapshots_with_limit_and_offset(self):
|
||||
def list_snapshots_with_limit_and_offset(is_admin):
|
||||
def stub_snapshot_get_all_by_project(context, project_id):
|
||||
return [
|
||||
stubs.stub_snapshot(1, display_name='backup1'),
|
||||
stubs.stub_snapshot(2, display_name='backup2'),
|
||||
stubs.stub_snapshot(3, display_name='backup3'),
|
||||
]
|
||||
|
||||
self.stubs.Set(db, 'snapshot_get_all_by_project',
|
||||
stub_snapshot_get_all_by_project)
|
||||
|
||||
req = fakes.HTTPRequest.blank('/v1/fake/snapshots?limit=1\
|
||||
&offset=1',
|
||||
use_admin_context=is_admin)
|
||||
res = self.controller.index(req)
|
||||
|
||||
self.assertTrue('snapshots' in res)
|
||||
self.assertEqual(1, len(res['snapshots']))
|
||||
self.assertEqual(2, res['snapshots'][0]['id'])
|
||||
|
||||
#admin case
|
||||
list_snapshots_with_limit_and_offset(is_admin=True)
|
||||
#non_admin case
|
||||
list_snapshots_with_limit_and_offset(is_admin=False)
|
||||
|
||||
def test_admin_list_snapshots_all_tenants(self):
|
||||
req = fakes.HTTPRequest.blank('/v1/fake/snapshots?all_tenants=1',
|
||||
use_admin_context=True)
|
||||
|
|
|
@ -546,6 +546,30 @@ class VolumeApiTest(test.TestCase):
|
|||
req,
|
||||
1)
|
||||
|
||||
def test_volume_detail_limit_offset(self):
|
||||
def volume_detail_limit_offset(is_admin):
|
||||
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('/v1/volumes/detail?limit=2\
|
||||
&offset=1',
|
||||
use_admin_context=is_admin)
|
||||
res_dict = self.controller.index(req)
|
||||
volumes = res_dict['volumes']
|
||||
self.assertEquals(len(volumes), 1)
|
||||
self.assertEquals(volumes[0]['id'], 2)
|
||||
|
||||
#admin case
|
||||
volume_detail_limit_offset(is_admin=True)
|
||||
#non_admin case
|
||||
volume_detail_limit_offset(is_admin=False)
|
||||
|
||||
def test_volume_delete(self):
|
||||
req = fakes.HTTPRequest.blank('/v1/volumes/1')
|
||||
resp = self.controller.delete(req, 1)
|
||||
|
|
|
@ -325,6 +325,32 @@ class SnapshotApiTest(test.TestCase):
|
|||
self.assertTrue('snapshots' in res)
|
||||
self.assertEqual(1, len(res['snapshots']))
|
||||
|
||||
def test_list_snapshots_with_limit_and_offset(self):
|
||||
def list_snapshots_with_limit_and_offset(is_admin):
|
||||
def stub_snapshot_get_all_by_project(context, project_id):
|
||||
return [
|
||||
stubs.stub_snapshot(1, display_name='backup1'),
|
||||
stubs.stub_snapshot(2, display_name='backup2'),
|
||||
stubs.stub_snapshot(3, display_name='backup3'),
|
||||
]
|
||||
|
||||
self.stubs.Set(db, 'snapshot_get_all_by_project',
|
||||
stub_snapshot_get_all_by_project)
|
||||
|
||||
req = fakes.HTTPRequest.blank('/v2/fake/snapshots?limit=1\
|
||||
&offset=1',
|
||||
use_admin_context=is_admin)
|
||||
res = self.controller.index(req)
|
||||
|
||||
self.assertTrue('snapshots' in res)
|
||||
self.assertEqual(1, len(res['snapshots']))
|
||||
self.assertEqual(2, res['snapshots'][0]['id'])
|
||||
|
||||
#admin case
|
||||
list_snapshots_with_limit_and_offset(is_admin=True)
|
||||
#non_admin case
|
||||
list_snapshots_with_limit_and_offset(is_admin=False)
|
||||
|
||||
def test_admin_list_snapshots_all_tenants(self):
|
||||
req = fakes.HTTPRequest.blank('/v2/fake/snapshots?all_tenants=1',
|
||||
use_admin_context=True)
|
||||
|
|
|
@ -521,6 +521,13 @@ class VolumeApiTest(test.TestCase):
|
|||
self.assertEquals(len(volumes), 1)
|
||||
self.assertEquals(volumes[0]['id'], 2)
|
||||
|
||||
req = fakes.HTTPRequest.blank('/v2/volumes/detail?limit=2&offset=1',
|
||||
use_admin_context=True)
|
||||
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,
|
||||
|
|
Loading…
Reference in New Issue