openstack
/
patrole
Code Issues Proposed changes

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
This commit is contained in:
Felipe Monteiro 2017-05-05 00:16:53 +01:00
parent beff0ca8c3
commit b35de585b8
7 changed files with 56 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
patrole_tempest_plugin/rbac_utils.py
View File

@ -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

5
patrole_tempest_plugin/tests/api/compute/rbac_base.py
View File

@ -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):

6
patrole_tempest_plugin/tests/api/identity/rbac_base.py
View File

@ -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):

8
patrole_tempest_plugin/tests/api/image/rbac_base.py
View File

@ -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)

5
patrole_tempest_plugin/tests/api/network/rbac_base.py
View File

@ -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)

5
patrole_tempest_plugin/tests/api/volume/rbac_base.py
View File

@ -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,

81
patrole_tempest_plugin/tests/unit/test_rbac_utils.py
View File

@ -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):