Use keystoneauth for Ironic and Swift clients

This patch does not change the options in config file yet to showcase
backward compatibility with old config options.

Change-Id: I1da93b59b2f4813c42008277bd6479dc6673e7f1

example.conf

@@ -387,59 +387,149 @@
 # From ironic_inspector.common.ironic
 #
 
-# Keystone authentication endpoint for accessing Ironic API. Use
+# Authentication URL (unknown value)
-# [keystone_authtoken]/auth_uri for keystone authentication. (string
+#auth_url = <None>
-# value)
-# Deprecated group/name - [discoverd]/os_auth_url
-#os_auth_url =
 
-# User name for accessing Ironic API. Use
+# Method to use for authentication: noauth or keystone. (string value)
-# [keystone_authtoken]/admin_user for keystone authentication. (string
+# Allowed values: keystone, noauth
-# value)
+#auth_strategy = keystone
-# Deprecated group/name - [discoverd]/os_username
-#os_username =
 
-# Password for accessing Ironic API. Use
+# Authentication type to load (unknown value)
-# [keystone_authtoken]/admin_password for keystone authentication.
+# Deprecated group/name - [DEFAULT]/auth_plugin
-# (string value)
+#auth_type = <None>
-# Deprecated group/name - [discoverd]/os_password
-#os_password =
 
-# Tenant name for accessing Ironic API. Use
+# PEM encoded Certificate Authority to use when verifying HTTPs
-# [keystone_authtoken]/admin_tenant_name for keystone authentication.
+# connections. (string value)
-# (string value)
+#cafile = <None>
-# Deprecated group/name - [discoverd]/os_tenant_name
-#os_tenant_name =
 
-# Keystone admin endpoint. DEPRECATED: use
+# PEM encoded client certificate cert file (string value)
-# [keystone_authtoken]/identity_uri. (string value)
+#certfile = <None>
+
+# Optional domain ID to use with v3 and v2 parameters. It will be used
+# for both the user and project domain in v3 and ignored in v2
+# authentication. (unknown value)
+#default_domain_id = <None>
+
+# Optional domain name to use with v3 API and v2 parameters. It will
+# be used for both the user and project domain in v3 and ignored in v2
+# authentication. (unknown value)
+#default_domain_name = <None>
+
+# Domain ID to scope to (unknown value)
+#domain_id = <None>
+
+# Domain name to scope to (unknown value)
+#domain_name = <None>
+
+# Keystone admin endpoint. DEPRECATED: Use [keystone_authtoken]
+# section for keystone token validation. (string value)
 # Deprecated group/name - [discoverd]/identity_uri
 # This option is deprecated for removal.
 # Its value may be silently ignored in the future.
 #identity_uri =
 
-# Method to use for authentication: noauth or keystone. (string value)
+# Verify HTTPS connections. (boolean value)
-# Allowed values: keystone, noauth
+#insecure = false
-#auth_strategy = keystone
 
 # Ironic API URL, used to set Ironic API URL when auth_strategy option
 # is noauth to work with standalone Ironic without keystone. (string
 # value)
 #ironic_url = http://localhost:6385/
 
-# Ironic service type. (string value)
+# PEM encoded client certificate key file (string value)
-#os_service_type = baremetal
+#keyfile = <None>
+
+# Maximum number of retries in case of conflict error (HTTP 409).
+# (integer value)
+#max_retries = 30
+
+# Keystone authentication endpoint for accessing Ironic API. Use
+# [keystone_authtoken] section for keystone token validation. (string
+# value)
+# Deprecated group/name - [discoverd]/os_auth_url
+# This option is deprecated for removal.
+# Its value may be silently ignored in the future.
+# Reason: Use options presented by configured keystone auth plugin.
+#os_auth_url =
 
 # Ironic endpoint type. (string value)
 #os_endpoint_type = internalURL
 
+# Password for accessing Ironic API. Use [keystone_authtoken] section
+# for keystone token validation. (string value)
+# Deprecated group/name - [discoverd]/os_password
+# This option is deprecated for removal.
+# Its value may be silently ignored in the future.
+# Reason: Use options presented by configured keystone auth plugin.
+#os_password =
+
+# Keystone region used to get Ironic endpoints. (string value)
+#os_region = <None>
+
+# Ironic service type. (string value)
+#os_service_type = baremetal
+
+# Tenant name for accessing Ironic API. Use [keystone_authtoken]
+# section for keystone token validation. (string value)
+# Deprecated group/name - [discoverd]/os_tenant_name
+# This option is deprecated for removal.
+# Its value may be silently ignored in the future.
+# Reason: Use options presented by configured keystone auth plugin.
+#os_tenant_name =
+
+# User name for accessing Ironic API. Use [keystone_authtoken] section
+# for keystone token validation. (string value)
+# Deprecated group/name - [discoverd]/os_username
+# This option is deprecated for removal.
+# Its value may be silently ignored in the future.
+# Reason: Use options presented by configured keystone auth plugin.
+#os_username =
+
+# User's password (unknown value)
+#password = <None>
+
+# Domain ID containing project (unknown value)
+#project_domain_id = <None>
+
+# Domain name containing project (unknown value)
+#project_domain_name = <None>
+
+# Project ID to scope to (unknown value)
+# Deprecated group/name - [DEFAULT]/tenant-id
+#project_id = <None>
+
+# Project name to scope to (unknown value)
+# Deprecated group/name - [DEFAULT]/tenant-name
+#project_name = <None>
+
 # Interval between retries in case of conflict error (HTTP 409).
 # (integer value)
 #retry_interval = 2
 
