diff --git a/cinder/tests/unit/volume/test_snapshot.py b/cinder/tests/unit/volume/test_snapshot.py index 74dcf3b16e9..8b57cba43ea 100644 --- a/cinder/tests/unit/volume/test_snapshot.py +++ b/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) diff --git a/cinder/volume/api.py b/cinder/volume/api.py index dbe7599cbc2..d2aa98be760 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -675,9 +675,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) diff --git a/releasenotes/notes/revert-snapshot-non-admin-8485be55060eab0d.yaml b/releasenotes/notes/revert-snapshot-non-admin-8485be55060eab0d.yaml new file mode 100644 index 00000000000..0ce87b44f48 --- /dev/null +++ b/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).