diff --git a/ironicclient/client.py b/ironicclient/client.py index b785fafa3..83b0e148a 100644 --- a/ironicclient/client.py +++ b/ironicclient/client.py @@ -100,14 +100,22 @@ def get_client(api_version, auth_type=None, os_ironic_api_version=None, session = session_loader.load_from_options(auth=auth_plugin, **session_opts) + # Make sure we also pass the endpoint interface to the HTTP client. + # NOTE(gyee): we are supposed to be using valid_interfaces as interface + # is deprecated. + interface = kwargs.get('interface') + endpoint = kwargs.get('endpoint') if not endpoint: try: # endpoint will be used to get hostname # and port that will be used for API version caching. + # NOTE(gyee): KSA defaults interface to 'public' if it is + # empty or None so there's no need to set it to publicURL + # explicitly. endpoint = session.get_endpoint( service_type=kwargs.get('service_type') or 'baremetal', - interface=kwargs.get('interface') or 'publicURL', + interface=interface, region_name=kwargs.get('region_name') ) except Exception as e: @@ -120,7 +128,8 @@ def get_client(api_version, auth_type=None, os_ironic_api_version=None, 'max_retries': max_retries, 'retry_interval': retry_interval, 'session': session, - 'endpoint_override': endpoint + 'endpoint_override': endpoint, + 'interface': interface } return Client(api_version, **ironicclient_kwargs) diff --git a/ironicclient/tests/unit/test_client.py b/ironicclient/tests/unit/test_client.py index 072ce3738..7d10701f4 100644 --- a/ironicclient/tests/unit/test_client.py +++ b/ironicclient/tests/unit/test_client.py @@ -32,7 +32,8 @@ class ClientTest(utils.BaseTestCase): @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, **kwargs): + auth='password', warn_mock_call_count=0, + 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' @@ -60,7 +61,7 @@ class ClientTest(utils.BaseTestCase): if not {'endpoint', 'ironic_url'}.intersection(kwargs): session.get_endpoint.assert_called_once_with( service_type=kwargs.get('service_type') or 'baremetal', - interface=kwargs.get('interface') or 'publicURL', + interface=expected_interface, region_name=kwargs.get('region_name')) if 'os_ironic_api_version' in kwargs: # NOTE(TheJulia): This does not test the negotiation logic @@ -76,6 +77,15 @@ class ClientTest(utils.BaseTestCase): port='6385') self.assertEqual(version or v1.DEFAULT_VER, client.http_client.os_ironic_api_version) + + # make sure the interface is conveyed to the client + 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): @@ -128,6 +138,32 @@ class ClientTest(utils.BaseTestCase): } self._test_get_client(warn_mock_call_count=4, **kwargs) + def test_get_client_and_endpoint_type(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' + } + self._test_get_client(warn_mock_call_count=5, + expected_interface='adminURL', **kwargs) + + def test_get_client_and_interface(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': '', + 'interface': 'internal' + } + self._test_get_client(warn_mock_call_count=4, + expected_interface='internal', **kwargs) + def test_get_client_with_region_no_auth_token(self): kwargs = { 'os_project_name': 'PROJECT_NAME', @@ -214,7 +250,7 @@ class ClientTest(utils.BaseTestCase): } iroclient.get_client('1', **kwargs) session.get_endpoint.assert_called_once_with(service_type='baremetal', - interface='publicURL', + interface=None, region_name=None) def test_get_client_incorrect_session_passed(self): @@ -229,7 +265,7 @@ class ClientTest(utils.BaseTestCase): @mock.patch.object(kaloading.session, 'Session', autospec=True) def _test_loader_arguments_passed_correctly( self, mock_ks_session, passed_kwargs, expected_kwargs, - loader_class): + 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' @@ -246,9 +282,8 @@ class ClientTest(utils.BaseTestCase): auth=mock.ANY, **session_opts) if 'ironic_url' not in passed_kwargs: service_type = passed_kwargs.get('service_type') or 'baremetal' - interface = passed_kwargs.get('interface') or 'publicURL' session.get_endpoint.assert_called_once_with( - service_type=service_type, interface=interface, + service_type=service_type, interface=expected_interface, region_name=passed_kwargs.get('region_name')) def test_loader_arguments_admin_token(self): @@ -282,6 +317,24 @@ class ClientTest(utils.BaseTestCase): 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', @@ -344,7 +397,8 @@ class ClientTest(utils.BaseTestCase): 'max_retries': None, 'retry_interval': None, 'session': session, - 'endpoint_override': 'http://ironic.example.org:6385/'} + 'endpoint_override': 'http://ironic.example.org:6385/', + 'interface': None} ) self.assertFalse(session.get_endpoint.called) @@ -368,7 +422,8 @@ class ClientTest(utils.BaseTestCase): 'max_retries': None, 'retry_interval': None, 'session': session, - 'endpoint_override': session.get_endpoint.return_value} + 'endpoint_override': session.get_endpoint.return_value, + 'interface': None} ) @mock.patch.object(iroclient, 'Client', autospec=True) @@ -385,7 +440,8 @@ class ClientTest(utils.BaseTestCase): 'max_retries': None, 'retry_interval': None, 'session': session, - 'endpoint_override': session.get_endpoint.return_value} + 'endpoint_override': session.get_endpoint.return_value, + 'interface': None} ) self.assertFalse(mock_ks_session.called) diff --git a/releasenotes/notes/pass-interface-argument-deb92e3feb0bf051.yaml b/releasenotes/notes/pass-interface-argument-deb92e3feb0bf051.yaml new file mode 100644 index 000000000..afebe7eba --- /dev/null +++ b/releasenotes/notes/pass-interface-argument-deb92e3feb0bf051.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + The ``interface`` argument was being ignored so that the HTTP client was + always using the public endpoint for Ironic. This fixes it so that the + ``interface`` argument is taken into consideration. See + `story 2005118 `_.