-# Maximum number of retries in case of conflict error (HTTP 409).
+# Tenant ID (unknown value)
-# (integer value)
+#tenant_id = <None>
-#max_retries = 30
+
+# Tenant Name (unknown value)
+#tenant_name = <None>
+
+# Timeout value for http requests (integer value)
+#timeout = <None>
+
+# Trust ID (unknown value)
+#trust_id = <None>
+
+# User's domain id (unknown value)
+#user_domain_id = <None>
+
+# User's domain name (unknown value)
+#user_domain_name = <None>
+
+# User id (unknown value)
+#user_id = <None>
+
+# Username (unknown value)
+# Deprecated group/name - [DEFAULT]/username
+#username = <None>
 
 
 [keystone_authtoken]
@@ -676,34 +766,112 @@
 # From ironic_inspector.common.swift
 #
 
-# Maximum number of times to retry a Swift request, before failing.
+# Authentication URL (unknown value)
-# (integer value)
+#auth_url = <None>
-#max_retries = 2
+
+# Authentication type to load (unknown value)
+# Deprecated group/name - [DEFAULT]/auth_plugin
+#auth_type = <None>
+
+# PEM encoded Certificate Authority to use when verifying HTTPs
+# connections. (string value)
+#cafile = <None>
+
+# PEM encoded client certificate cert file (string value)
+#certfile = <None>
+
+# Default Swift container to use when creating objects. (string value)
+#container = ironic-inspector
+
+# Optional domain ID to use with v3 and v2 parameters. It will be used
+# for both the user and project domain in v3 and ignored in v2
+# authentication. (unknown value)
+#default_domain_id = <None>
+
+# Optional domain name to use with v3 API and v2 parameters. It will
+# be used for both the user and project domain in v3 and ignored in v2
+# authentication. (unknown value)
+#default_domain_name = <None>
 
 # Number of seconds that the Swift object will last before being
 # deleted. (set to 0 to never delete the object). (integer value)
 #delete_after = 0
 
-# Default Swift container to use when creating objects. (string value)
+# Domain ID to scope to (unknown value)
-#container = ironic-inspector
+#domain_id = <None>
 
-# User name for accessing Swift API. (string value)
+# Domain name to scope to (unknown value)
-#username =
+#domain_name = <None>
 
-# Password for accessing Swift API. (string value)
+# Verify HTTPS connections. (boolean value)
-#password =
+#insecure = false
 
-# Tenant name for accessing Swift API. (string value)
+# PEM encoded client certificate key file (string value)
-#tenant_name =
+#keyfile = <None>
 
-# Keystone authentication API version (string value)
+# Maximum number of times to retry a Swift request, before failing.
-#os_auth_version = 2
+# (integer value)
+#max_retries = 2
 
 # Keystone authentication URL (string value)
+# This option is deprecated for removal.
+# Its value may be silently ignored in the future.
+# Reason: Use options presented by configured keystone auth plugin.
 #os_auth_url =
 
+# Keystone authentication API version (string value)
+# This option is deprecated for removal.
+# Its value may be silently ignored in the future.
+# Reason: Use options presented by configured keystone auth plugin.
+#os_auth_version = 2
+
+# Swift endpoint type. (string value)
+#os_endpoint_type = internalURL
+
+# Keystone region to get endpoint for. (string value)
+#os_region = <None>
+
 # Swift service type. (string value)
 #os_service_type = object-store
 
-# Swift endpoint type. (string value)
+# User's password (unknown value)
-#os_endpoint_type = internalURL
+#password = <None>
+
+# Domain ID containing project (unknown value)
+#project_domain_id = <None>
+
+# Domain name containing project (unknown value)
+#project_domain_name = <None>
+
+# Project ID to scope to (unknown value)
+# Deprecated group/name - [DEFAULT]/tenant-id
+#project_id = <None>
+
+# Project name to scope to (unknown value)
+# Deprecated group/name - [DEFAULT]/tenant-name
+#project_name = <None>
+
+# Tenant ID (unknown value)
+#tenant_id = <None>
+
+# Tenant Name (unknown value)
+#tenant_name = <None>
+
+# Timeout value for http requests (integer value)
+#timeout = <None>
+
+# Trust ID (unknown value)
+#trust_id = <None>
+
+# User's domain id (unknown value)
+#user_domain_id = <None>
+
+# User's domain name (unknown value)
+#user_domain_name = <None>
+
+# User id (unknown value)
+#user_id = <None>
+
+# Username (unknown value)
+# Deprecated group/name - [DEFAULT]/username
+#username = <None>

ironic_inspector/common/ironic.py

@@ -14,10 +14,10 @@
 import socket
 
 from ironicclient import client
-from keystoneclient import client as keystone_client
 from oslo_config import cfg
 
 from ironic_inspector.common.i18n import _
+from ironic_inspector.common import keystone
 from ironic_inspector import utils
 
 CONF = cfg.CONF
