Merge "Remove deprecated keystone arguments"

3 files changed, 59 insertions, 308 deletions

diff --git a/ironicclient/client.py b/ironicclient/client.py
index 66fd6d0..dbcd5c5 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 d9c77f9..754119c 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 0000000..3cd05d0
--- /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.