Remove legacy auth loading

remove support for specifying client auth in keystone_authtoken config section.
This was deprecated about a year ago and now can safely be removed.

Also, fill the [cinder] section with auth options in devstack.

Change-Id: I0c45d12d80eff45e643af29cded178644071c9fe

devstack/lib/ironic

@@ -1111,6 +1111,7 @@ function configure_ironic_conductor {
     configure_auth_for swift
     configure_auth_for glance
     configure_auth_for inspector
+    configure_auth_for cinder
     # this one is needed for lookup of Ironic API endpoint via Keystone
     configure_auth_for service_catalog
 

ironic/cmd/conductor.py

@@ -28,36 +28,12 @@ from oslo_service import service
 
 from ironic.common import rpc_service
 from ironic.common import service as ironic_service
-from ironic.conf import auth
 from ironic import version
 
 CONF = cfg.CONF
 
 LOG = log.getLogger(__name__)
 
-SECTIONS_WITH_AUTH = (
-    'service_catalog', 'neutron', 'glance', 'swift', 'cinder', 'inspector')
-
-
-# TODO(pas-ha) remove this check after deprecation period
-def check_auth_options(conf):
-    missing = []
-    for section in SECTIONS_WITH_AUTH:
-        if not auth.load_auth(conf, section):
-            missing.append('[%s]' % section)
-    if missing:
-        link = "http://docs.openstack.org/releasenotes/ironic/newton.html"
-        LOG.warning("Failed to load authentification credentials from "
-                    "%(missing)s config sections. "
-                    "The corresponding service users' credentials "
-                    "will be loaded from [%(old)s] config section, "
-                    "which is deprecated for this purpose. "
-                    "Please update the config file. "
-                    "For more info see %(link)s.",
-                    dict(missing=", ".join(missing),
-                         old=auth.LEGACY_SECTION,
-                         link=link))
-
 
 def warn_about_unsafe_shred_parameters(conf):
     iterations = conf.deploy.shred_random_overwrite_iterations
@@ -78,7 +54,6 @@ def warn_about_missing_default_boot_option(conf):
 
 
 def issue_startup_warnings(conf):
-    check_auth_options(conf)
     warn_about_unsafe_shred_parameters(conf)
     warn_about_missing_default_boot_option(conf)
 

ironic/common/keystone.py

@@ -18,34 +18,14 @@ from keystoneauth1 import exceptions as kaexception
 from keystoneauth1 import loading as kaloading
 from oslo_log import log as logging
 import six
-from six.moves.urllib import parse  # for legacy options loading only
 
 from ironic.common import exception
-from ironic.common.i18n import _
-from ironic.conf import auth as ironic_auth
 from ironic.conf import CONF
 
 
 LOG = logging.getLogger(__name__)
 
 
-# FIXME(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 == 'v3.0' or '/v3' in parse.urlparse(auth_url).path
-
-
 def ks_exceptions(f):
     """Wraps keystoneclient functions and centralizes exception handling."""
     @six.wraps(f)
@@ -71,48 +51,20 @@ def ks_exceptions(f):
 
 @ks_exceptions
 def get_session(group):
-    auth = ironic_auth.load_auth(CONF, group) or _get_legacy_auth()
+    try:
-    if not auth:
+        auth = kaloading.load_auth_from_conf_options(CONF, group)
-        msg = _("Failed to load auth from either [%(new)s] or [%(old)s] "
+    except kaexception.MissingRequiredOptions:
-                "config sections.")
+        LOG.error('Failed to load auth plugin from group %s', group)
-        raise exception.ConfigInvalid(message=msg, new=group,
+        raise
-                                      old=ironic_auth.LEGACY_SECTION)
     session = kaloading.load_session_from_conf_options(
         CONF, group, auth=auth)
     return session
 
 
-# FIXME(pas-ha) remove legacy path after deprecation
+# TODO(pas-ha) we actually should barely need this at all:
-def _get_legacy_auth():
+# if we instantiate a identity.Token auth plugin from incoming
-    """Load auth from keystone_authtoken config section
+# request context we could build a session with it, and each client
-
+# would know its service_type already, looking up the endpoint by itself
-    Used only to provide backward compatibility with old configs.
-    """
-    conf = getattr(CONF, ironic_auth.LEGACY_SECTION)
-    # NOTE(pas-ha) first try to load auth from legacy section
-    # using the new keystoneauth options that might be already set there
-    auth = ironic_auth.load_auth(CONF, ironic_auth.LEGACY_SECTION)
-    if auth:
-        return auth
-    # NOTE(pas-ha) now we surely have legacy config section for auth
-    # and with legacy options set in it, deal with it.
-    legacy_loader = kaloading.get_plugin_loader('password')
-    auth_params = {
-        'auth_url': conf.auth_uri,
-        'username': conf.admin_user,
-        'password': conf.admin_password,
-        'tenant_name': conf.admin_tenant_name
-    }
-    api_v3 = _is_apiv3(conf.auth_uri, conf.auth_version)
-    if api_v3:
-        # NOTE(pas-ha): mimic defaults of keystoneclient
-        auth_params.update({
-            'project_domain_id': 'default',
-            'user_domain_id': 'default',
-        })
-    return legacy_loader.load_from_options(**auth_params)
-
-
 @ks_exceptions
 def get_service_url(session, service_type='baremetal',
                     endpoint_type='internal'):
@@ -131,12 +83,11 @@ def get_service_url(session, service_type='baremetal',
                                 region_name=CONF.keystone.region_name)
 
 
+# TODO(pas-ha) move all clients to sessions, then we do not need this
 @ks_exceptions
 def get_admin_auth_token(session):
     """Get admin token.
 
     Currently used for inspector, glance and swift clients.
-    Only swift client does not actually support using sessions directly,
-    LP #1518938, others will be updated in ironic code.
     """
     return session.get_token()

ironic/conf/auth.py

@@ -14,34 +14,11 @@
 
 import copy
 
-from keystoneauth1 import exceptions as kaexception
 from keystoneauth1 import loading as kaloading
-from oslo_config import cfg
 from oslo_log import log
 
 
 LOG = log.getLogger(__name__)
-LEGACY_SECTION = 'keystone_authtoken'
-OLD_SESSION_OPTS = {
-    'certfile': [cfg.DeprecatedOpt('certfile', LEGACY_SECTION)],
-    'keyfile': [cfg.DeprecatedOpt('keyfile', LEGACY_SECTION)],
-    'cafile': [cfg.DeprecatedOpt('cafile', LEGACY_SECTION)],
-    'insecure': [cfg.DeprecatedOpt('insecure', LEGACY_SECTION)],
-    'timeout': [cfg.DeprecatedOpt('timeout', LEGACY_SECTION)],
-}
-
-# FIXME(pas-ha) remove import of auth_token section after deprecation period
-cfg.CONF.import_group(LEGACY_SECTION, 'keystonemiddleware.auth_token')
-
-
-def load_auth(conf, group):
-    try:
-        auth = kaloading.load_auth_from_conf_options(conf, group)
-    except kaexception.MissingRequiredOptions as exc:
-        LOG.debug('Failed to load credentials from group %(grp)s: %(err)s',
-                  {'grp': group, 'err': exc})
-        auth = None
-    return auth
 
 
 def register_auth_opts(conf, group):
@@ -50,8 +27,7 @@ def register_auth_opts(conf, group):
     Registers only basic auth options shared by all auth plugins.
     The rest are registered at runtime depending on auth plugin used.
     """
-    kaloading.register_session_conf_options(
+    kaloading.register_session_conf_options(conf, group)
-        conf, group, deprecated_opts=OLD_SESSION_OPTS)
     kaloading.register_auth_conf_options(conf, group)
 
 

ironic/tests/unit/common/test_keystone.py

@@ -13,6 +13,7 @@
 #    under the License.
 
 from keystoneauth1 import exceptions as ksexception
+from keystoneauth1 import identity as kaidentity
 from keystoneauth1 import loading as kaloading
 import mock
 from oslo_config import cfg
@@ -85,89 +86,18 @@ class KeystoneTestCase(base.TestCase):
         self.assertRaises(exception.KeystoneUnauthorized,
                           keystone.get_admin_auth_token, mock_sess)
 
-    @mock.patch.object(ironic_auth, 'load_auth')
+    def test_get_session(self):
-    def test_get_session(self, auth_get_mock):
-        auth_mock = mock.Mock()
-        auth_get_mock.return_value = auth_mock
         session = keystone.get_session(self.test_group)
-        self.assertEqual(auth_mock, session.auth)
+        # NOTE(pas-ha) 'password' auth_plugin is used
+        self.assertIsInstance(session.auth,
+                              kaidentity.generic.password.Password)
+        self.assertEqual('http://127.0.0.1:9898', session.auth.auth_url)
 
-    @mock.patch.object(keystone, '_get_legacy_auth', return_value=None)
+    def test_get_session_fail(self):
-    @mock.patch.object(ironic_auth, 'load_auth', return_value=None)
+        # NOTE(pas-ha) 'password' auth_plugin is used,
-    def test_get_session_fail(self, auth_get_mock, legacy_get_mock):
+        # so when we set the required auth_url to None,
-        self.assertRaisesRegex(exception.KeystoneFailure,
+        # MissingOption is raised
-                               "Failed to load auth from either",
+        self.config(auth_url=None, group=self.test_group)
-                               keystone.get_session, self.test_group)
+        self.assertRaises(exception.ConfigInvalid,
-
+                          keystone.get_session,
-    @mock.patch('keystoneauth1.loading.load_auth_from_conf_options')
+                          self.test_group)
-    @mock.patch('ironic.common.keystone._get_legacy_auth')
-    def test_get_session_failed_new_auth(self, legacy_get_mock, load_mock):
-        legacy_mock = mock.Mock()
-        legacy_get_mock.return_value = legacy_mock
-        load_mock.side_effect = [None, ksexception.MissingRequiredOptions]
-        self.assertEqual(legacy_mock,
-                         keystone.get_session(self.test_group).auth)
-
-
-@mock.patch('keystoneauth1.loading._plugins.identity.generic.Password.'
-            'load_from_options')
-class KeystoneLegacyTestCase(base.TestCase):
-    def setUp(self):
-        super(KeystoneLegacyTestCase, self).setUp()
-        self.test_group = 'test_group'
-        self.cfg_fixture.conf.register_group(cfg.OptGroup(self.test_group))
-        self.config(group=ironic_auth.LEGACY_SECTION,
-                    auth_uri='http://127.0.0.1:9898',
-                    admin_user='fake_user',
-                    admin_password='fake_pass',
-                    admin_tenant_name='fake_tenant')
-        ironic_auth.register_auth_opts(self.cfg_fixture.conf, self.test_group)
-        self.config(group=self.test_group,
-                    auth_type=None)
-        self.expected = dict(
-            auth_url='http://127.0.0.1:9898',
-            username='fake_user',
-            password='fake_pass',
-            tenant_name='fake_tenant')
-
-    def _set_config(self):
-        self.cfg_fixture = self.useFixture(fixture.Config())
-        self.addCleanup(cfg.CONF.reset)
-
-    @mock.patch.object(ironic_auth, 'load_auth', return_value=None)
-    def test_legacy_loading_v2(self, load_auth_mock, load_mock):
-        keystone.get_session(self.test_group)
-        load_mock.assert_called_once_with(**self.expected)
-        self.assertEqual(2, load_auth_mock.call_count)
-
-    @mock.patch.object(ironic_auth, 'load_auth', return_value=None)
-    def test_legacy_loading_v3(self, load_auth_mock, load_mock):
-        self.config(
-            auth_version='v3.0',
-            group=ironic_auth.LEGACY_SECTION)
-        self.expected.update(dict(
-            project_domain_id='default',
-            user_domain_id='default'))
-        keystone.get_session(self.test_group)
-        load_mock.assert_called_once_with(**self.expected)
-        self.assertEqual(2, load_auth_mock.call_count)
-
-    @mock.patch.object(ironic_auth, 'load_auth')
-    def test_legacy_loading_new_in_legacy(self, load_auth_mock, load_mock):
-        # NOTE(pas-ha) this is due to auth_plugin options
-        # being dynamically registered on first load,
-        # but we need to set the config before
-        plugin = kaloading.get_plugin_loader('password')
-        opts = kaloading.get_auth_plugin_conf_options(plugin)
-        self.cfg_fixture.register_opts(opts, group=ironic_auth.LEGACY_SECTION)
-        self.config(group=ironic_auth.LEGACY_SECTION,
-                    auth_uri='http://127.0.0.1:9898',
-                    username='fake_user',
-                    password='fake_pass',
-                    project_name='fake_tenant',
-                    auth_url='http://127.0.0.1:9898',
-                    auth_type='password')
-        load_auth_mock.side_effect = [None, mock.Mock()]
-        keystone.get_session(self.test_group)
-        self.assertFalse(load_mock.called)
-        self.assertEqual(2, load_auth_mock.call_count)

ironic/tests/unit/conf/test_auth.py

@@ -12,7 +12,6 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-from keystoneauth1 import identity as kaidentity
 from keystoneauth1 import loading as kaloading
 from oslo_config import cfg
 
@@ -54,17 +53,3 @@ class AuthConfTestCase(base.TestCase):
                     'tenant_name', 'project_name', 'trust_id',
                     'domain_id', 'user_domain_id', 'project_domain_id'}
         self.assertTrue(expected.issubset(names))
-
-    def test_load_auth(self):
-        auth = ironic_auth.load_auth(self.cfg_fixture.conf, self.test_group)
-        # NOTE(pas-ha) 'password' auth_plugin is used
-        self.assertIsInstance(auth, kaidentity.generic.password.Password)
-        self.assertEqual('http://127.0.0.1:9898', auth.auth_url)
-
-    def test_load_auth_missing_options(self):
-        # NOTE(pas-ha) 'password' auth_plugin is used,
-        # so when we set the required auth_url to None,
-        # MissingOption is raised
-        self.config(auth_url=None, group=self.test_group)
-        self.assertIsNone(ironic_auth.load_auth(
-            self.cfg_fixture.conf, self.test_group))

ironic/tests/unit/dhcp/test_neutron.py

@@ -17,7 +17,6 @@
 import mock
 
 from neutronclient.common import exceptions as neutron_client_exc
-from neutronclient.v2_0 import client
 from oslo_utils import uuidutils
 
 from ironic.common import dhcp_factory
@@ -41,17 +40,6 @@ class TestNeutron(db_base.DbTestCase):
         self.config(enabled_drivers=['fake'])
         self.config(dhcp_provider='neutron',
                     group='dhcp')
-        self.config(url='test-url',
-                    url_timeout=30,
-                    retries=2,
-                    group='neutron')
-        self.config(insecure=False,
-                    certfile='test-file',
-                    admin_user='test-admin-user',
-                    admin_tenant_name='test-admin-tenant',
-                    admin_password='test-admin-password',
-                    auth_uri='test-auth-uri',
-                    group='keystone_authtoken')
         self.node = object_utils.create_test_node(self.context)
         self.ports = [
             object_utils.create_test_port(
@@ -64,9 +52,8 @@ class TestNeutron(db_base.DbTestCase):
 
         dhcp_factory.DHCPFactory._dhcp_provider = None
 
-    @mock.patch.object(client.Client, 'update_port')
+    @mock.patch('ironic.common.neutron.get_client', autospec=True)
-    @mock.patch.object(client.Client, "__init__")
+    def test_update_port_dhcp_opts(self, client_mock):
-    def test_update_port_dhcp_opts(self, mock_client_init, mock_update_port):
         opts = [{'opt_name': 'bootfile-name',
                  'opt_value': 'pxelinux.0'},
                 {'opt_name': 'tftp-server',
@@ -76,19 +63,16 @@ class TestNeutron(db_base.DbTestCase):
         port_id = 'fake-port-id'
         expected = {'port': {'extra_dhcp_opts': opts}}
 
-        mock_client_init.return_value = None
         api = dhcp_factory.DHCPFactory()
         api.provider.update_port_dhcp_opts(port_id, opts)
-        mock_update_port.assert_called_once_with(port_id, expected)
+        client_mock.return_value.update_port.assert_called_once_with(
+            port_id, expected)
 
-    @mock.patch.object(client.Client, 'update_port')
+    @mock.patch('ironic.common.neutron.get_client', autospec=True)
-    @mock.patch.object(client.Client, "__init__")
+    def test_update_port_dhcp_opts_with_exception(self, client_mock):
-    def test_update_port_dhcp_opts_with_exception(self, mock_client_init,
-                                                  mock_update_port):
         opts = [{}]
         port_id = 'fake-port-id'
-        mock_client_init.return_value = None
+        client_mock.return_value.update_port.side_effect = (
-        mock_update_port.side_effect = (
             neutron_client_exc.NeutronClientException())
 
         api = dhcp_factory.DHCPFactory()
@@ -379,8 +363,9 @@ class TestNeutron(db_base.DbTestCase):
     def test__get_ip_addresses_portgroup_int_info(self):
         self._test__get_ip_addresses_portgroup('internal_info')
 
+    @mock.patch('ironic.common.neutron.get_client', autospec=True)
     @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_port_ip_address')
-    def test_get_ip_addresses(self, get_ip_mock):
+    def test_get_ip_addresses(self, get_ip_mock, client_mock):
         ip_address = '10.10.0.1'
         expected = [ip_address]
 
@@ -390,11 +375,13 @@ class TestNeutron(db_base.DbTestCase):
             api = dhcp_factory.DHCPFactory().provider
             result = api.get_ip_addresses(task)
             get_ip_mock.assert_called_once_with(task, task.ports[0],
-                                                mock.ANY)
+                                                client_mock.return_value)
         self.assertEqual(expected, result)
 
+    @mock.patch('ironic.common.neutron.get_client', autospec=True)
     @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_port_ip_address')
-    def test_get_ip_addresses_for_port_and_portgroup(self, get_ip_mock):
+    def test_get_ip_addresses_for_port_and_portgroup(self, get_ip_mock,
+                                                     client_mock):
         object_utils.create_test_portgroup(
             self.context, node_id=self.node.id, address='aa:bb:cc:dd:ee:ff',
             uuid=uuidutils.generate_uuid(),
@@ -404,5 +391,6 @@ class TestNeutron(db_base.DbTestCase):
             api = dhcp_factory.DHCPFactory().provider
             api.get_ip_addresses(task)
             get_ip_mock.assert_has_calls(
-                [mock.call(task, task.ports[0], mock.ANY),
+                [mock.call(task, task.ports[0], client_mock.return_value),
-                 mock.call(task, task.portgroups[0], mock.ANY)])
+                 mock.call(task, task.portgroups[0], client_mock.return_value)]
+            )

ironic/tests/unit/drivers/modules/test_inspector.py

@@ -65,6 +65,8 @@ class BaseTestCase(db_base.DbTestCase):
         self.task.node = self.node
         self.task.driver = self.driver
         self.api_version = (1, 0)
+        # NOTE(pas-ha) force-reset  global inspector session object
+        inspector._INSPECTOR_SESSION = None
 
 
 class CommonFunctionsTestCase(BaseTestCase):

releasenotes/notes/no-more-legacy-auth-eeb32f907d0ab5de.yaml

@@ -0,0 +1,14 @@
+---
+upgrade:
+  - |
+    Ironic no longer falls back to loading authentication configuration
+    options for accessing other services from the ``[keystone_authtoken]``
+    section.
+    As a result, the following configuration sections now must contain
+    proper authentication options for appropriate service:
+
+      - glance
+      - neutron
+      - swift
+      - inspector
+      - service_catalog