From 79ecb6f53c157b651d13cd97d5dfdb7b8437042b Mon Sep 17 00:00:00 2001 From: Yves-Gwenael Bourhis Date: Fri, 15 Mar 2019 18:53:08 +0100 Subject: [PATCH] To get tenant, use openstacksdk instead of shade. Shade retrieves the list of all tenants and filters the result. This can be a huge issue on platforms with tons of tenants. OpenstackSDK's get_project method acts also like shade's get_project method, but OpenstackSDK's identity manager just recovers the desired tenant, so we use this. Also we pass the OpenStackConfig instance to shade or openstacksdk because if we don't, they re-instantiate one uselessly. Closes-Bug: #1820616 Change-Id: I737b031fa9f2e4394d58ac204bf28b422cec1c28 --- ospurge/main.py | 43 ++++++++++++------- ospurge/tests/test_main.py | 88 ++++++++++++++++++++++---------------- 2 files changed, 79 insertions(+), 52 deletions(-) diff --git a/ospurge/main.py b/ospurge/main.py index cfb6b37..0809887 100644 --- a/ospurge/main.py +++ b/ospurge/main.py @@ -18,6 +18,9 @@ import sys import threading import typing +from openstack import connection +from openstack import exceptions as os_exceptions + import os_client_config import shade @@ -83,39 +86,47 @@ def create_argument_parser(): class CredentialsManager(object): - def __init__(self, options): + def __init__(self, options, config): self.options = options + self.config = config self.revoke_role_after_purge = False self.disable_project_after_purge = False self.cloud = None # type: Optional[shade.OpenStackCloud] - self.operator_cloud = None # type: Optional[shade.OperatorCloud] + self.connection = None # type: Optional[connection.Connection] if options.purge_own_project: - self.cloud = shade.openstack_cloud(argparse=options) + self.cloud = shade.openstack_cloud(argparse=options, config=config) self.user_id = self.cloud.keystone_session.get_user_id() self.project_id = self.cloud.keystone_session.get_project_id() else: - self.operator_cloud = shade.operator_cloud(argparse=options) - self.user_id = self.operator_cloud.keystone_session.get_user_id() + self.connection = connection.Connection( + config=config.get_one(argparse=options) + ) + self.user_id = self.connection.identity.get_user_id() - project = self.operator_cloud.get_project(options.purge_project) - if not project: + try: + # Only admins can do that. + project = self.connection.identity.get_tenant( + options.purge_project) + except ( + os_exceptions.ResourceNotFound, os_exceptions.NotFoundException + ): raise exceptions.OSProjectNotFound( "Unable to find project '{}'".format(options.purge_project) ) - self.project_id = project['id'] + self.project_id = project.id # If project is not enabled, we must disable it after purge. - self.disable_project_after_purge = not project.enabled + self.disable_project_after_purge = not project.is_enabled # Reuse the information passed to get the `OperatorCloud` but # change the project. This way we bind/re-scope to the project # we want to purge, not the project we authenticated to. self.cloud = shade.openstack_cloud( **utils.replace_project_info( - self.operator_cloud.cloud_config.config, + self.connection.config.config, self.project_id ) ) @@ -128,7 +139,7 @@ class CredentialsManager(object): ) def ensure_role_on_project(self): - if self.operator_cloud and self.operator_cloud.grant_role( + if self.connection and self.connection.grant_role( self.options.admin_role_name, project=self.options.purge_project, user=self.user_id ): @@ -139,7 +150,7 @@ class CredentialsManager(object): self.revoke_role_after_purge = True def revoke_role_on_project(self): - self.operator_cloud.revoke_role( + self.connection.revoke_role( self.options.admin_role_name, user=self.user_id, project=self.options.purge_project) logging.warning( @@ -148,13 +159,13 @@ class CredentialsManager(object): ) def ensure_enabled_project(self): - if self.operator_cloud and self.disable_project_after_purge: - self.operator_cloud.update_project(self.project_id, enabled=True) + if self.connection and self.disable_project_after_purge: + self.connection.update_project(self.project_id, enabled=True) logging.warning("Project '%s' was disabled before purge and it is " "now enabled", self.options.purge_project) def disable_project(self): - self.operator_cloud.update_project(self.project_id, enabled=False) + self.connection.update_project(self.project_id, enabled=False) logging.warning("Project '%s' was disabled before purge and it is " "now also disabled", self.options.purge_project) @@ -221,7 +232,7 @@ def main(): options = parser.parse_args() configure_logging(options.verbose) - creds_manager = CredentialsManager(options=options) + creds_manager = CredentialsManager(options=options, config=cloud_config) creds_manager.ensure_enabled_project() creds_manager.ensure_role_on_project() diff --git a/ospurge/tests/test_main.py b/ospurge/tests/test_main.py index c00dc17..07690e5 100644 --- a/ospurge/tests/test_main.py +++ b/ospurge/tests/test_main.py @@ -16,6 +16,8 @@ import unittest import shade.exc +from openstack import exceptions as os_exceptions + from ospurge import exceptions from ospurge import main from ospurge.resources.base import ServiceResource @@ -142,6 +144,7 @@ class TestFunctions(unittest.TestCase): self.assertEqual(1, resource_manager.list.call_count) self.assertFalse(exit.set.called) + @mock.patch.object(main, 'connection') @mock.patch.object(main, 'os_client_config', autospec=True) @mock.patch.object(main, 'shade') @mock.patch('argparse.ArgumentParser.parse_args') @@ -149,12 +152,12 @@ class TestFunctions(unittest.TestCase): @mock.patch('concurrent.futures.ThreadPoolExecutor', autospec=True) @mock.patch('sys.exit', autospec=True) def test_main(self, m_sys_exit, m_tpe, m_event, m_parse_args, m_shade, - m_oscc): + m_oscc, m_conn): m_tpe.return_value.__enter__.return_value.map.side_effect = \ KeyboardInterrupt m_parse_args.return_value.purge_own_project = False m_parse_args.return_value.resource = None - m_shade.operator_cloud().get_project().enabled = False + m_conn.Connection().identity.get_tenant().is_enabled = False main.main() @@ -177,6 +180,7 @@ class TestFunctions(unittest.TestCase): m_event.return_value.is_set.assert_called_once_with() self.assertIsInstance(m_sys_exit.call_args[0][0], int) + @mock.patch.object(main, 'connection') @mock.patch.object(main, 'os_client_config', autospec=True) @mock.patch.object(main, 'shade') @mock.patch('argparse.ArgumentParser.parse_args') @@ -184,12 +188,12 @@ class TestFunctions(unittest.TestCase): @mock.patch('concurrent.futures.ThreadPoolExecutor', autospec=True) @mock.patch('sys.exit', autospec=True) def test_main_resource(self, m_sys_exit, m_tpe, m_event, m_parse_args, - m_shade, m_oscc): + m_shade, m_oscc, m_conn): m_tpe.return_value.__enter__.return_value.map.side_effect = \ KeyboardInterrupt m_parse_args.return_value.purge_own_project = False m_parse_args.return_value.resource = "Networks" - m_shade.operator_cloud().get_project().enabled = False + m_conn.Connection().identity.get_tenant().is_enabled = False main.main() m_tpe.return_value.__enter__.assert_called_once_with() executor = m_tpe.return_value.__enter__.return_value @@ -198,19 +202,23 @@ class TestFunctions(unittest.TestCase): self.assertIsInstance(obj, ServiceResource) +@mock.patch.object(main, 'os_client_config') +@mock.patch.object(main, 'connection') @mock.patch.object(main, 'shade') class TestCredentialsManager(unittest.TestCase): - def test_init_with_purge_own_project(self, m_shade): + def test_init_with_purge_own_project(self, m_shade, m_conn, m_osc): _options = SimpleNamespace( purge_own_project=True, purge_project=None) - creds_mgr = main.CredentialsManager(_options) + _config = m_osc.OpenStackConfig() + creds_mgr = main.CredentialsManager(_options, _config) self.assertEqual(_options, creds_mgr.options) self.assertEqual(False, creds_mgr.revoke_role_after_purge) self.assertEqual(False, creds_mgr.disable_project_after_purge) - self.assertIsNone(creds_mgr.operator_cloud) + self.assertIsNone(creds_mgr.connection) - m_shade.openstack_cloud.assert_called_once_with(argparse=_options) + m_shade.openstack_cloud.assert_called_once_with( + argparse=_options, config=_config) self.assertEqual(m_shade.openstack_cloud.return_value, creds_mgr.cloud) @@ -226,24 +234,25 @@ class TestCredentialsManager(unittest.TestCase): creds_mgr.cloud.cloud_config.get_auth_args.assert_called_once_with() @mock.patch.object(utils, 'replace_project_info') - def test_init_with_purge_project(self, m_replace, m_shade): + def test_init_with_purge_project(self, m_replace, m_shade, m_conn, m_osc): + _config = m_osc.OpenStackConfig() _options = SimpleNamespace( purge_own_project=False, purge_project=mock.sentinel.purge_project) - creds_mgr = main.CredentialsManager(_options) + creds_mgr = main.CredentialsManager(_options, _config) - m_shade.operator_cloud.assert_called_once_with(argparse=_options) - self.assertEqual(m_shade.operator_cloud.return_value, - creds_mgr.operator_cloud) + m_conn.Connection.assert_called_once_with(config=_config.get_one()) + self.assertEqual(m_conn.Connection.return_value, + creds_mgr.connection) - creds_mgr.operator_cloud.get_project.assert_called_once_with( + creds_mgr.connection.identity.get_tenant.assert_called_once_with( _options.purge_project) self.assertEqual( - creds_mgr.operator_cloud.keystone_session.get_user_id.return_value, + creds_mgr.connection.identity.get_user_id.return_value, creds_mgr.user_id ) self.assertEqual( - creds_mgr.operator_cloud.get_project()['id'], + creds_mgr.connection.identity.get_tenant().id, creds_mgr.project_id ) self.assertFalse(creds_mgr.disable_project_after_purge) @@ -252,65 +261,72 @@ class TestCredentialsManager(unittest.TestCase): creds_mgr.cloud ) m_replace.assert_called_once_with( - creds_mgr.operator_cloud.cloud_config.config, + creds_mgr.connection.config.config, creds_mgr.project_id ) creds_mgr.cloud.cloud_config.get_auth_args.assert_called_once_with() - def test_init_with_project_not_found(self, m_shade): - m_shade.operator_cloud.return_value.get_project.return_value = None + def test_init_with_project_not_found(self, m_shade, m_conn, m_osc): + m_conn.Connection.return_value.identity.get_tenant\ + .side_effect = os_exceptions.NotFoundException self.assertRaises( exceptions.OSProjectNotFound, - main.CredentialsManager, mock.Mock(purge_own_project=False) + main.CredentialsManager, + mock.Mock(purge_own_project=False), m_osc.OpenStackConfig() ) - def test_ensure_role_on_project(self, m_shade): + def test_ensure_role_on_project(self, m_shade, m_conn, m_osc): options = mock.Mock(purge_own_project=False) - creds_manager = main.CredentialsManager(options) + config = m_osc.OpenStackConfig() + creds_manager = main.CredentialsManager(options, config) creds_manager.ensure_role_on_project() - m_shade.operator_cloud.return_value.grant_role.assert_called_once_with( + m_conn.Connection.return_value.grant_role.assert_called_once_with( options.admin_role_name, project=options.purge_project, user=mock.ANY) self.assertEqual(True, creds_manager.revoke_role_after_purge) # If purge_own_project is not False, we purge our own project # so no need to revoke role after purge - creds_manager = main.CredentialsManager(mock.Mock()) + creds_manager = main.CredentialsManager( + mock.Mock(), m_osc.OpenStackConfig()) creds_manager.ensure_role_on_project() self.assertEqual(False, creds_manager.revoke_role_after_purge) - def test_revoke_role_on_project(self, m_shade): + def test_revoke_role_on_project(self, m_shade, m_conn, m_osc): options = mock.Mock(purge_own_project=False) - creds_manager = main.CredentialsManager(options) + config = m_osc.OpenStackConfig() + creds_manager = main.CredentialsManager(options, config) creds_manager.revoke_role_on_project() - m_shade.operator_cloud().revoke_role.assert_called_once_with( + m_conn.Connection().revoke_role.assert_called_once_with( options.admin_role_name, project=options.purge_project, user=mock.ANY) - def test_ensure_enabled_project(self, m_shade): - m_shade.operator_cloud().get_project().enabled = False + def test_ensure_enabled_project(self, m_shade, m_conn, m_osc): + m_conn.Connection().identity.get_tenant().is_enabled = False creds_manager = main.CredentialsManager( - mock.Mock(purge_own_project=False)) + mock.Mock(purge_own_project=False), m_osc.OpenStackConfig()) creds_manager.ensure_enabled_project() self.assertEqual(True, creds_manager.disable_project_after_purge) - m_shade.operator_cloud().update_project.assert_called_once_with( + m_conn.Connection().update_project.assert_called_once_with( mock.ANY, enabled=True) # If project is enabled before purge, no need to disable it after # purge - creds_manager = main.CredentialsManager(mock.Mock()) + creds_manager = main.CredentialsManager( + mock.Mock(), m_osc.OpenStackConfig()) creds_manager.ensure_enabled_project() self.assertEqual(False, creds_manager.disable_project_after_purge) - self.assertEqual(1, m_shade.operator_cloud().update_project.call_count) + self.assertEqual(1, m_conn.Connection().update_project.call_count) - def test_disable_project(self, m_shade): + def test_disable_project(self, m_shade, m_conn, m_osc): options = mock.Mock(purge_own_project=False) - creds_manager = main.CredentialsManager(options) + config = m_osc.OpenStackConfig() + creds_manager = main.CredentialsManager(options, config) creds_manager.disable_project() - m_shade.operator_cloud().update_project.assert_called_once_with( + m_conn.Connection().update_project.assert_called_once_with( mock.ANY, enabled=False )