From aed94a76cf62fdaedff793c7a9d998920b56c072 Mon Sep 17 00:00:00 2001 From: Rajat Dhasmana Date: Mon, 15 Jun 2020 07:10:05 +0000 Subject: [PATCH] Fix: listing volumes with filters This patch includes 2 fix for the following issues: 1) ast.literal_eval() doesn't work with int and float values See comment inline 2) Do not traverse and modify the same dict: while traversing filters dict, we're modifying it inside the loop which distorts the order of elements (adding modified elements in the end). to fix this, i've used a temp dict that will be used to traverse and modification will be done in the filters dict. Closes-Bug: #1883490 Change-Id: I18b4b0b1b71904b766f7b89df49f5539e3c7662a --- cinder/tests/unit/api/v3/test_volumes.py | 17 +++++++++++++++++ cinder/volume/api.py | 9 +++++++-- ...-list-volume-filtering-3f2bf93ab9b98974.yaml | 5 +++++ 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/fix-list-volume-filtering-3f2bf93ab9b98974.yaml diff --git a/cinder/tests/unit/api/v3/test_volumes.py b/cinder/tests/unit/api/v3/test_volumes.py index 300692b3883..8f5e793d0b3 100644 --- a/cinder/tests/unit/api/v3/test_volumes.py +++ b/cinder/tests/unit/api/v3/test_volumes.py @@ -292,6 +292,23 @@ class VolumeApiTest(test.TestCase): else: self.assertNotIn('count', res_dict) + def test_list_volume_with_multiple_filters(self): + metadata = {'key_X': 'value_X'} + self._create_multiple_volumes_with_different_project() + test_utils.create_volume(self.ctxt, metadata=metadata) + + self.mock_object(ViewBuilder, '_get_volume_type', + v2_fakes.fake_volume_type_name_get) + # Request with 'all_tenants' and 'metadata' + req = fakes.HTTPRequest.blank( + "/v3/volumes/detail?all_tenants=1" + "&metadata=%7B%27key_X%27%3A+%27value_X%27%7D") + ctxt = context.RequestContext(fake.USER_ID, fake.PROJECT_ID, False) + req.environ['cinder.context'] = ctxt + res_dict = self.controller._get_volumes(req, is_detail=True) + self.assertEqual(1, len(res_dict['volumes'])) + self.assertEqual(metadata, res_dict['volumes'][0]['metadata']) + def test_volume_index_filter_by_group_id_in_unsupport_version(self): self._create_volume_with_group() req = fakes.HTTPRequest.blank(("/v3/volumes?group_id=%s") % diff --git a/cinder/volume/api.py b/cinder/volume/api.py index c132caae464..10a7b9e07bd 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -2033,8 +2033,8 @@ class API(base.Base): # To translate any true/false equivalent to True/False # which is only acceptable format in database queries. - - for key, val in filters.items(): + temp_dict = filters.copy() + for key, val in temp_dict.items(): try: if key in booleans: filters[key] = self._check_boolean_filter_value( @@ -2047,6 +2047,11 @@ class API(base.Base): # the filter becomes different from the user input. continue else: + # this is required as ast.literal_eval(/) + # raises exception. Eg: ast.literal_eval(5) generates + # ValueError: malformed node or string: 5 + if not isinstance(val, str): + val = str(val) filters[key] = ast.literal_eval(val) except (ValueError, SyntaxError): LOG.debug('Could not evaluate value %s, assuming string', val) diff --git a/releasenotes/notes/fix-list-volume-filtering-3f2bf93ab9b98974.yaml b/releasenotes/notes/fix-list-volume-filtering-3f2bf93ab9b98974.yaml new file mode 100644 index 00000000000..02dd00a8213 --- /dev/null +++ b/releasenotes/notes/fix-list-volume-filtering-3f2bf93ab9b98974.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + `Bug #1883490 `_: + Fixed incorrect response of listing volumes with filters. \ No newline at end of file