openstack/python-ironicclient
Code Issues Proposed changes

Get rid of the oslo.config requirement

Browse Source 
It is only used to populate the list of supported Session options.
Use openstacksdk to create a session with all arguments instead.
Since it's already a dependency of osc-lib, this changes adds no
new dependencies, while substantially simplifying the code.

Change-Id: I6100f94c18cbf3e90d4a6d3ec2a3fc74ebea53a5
This commit is contained in:
Dmitry Tantsur 2020-04-06 13:49:27 +02:00
parent 565a256f4e
commit b21dc1dddd
4 changed files with 45 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
ironicclient/client.py
View File

@ -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(

47
ironicclient/tests/unit/test_client.py
View File

@ -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(

6
releasenotes/notes/session-create-092172964afdb71b.yaml Normal file
View File

@ -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.

1
requirements.txt
View File

@ -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