openstack/shade
Code Issues Proposed changes

Don't glob match name_or_id

Browse Source 
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
This commit is contained in:
Ian Wienand 2017-03-03 14:06:34 +11:00
parent b06b1fff9b
commit 0ecbb22547
2 changed files with 8 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
shade/_utils.py
View File

@ -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

19
shade/tests/unit/test__utils.py
View File

@ -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')