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
This commit is contained in:
Rajat Dhasmana 2021-03-03 02:09:38 -05:00
parent 85975fb2b6
commit b675b66d4f
3 changed files with 20 additions and 1 deletions

View File

@ -446,7 +446,11 @@ def reject_invalid_filters(context, filters, resource,
invalid_filters.append(key) invalid_filters.append(key)
if invalid_filters: if invalid_filters:
if 'all_tenants' in 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') invalid_filters.remove('all_tenants')
filters.pop('all_tenants')
if len(invalid_filters) == 0: if len(invalid_filters) == 0:
return return
raise webob.exc.HTTPBadRequest( raise webob.exc.HTTPBadRequest(

View File

@ -417,7 +417,15 @@ class GeneralFiltersTest(test.TestCase):
'is_admin': False, 'is_admin': False,
'result': {'volume_type': ["is_public"]}, 'result': {'volume_type': ["is_public"]},
'expected': {'is_public': True}, '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 @ddt.unpack
@mock.patch('cinder.api.common.get_enabled_resource_filters') @mock.patch('cinder.api.common.get_enabled_resource_filters')
def test_reject_invalid_filters(self, mock_get, filters, def test_reject_invalid_filters(self, mock_get, filters,

View File

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