diff --git a/ironicclient/client.py b/ironicclient/client.py index 29cf335b6..f556847a0 100644 --- a/ironicclient/client.py +++ b/ironicclient/client.py @@ -12,7 +12,7 @@ import logging -from keystoneauth1 import loading as kaloading +from openstack import config from oslo_utils import importutils from ironicclient.common.i18n import _ @@ -22,7 +22,9 @@ LOG = logging.getLogger(__name__) def get_client(api_version, auth_type=None, os_ironic_api_version=None, - max_retries=None, retry_interval=None, session=None, **kwargs): + max_retries=None, retry_interval=None, session=None, + valid_interfaces=None, interface=None, service_type=None, + region_name=None, **kwargs): """Get an authenticated client, based on the credentials. :param api_version: the API version to use. Valid value: '1'. @@ -33,6 +35,12 @@ def get_client(api_version, auth_type=None, os_ironic_api_version=None, of conflict error. :param session: An existing keystoneauth session. Will be created from kwargs if not provided. + :param valid_interfaces: List of valid endpoint interfaces to use if + the bare metal endpoint is not provided. + :param interface: An alias for valid_interfaces. + :param service_type: Bare metal endpoint service type. + :param region_name: Name of the region to use when searching the bare metal + endpoint. :param kwargs: all the other params that are passed to keystoneauth. """ # TODO(TheJulia): At some point, we should consider possibly noting @@ -50,30 +58,22 @@ def get_client(api_version, auth_type=None, os_ironic_api_version=None, auth_type = 'token' else: auth_type = 'password' + if not session: - loader = kaloading.get_plugin_loader(auth_type) - loader_options = loader.get_options() - # option.name looks like 'project-name', while dest will be the actual - # argument name to which the value will be passed to (project_name) - auth_options = [o.dest for o in loader_options] - # Include deprecated names as well - auth_options.extend([d.dest for o in loader_options - for d in o.deprecated]) - auth_kwargs = {k: v for (k, v) in kwargs.items() if k in auth_options} - auth_plugin = loader.load_from_options(**auth_kwargs) - # Let keystoneauth do the necessary parameter conversions - session_loader = kaloading.session.Session() - session_opts = {k: v for (k, v) in kwargs.items() if k in - [o.dest for o in session_loader.get_conf_options()]} - session = session_loader.load_from_options(auth=auth_plugin, - **session_opts) + # TODO(dtantsur): consider flipping load_yaml_config to True to support + # the clouds.yaml format. + region = config.get_cloud_region(load_yaml_config=False, + load_envvars=False, + auth_type=auth_type, + **kwargs) + session = region.get_session() # Make sure we also pass the endpoint interface to the HTTP client. # NOTE(gyee/efried): 'interface' in ksa config is deprecated in favor of # 'valid_interfaces'. So, since the caller may be deriving kwargs from # conf, accept 'valid_interfaces' first. But keep support for 'interface', # in case the caller is deriving kwargs from, say, an existing Adapter. - interface = kwargs.get('valid_interfaces', kwargs.get('interface')) + interface = valid_interfaces or interface endpoint = kwargs.get('endpoint') if not endpoint: @@ -84,9 +84,9 @@ def get_client(api_version, auth_type=None, os_ironic_api_version=None, # 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', + service_type=service_type or 'baremetal', interface=interface, - region_name=kwargs.get('region_name') + region_name=region_name, ) except Exception as e: raise exc.AmbiguousAuthSystem( diff --git a/ironicclient/tests/unit/test_client.py b/ironicclient/tests/unit/test_client.py index fc43aa71d..048afbc5e 100644 --- a/ironicclient/tests/unit/test_client.py +++ b/ironicclient/tests/unit/test_client.py @@ -12,7 +12,7 @@ import mock -from keystoneauth1 import loading as kaloading +from openstack import config from ironicclient import client as iroclient from ironicclient.common import filecache @@ -25,50 +25,39 @@ from ironicclient.v1 import client as v1 class ClientTest(utils.BaseTestCase): @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, version=None, - auth='password', expected_interface=None, **kwargs): - session = mock_ks_session.return_value.load_from_options.return_value + @mock.patch.object(config, 'get_cloud_region', autospec=True) + def _test_get_client(self, mock_cloud_region, mock_retrieve_data, + version=None, auth='password', + expected_interface=None, **kwargs): + session = mock_cloud_region.return_value.get_session.return_value session.get_endpoint.return_value = 'http://localhost:6385/v1/f14b4123' - - class Opt(object): - def __init__(self, name): - self.dest = name - - session_loader_options = [ - Opt('insecure'), Opt('cafile'), Opt('certfile'), Opt('keyfile'), - Opt('timeout')] - mock_ks_session.return_value.get_conf_options.return_value = ( - session_loader_options) - mock_ks_loader.return_value.load_from_options.return_value = 'auth' mock_retrieve_data.return_value = version client = iroclient.get_client('1', **kwargs) - mock_ks_loader.assert_called_once_with(auth) - session_opts = {k: v for (k, v) in kwargs.items() if k in - [o.dest for o in session_loader_options]} - mock_ks_session.return_value.load_from_options.assert_called_once_with( - auth='auth', **session_opts) + expected_version = kwargs.pop('os_ironic_api_version', None) + kwargs.pop('interface', None) + kwargs.pop('valid_interfaces', None) + get_endpoint_call = mock.call( - service_type=kwargs.get('service_type') or 'baremetal', + service_type=kwargs.pop('service_type', None) or 'baremetal', interface=expected_interface, - region_name=kwargs.get('region_name')) - if not {'endpoint'}.intersection(kwargs): + region_name=kwargs.pop('region_name', None)) + mock_cloud_region.assert_called_once_with(load_yaml_config=False, + load_envvars=False, + auth_type=auth, **kwargs) + if 'endpoint' not in kwargs: self.assertEqual([get_endpoint_call], session.get_endpoint.call_args_list) else: # we use adaper.get_endpoint instead of session.get_endpoint self.assertFalse(session.get_endpoint.called) - if 'os_ironic_api_version' in kwargs: + if expected_version is not None: # NOTE(TheJulia): This does not test the negotiation logic # as a request must be triggered in order for any version # negotiation actions to occur. self.assertEqual(0, mock_retrieve_data.call_count) - self.assertEqual(kwargs['os_ironic_api_version'], - client.current_api_version) + self.assertEqual(expected_version, client.current_api_version) self.assertFalse(client.is_api_version_negotiated) else: mock_retrieve_data.assert_called_once_with( diff --git a/releasenotes/notes/session-create-092172964afdb71b.yaml b/releasenotes/notes/session-create-092172964afdb71b.yaml new file mode 100644 index 000000000..b8429fad9 --- /dev/null +++ b/releasenotes/notes/session-create-092172964afdb71b.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - | + When no session is provided in the ``get_client`` call, a session is now + created using OpenStackSDK. Only arguments that are supported by it are + supported now. diff --git a/requirements.txt b/requirements.txt index 6cc183636..40b1bf233 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,7 +9,6 @@ jsonschema>=2.6.0 # MIT keystoneauth1>=3.4.0 # Apache-2.0 openstacksdk>=0.18.0 # Apache-2.0 osc-lib>=1.10.0 # Apache-2.0 -oslo.config>=5.2.0 # Apache-2.0 oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0 oslo.utils>=3.33.0 # Apache-2.0 PyYAML>=3.12 # MIT