diff --git a/patrole_tempest_plugin/rbac_utils.py b/patrole_tempest_plugin/rbac_utils.py index 4cddb8d1..fe2d99f8 100644 --- a/patrole_tempest_plugin/rbac_utils.py +++ b/patrole_tempest_plugin/rbac_utils.py @@ -18,7 +18,6 @@ import time 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 @@ CONF = config.CONF 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 @@ class RbacUtils(object): 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 @@ class RbacUtils(object): 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 457d08d8..ff5b46fb 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 import config 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 @@ class BaseV2ComputeRbacTest(compute_base.BaseV2ComputeTest): 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 8fef4877..6cb3d2ff 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 import config 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 @@ class BaseIdentityRbacTest(base.BaseIdentityTest): 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 a825c713..72705609 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 @@ class BaseV1ImageRbacTest(image_base.BaseV1ImageTest): 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 @@ class BaseV2ImageRbacTest(image_base.BaseV2ImageTest): 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 d0331741..3ee31b7b 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 @@ class BaseNetworkRbacTest(network_base.BaseNetworkTest): 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 ccadad18..742665bd 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 import config 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 @@ class BaseVolumeRbacTest(vol_base.BaseVolumeTest): 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 b7f6ba80..0f38b1e3 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 mock 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 @@ CONF = config.CONF 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 @@ class RBACUtilsTest(base.TestCase): **{'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 @@ class RBACUtilsTest(base.TestCase): 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 @@ class RBACUtilsTest(base.TestCase): '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 @@ class RBACUtilsTest(base.TestCase): 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 @@ class RBACUtilsTest(base.TestCase): '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 @@ class RBACUtilsTest(base.TestCase): '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 @@ class RBACUtilsTest(base.TestCase): 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):