@@ -32,35 +32,50 @@ DEFAULT_IRONIC_API_VERSION = '1.11'
 IRONIC_GROUP = 'ironic'
 
 IRONIC_OPTS = [
+    cfg.StrOpt('os_region',
+               help='Keystone region used to get Ironic endpoints.'),
     cfg.StrOpt('os_auth_url',
                default='',
                help='Keystone authentication endpoint for accessing Ironic '
-                    'API. Use [keystone_authtoken]/auth_uri for keystone '
+                    'API. Use [keystone_authtoken] section for keystone '
-                    'authentication.',
+                    'token validation.',
-               deprecated_group='discoverd'),
+               deprecated_group='discoverd',
+               deprecated_for_removal=True,
+               deprecated_reason='Use options presented by configured '
+                                 'keystone auth plugin.'),
     cfg.StrOpt('os_username',
                default='',
                help='User name for accessing Ironic API. '
-                    'Use [keystone_authtoken]/admin_user for keystone '
+                    'Use [keystone_authtoken] section for keystone '
-                    'authentication.',
+                    'token validation.',
-               deprecated_group='discoverd'),
+               deprecated_group='discoverd',
+               deprecated_for_removal=True,
+               deprecated_reason='Use options presented by configured '
+                                 'keystone auth plugin.'),
     cfg.StrOpt('os_password',
                default='',
                help='Password for accessing Ironic API. '
-                    'Use [keystone_authtoken]/admin_password for keystone '
+                    'Use [keystone_authtoken] section for keystone '
-                    'authentication.',
+                    'token validation.',
                secret=True,
-               deprecated_group='discoverd'),
+               deprecated_group='discoverd',
+               deprecated_for_removal=True,
+               deprecated_reason='Use options presented by configured '
+                                 'keystone auth plugin.'),
     cfg.StrOpt('os_tenant_name',
                default='',
                help='Tenant name for accessing Ironic API. '
-                    'Use [keystone_authtoken]/admin_tenant_name for keystone '
+                    'Use [keystone_authtoken] section for keystone '
-                    'authentication.',
+                    'token validation.',
-               deprecated_group='discoverd'),
+               deprecated_group='discoverd',
+               deprecated_for_removal=True,
+               deprecated_reason='Use options presented by configured '
+                                 'keystone auth plugin.'),
     cfg.StrOpt('identity_uri',
                default='',
                help='Keystone admin endpoint. '
-                    'DEPRECATED: use [keystone_authtoken]/identity_uri.',
+                    'DEPRECATED: Use [keystone_authtoken] section for '
+                    'keystone token validation.',
                deprecated_group='discoverd',
                deprecated_for_removal=True),
     cfg.StrOpt('auth_strategy',
@@ -90,6 +105,24 @@ IRONIC_OPTS = [
 
 
 CONF.register_opts(IRONIC_OPTS, group=IRONIC_GROUP)
+keystone.register_auth_opts(IRONIC_GROUP)
+
+IRONIC_SESSION = None
+LEGACY_MAP = {
+    'auth_url': 'os_auth_url',
+    'username': 'os_username',
+    'password': 'os_password',
+    'tenant_name': 'os_tenant_name'
+}
+
+
+def reset_ironic_session():
+    """Reset the global session variable.
+
+    Mostly useful for unit tests.
+    """
+    global IRONIC_SESSION
+    IRONIC_SESSION = None
 
 
 def get_ipmi_address(node):
@@ -114,33 +147,28 @@ def get_client(token=None,
     """Get Ironic client instance."""
     # NOTE: To support standalone ironic without keystone
     if CONF.ironic.auth_strategy == 'noauth':
-        args = {'os_auth_token': 'noauth',
+        args = {'token': 'noauth',
-                'ironic_url': CONF.ironic.ironic_url}
+                'endpoint': CONF.ironic.ironic_url}
-    elif token is None:
-        args = {'os_password': CONF.ironic.os_password,
-                'os_username': CONF.ironic.os_username,
-                'os_auth_url': CONF.ironic.os_auth_url,
-                'os_tenant_name': CONF.ironic.os_tenant_name,
-                'os_service_type': CONF.ironic.os_service_type,
-                'os_endpoint_type': CONF.ironic.os_endpoint_type}
     else:
-        keystone_creds = {'password': CONF.ironic.os_password,
+        global IRONIC_SESSION
-                          'username': CONF.ironic.os_username,
+        if not IRONIC_SESSION:
-                          'auth_url': CONF.ironic.os_auth_url,
+            IRONIC_SESSION = keystone.get_session(
-                          'tenant_name': CONF.ironic.os_tenant_name}
+                IRONIC_GROUP, legacy_mapping=LEGACY_MAP)
-        keystone = keystone_client.Client(**keystone_creds)
+        if token is None:
-        # FIXME(sambetts): Work around for Bug 1539839 as client.authenticate
+            args = {'session': IRONIC_SESSION,
-        # is not called.
+                    'region_name': CONF.ironic.os_region}
-        keystone.authenticate()
+        else:
-        ironic_url = keystone.service_catalog.url_for(
+            ironic_url = IRONIC_SESSION.get_endpoint(
                 service_type=CONF.ironic.os_service_type,
-            endpoint_type=CONF.ironic.os_endpoint_type)
+                endpoint_type=CONF.ironic.os_endpoint_type,
-        args = {'os_auth_token': token,
+                region_name=CONF.ironic.os_region
-                'ironic_url': ironic_url}
+            )
+            args = {'token': token,
+                    'endpoint': ironic_url}
     args['os_ironic_api_version'] = api_version
     args['max_retries'] = CONF.ironic.max_retries
     args['retry_interval'] = CONF.ironic.retry_interval
-    return client.get_client(1, **args)
+    return client.Client(1, **args)
 
 
 def check_provision_state(node, with_credentials=False):
@@ -173,4 +201,4 @@ def dict_to_capabilities(caps_dict):
 
 
 def list_opts():
-    return [(IRONIC_GROUP, IRONIC_OPTS)]
+    return keystone.add_auth_options(IRONIC_OPTS, IRONIC_GROUP)

ironic_inspector/common/keystone.py

@@ -0,0 +1,129 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import copy
+
+from keystoneauth1 import exceptions
+from keystoneauth1 import loading
+from oslo_config import cfg
+from oslo_log import log
+from six.moves.urllib import parse  # for legacy options loading only
+
+from ironic_inspector.common.i18n import _LW
+
+CONF = cfg.CONF
+LOG = log.getLogger(__name__)
+
+
+def register_auth_opts(group):
+    loading.register_session_conf_options(CONF, group)
+    loading.register_auth_conf_options(CONF, group)
+    CONF.set_default('auth_type', default='password', group=group)
+
+
+def get_session(group, legacy_mapping=None, legacy_auth_opts=None):
+    auth = _get_auth(group, legacy_mapping, legacy_auth_opts)
+    session = loading.load_session_from_conf_options(
+        CONF, group, auth=auth)
+    return session
+
+
+def _get_auth(group, legacy_mapping=None, legacy_opts=None):
+    try:
+        auth = loading.load_auth_from_conf_options(CONF, group)
+    except exceptions.MissingRequiredOptions:
+        auth = _get_legacy_auth(group, legacy_mapping, legacy_opts)
+    else:
+        if auth is None:
+            auth = _get_legacy_auth(group, legacy_mapping, legacy_opts)
+    return auth
+
+
+def _get_legacy_auth(group, legacy_mapping, legacy_opts):
+    """Load auth plugin from legacy options.
+
+    If legacy_opts is not empty, these options will be registered first.
+
+    legacy_mapping is a dict that maps the following keys to legacy option
+    names:
+        auth_url
+        username
+        password
+        tenant_name
+    """
+    LOG.warning(_LW("Group [%s]: Using legacy auth loader is deprecated. "
+                    "Consider specifying appropriate keystone auth plugin as "
+                    "'auth_type' and corresponding plugin options."), group)
+    if legacy_opts:
+        for opt in legacy_opts:
+            try:
+                CONF.register_opt(opt, group=group)
+            except cfg.DuplicateOptError:
+                pass
+
+    conf = getattr(CONF, group)
+    auth_params = {a: getattr(conf, legacy_mapping[a])
+                   for a in legacy_mapping}
+    legacy_loader = loading.get_plugin_loader('password')
+    # NOTE(pas-ha) only Swift had this option, take it into account
+    try:
+        auth_version = conf.get('os_auth_version')
+    except cfg.NoSuchOptError:
+        auth_version = None
+    # NOTE(pas-ha) mimic defaults of keystoneclient
+    if _is_apiv3(auth_params['auth_url'], auth_version):
+        auth_params.update({
+            'project_domain_id': 'default',
+            'user_domain_id': 'default'})
+    return legacy_loader.load_from_options(**auth_params)
+
+
+# NOTE(pas-ha): for backward compat with legacy options loading only
+def _is_apiv3(auth_url, auth_version):
+    """Check if V3 version of API is being used or not.
+
+    This method inspects auth_url and auth_version, and checks whether V3
+    version of the API is being used or not.
+    When no auth_version is specified and auth_url is not a versioned
+    endpoint, v2.0 is assumed.
+    :param auth_url: a http or https url to be inspected (like
+        'http://127.0.0.1:9898/').
+    :param auth_version: a string containing the version (like 'v2', 'v3.0')
+                         or None
+    :returns: True if V3 of the API is being used.
+    """
+    return (auth_version in ('v3.0', '3') or
+            '/v3' in parse.urlparse(auth_url).path)
+
+
+def add_auth_options(options, group):
+
+    def add_options(opts, opts_to_add):
+        for new_opt in opts_to_add:
+            for opt in opts:
+                if opt.name == new_opt.name:
+                    break
+            else:
+                opts.append(new_opt)
+
+    opts = copy.deepcopy(options)
+    opts.insert(0, loading.get_auth_common_conf_options()[0])
+    # NOTE(dims): There are a lot of auth plugins, we just generate
+    # the config options for a few common ones
+    plugins = ['password', 'v2password', 'v3password']
+    for name in plugins:
+        plugin = loading.get_plugin_loader(name)
+        add_options(opts, loading.get_auth_plugin_conf_options(plugin))
+    add_options(opts, loading.get_session_conf_options())
+    opts.sort(key=lambda x: x.name)
+    return [(group, opts)]

ironic_inspector/common/swift.py

@@ -17,10 +17,12 @@ import json
 
 from oslo_config import cfg
 from oslo_log import log
+import six
 from swiftclient import client as swift_client
 from swiftclient import exceptions as swift_exceptions
 
 from ironic_inspector.common.i18n import _
+from ironic_inspector.common import keystone
 from ironic_inspector import utils
 
 CONF = cfg.CONF
@@ -28,7 +30,7 @@ CONF = cfg.CONF
 
 LOG = log.getLogger('ironic_inspector.common.swift')
 
-
+SWIFT_GROUP = 'swift'
 SWIFT_OPTS = [
     cfg.IntOpt('max_retries',
                default=2,
@@ -41,6 +43,32 @@ SWIFT_OPTS = [
     cfg.StrOpt('container',
                default='ironic-inspector',
                help='Default Swift container to use when creating objects.'),
+    cfg.StrOpt('os_auth_version',
+               default='2',
+               help='Keystone authentication API version',
+               deprecated_for_removal=True,
+               deprecated_reason='Use options presented by configured '
+                                 'keystone auth plugin.'),
+    cfg.StrOpt('os_auth_url',
+               default='',
+               help='Keystone authentication URL',
+               deprecated_for_removal=True,
+               deprecated_reason='Use options presented by configured '
+                                 'keystone auth plugin.'),
+    cfg.StrOpt('os_service_type',
+               default='object-store',
+               help='Swift service type.'),
+    cfg.StrOpt('os_endpoint_type',
+               default='internalURL',
+               help='Swift endpoint type.'),
+    cfg.StrOpt('os_region',
+               help='Keystone region to get endpoint for.'),
+]
+
+# NOTE(pas-ha) these old options conflict with options exported by
+# most used keystone auth plugins. Need to register them manually
+# for the backward-compat case.
+LEGACY_OPTS = [
     cfg.StrOpt('username',
                default='',
                help='User name for accessing Swift API.'),
@@ -51,59 +79,67 @@ SWIFT_OPTS = [
     cfg.StrOpt('tenant_name',
                default='',
                help='Tenant name for accessing Swift API.'),
-    cfg.StrOpt('os_auth_version',
-               default='2',
-               help='Keystone authentication API version'),
-    cfg.StrOpt('os_auth_url',
-               default='',
-               help='Keystone authentication URL'),
-    cfg.StrOpt('os_service_type',
-               default='object-store',
-               help='Swift service type.'),
-    cfg.StrOpt('os_endpoint_type',
-               default='internalURL',
-               help='Swift endpoint type.'),
 ]
 
-
+CONF.register_opts(SWIFT_OPTS, group=SWIFT_GROUP)
-def list_opts():
+keystone.register_auth_opts(SWIFT_GROUP)
-    return [
-        ('swift', SWIFT_OPTS)
-    ]
-
-CONF.register_opts(SWIFT_OPTS, group='swift')
 
 OBJECT_NAME_PREFIX = 'inspector_data'
+SWIFT_SESSION = None
+LEGACY_MAP = {
+    'auth_url': 'os_auth_url',
+    'username': 'username',
+    'password': 'password',
+    'tenant_name': 'tenant_name',
+}
+
+
+def reset_swift_session():
+    """Reset the global session variable.
+
+    Mostly useful for unit tests.
+    """
+    global SWIFT_SESSION
+    SWIFT_SESSION = None
 
 
 class SwiftAPI(object):
     """API for communicating with Swift."""
 
-    def __init__(self, user=None, tenant_name=None, key=None,
+    def __init__(self):
-                 auth_url=None, auth_version=None,
-                 service_type=None, endpoint_type=None):
         """Constructor for creating a SwiftAPI object.
 
-        :param user: the name of the user for Swift account
+        Authentification is loaded from config file.
-        :param tenant_name: the name of the tenant for Swift account
-        :param key: the 'password' or key to authenticate with
-        :param auth_url: the url for authentication
-        :param auth_version: the version of api to use for authentication
-        :param service_type: service type in the service catalog
-        :param endpoint_type: service endpoint type
         """
-        self.connection = swift_client.Connection(
+        global SWIFT_SESSION
-            retries=CONF.swift.max_retries,
+        if not SWIFT_SESSION:
-            user=user or CONF.swift.username,
+            SWIFT_SESSION = keystone.get_session(
-            tenant_name=tenant_name or CONF.swift.tenant_name,
+                SWIFT_GROUP, legacy_mapping=LEGACY_MAP,
-            key=key or CONF.swift.password,
+                legacy_auth_opts=LEGACY_OPTS)
-            authurl=auth_url or CONF.swift.os_auth_url,
+        # TODO(pas-ha): swiftclient does not support keystone sessions ATM.
-            auth_version=auth_version or CONF.swift.os_auth_version,
+        # Must be reworked when LP bug #1518938 is fixed.
-            os_options={
+        swift_url = SWIFT_SESSION.get_endpoint(
-                'service_type': service_type or CONF.swift.os_service_type,
+            service_type=CONF.swift.os_service_type,
-                'endpoint_type': endpoint_type or CONF.swift.os_endpoint_type
+            endpoint_type=CONF.swift.os_endpoint_type,
-            }
+            region_name=CONF.swift.os_region
         )
+        token = SWIFT_SESSION.get_token()
+        params = dict(retries=CONF.swift.max_retries,
+                      preauthurl=swift_url,
+                      preauthtoken=token)
+        # NOTE(pas-ha):session.verify is for HTTPS urls and can be
+        # - False (do not verify)
+        # - True (verify but try to locate system CA certificates)
+        # - Path (verify using specific CA certificate)
+        # This is normally handled inside the Session instance,
+        # but swiftclient still does not support sessions,
+        # so we need to reconstruct these options from Session here.
+        verify = SWIFT_SESSION.verify
+        params['insecure'] = not verify
+        if verify and isinstance(verify, six.string_types):
+            params['cacert'] = verify
+
+        self.connection = swift_client.Connection(**params)
 
     def create_object(self, object, data, container=CONF.swift.container,
                       headers=None):
@@ -182,3 +218,7 @@ def get_introspection_data(uuid):
     swift_api = SwiftAPI()
     swift_object_name = '%s-%s' % (OBJECT_NAME_PREFIX, uuid)
     return swift_api.get_object(swift_object_name)
+
+
+def list_opts():
+    return keystone.add_auth_options(SWIFT_OPTS, SWIFT_GROUP)

ironic_inspector/main.py

@@ -351,7 +351,6 @@ class Service(object):
 
         log.set_defaults(default_log_levels=[
             'sqlalchemy=WARNING',
-            'keystoneclient=INFO',
             'iso8601=WARNING',
             'requests=WARNING',
             'urllib3.connectionpool=WARNING',

ironic_inspector/test/test_common_ironic.py

@@ -16,10 +16,10 @@ import socket
 import unittest
 
 from ironicclient import client
-from keystoneclient import client as keystone_client
 from oslo_config import cfg
 
 from ironic_inspector.common import ironic as ir_utils
+from ironic_inspector.common import keystone
 from ironic_inspector.test import base
 from ironic_inspector import utils
 
@@ -27,37 +27,44 @@ from ironic_inspector import utils
 CONF = cfg.CONF
 
 
+@mock.patch.object(keystone, 'register_auth_opts')
+@mock.patch.object(keystone, 'get_session')
+@mock.patch.object(client, 'Client')
 class TestGetClient(base.BaseTest):
     def setUp(self):
         super(TestGetClient, self).setUp()
-        CONF.set_override('auth_strategy', 'keystone')
+        ir_utils.reset_ironic_session()
+        self.cfg.config(auth_strategy='keystone')
+        self.cfg.config(os_region='somewhere', group='ironic')
+        self.addCleanup(ir_utils.reset_ironic_session)
 
-    @mock.patch.object(client, 'get_client')
+    def test_get_client_with_auth_token(self, mock_client, mock_load,
-    @mock.patch.object(keystone_client, 'Client')
+                                        mock_opts):
-    def test_get_client_with_auth_token(self, mock_keystone_client,
-                                        mock_client):
         fake_token = 'token'
         fake_ironic_url = 'http://127.0.0.1:6385'
-        mock_keystone_client().service_catalog.url_for.return_value = (
+        mock_sess = mock.Mock()
-            fake_ironic_url)
+        mock_sess.get_endpoint.return_value = fake_ironic_url
+        mock_load.return_value = mock_sess
         ir_utils.get_client(fake_token)
-        args = {'os_auth_token': fake_token,
+        mock_sess.get_endpoint.assert_called_once_with(
-                'ironic_url': fake_ironic_url,
+            endpoint_type=CONF.ironic.os_endpoint_type,
-                'os_ironic_api_version': '1.11',
+            service_type=CONF.ironic.os_service_type,
+            region_name=CONF.ironic.os_region)
+        args = {'token': fake_token,
+                'endpoint': fake_ironic_url,
+                'os_ironic_api_version': ir_utils.DEFAULT_IRONIC_API_VERSION,
                 'max_retries': CONF.ironic.max_retries,
                 'retry_interval': CONF.ironic.retry_interval}
         mock_client.assert_called_once_with(1, **args)
 
-    @mock.patch.object(client, 'get_client')
+    def test_get_client_without_auth_token(self, mock_client, mock_load,
-    def test_get_client_without_auth_token(self, mock_client):
+                                           mock_opts):
+        mock_sess = mock.Mock()
+        mock_load.return_value = mock_sess
         ir_utils.get_client(None)
-        args = {'os_password': CONF.ironic.os_password,
+        args = {'session': mock_sess,
-                'os_username': CONF.ironic.os_username,
+                'region_name': 'somewhere',
-                'os_auth_url': CONF.ironic.os_auth_url,
+                'os_ironic_api_version': ir_utils.DEFAULT_IRONIC_API_VERSION,
-                'os_tenant_name': CONF.ironic.os_tenant_name,
-                'os_endpoint_type': CONF.ironic.os_endpoint_type,
-                'os_service_type': CONF.ironic.os_service_type,
-                'os_ironic_api_version': '1.11',
                 'max_retries': CONF.ironic.max_retries,
                 'retry_interval': CONF.ironic.retry_interval}
         mock_client.assert_called_once_with(1, **args)
@@ -92,7 +99,7 @@ class TestGetIpmiAddress(base.BaseTest):
                          driver_info={'foo': '192.168.1.1'})
         self.assertIsNone(ir_utils.get_ipmi_address(node))
 
-        CONF.set_override('ipmi_address_fields', ['foo', 'bar', 'baz'])
+        self.cfg.config(ipmi_address_fields=['foo', 'bar', 'baz'])
         ip = ir_utils.get_ipmi_address(node)
         self.assertEqual(ip, '192.168.1.1')
 

ironic_inspector/test/test_keystone.py

@@ -0,0 +1,115 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import mock
+
+from keystoneauth1 import exceptions as kaexc
+from keystoneauth1 import loading as kaloading
+from oslo_config import cfg
+
+from ironic_inspector.common import keystone
+from ironic_inspector.test import base
+
+
+CONF = cfg.CONF
+TESTGROUP = 'keystone_test'
+
+
+class KeystoneTest(base.BaseTest):
+
+    def setUp(self):
+        super(KeystoneTest, self).setUp()
+        self.cfg.conf.register_group(cfg.OptGroup(TESTGROUP))
+
+    def test_register_auth_opts(self):
+        keystone.register_auth_opts(TESTGROUP)
+        auth_opts = ['auth_type', 'auth_section']
+        sess_opts = ['certfile', 'keyfile', 'insecure', 'timeout', 'cafile']
+        for o in auth_opts + sess_opts:
+            self.assertIn(o, self.cfg.conf[TESTGROUP])
+        self.assertEqual('password', self.cfg.conf[TESTGROUP]['auth_type'])
+
+    @mock.patch.object(keystone, '_get_auth')
+    def test_get_session(self, auth_mock):
+        keystone.register_auth_opts(TESTGROUP)
+        self.cfg.config(group=TESTGROUP,
+                        cafile='/path/to/ca/file')
+        auth1 = mock.Mock()
+        auth_mock.return_value = auth1
+        sess = keystone.get_session(TESTGROUP)
+        self.assertEqual('/path/to/ca/file', sess.verify)
+        self.assertEqual(auth1, sess.auth)
+
+    @mock.patch('keystoneauth1.loading.load_auth_from_conf_options')
+    @mock.patch.object(keystone, '_get_legacy_auth')
+    def test__get_auth(self, legacy_mock, load_mock):
+        auth1 = mock.Mock()
+        load_mock.side_effect = [
+            auth1,
+            None,
+            kaexc.MissingRequiredOptions([kaloading.Opt('spam')])]
+        auth2 = mock.Mock()
+        legacy_mock.return_value = auth2
+        self.assertEqual(auth1, keystone._get_auth(TESTGROUP))
+        self.assertEqual(auth2, keystone._get_auth(TESTGROUP))
+        self.assertEqual(auth2, keystone._get_auth(TESTGROUP))
+
+    @mock.patch('keystoneauth1.loading._plugins.identity.generic.Password.'
+                'load_from_options')
+    def test__get_legacy_auth(self, load_mock):
+        self.cfg.register_opts(
+            [cfg.StrOpt('identity_url'),
+             cfg.StrOpt('old_user'),
+             cfg.StrOpt('old_password')],
+            group=TESTGROUP)
+        self.cfg.config(group=TESTGROUP,
+                        identity_url='http://fake:5000/v3',
+                        old_password='ham',
+                        old_user='spam')
+        options = [cfg.StrOpt('old_tenant_name', default='fake'),
+                   cfg.StrOpt('old_user')]
+        mapping = {'username': 'old_user',
+                   'password': 'old_password',
+                   'auth_url': 'identity_url',
+                   'tenant_name': 'old_tenant_name'}
+
+        keystone._get_legacy_auth(TESTGROUP, mapping, options)
+        load_mock.assert_called_once_with(username='spam',
+                                          password='ham',
+                                          tenant_name='fake',
+                                          user_domain_id='default',
+                                          project_domain_id='default',
+                                          auth_url='http://fake:5000/v3')
+
+    def test__is_api_v3(self):
+        cases = ((False, 'http://fake:5000', None),
+                 (False, 'http://fake:5000/v2.0', None),
+                 (True, 'http://fake:5000/v3', None),
+                 (True, 'http://fake:5000', '3'),
+                 (True, 'http://fake:5000', 'v3.0'))
+        for case in cases:
+            result, url, version = case
+            self.assertEqual(result, keystone._is_apiv3(url, version))
+
+    def test_add_auth_options(self):
+        group, opts = keystone.add_auth_options([], TESTGROUP)[0]
+        self.assertEqual(TESTGROUP, group)
+        # check that there is no duplicates
+        names = {o.dest for o in opts}
+        self.assertEqual(len(names), len(opts))
+        # NOTE(pas-ha) checking for most standard auth and session ones only
+        expected = {'timeout', 'insecure', 'cafile', 'certfile', 'keyfile',
+                    'auth_type', 'auth_url', 'username', 'password',
+                    'tenant_name', 'project_name', 'trust_id',
+                    'domain_id', 'user_domain_id', 'project_domain_id'}
+        self.assertTrue(expected.issubset(names))

ironic_inspector/test/test_swift.py

@@ -14,23 +14,18 @@
 
 # Mostly copied from ironic/tests/test_swift.py
 
-import sys
-
 try:
     from unittest import mock
 except ImportError:
     import mock
-from oslo_config import cfg
-from six.moves import reload_module
 from swiftclient import client as swift_client
 from swiftclient import exceptions as swift_exception
 
+from ironic_inspector.common import keystone
 from ironic_inspector.common import swift
 from ironic_inspector.test import base as test_base
 from ironic_inspector import utils
 
-CONF = cfg.CONF
-
 
 class BaseTest(test_base.NodeTest):
     def setUp(self):
@@ -52,61 +47,43 @@ class BaseTest(test_base.NodeTest):
         }
 
 
+@mock.patch.object(keystone, 'register_auth_opts')
+@mock.patch.object(keystone, 'get_session')
 @mock.patch.object(swift_client, 'Connection', autospec=True)
 class SwiftTestCase(BaseTest):
 
     def setUp(self):
         super(SwiftTestCase, self).setUp()
+        swift.reset_swift_session()
         self.swift_exception = swift_exception.ClientException('', '')
+        self.cfg.config(group='swift',
+                        os_service_type='object-store',
+                        os_endpoint_type='internalURL',
+                        os_region='somewhere',
+                        max_retries=2)
+        self.addCleanup(swift.reset_swift_session)
 
-        CONF.set_override('username', 'swift', 'swift')
+    def test___init__(self, connection_mock, load_mock, opts_mock):
-        CONF.set_override('tenant_name', 'tenant', 'swift')
+        swift_url = 'http://swiftapi'
-        CONF.set_override('password', 'password', 'swift')
+        token = 'secret_token'
-        CONF.set_override('os_auth_url', 'http://authurl/v2.0', 'swift')
+        mock_sess = mock.Mock()
-        CONF.set_override('os_auth_version', '2', 'swift')
+        mock_sess.get_token.return_value = token
-        CONF.set_override('max_retries', 2, 'swift')
+        mock_sess.get_endpoint.return_value = swift_url
-        CONF.set_override('os_service_type', 'object-store', 'swift')
+        mock_sess.verify = False
-        CONF.set_override('os_endpoint_type', 'internalURL', 'swift')
+        load_mock.return_value = mock_sess
-
-        # The constructor of SwiftAPI accepts arguments whose
-        # default values are values of some config options above. So reload
-        # the module to make sure the required values are set.
-        reload_module(sys.modules['ironic_inspector.common.swift'])
-
-    def test___init__(self, connection_mock):
-        swift.SwiftAPI(user=CONF.swift.username,
-                       tenant_name=CONF.swift.tenant_name,
-                       key=CONF.swift.password,
-                       auth_url=CONF.swift.os_auth_url,
-                       auth_version=CONF.swift.os_auth_version)
-        params = {'retries': 2,
-                  'user': 'swift',
-                  'tenant_name': 'tenant',
-                  'key': 'password',
-                  'authurl': 'http://authurl/v2.0',
-                  'auth_version': '2',
-                  'os_options': {'service_type': 'object-store',
-                                 'endpoint_type': 'internalURL'}}
-        connection_mock.assert_called_once_with(**params)
-
-    def test___init__defaults(self, connection_mock):
         swift.SwiftAPI()
         params = {'retries': 2,
-                  'user': 'swift',
+                  'preauthurl': swift_url,
-                  'tenant_name': 'tenant',
+                  'preauthtoken': token,
-                  'key': 'password',
+                  'insecure': True}
-                  'authurl': 'http://authurl/v2.0',
-                  'auth_version': '2',
-                  'os_options': {'service_type': 'object-store',
-                                 'endpoint_type': 'internalURL'}}
         connection_mock.assert_called_once_with(**params)
+        mock_sess.get_endpoint.assert_called_once_with(
+            service_type='object-store',
+            endpoint_type='internalURL',
+            region_name='somewhere')
 
-    def test_create_object(self, connection_mock):
+    def test_create_object(self, connection_mock, load_mock, opts_mock):
-        swiftapi = swift.SwiftAPI(user=CONF.swift.username,
+        swiftapi = swift.SwiftAPI()
-                                  tenant_name=CONF.swift.tenant_name,
-                                  key=CONF.swift.password,
-                                  auth_url=CONF.swift.os_auth_url,
-                                  auth_version=CONF.swift.os_auth_version)
         connection_obj_mock = connection_mock.return_value
 
         connection_obj_mock.put_object.return_value = 'object-uuid'
@@ -119,12 +96,9 @@ class SwiftTestCase(BaseTest):
             'ironic-inspector', 'object', 'some-string-data', headers=None)
         self.assertEqual('object-uuid', object_uuid)
 
-    def test_create_object_create_container_fails(self, connection_mock):
+    def test_create_object_create_container_fails(self, connection_mock,
-        swiftapi = swift.SwiftAPI(user=CONF.swift.username,
+                                                  load_mock, opts_mock):
-                                  tenant_name=CONF.swift.tenant_name,
+        swiftapi = swift.SwiftAPI()
-                                  key=CONF.swift.password,
-                                  auth_url=CONF.swift.os_auth_url,
-                                  auth_version=CONF.swift.os_auth_version)
         connection_obj_mock = connection_mock.return_value
         connection_obj_mock.put_container.side_effect = self.swift_exception
         self.assertRaises(utils.Error, swiftapi.create_object, 'object',
@@ -133,12 +107,9 @@ class SwiftTestCase(BaseTest):
                                                                   'inspector')
         self.assertFalse(connection_obj_mock.put_object.called)
 
-    def test_create_object_put_object_fails(self, connection_mock):
+    def test_create_object_put_object_fails(self, connection_mock, load_mock,
-        swiftapi = swift.SwiftAPI(user=CONF.swift.username,
+                                            opts_mock):
-                                  tenant_name=CONF.swift.tenant_name,
+        swiftapi = swift.SwiftAPI()
-                                  key=CONF.swift.password,
-                                  auth_url=CONF.swift.os_auth_url,
-                                  auth_version=CONF.swift.os_auth_version)
         connection_obj_mock = connection_mock.return_value
         connection_obj_mock.put_object.side_effect = self.swift_exception
         self.assertRaises(utils.Error, swiftapi.create_object, 'object',
@@ -148,12 +119,8 @@ class SwiftTestCase(BaseTest):
         connection_obj_mock.put_object.assert_called_once_with(
             'ironic-inspector', 'object', 'some-string-data', headers=None)
 
-    def test_get_object(self, connection_mock):
+    def test_get_object(self, connection_mock, load_mock, opts_mock):
-        swiftapi = swift.SwiftAPI(user=CONF.swift.username,
+        swiftapi = swift.SwiftAPI()
-                                  tenant_name=CONF.swift.tenant_name,
-                                  key=CONF.swift.password,
-                                  auth_url=CONF.swift.os_auth_url,
-                                  auth_version=CONF.swift.os_auth_version)
         connection_obj_mock = connection_mock.return_value
 
         expected_obj = self.data
@@ -165,12 +132,8 @@ class SwiftTestCase(BaseTest):
             'ironic-inspector', 'object')
         self.assertEqual(expected_obj, swift_obj)
 
-    def test_get_object_fails(self, connection_mock):
+    def test_get_object_fails(self, connection_mock, load_mock, opts_mock):
-        swiftapi = swift.SwiftAPI(user=CONF.swift.username,
+        swiftapi = swift.SwiftAPI()
-                                  tenant_name=CONF.swift.tenant_name,
-                                  key=CONF.swift.password,
-                                  auth_url=CONF.swift.os_auth_url,
-                                  auth_version=CONF.swift.os_auth_version)
         connection_obj_mock = connection_mock.return_value
         connection_obj_mock.get_object.side_effect = self.swift_exception
         self.assertRaises(utils.Error, swiftapi.get_object,

releasenotes/notes/keystoneauth-plugins-aab6cbe1d0e884bf.yaml

@@ -0,0 +1,17 @@
+---
+features:
+  - Ironic-Inspector is now using keystoneauth and proper auth_plugins
+    instead of keystoneclient for communicating with Ironic and Swift.
+    It allows to finely tune authentification for each service independently.
+    For each service, the keystone session is created and reused, minimizing
+    the number of authentification requests to Keystone.
+upgrade:
+  - Operators are advised to specify a proper keystoneauth plugin
+    and its appropriate settings in [ironic] and [swift] config sections.
+    Backward compatibility with previous authentification options is included.
+    Using authentification informaiton for Ironic and Swift from
+    [keystone_authtoken] config section is no longer supported.
+deprecations:
+  - Most of current authentification options for either Ironic or Swift are
+    deprecated and will be removed in a future release. Please configure
+    the keystoneauth auth plugin authentification instead.

requirements.txt

@@ -8,11 +8,11 @@ Flask<1.0,>=0.10 # BSD
 futurist>=0.11.0 # Apache-2.0
 jsonpath-rw<2.0,>=1.2.0 # Apache-2.0
 jsonschema!=2.5.0,<3.0.0,>=2.0.0 # MIT
+keystoneauth1>=2.1.0  # Apache-2.0
 keystonemiddleware!=4.1.0,>=4.0.0 # Apache-2.0
 netaddr!=0.7.16,>=0.7.12 # BSD
 pbr>=1.6 # Apache-2.0
 python-ironicclient>=1.1.0 # Apache-2.0
-python-keystoneclient!=1.8.0,!=2.1.0,>=1.6.0 # Apache-2.0
 python-swiftclient>=2.2.0 # Apache-2.0
 oslo.concurrency>=3.5.0 # Apache-2.0
 oslo.config>=3.7.0 # Apache-2.0