Remove singleton from RbacUtils constructor
Currently, the RbacUtils class in rbac_utils is a singleton,
which means the constructor is only called once. The problem with
that is when we instantiate RbacUtils in each rbac_base class,
we have to then also call switch_role(toggle_rbac_role=False).
We could do that inside the constructor to simplify the code,
but only if RbacUtils constructor stops being a singleton -- or
else admin credentials are not guaranteed during set up across
all test classes.
In addition, setting "rbac_utils = RbacUtils" at the end of rbac_utils
is pointless and only makes the code harder to read. This patch
removes that line of code and refactors the imports for rbac_utils where
necessary.
Change-Id: I778ae19b4bd0b71ab77984ae57dd96fd829a1fc4
Closes-Bug: #1688079
diff --git a/patrole_tempest_plugin/rbac_utils.py b/patrole_tempest_plugin/rbac_utils.py
index 4cddb8d..fe2d99f 100644
--- a/patrole_tempest_plugin/rbac_utils.py
+++ b/patrole_tempest_plugin/rbac_utils.py
@@ -18,7 +18,6 @@
from oslo_log import log as logging
import oslo_utils.uuidutils as uuid_utils
-import six
from tempest.common import credentials_factory as credentials
from tempest import config
@@ -29,19 +28,11 @@
LOG = logging.getLogger(__name__)
-class Singleton(type):
- _instances = {}
-
- def __call__(cls, *args, **kwargs):
- if cls not in cls._instances:
- cls._instances[cls] = super(Singleton, cls).__call__(*args,
- **kwargs)
- return cls._instances[cls]
-
-
-@six.add_metaclass(Singleton)
class RbacUtils(object):
+ def __init__(self, test_obj):
+ self.switch_role(test_obj, toggle_rbac_role=False)
+
# References the last value of `toggle_rbac_role` that was passed to
# `switch_role`. Used for ensuring that `switch_role` is correctly used
# in a test file, so that false positives are prevented. The key used
@@ -70,7 +61,7 @@
if not self.admin_role_id or not self.rbac_role_id:
self._get_roles()
- rbac_utils._validate_switch_role(self, test_obj, toggle_rbac_role)
+ self._validate_switch_role(test_obj, toggle_rbac_role)
if toggle_rbac_role:
self._add_role_to_user(self.rbac_role_id)
@@ -171,5 +162,3 @@
self.admin_role_id = admin_role_id
self.rbac_role_id = rbac_role_id
-
-rbac_utils = RbacUtils
diff --git a/patrole_tempest_plugin/tests/api/compute/rbac_base.py b/patrole_tempest_plugin/tests/api/compute/rbac_base.py
index 457d08d..ff5b46f 100644
--- a/patrole_tempest_plugin/tests/api/compute/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/compute/rbac_base.py
@@ -16,7 +16,7 @@
from tempest.lib.common.utils import data_utils
from tempest.lib.common.utils import test_utils
-from patrole_tempest_plugin.rbac_utils import rbac_utils
+from patrole_tempest_plugin import rbac_utils
CONF = config.CONF
@@ -36,8 +36,7 @@
def setup_clients(cls):
super(BaseV2ComputeRbacTest, cls).setup_clients()
cls.auth_provider = cls.os_primary.auth_provider
- cls.rbac_utils = rbac_utils()
- cls.rbac_utils.switch_role(cls, toggle_rbac_role=False)
+ cls.rbac_utils = rbac_utils.RbacUtils(cls)
@classmethod
def resource_setup(cls):
diff --git a/patrole_tempest_plugin/tests/api/identity/rbac_base.py b/patrole_tempest_plugin/tests/api/identity/rbac_base.py
index 8fef487..6cb3d2f 100644
--- a/patrole_tempest_plugin/tests/api/identity/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/identity/rbac_base.py
@@ -20,7 +20,7 @@
from tempest.lib.common.utils import data_utils
from tempest.lib.common.utils import test_utils
-from patrole_tempest_plugin.rbac_utils import rbac_utils
+from patrole_tempest_plugin import rbac_utils
CONF = config.CONF
LOG = logging.getLogger(__name__)
@@ -41,9 +41,7 @@
def setup_clients(cls):
super(BaseIdentityRbacTest, cls).setup_clients()
cls.auth_provider = cls.os_primary.auth_provider
-
- cls.rbac_utils = rbac_utils()
- cls.rbac_utils.switch_role(cls, toggle_rbac_role=False)
+ cls.rbac_utils = rbac_utils.RbacUtils(cls)
@classmethod
def resource_setup(cls):
diff --git a/patrole_tempest_plugin/tests/api/image/rbac_base.py b/patrole_tempest_plugin/tests/api/image/rbac_base.py
index a825c71..7270560 100644
--- a/patrole_tempest_plugin/tests/api/image/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/image/rbac_base.py
@@ -14,7 +14,7 @@
from tempest.api.image import base as image_base
from tempest import config
-from patrole_tempest_plugin.rbac_utils import rbac_utils
+from patrole_tempest_plugin import rbac_utils
CONF = config.CONF
@@ -34,8 +34,7 @@
def setup_clients(cls):
super(BaseV1ImageRbacTest, cls).setup_clients()
cls.auth_provider = cls.os_primary.auth_provider
- cls.rbac_utils = rbac_utils()
- cls.rbac_utils.switch_role(cls, toggle_rbac_role=False)
+ cls.rbac_utils = rbac_utils.RbacUtils(cls)
class BaseV2ImageRbacTest(image_base.BaseV2ImageTest):
@@ -53,5 +52,4 @@
def setup_clients(cls):
super(BaseV2ImageRbacTest, cls).setup_clients()
cls.auth_provider = cls.os_primary.auth_provider
- cls.rbac_utils = rbac_utils()
- cls.rbac_utils.switch_role(cls, toggle_rbac_role=False)
+ cls.rbac_utils = rbac_utils.RbacUtils(cls)
diff --git a/patrole_tempest_plugin/tests/api/network/rbac_base.py b/patrole_tempest_plugin/tests/api/network/rbac_base.py
index d033174..3ee31b7 100644
--- a/patrole_tempest_plugin/tests/api/network/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/network/rbac_base.py
@@ -16,7 +16,7 @@
from tempest.api.network import base as network_base
from tempest import config
-from patrole_tempest_plugin.rbac_utils import rbac_utils
+from patrole_tempest_plugin import rbac_utils
CONF = config.CONF
@@ -36,5 +36,4 @@
def setup_clients(cls):
super(BaseNetworkRbacTest, cls).setup_clients()
cls.auth_provider = cls.os_primary.auth_provider
- cls.rbac_utils = rbac_utils()
- cls.rbac_utils.switch_role(cls, toggle_rbac_role=False)
+ cls.rbac_utils = rbac_utils.RbacUtils(cls)
diff --git a/patrole_tempest_plugin/tests/api/volume/rbac_base.py b/patrole_tempest_plugin/tests/api/volume/rbac_base.py
index ccadad1..742665b 100644
--- a/patrole_tempest_plugin/tests/api/volume/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/volume/rbac_base.py
@@ -16,7 +16,7 @@
from tempest.lib.common.utils import data_utils
from tempest.lib.common.utils import test_utils
-from patrole_tempest_plugin.rbac_utils import rbac_utils
+from patrole_tempest_plugin import rbac_utils
CONF = config.CONF
@@ -37,8 +37,7 @@
super(BaseVolumeRbacTest, cls).setup_clients()
cls.auth_provider = cls.os_primary.auth_provider
- cls.rbac_utils = rbac_utils()
- cls.rbac_utils.switch_role(cls, toggle_rbac_role=False)
+ cls.rbac_utils = rbac_utils.RbacUtils(cls)
version_checker = {
1: [cls.os_primary.volume_hosts_client,
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_utils.py b/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
index b7f6ba8..0f38b1e 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
@@ -17,6 +17,7 @@
import testtools
from tempest import config
+from tempest.lib import base as lib_base
from tempest.lib import exceptions as lib_exc
from tempest.tests import base
@@ -28,33 +29,43 @@
class RBACUtilsTest(base.TestCase):
- def setUp(self):
+ available_roles = {
+ 'roles': [
+ {'name': 'admin', 'id': 'admin_id'},
+ {'name': 'Member', 'id': 'member_id'}
+ ]
+ }
+
+ @mock.patch.object(rbac_utils, 'credentials', autospec=True,
+ **{'is_admin_available.return_value': True})
+ @mock.patch.object(rbac_utils.RbacUtils, '_clear_user_roles',
+ autospec=True, return_value=False)
+ def setUp(self, *args):
super(RBACUtilsTest, self).setUp()
- available_roles = {
- 'roles': [
- {'name': 'admin', 'id': 'admin_id'},
- {'name': 'Member', 'id': 'member_id'}
- ]
- }
- # Because rbac_utils is a singleton, reset all of its role-related
- # parameters to the correct values for each test run.
- self.rbac_utils = rbac_utils.rbac_utils()
- self.rbac_utils.switch_role_history = {}
- self.rbac_utils.admin_role_id = 'admin_id'
- self.rbac_utils.rbac_role_id = 'member_id'
-
- self.mock_test_obj = mock.Mock()
+ self.mock_test_obj = mock.Mock(spec=lib_base.BaseTestCase)
self.mock_test_obj.auth_provider = mock.Mock(
**{'credentials.user_id': mock.sentinel.user_id,
'credentials.tenant_id': mock.sentinel.project_id})
self.mock_test_obj.os_admin = mock.Mock(
- **{'roles_v3_client.list_roles.return_value': available_roles})
+ **{'roles_v3_client.list_roles.return_value': self.available_roles}
+ )
+ self.mock_test_obj.get_identity_version = mock.Mock(return_value=3)
+
+ with mock.patch.object(rbac_utils.RbacUtils, '_validate_switch_role'):
+ self.rbac_utils = rbac_utils.RbacUtils(self.mock_test_obj)
+ self.rbac_utils.switch_role_history = {}
+ self.rbac_utils.admin_role_id = 'admin_id'
+ self.rbac_utils.rbac_role_id = 'member_id'
CONF.set_override('admin_role', 'admin', group='identity')
CONF.set_override('auth_version', 'v3', group='identity')
CONF.set_override('rbac_test_role', 'Member', group='rbac')
+ roles_client = self.mock_test_obj.os_admin.roles_v3_client
+ roles_client.create_user_role_on_project.reset_mock()
+ self.mock_test_obj.auth_provider.reset_mock()
+
self.addCleanup(CONF.clear_override, 'rbac_test_role', group='rbac')
self.addCleanup(CONF.clear_override, 'admin_role', group='identity')
self.addCleanup(CONF.clear_override, 'auth_version', group='identity')
@@ -65,7 +76,7 @@
**{'roles_client.list_user_roles_on_project.'
'return_value': {'roles': [{'id': return_value}]}})
- @mock.patch.object(rbac_utils.rbac_utils, '_clear_user_roles',
+ @mock.patch.object(rbac_utils.RbacUtils, '_clear_user_roles',
autospec=True, return_value=False)
def test_initialization_with_missing_admin_role(self, _):
self.mock_test_obj.os_admin = mock.Mock(
@@ -79,7 +90,7 @@
self.assertIn("Role with name 'admin' does not exist in the system.",
e.__str__())
- @mock.patch.object(rbac_utils.rbac_utils, '_clear_user_roles',
+ @mock.patch.object(rbac_utils.RbacUtils, '_clear_user_roles',
autospec=True, return_value=False)
def test_initialization_with_missing_rbac_role(self, _):
self.mock_test_obj.os_admin = mock.Mock(
@@ -116,43 +127,33 @@
'member_id'),
])
- @mock.patch.object(rbac_utils.rbac_utils, '_clear_user_roles',
+ @mock.patch.object(rbac_utils.RbacUtils, '_clear_user_roles',
autospec=True, return_value=False)
@mock.patch.object(rbac_utils, 'time', autospec=True)
- def test_rbac_utils_switch_role_to_admin_role(self, mock_time,
- mock_clear_user_roles):
+ def test_rbac_utils_switch_role_to_admin_role(self, mock_time, _):
self.rbac_utils.prev_switch_role = True
self._mock_list_user_roles_on_project('admin_id')
roles_client = self.mock_test_obj.os_admin.roles_v3_client
self.rbac_utils.switch_role(self.mock_test_obj, False)
- roles_client.create_user_role_on_project.\
- assert_called_once_with(mock.sentinel.project_id,
- mock.sentinel.user_id,
- 'admin_id')
- mock_clear_user_roles.assert_called_once_with(
- self.rbac_utils, 'admin_id')
+ roles_client.create_user_role_on_project.assert_called_once_with(
+ mock.sentinel.project_id, mock.sentinel.user_id, 'admin_id')
self.mock_test_obj.auth_provider.clear_auth.assert_called_once_with()
self.mock_test_obj.auth_provider.set_auth.assert_called_once_with()
mock_time.sleep.assert_called_once_with(1)
- @mock.patch.object(rbac_utils.rbac_utils, '_clear_user_roles',
+ @mock.patch.object(rbac_utils.RbacUtils, '_clear_user_roles',
autospec=True, return_value=False)
@mock.patch.object(rbac_utils, 'time', autospec=True)
- def test_rbac_utils_switch_role_to_rbac_role(self, mock_time,
- mock_clear_user_roles):
+ def test_rbac_utils_switch_role_to_rbac_role(self, mock_time, _):
self._mock_list_user_roles_on_project('member_id')
roles_client = self.mock_test_obj.os_admin.roles_v3_client
self.rbac_utils.switch_role(self.mock_test_obj, True)
- roles_client.create_user_role_on_project.\
- assert_called_once_with(mock.sentinel.project_id,
- mock.sentinel.user_id,
- 'member_id')
- mock_clear_user_roles.assert_called_once_with(
- self.rbac_utils, 'member_id')
+ roles_client.create_user_role_on_project.assert_called_once_with(
+ mock.sentinel.project_id, mock.sentinel.user_id, 'member_id')
self.mock_test_obj.auth_provider.clear_auth.assert_called_once_with()
self.mock_test_obj.auth_provider.set_auth.assert_called_once_with()
mock_time.sleep.assert_called_once_with(1)
@@ -165,7 +166,7 @@
self.rbac_utils.switch_role, self.mock_test_obj,
None)
- @mock.patch.object(rbac_utils.rbac_utils, '_clear_user_roles',
+ @mock.patch.object(rbac_utils.RbacUtils, '_clear_user_roles',
autospec=True, return_value=False)
def test_rbac_utils_switch_roles_with_false_value_twice(self, _):
self._mock_list_user_roles_on_project('admin_id')
@@ -178,7 +179,7 @@
'twice. Make sure that you included a rbac_utils.switch_role '
'method call inside the test.', str(e))
- @mock.patch.object(rbac_utils.rbac_utils, '_clear_user_roles',
+ @mock.patch.object(rbac_utils.RbacUtils, '_clear_user_roles',
autospec=True, return_value=False)
def test_rbac_utils_switch_roles_with_true_value_twice(self, _):
self._mock_list_user_roles_on_project('admin_id')
@@ -191,7 +192,7 @@
'twice. Make sure that you included a rbac_utils.switch_role '
'method call inside the test.', str(e))
- @mock.patch.object(rbac_utils.rbac_utils, '_clear_user_roles',
+ @mock.patch.object(rbac_utils.RbacUtils, '_clear_user_roles',
autospec=True, return_value=False)
@mock.patch.object(rbac_utils, 'LOG', autospec=True)
@mock.patch.object(rbac_utils, 'sys', autospec=True)
@@ -228,7 +229,7 @@
self.rbac_utils.switch_role(self.mock_test_obj, True)
mock_log.error.assert_not_called()
- @mock.patch.object(rbac_utils.rbac_utils, '_clear_user_roles',
+ @mock.patch.object(rbac_utils.RbacUtils, '_clear_user_roles',
autospec=True, return_value=False)
def test_rbac_utils_switch_role_except_exception(self,
mock_clear_user_roles):