Fix revert to snapshot for non admins

When a non user calls revert to snapshot using the snapshot name the
client returns a failure status:

  $ cinder --os-volume-api-version 3.59 revert-to-snapshot snap1
    ERROR: No snapshot with a name or ID of 'snap1' exists.

The revert to snapshot API requires an UUID, so the cinderclient lists
the snapshots passing "all_tenants=1" and the name of the snapshot to
find its first when it's not a UUID.

The problem that we were only removing the "all_tenants" filter for
admins, leaving it for normal users and passing it to the DB which
wouldn't understand that filter and return None.

This patch ensure we always remove the "all_tenants" filter (as we do in
other places) before calling the OVO method to list snapshots.

Change-Id: I5fd3d7840b36f6805143cd1b837258232e7bc58a
Fixes-Bug: #1889758

cinder/tests/unit/volume/test_snapshot.py

@@ -630,3 +630,38 @@ class SnapshotTestCase(base.BaseVolumeTestCase):
         snapshot.refresh()
         self.assertEqual(fields.SnapshotStatus.ERROR_DELETING,
                          snapshot.status)
+
+    @ddt.data({'all_tenants': '1', 'name': 'snap1'},
+              {'all_tenants': 'true', 'name': 'snap1'},
+              {'all_tenants': 'false', 'name': 'snap1'},
+              {'all_tenants': '0', 'name': 'snap1'},
+              {'name': 'snap1'})
+    @mock.patch.object(objects, 'SnapshotList')
+    @mock.patch.object(context.RequestContext, 'authorize')
+    def test_get_all_snapshots_non_admin(self, search_opts, auth_mock,
+                                         snaplist_mock):
+        ctxt = context.RequestContext(user_id=None, is_admin=False,
+                                      project_id=mock.sentinel.project_id,
+                                      read_deleted='no', overwrite=False)
+        volume_api = cinder.volume.api.API()
+        res = volume_api.get_all_snapshots(ctxt,
+                                           search_opts,
+                                           mock.sentinel.marker,
+                                           mock.sentinel.limit,
+                                           mock.sentinel.sort_keys,
+                                           mock.sentinel.sort_dirs,
+                                           mock.sentinel.offset)
+
+        auth_mock.assert_called_once_with(
+            cinder.volume.api.snapshot_policy.GET_ALL_POLICY)
+        snaplist_mock.get_all.assert_not_called()
+        snaplist_mock.get_all_by_project.assert_called_once_with(
+            ctxt,
+            mock.sentinel.project_id,
+            {'name': 'snap1'},
+            mock.sentinel.marker,
+            mock.sentinel.limit,
+            mock.sentinel.sort_keys,
+            mock.sentinel.sort_dirs,
+            mock.sentinel.offset)
+        self.assertEqual(snaplist_mock.get_all_by_project.return_value, res)

cinder/volume/api.py

@@ -678,9 +678,10 @@ class API(base.Base):
 
         search_opts = search_opts or {}
 
-        if context.is_admin and 'all_tenants' in search_opts:
-            # Need to remove all_tenants to pass the filtering below.
-            del search_opts['all_tenants']
+        # Need to remove all_tenants to pass the filtering below.
+        all_tenants = strutils.bool_from_string(search_opts.pop('all_tenants',
+                                                                'false'))
+        if context.is_admin and all_tenants:
             snapshots = objects.SnapshotList.get_all(
                 context, search_opts, marker, limit, sort_keys, sort_dirs,
                 offset)

releasenotes/notes/revert-snapshot-non-admin-8485be55060eab0d.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fix revert to snapshot not working for non admin users when using the
+    snapshot's name (bug #1889758).