From: Avishay Traeger Date: Thu, 18 Dec 2014 13:55:26 +0000 (+0100) Subject: Fix issue with passing lists in filters X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=4aaf40ba1aab4d7c347b05750d0fe21f8d1bcc68;p=openstack-build%2Fcinder-build.git Fix issue with passing lists in filters The Cinder db code already allows searching for a value in a list, for example, id in ['id1', 'id2', 'id3']. The problem is when we get the value at the API it comes as a string (e.g. "['id1', 'id2', 'id3']", and therefore the filter fails. This was already fixed for metadata filters, so just apply it to all filters. We do a try/except because if it really is a string, then the literal_eval will fail because we don't pass double quotes, such as "'string'". The current code therefore will now cover lists and fall back to the original behavior for strings. Change-Id: Idfd8298401a330bb99ddcd130268fabb6ef9d162 Closes-Bug: 1403875 --- diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py index bc50c0b46..3821cac7a 100644 --- a/cinder/api/v1/volumes.py +++ b/cinder/api/v1/volumes.py @@ -270,8 +270,11 @@ class VolumeController(wsgi.Controller): search_opts.pop('limit', None) search_opts.pop('offset', None) - if 'metadata' in search_opts: - search_opts['metadata'] = ast.literal_eval(search_opts['metadata']) + for k, v in search_opts.iteritems(): + try: + search_opts[k] = ast.literal_eval(v) + except ValueError: + LOG.debug('Could not evaluate value %s, assuming string', v) context = req.environ['cinder.context'] utils.remove_invalid_filter_options(context, diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py index 0f30eea02..afe8ba4f9 100644 --- a/cinder/api/v2/volumes.py +++ b/cinder/api/v2/volumes.py @@ -230,8 +230,11 @@ class VolumeController(wsgi.Controller): filters['display_name'] = filters['name'] del filters['name'] - if 'metadata' in filters: - filters['metadata'] = ast.literal_eval(filters['metadata']) + for k, v in filters.iteritems(): + try: + filters[k] = ast.literal_eval(v) + except ValueError: + LOG.debug('Could not evaluate value %s, assuming string', v) volumes = self.volume_api.get_all(context, marker, limit, sort_key, sort_dir, filters, diff --git a/cinder/tests/api/v1/test_volumes.py b/cinder/tests/api/v1/test_volumes.py index e3a7b11b9..8975bdf07 100644 --- a/cinder/tests/api/v1/test_volumes.py +++ b/cinder/tests/api/v1/test_volumes.py @@ -16,6 +16,7 @@ import datetime from lxml import etree +import mock from oslo.config import cfg import webob @@ -761,6 +762,35 @@ class VolumeApiTest(test.TestCase): self.assertIn('volumes', res) self.assertEqual(1, len(res['volumes'])) + @mock.patch('cinder.utils.add_visible_admin_metadata') + @mock.patch('cinder.volume.api.API.get_all') + def test_get_volumes_filter_with_string(self, get_all, add_meta): + req = mock.MagicMock() + req.GET.copy.return_value = {'display_name': 'Volume-573108026'} + context = mock.Mock() + req.environ = {'cinder.context': context} + self.controller._items(req, mock.Mock) + get_all.assert_any_call(context, sort_dir='desc', + viewable_admin_meta=True, + sort_key='created_at', + limit=None, + filters={'display_name': 'Volume-573108026'}, + marker=None) + + @mock.patch('cinder.utils.add_visible_admin_metadata') + @mock.patch('cinder.volume.api.API.get_all') + def test_get_volumes_filter_with_list(self, get_all, add_meta): + req = mock.MagicMock() + req.GET.copy.return_value = {'id': "['1', '2', '3']"} + context = mock.Mock() + req.environ = {'cinder.context': context} + self.controller._items(req, mock.Mock) + get_all.assert_any_call(context, sort_dir='desc', + viewable_admin_meta=True, + sort_key='created_at', + limit=None, filters={'id': ['1', '2', '3']}, + marker=None) + class VolumeSerializerTest(test.TestCase): def _verify_volume_attachment(self, attach, tree): diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py index 1c38dda11..55e594280 100644 --- a/cinder/tests/api/v2/test_volumes.py +++ b/cinder/tests/api/v2/test_volumes.py @@ -17,6 +17,7 @@ import datetime from lxml import etree +import mock from oslo.config import cfg import six import six.moves.urllib.parse as urlparse @@ -1541,6 +1542,32 @@ class VolumeApiTest(test.TestCase): body = {'volume': 'string'} self._create_volume_bad_request(body=body) + @mock.patch('cinder.utils.add_visible_admin_metadata') + @mock.patch('cinder.volume.api.API.get_all') + def test_get_volumes_filter_with_string(self, get_all, add_meta): + req = mock.MagicMock() + context = mock.Mock() + req.environ = {'cinder.context': context} + req.params = {'display_name': 'Volume-573108026'} + self.controller._view_builder.detail_list = mock.Mock() + self.controller._get_volumes(req, True) + get_all.assert_any_call(context, None, None, 'created_at', 'desc', + {'display_name': 'Volume-573108026'}, + viewable_admin_meta=True) + + @mock.patch('cinder.utils.add_visible_admin_metadata') + @mock.patch('cinder.volume.api.API.get_all') + def test_get_volumes_filter_with_list(self, get_all, add_meta): + req = mock.MagicMock() + context = mock.Mock() + req.environ = {'cinder.context': context} + req.params = {'id': "['1', '2', '3']"} + self.controller._view_builder.detail_list = mock.Mock() + self.controller._get_volumes(req, True) + get_all.assert_any_call(context, None, None, 'created_at', 'desc', + {'id': ['1', '2', '3']}, + viewable_admin_meta=True) + class VolumeSerializerTest(test.TestCase): def _verify_volume_attachment(self, attach, tree):