diff --git a/ironic/common/cinder.py b/ironic/common/cinder.py index 48d8461cf2..66801bf604 100644 --- a/ironic/common/cinder.py +++ b/ironic/common/cinder.py @@ -35,7 +35,8 @@ _CINDER_SESSION = None def _get_cinder_session(): global _CINDER_SESSION if not _CINDER_SESSION: - _CINDER_SESSION = keystone.get_session('cinder') + auth = keystone.get_auth('cinder') + _CINDER_SESSION = keystone.get_session('cinder', auth=auth) return _CINDER_SESSION diff --git a/ironic/common/glance_service/service_utils.py b/ironic/common/glance_service/service_utils.py index 1320177f3b..d4771064f0 100644 --- a/ironic/common/glance_service/service_utils.py +++ b/ironic/common/glance_service/service_utils.py @@ -108,7 +108,8 @@ def _get_api_server_iterator(): api_servers = [] if not CONF.glance.glance_api_servers and not CONF.glance.glance_host: - session = keystone.get_session('service_catalog') + session = keystone.get_session('glance', + auth=keystone.get_auth('glance')) api_servers = [keystone.get_service_url(session, service_type='image', endpoint_type='public')] else: diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py index 028020c6e7..c85c1f11c5 100644 --- a/ironic/common/image_service.py +++ b/ironic/common/image_service.py @@ -41,7 +41,8 @@ _GLANCE_SESSION = None def _get_glance_session(): global _GLANCE_SESSION if not _GLANCE_SESSION: - _GLANCE_SESSION = keystone.get_session('glance') + auth = keystone.get_auth('glance') + _GLANCE_SESSION = keystone.get_session('glance', auth=auth) return _GLANCE_SESSION @@ -55,8 +56,7 @@ def GlanceImageService(client=None, version=None, context=None): service_class = getattr(module, 'GlanceImageService') if (context is not None and CONF.glance.auth_strategy == 'keystone' and not context.auth_token): - session = _get_glance_session() - context.auth_token = keystone.get_admin_auth_token(session) + context.auth_token = _get_glance_session().get_token() return service_class(client, version, context) diff --git a/ironic/common/keystone.py b/ironic/common/keystone.py index 267df25dda..52045c9ada 100644 --- a/ironic/common/keystone.py +++ b/ironic/common/keystone.py @@ -50,44 +50,60 @@ def ks_exceptions(f): @ks_exceptions -def get_session(group): +def get_session(group, **session_kwargs): + """Loads session object from options in a configuration file section. + + The session_kwargs will be passed directly to keystoneauth1 Session + and will override the values loaded from config. + Consult keystoneauth1 docs for available options. + + :param group: name of the config section to load session options from + + """ + return kaloading.load_session_from_conf_options( + CONF, group, **session_kwargs) + + +@ks_exceptions +def get_auth(group, **auth_kwargs): + """Loads auth plugin from options in a configuration file section. + + The auth_kwargs will be passed directly to keystoneauth1 auth plugin + and will override the values loaded from config. + Note that the accepted kwargs will depend on auth plugin type as defined + by [group]auth_type option. + Consult keystoneauth1 docs for available auth plugins and their options. + + :param group: name of the config section to load auth plugin options from + + """ try: - auth = kaloading.load_auth_from_conf_options(CONF, group) + auth = kaloading.load_auth_from_conf_options(CONF, group, + **auth_kwargs) except kaexception.MissingRequiredOptions: LOG.error('Failed to load auth plugin from group %s', group) raise - session = kaloading.load_session_from_conf_options( - CONF, group, auth=auth) - return session + return auth -# TODO(pas-ha) we actually should barely need this at all: -# if we instantiate a identity.Token auth plugin from incoming -# request context we could build a session with it, and each client -# would know its service_type already, looking up the endpoint by itself +# NOTE(pas-ha) Used by neutronclient and resolving ironic API only +# FIXME(pas-ha) remove this while moving to kesytoneauth adapters @ks_exceptions -def get_service_url(session, service_type='baremetal', - endpoint_type='internal'): - """Wrapper for get service url from keystone service catalog. +def get_service_url(session, **kwargs): + """Find endpoint for given service in keystone catalog. - Given a service_type and an endpoint_type, this method queries - keystone service catalog and provides the url for the desired - endpoint. + If 'interrace' is provided, fetches service url of this interface. + Otherwise, first tries to fetch 'internal' endpoint, + and then the 'public' one. + + :param session: keystoneauth Session object + :param kwargs: any other arguments accepted by Session.get_endpoint method - :param service_type: the keystone service for which url is required. - :param endpoint_type: the type of endpoint for the service. - :returns: an http/https url for the desired endpoint. """ - return session.get_endpoint(service_type=service_type, - interface=endpoint_type, - 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. - """ - return session.get_token() + if 'interface' in kwargs: + return session.get_endpoint(**kwargs) + try: + return session.get_endpoint(interface='internal', **kwargs) + except kaexception.EndpointNotFound: + return session.get_endpoint(interface='public', **kwargs) diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index 8e5c27f17c..a60a658f63 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -30,7 +30,8 @@ _NEUTRON_SESSION = None def _get_neutron_session(): global _NEUTRON_SESSION if not _NEUTRON_SESSION: - _NEUTRON_SESSION = keystone.get_session('neutron') + auth = keystone.get_auth('neutron') + _NEUTRON_SESSION = keystone.get_session('neutron', auth=auth) return _NEUTRON_SESSION @@ -60,7 +61,9 @@ def get_client(token=None): else: params['token'] = token params['endpoint_url'] = url or keystone.get_service_url( - session, service_type='network') + session, + service_type='network', + region_name=CONF.keystone.region_name) params.update({ 'timeout': CONF.neutron.url_timeout or CONF.neutron.timeout, 'insecure': CONF.neutron.insecure, diff --git a/ironic/common/swift.py b/ironic/common/swift.py index 1203ed0732..8168870eef 100644 --- a/ironic/common/swift.py +++ b/ironic/common/swift.py @@ -31,7 +31,8 @@ _SWIFT_SESSION = None def _get_swift_session(): global _SWIFT_SESSION if not _SWIFT_SESSION: - _SWIFT_SESSION = keystone.get_session('swift') + auth = keystone.get_auth('swift') + _SWIFT_SESSION = keystone.get_session('swift', auth=auth) return _SWIFT_SESSION diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index ffdb53bd29..7d79403a49 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -77,7 +77,9 @@ _IRONIC_SESSION = None def _get_ironic_session(): global _IRONIC_SESSION if not _IRONIC_SESSION: - _IRONIC_SESSION = keystone.get_session('service_catalog') + auth = keystone.get_auth('service_catalog') + _IRONIC_SESSION = keystone.get_session('service_catalog', + auth=auth) return _IRONIC_SESSION diff --git a/ironic/drivers/modules/inspector.py b/ironic/drivers/modules/inspector.py index 49092dc399..4edfd956ba 100644 --- a/ironic/drivers/modules/inspector.py +++ b/ironic/drivers/modules/inspector.py @@ -45,7 +45,8 @@ def _get_inspector_session(): global _INSPECTOR_SESSION if not _INSPECTOR_SESSION: - _INSPECTOR_SESSION = keystone.get_session('inspector') + _INSPECTOR_SESSION = keystone.get_session( + 'inspector', auth=keystone.get_auth('inspector')) return _INSPECTOR_SESSION diff --git a/ironic/tests/unit/common/test_cinder.py b/ironic/tests/unit/common/test_cinder.py index ba598aaa63..b7fb3786b8 100644 --- a/ironic/tests/unit/common/test_cinder.py +++ b/ironic/tests/unit/common/test_cinder.py @@ -30,6 +30,7 @@ from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils as object_utils +@mock.patch.object(keystone, 'get_auth', autospec=True) @mock.patch.object(keystone, 'get_session', autospec=True) class TestCinderSession(base.TestCase): @@ -39,17 +40,21 @@ class TestCinderSession(base.TestCase): retries=2, group='cinder') - def test__get_cinder_session(self, mock_keystone_session): + def test__get_cinder_session(self, mock_keystone_session, mock_auth): """Check establishing new session when no session exists.""" mock_keystone_session.return_value = 'session1' self.assertEqual('session1', cinder._get_cinder_session()) - mock_keystone_session.assert_called_once_with('cinder') + mock_keystone_session.assert_called_once_with( + 'cinder', auth=mock_auth.return_value) + mock_auth.assert_called_once_with('cinder') """Check if existing session is used.""" mock_keystone_session.reset_mock() + mock_auth.reset_mock() mock_keystone_session.return_value = 'session2' self.assertEqual('session1', cinder._get_cinder_session()) self.assertFalse(mock_keystone_session.called) + self.assertFalse(mock_auth.called) @mock.patch.object(cinder, '_get_cinder_session', autospec=True) diff --git a/ironic/tests/unit/common/test_glance_service.py b/ironic/tests/unit/common/test_glance_service.py index cd6793fc7e..b166e044e2 100644 --- a/ironic/tests/unit/common/test_glance_service.py +++ b/ironic/tests/unit/common/test_glance_service.py @@ -859,14 +859,17 @@ class TestGlanceAPIServers(base.TestCase): @mock.patch.object(service_utils.keystone, 'get_service_url', autospec=True) @mock.patch.object(service_utils.keystone, 'get_session', autospec=True) - def test__get_api_servers_with_keystone(self, mock_session, + @mock.patch.object(service_utils.keystone, 'get_auth', autospec=True) + def test__get_api_servers_with_keystone(self, mock_auth, mock_session, mock_service_url): mock_service_url.return_value = 'https://example.com' endpoint = service_utils._get_api_server() self.assertEqual('https://example.com', endpoint) - mock_session.assert_called_once_with('service_catalog') + mock_auth.assert_called_once_with('glance') + mock_session.assert_called_once_with('glance', + auth=mock_auth.return_value) mock_service_url.assert_called_once_with(mock_session.return_value, service_type='image', endpoint_type='public') diff --git a/ironic/tests/unit/common/test_keystone.py b/ironic/tests/unit/common/test_keystone.py index 2365dea0f1..1b1a420706 100644 --- a/ironic/tests/unit/common/test_keystone.py +++ b/ironic/tests/unit/common/test_keystone.py @@ -13,7 +13,6 @@ # 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 @@ -52,52 +51,45 @@ class KeystoneTestCase(base.TestCase): self.cfg_fixture = self.useFixture(fixture.Config()) self.addCleanup(cfg.CONF.reset) - def test_get_url(self): - fake_url = 'http://127.0.0.1:6385' - mock_sess = mock.Mock() - mock_sess.get_endpoint.return_value = fake_url - res = keystone.get_service_url(mock_sess) - mock_sess.get_endpoint.assert_called_with( - interface='internal', region_name='fake_region', - service_type='baremetal') - self.assertEqual(fake_url, res) - - def test_get_url_failure(self): - exc_map = ( - (ksexception.Unauthorized, exception.KeystoneUnauthorized), - (ksexception.EndpointNotFound, exception.CatalogNotFound), - (ksexception.EmptyCatalog, exception.CatalogNotFound), - (ksexception.Unauthorized, exception.KeystoneUnauthorized), - ) - for kexc, irexc in exc_map: - mock_sess = mock.Mock() - mock_sess.get_endpoint.side_effect = kexc - self.assertRaises(irexc, keystone.get_service_url, mock_sess) - - def test_get_admin_auth_token(self): - mock_sess = mock.Mock() - mock_sess.get_token.return_value = 'fake_token' - self.assertEqual('fake_token', - keystone.get_admin_auth_token(mock_sess)) - - def test_get_admin_auth_token_failure(self): - mock_sess = mock.Mock() - mock_sess.get_token.side_effect = ksexception.Unauthorized - self.assertRaises(exception.KeystoneUnauthorized, - keystone.get_admin_auth_token, mock_sess) - def test_get_session(self): - session = keystone.get_session(self.test_group) - # 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) + self.config(timeout=10, group=self.test_group) + session = keystone.get_session(self.test_group, timeout=20) + self.assertEqual(20, session.timeout) - def test_get_session_fail(self): + def test_get_auth(self): + auth = keystone.get_auth(self.test_group) + self.assertEqual('http://127.0.0.1:9898', auth.auth_url) + + def test_get_auth_fail(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.assertRaises(exception.ConfigInvalid, - keystone.get_session, + keystone.get_auth, self.test_group) + + def test_get_service_url_with_interface(self): + session = mock.Mock() + session.get_endpoint.return_value = 'spam' + params = {'interface': 'admin', 'ham': 'eggs'} + self.assertEqual('spam', keystone.get_service_url(session, **params)) + session.get_endpoint.assert_called_once_with(**params) + + def test_get_service_url_internal(self): + session = mock.Mock() + session.get_endpoint.return_value = 'spam' + params = {'ham': 'eggs'} + self.assertEqual('spam', keystone.get_service_url(session, **params)) + session.get_endpoint.assert_called_once_with(interface='internal', + **params) + + def test_get_service_url_internal_fail(self): + session = mock.Mock() + session.get_endpoint.side_effect = [ksexception.EndpointNotFound(), + 'spam'] + params = {'ham': 'eggs'} + self.assertEqual('spam', keystone.get_service_url(session, **params)) + session.get_endpoint.assert_has_calls([ + mock.call(interface='internal', **params), + mock.call(interface='public', **params)]) diff --git a/ironic/tests/unit/drivers/modules/test_inspector.py b/ironic/tests/unit/drivers/modules/test_inspector.py index b843d7f5e2..bcd197b4c8 100644 --- a/ironic/tests/unit/drivers/modules/test_inspector.py +++ b/ironic/tests/unit/drivers/modules/test_inspector.py @@ -89,8 +89,10 @@ class CommonFunctionsTestCase(BaseTestCase): self.assertTrue(warn_mock.called) +@mock.patch('ironic.common.keystone.get_auth', + lambda _section, **kwargs: mock.sentinel.auth) @mock.patch('ironic.common.keystone.get_session', - lambda _n: mock.sentinel.session) + lambda _section, **kwargs: mock.sentinel.session) @mock.patch.object(eventlet, 'spawn_n', lambda f, *a, **kw: f(*a, **kw)) @mock.patch.object(client.ClientV1, 'introspect') @mock.patch.object(client.ClientV1, '__init__', return_value=None) @@ -134,8 +136,10 @@ class InspectHardwareTestCase(BaseTestCase): mock_introspect.assert_called_once_with(self.node.uuid) +@mock.patch('ironic.common.keystone.get_auth', + lambda _section, **kwargs: mock.sentinel.auth) @mock.patch('ironic.common.keystone.get_session', - lambda _n: mock.sentinel.session) + lambda _section, **kwargs: mock.sentinel.session) @mock.patch.object(client.ClientV1, 'get_status') @mock.patch.object(client.ClientV1, '__init__', return_value=None) class CheckStatusTestCase(BaseTestCase):