Fix volume filtering for quoted display name

Cinder v2 API allows creating a volume with a quoted (single or double
quotes or even unbalanced number of quotes) display name. But when we
try to get info for such volume, we end up getting an error message
saying that no volume with such a name or ID exists. This error is due
to the inadvertent stripping of quotes from the filter in the api layer.

The api call eventually comes to check_volume_filters() in
cinder/volume/api.py. The invocation of ast.literal_eval() inside this
method strips the quotes for certain quoted strings leading to this
incorrect filtering. ast.literal_eval() is used to convert string
representations into python objects which are then used to frame the
SQL queries in the db layer. For example, the string "[1,2]" for a
filter (not the display name filter) gets converted to a list object
and results in an "IN" operation being emitted in the SQL query as
opposed to an exact match.

When display_name does not contain any quotes or contains an unbalanced
number of quotes, then ast.literal_eval() throws (just like the Python
interpreter would throw for an unquoted string literal or one with
unbalanced number of quotes). We handle this by ignoring the exception
and using the raw input value as the filter and moving on. For string
containing balanced number of quotes, such as, '"foo"',
ast.literal_eval() succeeds and returns the input with the surrounding
quotes stripped (just like how the python interpreter strips quotes
from a string literal to initialize a string var's value in memory).
To always use the raw user input string as the filter value, we can
either not pass string inputs to ast.literal_eval() or encode the
string using encode("string-escape") so that we get the original string
back after passing through ast.literal_eval(). We choose the former as
the latter buys us nothing.

Change-Id: I48e0aea801ccb011cb974eea3d685bb9f35c61b2
Closes-Bug: #1503485

diff --git a/cinder/tests/unit/api/v2/test_volumes.py b/cinder/tests/unit/api/v2/test_volumes.py
index 6dafffeab8bd61b510e426f3b61316bdb3b15bcd..2652869488b1d166d86cad26b94fd644db277220 100644 (file)
--- a/cinder/tests/unit/api/v2/test_volumes.py
+++ b/cinder/tests/unit/api/v2/test_volumes.py
@@ -1362,20 +1362,46 @@ class VolumeApiTest(test.TestCase):
         body = {'volume': 'string'}
         self._create_volume_bad_request(body=body)
 
-    @mock.patch('cinder.volume.api.API.get_all')
-    def test_get_volumes_filter_with_string(self, get_all):
+    def _test_get_volumes_by_name(self, get_all, display_name):
         req = mock.MagicMock()
         context = mock.Mock()
         req.environ = {'cinder.context': context}
-        req.params = {'display_name': 'Volume-573108026'}
+        req.params = {'display_name': display_name}
         self.controller._view_builder.detail_list = mock.Mock()
         self.controller._get_volumes(req, True)
         get_all.assert_called_once_with(
             context, None, CONF.osapi_max_limit,
             sort_keys=['created_at'], sort_dirs=['desc'],
-            filters={'display_name': 'Volume-573108026'},
+            filters={'display_name': display_name},
             viewable_admin_meta=True, offset=0)
 
+    @mock.patch('cinder.volume.api.API.get_all')
+    def test_get_volumes_filter_with_string(self, get_all):
+        """Test to get a volume with an alpha-numeric display name."""
+        self._test_get_volumes_by_name(get_all, 'Volume-573108026')
+
+    @mock.patch('cinder.volume.api.API.get_all')
+    def test_get_volumes_filter_with_double_quoted_string(self, get_all):
+        """Test to get a volume with a double-quoted display name."""
+        self._test_get_volumes_by_name(get_all, '"Volume-573108026"')
+
+    @mock.patch('cinder.volume.api.API.get_all')
+    def test_get_volumes_filter_with_single_quoted_string(self, get_all):
+        """Test to get a volume with a single-quoted display name."""
+        self._test_get_volumes_by_name(get_all, "'Volume-573108026'")
+
+    @mock.patch('cinder.volume.api.API.get_all')
+    def test_get_volumes_filter_with_quote_in_between_string(self, get_all):
+        """Test to get a volume with a quote in between the display name."""
+        self._test_get_volumes_by_name(get_all, 'Volu"me-573108026')
+
+    @mock.patch('cinder.volume.api.API.get_all')
+    def test_get_volumes_filter_with_mixed_quoted_string(self, get_all):
+        """Test to get a volume with a mix of single and double quotes. """
+        # The display name starts with a single quote and ends with a
+        # double quote
+        self._test_get_volumes_by_name(get_all, '\'Volume-573108026"')
+
     @mock.patch('cinder.volume.api.API.get_all')
     def test_get_volumes_filter_with_true(self, get_all):
         req = mock.MagicMock()

diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index 3d3f9429cb29eda067c600fb95bf5c42be42c7be..e25dc28627a5244a0494c38f6c365741d1ce97d1 100644 (file)
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -1667,6 +1667,13 @@ class API(base.Base):
                         filters[k] = True
                     else:
                         filters[k] = bool(v)
+                elif k == 'display_name':
+                    # Use the raw value of display name as is for the filter
+                    # without passing it through ast.literal_eval(). If the
+                    # display name is a properly quoted string (e.g. '"foo"')
+                    # then literal_eval() strips the quotes (i.e. 'foo'), so
+                    # the filter becomes different from the user input.
+                    continue
                 else:
                     filters[k] = ast.literal_eval(v)
             except (ValueError, SyntaxError):

diff --git a/releasenotes/notes/volume-filtering-for-quoted-display-name-7f5e8ac888a73001.yaml b/releasenotes/notes/volume-filtering-for-quoted-display-name-7f5e8ac888a73001.yaml
new file mode 100644 (file)
index 0000000..8192b26
--- /dev/null
+++ b/releasenotes/notes/volume-filtering-for-quoted-display-name-7f5e8ac888a73001.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - Filtering volumes by their display name now
+    correctly handles display names with single and
+    double quotes.