From b675b66d4f798786a30dbec00504fd2080a2e27f Mon Sep 17 00:00:00 2001
From: Rajat Dhasmana <rajatdhasmana@gmail.com>
Date: Wed, 3 Mar 2021 02:09:38 -0500
Subject: [PATCH] Fix: show volume by name for non-admins

Currently we don't pop the all_tenants field for non-admins resulting
in database not able to process it and no records are displayed.
Cinderclient passes it for most APIs since it doesn't have the context
and cannot determine if the user is admin or non-admin.
On the API side, we need to filter out the all_tenants key for non-admins
since it is only valid for admin operations.
This only happens incase of general filtering API which was introduced
in microversion 3.31, prior to this all invalid filters were removed.

Closes-Bug: #1917574
Change-Id: Idf619b4dbb64318f6e2231b13b8bacce3deaa43b
---
 cinder/api/common.py                                   |  4 ++++
 cinder/tests/unit/api/test_common.py                   | 10 +++++++++-
 .../fix-show-volume-non-admins-1bc5238398e73981.yaml   |  7 +++++++
 3 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/fix-show-volume-non-admins-1bc5238398e73981.yaml

diff --git a/cinder/api/common.py b/cinder/api/common.py
index 6b1099c747c..4320527862c 100644
--- a/cinder/api/common.py
+++ b/cinder/api/common.py
@@ -446,7 +446,11 @@ def reject_invalid_filters(context, filters, resource,
                 invalid_filters.append(key)
     if invalid_filters:
         if 'all_tenants' in invalid_filters:
+            # NOTE: this is a special case: the cinderclient always adds
+            # 'all_tenants', so we don't want to hold that against a non-admin
+            # user and we silently ignore it.  See Bug #1917574.
             invalid_filters.remove('all_tenants')
+            filters.pop('all_tenants')
         if len(invalid_filters) == 0:
             return
         raise webob.exc.HTTPBadRequest(
diff --git a/cinder/tests/unit/api/test_common.py b/cinder/tests/unit/api/test_common.py
index c3abaa57554..ecd38934096 100644
--- a/cinder/tests/unit/api/test_common.py
+++ b/cinder/tests/unit/api/test_common.py
@@ -417,7 +417,15 @@ class GeneralFiltersTest(test.TestCase):
                'is_admin': False,
                'result': {'volume_type': ["is_public"]},
                'expected': {'is_public': True},
-               'resource': 'volume_type'})
+               'resource': 'volume_type'},
+              {'filters': {'key1': 'value1',
+                           'all_tenants': 'value2',
+                           'key3': 'value3'},
+               'is_admin': False,
+               'result': {'fake_resource': ['key1', 'key3']},
+               'expected': {'key1': 'value1',
+                            'key3': 'value3'},
+               'resource': 'fake_resource'})
     @ddt.unpack
     @mock.patch('cinder.api.common.get_enabled_resource_filters')
     def test_reject_invalid_filters(self, mock_get, filters,
diff --git a/releasenotes/notes/fix-show-volume-non-admins-1bc5238398e73981.yaml b/releasenotes/notes/fix-show-volume-non-admins-1bc5238398e73981.yaml
new file mode 100644
index 00000000000..3b51d3174d8
--- /dev/null
+++ b/releasenotes/notes/fix-show-volume-non-admins-1bc5238398e73981.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    `Bug #1917574 <https://bugs.launchpad.net/cinder/+bug/1917574>`_:
+    Fixed issue when cinderclient requests to show volume by name for
+    non-admin users would result in the volume not being found for
+    microversions 3.31 or later.