From 0ecbb22547c470fca6d50cfad2ee94e1241f83f4 Mon Sep 17 00:00:00 2001 From: Ian Wienand Date: Fri, 3 Mar 2017 14:06:34 +1100 Subject: [PATCH] Don't glob match name_or_id I was trying to bring up a host on vexxhost using the image name "Ubuntu 16.04.1 LTS [2017-03-02]". As described in the change, because this gets run through fnmatch() directly it makes everything barf as "[2017-03-02]" gets matched as an incorrect glob character class. This is a little tricky because all the external list_*, search_*, get_* API functions basically pass name_or_id through directly. None of them have ever explicitly said that you could use globbing, although it probably would have worked. Any such use would have been relying on implementation details of the _util functions, and I feel like the fact it's _underscore_utils means that you probably had fair warning not to do that. And obviously the "filters" argument is meant for this type of filtering. Additionally, fnmatch() says that it normalises case depending on OS filename case-sensitivity, which seems like incorrect behaviour for these functions which are not actually related to filenames. Thus I believe the matching should be just be a straight string comparison. I have added a test for this, but we also have to remove the tests for globbing behaviour as they are incorrect now. Change-Id: Id89bbf6306adebe96c1db417a4b8b5ae51023af0 --- shade/_utils.py | 9 +++------ shade/tests/unit/test__utils.py | 19 +++++-------------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/shade/_utils.py b/shade/_utils.py index b3da5b5c9..cbb9b7a13 100644 --- a/shade/_utils.py +++ b/shade/_utils.py @@ -13,7 +13,6 @@ # limitations under the License. import contextlib -import fnmatch import inspect import jmespath import munch @@ -84,8 +83,7 @@ def _filter_list(data, name_or_id, filters): each dictionary contains an 'id' and 'name' key if a value for name_or_id is given. :param string name_or_id: - The name or ID of the entity being filtered. Can be a glob pattern, - such as 'nb01*'. + The name or ID of the entity being filtered. :param filters: A dictionary of meta data to use for further filtering. Elements of this dictionary may, themselves, be dictionaries. Example:: @@ -104,9 +102,8 @@ def _filter_list(data, name_or_id, filters): for e in data: e_id = e.get('id', None) e_name = e.get('name', None) - if ((e_id and fnmatch.fnmatch(str(e_id), str(name_or_id))) or - (e_name and fnmatch.fnmatch( - str(e_name), str(name_or_id)))): + if ((e_id and str(e_id) == str(name_or_id)) or + (e_name and str(e_name) == str(name_or_id))): identifier_matches.append(e) data = identifier_matches diff --git a/shade/tests/unit/test__utils.py b/shade/tests/unit/test__utils.py index 28235e8dd..6e611d73b 100644 --- a/shade/tests/unit/test__utils.py +++ b/shade/tests/unit/test__utils.py @@ -40,21 +40,12 @@ class TestUtils(base.TestCase): ret = _utils._filter_list(data, 'donald', None) self.assertEqual([el1], ret) - def test__filter_list_name_or_id_glob(self): + def test__filter_list_name_or_id_special(self): el1 = dict(id=100, name='donald') - el2 = dict(id=200, name='pluto') - el3 = dict(id=200, name='pluto-2') - data = [el1, el2, el3] - ret = _utils._filter_list(data, 'pluto*', None) - self.assertEqual([el2, el3], ret) - - def test__filter_list_name_or_id_glob_not_found(self): - el1 = dict(id=100, name='donald') - el2 = dict(id=200, name='pluto') - el3 = dict(id=200, name='pluto-2') - data = [el1, el2, el3] - ret = _utils._filter_list(data, 'q*', None) - self.assertEqual([], ret) + el2 = dict(id=200, name='pluto[2017-01-10]') + data = [el1, el2] + ret = _utils._filter_list(data, 'pluto[2017-01-10]', None) + self.assertEqual([el2], ret) def test__filter_list_filter(self): el1 = dict(id=100, name='donald', other='duck')