From 34101027c6fea35f8acbc0463a7e6c95f6dfeb66 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Tue, 18 Aug 2015 11:29:15 +0200 Subject: [PATCH] Fix backup list all_tenants=0 filtering for admin When a user with admin role lists backups specifying all_tenants=0 in the request, it will not behave as expected, it will return backups for all tenants just as if all_tenants=1 had been specified. This patch fixes this issue and will only return all tenants in backup listings if all_tenants is set to a value meaning True (1, true, yes, y). Change-Id: Ia74bc258a86f6dc48483dfe17bf5a312aec4058d Closes-Bug: #1485900 --- cinder/backup/api.py | 10 +++++--- cinder/tests/unit/test_backup.py | 40 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/cinder/backup/api.py b/cinder/backup/api.py index cc4c24ef4..ddfa871a4 100644 --- a/cinder/backup/api.py +++ b/cinder/backup/api.py @@ -24,6 +24,7 @@ from eventlet import greenthread from oslo_config import cfg from oslo_log import log as logging from oslo_utils import excutils +from oslo_utils import strutils from cinder.backup import rpcapi as backup_rpcapi from cinder import context @@ -103,9 +104,12 @@ 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. - search_opts.pop('all_tenants', None) + all_tenants = search_opts.pop('all_tenants', '0') + if not utils.is_valid_boolstr(all_tenants): + msg = _("all_tenants must be a boolean, got '%s'.") % all_tenants + raise exception.InvalidParameterValue(err=msg) + + if context.is_admin and strutils.bool_from_string(all_tenants): backups = objects.BackupList.get_all(context, filters=search_opts) else: backups = objects.BackupList.get_all_by_project( diff --git a/cinder/tests/unit/test_backup.py b/cinder/tests/unit/test_backup.py index 52278d715..238b52709 100644 --- a/cinder/tests/unit/test_backup.py +++ b/cinder/tests/unit/test_backup.py @@ -22,6 +22,7 @@ from oslo_config import cfg from oslo_utils import importutils from oslo_utils import timeutils +from cinder.backup import api from cinder.backup import manager from cinder import context from cinder import db @@ -890,3 +891,42 @@ class BackupTestCaseWithVerify(BaseBackupTest): self.backup_mgr.reset_status(self.ctxt, backup, 'error') backup = db.backup_get(self.ctxt, backup['id']) self.assertEqual('error', backup['status']) + + +class BackupAPITestCase(BaseBackupTest): + def setUp(self): + super(BackupAPITestCase, self).setUp() + self.api = api.API() + + def test_get_all_wrong_all_tenants_value(self): + self.assertRaises(exception.InvalidParameterValue, + self.api.get_all, self.ctxt, {'all_tenants': 'bad'}) + + @mock.patch.object(objects, 'BackupList') + def test_get_all_no_all_tenants_value(self, mock_backuplist): + result = self.api.get_all(self.ctxt, {'key': 'value'}) + self.assertFalse(mock_backuplist.get_all.called) + self.assertEqual(mock_backuplist.get_all_by_project.return_value, + result) + mock_backuplist.get_all_by_project.assert_called_once_with( + self.ctxt, self.ctxt.project_id, filters={'key': 'value'}) + + @mock.patch.object(objects, 'BackupList') + def test_get_all_false_value_all_tenants(self, mock_backuplist): + result = self.api.get_all(self.ctxt, {'all_tenants': '0', + 'key': 'value'}) + self.assertFalse(mock_backuplist.get_all.called) + self.assertEqual(mock_backuplist.get_all_by_project.return_value, + result) + mock_backuplist.get_all_by_project.assert_called_once_with( + self.ctxt, self.ctxt.project_id, filters={'key': 'value'}) + + @mock.patch.object(objects, 'BackupList') + def test_get_all_true_value_all_tenants(self, mock_backuplist): + result = self.api.get_all(self.ctxt, {'all_tenants': 'true', + 'key': 'value'}) + self.assertFalse(mock_backuplist.get_all_by_project.called) + self.assertEqual(mock_backuplist.get_all.return_value, + result) + mock_backuplist.get_all.assert_called_once_with( + self.ctxt, filters={'key': 'value'}) -- 2.45.2