From 2b141a3f3a7edf03ee8978b43601c3c2679aa77c Mon Sep 17 00:00:00 2001 From: Riccardo Pittau Date: Wed, 21 Aug 2019 14:21:26 +0200 Subject: [PATCH] Remove deprecated keystone arguments Removing deprecated keystone arguments in favor of standardized argument naming. Change-Id: Ic7a0cd96c9bd0a652fec78170038fd770f6abd52 Story: 2006422 Task: 36318 --- ironicclient/client.py | 33 -- ironicclient/tests/unit/test_client.py | 329 +++--------------- ...ecated-keystone-args-925ac5f3607a89a3.yaml | 5 + 3 files changed, 59 insertions(+), 308 deletions(-) create mode 100644 releasenotes/notes/remove-deprecated-keystone-args-925ac5f3607a89a3.yaml diff --git a/ironicclient/client.py b/ironicclient/client.py index 66fd6d09d..dbcd5c5eb 100644 --- a/ironicclient/client.py +++ b/ironicclient/client.py @@ -21,38 +21,6 @@ from ironicclient import exc LOG = logging.getLogger(__name__) -# TODO(vdrok): remove in Stein -def convert_keystoneauth_opts(kwargs): - old_to_new_names = { - ('os_auth_token',): 'token', - ('os_username',): 'username', - ('os_password',): 'password', - ('os_auth_url',): 'auth_url', - ('os_project_id',): 'project_id', - ('os_project_name',): 'project_name', - ('os_tenant_id',): 'tenant_id', - ('os_tenant_name',): 'tenant_name', - ('os_region_name',): 'region_name', - ('os_user_domain_id',): 'user_domain_id', - ('os_user_domain_name',): 'user_domain_name', - ('os_project_domain_id',): 'project_domain_id', - ('os_project_domain_name',): 'project_domain_name', - ('os_service_type',): 'service_type', - ('os_endpoint_type',): 'interface', - ('ironic_url',): 'endpoint', - ('os_cacert', 'ca_file'): 'cafile', - ('os_cert', 'cert_file'): 'certfile', - ('os_key', 'key_file'): 'keyfile' - } - for olds, new in old_to_new_names.items(): - for old in olds: - if kwargs.get(old): - LOG.warning('The argument "%s" passed to get_client is ' - 'deprecated and will be removed in Stein release, ' - 'please use "%s" instead.', old, new) - kwargs.setdefault(new, kwargs[old]) - - def get_client(api_version, auth_type=None, os_ironic_api_version=None, max_retries=None, retry_interval=None, **kwargs): """Get an authenticated client, based on the credentials. @@ -70,7 +38,6 @@ def get_client(api_version, auth_type=None, os_ironic_api_version=None, # auto-negotiate to the greatest available version, however we do not # have the ability yet for a caller to cap the version, and will hold # off doing so until then. - convert_keystoneauth_opts(kwargs) if auth_type is None: if 'endpoint' in kwargs: if 'token' in kwargs: diff --git a/ironicclient/tests/unit/test_client.py b/ironicclient/tests/unit/test_client.py index d9c77f965..754119c8a 100644 --- a/ironicclient/tests/unit/test_client.py +++ b/ironicclient/tests/unit/test_client.py @@ -12,9 +12,7 @@ import mock -from keystoneauth1 import identity from keystoneauth1 import loading as kaloading -from keystoneauth1 import token_endpoint from ironicclient import client as iroclient from ironicclient.common import filecache @@ -26,14 +24,12 @@ from ironicclient.v1 import client as v1 class ClientTest(utils.BaseTestCase): - @mock.patch.object(iroclient.LOG, 'warning', autospec=True) @mock.patch.object(filecache, 'retrieve_data', autospec=True) @mock.patch.object(kaloading.session, 'Session', autospec=True) @mock.patch.object(kaloading, 'get_plugin_loader', autospec=True) def _test_get_client(self, mock_ks_loader, mock_ks_session, - mock_retrieve_data, warn_mock, version=None, - auth='password', warn_mock_call_count=0, - expected_interface=None, **kwargs): + mock_retrieve_data, version=None, + auth='password', expected_interface=None, **kwargs): session = mock_ks_session.return_value.load_from_options.return_value session.get_endpoint.return_value = 'http://localhost:6385/v1/f14b4123' @@ -50,8 +46,6 @@ class ClientTest(utils.BaseTestCase): mock_retrieve_data.return_value = version client = iroclient.get_client('1', **kwargs) - self.assertEqual(warn_mock_call_count, warn_mock.call_count) - iroclient.convert_keystoneauth_opts(kwargs) mock_ks_loader.assert_called_once_with(auth) session_opts = {k: v for (k, v) in kwargs.items() if k in @@ -62,7 +56,7 @@ class ClientTest(utils.BaseTestCase): service_type=kwargs.get('service_type') or 'baremetal', interface=expected_interface, region_name=kwargs.get('region_name')) - if not {'endpoint', 'ironic_url'}.intersection(kwargs): + if not {'endpoint'}.intersection(kwargs): calls = [get_endpoint_call, mock.call(interface=client.http_client.interface, service_type=client.http_client.service_type, @@ -90,20 +84,9 @@ class ClientTest(utils.BaseTestCase): if expected_interface is not None: self.assertEqual(expected_interface, client.http_client.interface) - if kwargs.get('os_endpoint_type'): - self.assertEqual(kwargs['os_endpoint_type'], - client.http_client.interface) return client - def test_get_client_only_ironic_url(self): - kwargs = {'ironic_url': 'http://localhost:6385/v1'} - client = self._test_get_client(auth='none', - warn_mock_call_count=1, **kwargs) - self.assertIsInstance(client.http_client, http.SessionClient) - self.assertEqual('http://localhost:6385', - client.http_client.endpoint_override) - def test_get_client_only_endpoint(self): kwargs = {'endpoint': 'http://localhost:6385/v1'} client = self._test_get_client(auth='none', **kwargs) @@ -111,14 +94,13 @@ class ClientTest(utils.BaseTestCase): self.assertEqual('http://localhost:6385', client.http_client.endpoint_override) - def test_get_client_with_auth_token_ironic_url(self): + def test_get_client_with_auth_token_endpoint(self): kwargs = { - 'ironic_url': 'http://localhost:6385/v1', - 'os_auth_token': 'USER_AUTH_TOKEN', + 'endpoint': 'http://localhost:6385/v1', + 'token': 'USER_AUTH_TOKEN', } - client = self._test_get_client(auth='admin_token', - warn_mock_call_count=2, **kwargs) + client = self._test_get_client(auth='admin_token', **kwargs) self.assertIsInstance(client.http_client, http.SessionClient) self.assertEqual('http://localhost:6385', @@ -126,101 +108,89 @@ class ClientTest(utils.BaseTestCase): def test_get_client_no_auth_token(self): kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', } - self._test_get_client(warn_mock_call_count=4, **kwargs) + self._test_get_client(**kwargs) - def test_get_client_service_and_endpoint_type_defaults(self): + def test_get_client_service_and_interface_defaults(self): kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', - 'os_service_type': '', - 'os_endpoint_type': '' + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', + 'service_type': '', } - self._test_get_client(warn_mock_call_count=4, **kwargs) + self._test_get_client(**kwargs) - def test_get_client_and_endpoint_type(self): + def test_get_client_and_interface_adminurl(self): kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', - 'os_service_type': '', - 'os_endpoint_type': 'adminURL' + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', + 'service_type': '', + 'interface': 'adminURL' } - self._test_get_client(warn_mock_call_count=5, - expected_interface='adminURL', **kwargs) + self._test_get_client(expected_interface='adminURL', **kwargs) - def test_get_client_and_interface(self): + def test_get_client_and_interface_internal(self): kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', - 'os_service_type': '', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', + 'service_type': '', 'interface': 'internal' } - self._test_get_client(warn_mock_call_count=4, - expected_interface='internal', **kwargs) + self._test_get_client(expected_interface='internal', **kwargs) def test_get_client_and_valid_interfaces(self): kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', - 'os_service_type': '', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', + 'service_type': '', 'valid_interfaces': ['internal', 'public'] } - self._test_get_client(warn_mock_call_count=4, - expected_interface=['internal', 'public'], + self._test_get_client(expected_interface=['internal', 'public'], **kwargs) def test_get_client_and_interface_and_valid_interfaces(self): """Ensure 'valid_interfaces' takes precedence over 'interface'.""" kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_password': 'PASSWORD', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', - 'os_service_type': '', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'password': 'PASSWORD', + 'auth_url': 'http://localhost:35357/v2.0', + 'service_type': '', 'interface': ['ignored'], 'valid_interfaces': ['internal', 'public'] } - self._test_get_client(warn_mock_call_count=4, - expected_interface=['internal', 'public'], + self._test_get_client(expected_interface=['internal', 'public'], **kwargs) def test_get_client_with_region_no_auth_token(self): kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', 'os_password': 'PASSWORD', 'os_region_name': 'REGIONONE', - 'os_auth_url': 'http://localhost:35357/v2.0', - 'os_auth_token': '', + 'auth_url': 'http://localhost:35357/v2.0', } - self._test_get_client(warn_mock_call_count=5, **kwargs) + self._test_get_client(**kwargs) def test_get_client_incorrect_auth_params(self): kwargs = { - 'os_project_name': 'PROJECT_NAME', - 'os_username': 'USERNAME', - 'os_auth_url': 'http://localhost:35357/v2.0', + 'project_name': 'PROJECT_NAME', + 'username': 'USERNAME', + 'auth_url': 'http://localhost:35357/v2.0', } self.assertRaises(exc.AmbiguousAuthSystem, iroclient.get_client, - '1', warn_mock_call_count=3, **kwargs) + '1', **kwargs) def test_get_client_with_api_version_latest(self): kwargs = { @@ -238,7 +208,6 @@ class ClientTest(utils.BaseTestCase): 'username': 'USERNAME', 'password': 'PASSWORD', 'auth_url': 'http://localhost:35357/v2.0', - 'auth_token': '', 'os_ironic_api_version': ['1.1', '1.99'], } self._test_get_client(**kwargs) @@ -300,193 +269,3 @@ class ClientTest(utils.BaseTestCase): } self.assertRaises(exc.AmbiguousAuthSystem, iroclient.get_client, '1', **kwargs) - - @mock.patch.object(kaloading.session, 'Session', autospec=True) - def _test_loader_arguments_passed_correctly( - self, mock_ks_session, passed_kwargs, expected_kwargs, - loader_class, expected_interface=None): - session = mock_ks_session.return_value.load_from_options.return_value - session.get_endpoint.return_value = 'http://localhost:6385/v1/f14b4123' - - with mock.patch.object(loader_class, '__init__', - autospec=True) as init_mock: - init_mock.return_value = None - client = iroclient.get_client('1', **passed_kwargs) - iroclient.convert_keystoneauth_opts(passed_kwargs) - - init_mock.assert_called_once_with(mock.ANY, **expected_kwargs) - session_opts = {k: v for (k, v) in passed_kwargs.items() if k in - ['insecure', 'cacert', 'cert', 'key', 'timeout']} - mock_ks_session.return_value.load_from_options.assert_called_once_with( - auth=mock.ANY, **session_opts) - if 'ironic_url' not in passed_kwargs: - service_type = passed_kwargs.get('service_type') or 'baremetal' - endpoint_calls = [ - mock.call(service_type=service_type, - interface=expected_interface, - region_name=passed_kwargs.get('region_name')), - mock.call(service_type=client.http_client.service_type, - interface=client.http_client.interface, - region_name=client.http_client.region_name) - ] - self.assertEqual(endpoint_calls, - session.get_endpoint.call_args_list) - - def test_loader_arguments_admin_token(self): - passed_kwargs = { - 'ironic_url': 'http://localhost:6385/v1', - 'os_auth_token': 'USER_AUTH_TOKEN', - } - expected_kwargs = { - 'endpoint': 'http://localhost:6385/v1', - 'token': 'USER_AUTH_TOKEN' - } - self._test_loader_arguments_passed_correctly( - passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs, - loader_class=token_endpoint.Token - ) - - def test_loader_arguments_token(self): - passed_kwargs = { - 'os_auth_url': 'http://localhost:35357/v3', - 'os_region_name': 'REGIONONE', - 'os_auth_token': 'USER_AUTH_TOKEN', - 'os_project_name': 'admin' - } - expected_kwargs = { - 'auth_url': 'http://localhost:35357/v3', - 'project_name': 'admin', - 'token': 'USER_AUTH_TOKEN' - } - self._test_loader_arguments_passed_correctly( - passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs, - loader_class=identity.Token - ) - - def test_loader_arguments_interface(self): - passed_kwargs = { - 'os_auth_url': 'http://localhost:35357/v3', - 'os_region_name': 'REGIONONE', - 'os_auth_token': 'USER_AUTH_TOKEN', - 'os_project_name': 'admin', - 'interface': 'internal' - } - expected_kwargs = { - 'auth_url': 'http://localhost:35357/v3', - 'project_name': 'admin', - 'token': 'USER_AUTH_TOKEN' - } - self._test_loader_arguments_passed_correctly( - passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs, - loader_class=identity.Token, expected_interface='internal' - ) - - def test_loader_arguments_password_tenant_name(self): - passed_kwargs = { - 'os_auth_url': 'http://localhost:35357/v3', - 'os_region_name': 'REGIONONE', - 'os_project_name': 'PROJECT', - 'os_username': 'user', - 'os_password': '1234', - 'os_project_domain_id': 'DEFAULT', - 'os_user_domain_id': 'DEFAULT' - } - expected_kwargs = { - 'auth_url': 'http://localhost:35357/v3', - 'project_name': 'PROJECT', - 'user_domain_id': 'DEFAULT', - 'project_domain_id': 'DEFAULT', - 'username': 'user', - 'password': '1234' - } - self._test_loader_arguments_passed_correctly( - passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs, - loader_class=identity.Password - ) - - def test_loader_arguments_password_project_id(self): - passed_kwargs = { - 'os_auth_url': 'http://localhost:35357/v3', - 'os_region_name': 'REGIONONE', - 'os_project_id': '1000', - 'os_username': 'user', - 'os_password': '1234', - 'os_project_domain_name': 'domain1', - 'os_user_domain_name': 'domain1' - } - expected_kwargs = { - 'auth_url': 'http://localhost:35357/v3', - 'project_id': '1000', - 'user_domain_name': 'domain1', - 'project_domain_name': 'domain1', - 'username': 'user', - 'password': '1234' - } - self._test_loader_arguments_passed_correctly( - passed_kwargs=passed_kwargs, expected_kwargs=expected_kwargs, - loader_class=identity.Password - ) - - @mock.patch.object(iroclient, 'Client', autospec=True) - @mock.patch.object(kaloading.session, 'Session', autospec=True) - def test_correct_arguments_passed_to_client_constructor_noauth_mode( - self, mock_ks_session, mock_client): - session = mock_ks_session.return_value.load_from_options.return_value - kwargs = { - 'ironic_url': 'http://ironic.example.org:6385/', - 'os_auth_token': 'USER_AUTH_TOKEN', - 'os_ironic_api_version': 'latest', - } - iroclient.get_client('1', **kwargs) - mock_client.assert_called_once_with( - '1', **{'os_ironic_api_version': 'latest', - 'max_retries': None, - 'retry_interval': None, - 'session': session, - 'endpoint_override': 'http://ironic.example.org:6385/', - 'interface': None} - ) - self.assertFalse(session.get_endpoint.called) - - @mock.patch.object(iroclient, 'Client', autospec=True) - @mock.patch.object(kaloading.session, 'Session', autospec=True) - def test_correct_arguments_passed_to_client_constructor_session_created( - self, mock_ks_session, mock_client): - session = mock_ks_session.return_value.load_from_options.return_value - kwargs = { - 'os_auth_url': 'http://localhost:35357/v3', - 'os_region_name': 'REGIONONE', - 'os_project_id': '1000', - 'os_username': 'user', - 'os_password': '1234', - 'os_project_domain_name': 'domain1', - 'os_user_domain_name': 'domain1' - } - iroclient.get_client('1', **kwargs) - mock_client.assert_called_once_with( - '1', **{'os_ironic_api_version': None, - 'max_retries': None, - 'retry_interval': None, - 'session': session, - 'endpoint_override': session.get_endpoint.return_value, - 'interface': None} - ) - - @mock.patch.object(iroclient, 'Client', autospec=True) - @mock.patch.object(kaloading.session, 'Session', autospec=True) - def test_correct_arguments_passed_to_client_constructor_session_passed( - self, mock_ks_session, mock_client): - session = mock.Mock() - kwargs = { - 'session': session, - } - iroclient.get_client('1', **kwargs) - mock_client.assert_called_once_with( - '1', **{'os_ironic_api_version': None, - 'max_retries': None, - 'retry_interval': None, - 'session': session, - 'endpoint_override': session.get_endpoint.return_value, - 'interface': None} - ) - self.assertFalse(mock_ks_session.called) diff --git a/releasenotes/notes/remove-deprecated-keystone-args-925ac5f3607a89a3.yaml b/releasenotes/notes/remove-deprecated-keystone-args-925ac5f3607a89a3.yaml new file mode 100644 index 000000000..3cd05d038 --- /dev/null +++ b/releasenotes/notes/remove-deprecated-keystone-args-925ac5f3607a89a3.yaml @@ -0,0 +1,5 @@ +--- +upgrade: + - | + Removing deprecated keystone arguments in favor of standardized + argument naming.