diff --git a/nova/api/openstack/compute/baremetal_nodes.py b/nova/api/openstack/compute/baremetal_nodes.py index b68e2cf6a3..1aa7d752a6 100644 --- a/nova/api/openstack/compute/baremetal_nodes.py +++ b/nova/api/openstack/compute/baremetal_nodes.py @@ -56,6 +56,7 @@ class BareMetalNodeController(wsgi.Controller): # up to handle this and raise VirtDriverNotReady as appropriate. self._ironic_connection = utils.get_sdk_adapter( 'baremetal', + admin=True, check_service=True, ) return self._ironic_connection diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index efd8c0ffd5..8b9bb51a95 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -3753,7 +3753,7 @@ class LimitsCommands(): conf_utils.register_ksa_opts( CONF, 'keystone_authtoken', 'identity') keystone_api = utils.get_sdk_adapter( - 'identity', conf_group='keystone_authtoken') + 'identity', admin=True, conf_group='keystone_authtoken') # Service ID is required in unified limits APIs. service_id = keystone_api.find_service('nova').id diff --git a/nova/scheduler/client/report.py b/nova/scheduler/client/report.py index ec1edbc637..ba42277ebf 100644 --- a/nova/scheduler/client/report.py +++ b/nova/scheduler/client/report.py @@ -274,7 +274,8 @@ class SchedulerReportClient(object): """Create the HTTP session accessing the placement service.""" # Flush provider tree and associations so we start from a clean slate. self.clear_provider_cache(init=True) - client = self._adapter or utils.get_sdk_adapter('placement') + client = self._adapter or utils.get_sdk_adapter( + "placement", admin=True) # Set accept header on every request to ensure we notify placement # service of our response body media type preferences. client.additional_headers = {'accept': 'application/json'} diff --git a/nova/share/manila.py b/nova/share/manila.py index e0e8b55aa2..a2f37fb0f2 100644 --- a/nova/share/manila.py +++ b/nova/share/manila.py @@ -30,7 +30,7 @@ LOG = logging.getLogger(__name__) MIN_SHARE_FILE_SYSTEM_MICROVERSION = "2.82" -def manilaclient(context): +def _manilaclient(context, admin=False): """Constructs a manila client object for making API requests. :return: An openstack.proxy.Proxy object for the specified service_type. @@ -41,7 +41,9 @@ def manilaclient(context): return utils.get_sdk_adapter( "shared-file-system", + admin=admin, check_service=True, + context=context, shared_file_system_api_version=MIN_SHARE_FILE_SYSTEM_MICROVERSION, global_request_id=context.global_id ) @@ -208,7 +210,7 @@ class API(object): paths.append(export_location.path) return paths[0] - client = manilaclient(context) + client = _manilaclient(context, admin=False) LOG.debug("Get share id:'%s' data from manila", share_id) share = client.get_share(share_id) export_locations = client.export_locations(share.id) @@ -236,7 +238,8 @@ class API(object): LOG.debug("Get share access id for share id:'%s'", share_id) - access_list = manilaclient(context).access_rules(share_id) + access_list = _manilaclient( + context, admin=True).access_rules(share_id) for access in access_list: if ( @@ -279,7 +282,7 @@ class API(object): LOG.debug("Allow host access to share id:'%s'", share_id) - access = manilaclient(context).create_access_rule( + access = _manilaclient(context, admin=True).create_access_rule( share_id, access_type=access_type, access_to=access_to, @@ -311,8 +314,6 @@ class API(object): respond with a status code 202. """ - client = manilaclient(context) - access = self.get_access( context, share_id, @@ -321,6 +322,7 @@ class API(object): ) if access: + client = _manilaclient(context, admin=True) LOG.debug("Deny host access to share id:'%s'", share_id) resp = client.delete_access_rule(access.id, share_id) if resp.status_code != 202: diff --git a/nova/tests/fixtures/nova.py b/nova/tests/fixtures/nova.py index f0164a3902..56ee615104 100644 --- a/nova/tests/fixtures/nova.py +++ b/nova/tests/fixtures/nova.py @@ -2044,10 +2044,10 @@ class UnifiedLimitsFixture(fixtures.Fixture): self.mock_sdk_adapter = mock.Mock() real_get_sdk_adapter = utils.get_sdk_adapter - def fake_get_sdk_adapter(service_type, **kwargs): + def fake_get_sdk_adapter(service_type, admin, **kwargs): if service_type == 'identity': return self.mock_sdk_adapter - return real_get_sdk_adapter(service_type, **kwargs) + return real_get_sdk_adapter(service_type, admin, **kwargs) self.useFixture(fixtures.MockPatch( 'nova.utils.get_sdk_adapter', fake_get_sdk_adapter)) diff --git a/nova/tests/unit/test_manila.py b/nova/tests/unit/test_manila.py index 7dc9ad890d..9993dff8f4 100644 --- a/nova/tests/unit/test_manila.py +++ b/nova/tests/unit/test_manila.py @@ -13,6 +13,8 @@ from requests import Response import fixtures +from keystoneauth1 import loading as ks_loading + import nova.conf from nova import context as nova_context from nova import exception @@ -156,9 +158,9 @@ class BaseManilaTestCase(object): self.mock_get_confgrp = self.useFixture(fixtures.MockPatch( 'nova.utils._get_conf_group')).mock - self.mock_get_auth_sess = self.useFixture(fixtures.MockPatch( - 'nova.utils._get_auth_and_session')).mock - self.mock_get_auth_sess.return_value = (None, mock.sentinel.session) + self.mock_ks_loading = self.useFixture( + fixtures.MockPatchObject(ks_loading, 'load_auth_from_conf_options') + ).mock self.service_type = 'shared-file-system' self.mock_connection = self.useFixture( @@ -223,16 +225,39 @@ class BaseManilaTestCase(object): return FakeConnection() - def create_client(self, context): - return manila.manilaclient(context) - - def test_client(self): - client = self.create_client(self.context) + @mock.patch('nova.utils.get_sdk_adapter') + def test_client(self, mock_get_sdk_adapter): + client = manila._manilaclient(self.context) self.assertTrue(hasattr(client, 'get_share')) self.assertTrue(hasattr(client, 'export_locations')) self.assertTrue(hasattr(client, 'access_rules')) self.assertTrue(hasattr(client, 'create_access_rule')) self.assertTrue(hasattr(client, 'delete_access_rule')) + mock_get_sdk_adapter.assert_called_once_with( + "shared-file-system", + admin=False, + check_service=True, + context=self.context, + shared_file_system_api_version="2.82", + global_request_id=self.context.global_id, + ) + + @mock.patch('nova.utils.get_sdk_adapter') + def test_client_admin(self, mock_get_sdk_adapter): + client = manila._manilaclient(self.context, admin=True) + self.assertTrue(hasattr(client, 'get_share')) + self.assertTrue(hasattr(client, 'export_locations')) + self.assertTrue(hasattr(client, 'access_rules')) + self.assertTrue(hasattr(client, 'create_access_rule')) + self.assertTrue(hasattr(client, 'delete_access_rule')) + mock_get_sdk_adapter.assert_called_once_with( + "shared-file-system", + admin=True, + check_service=True, + context=self.context, + shared_file_system_api_version="2.82", + global_request_id=self.context.global_id, + ) class ManilaTestCase(BaseManilaTestCase, test.NoDBTestCase): @@ -246,10 +271,20 @@ class ManilaTestCase(BaseManilaTestCase, test.NoDBTestCase): self.assertIn("Share nonexisting could not be found.", exc.message) - def test_get_share(self): + @mock.patch( + 'nova.utils.get_sdk_adapter', side_effect=nova.utils.get_sdk_adapter) + def test_get_share(self, mock_get_sdk_adapter): """Tests that we manage to get a share. """ share = self.api.get(self.context, '1234') + mock_get_sdk_adapter.assert_called_once_with( + "shared-file-system", + admin=False, + check_service=True, + context=self.context, + shared_file_system_api_version="2.82", + global_request_id=self.context.global_id, + ) self.assertIsInstance(share, manila.Share) self.assertEqual('1234', share.id) self.assertEqual(1, share.size) @@ -304,12 +339,21 @@ class ManilaTestCase(BaseManilaTestCase, test.NoDBTestCase): self.assertIn("Share nonexisting2 could not be found.", exc.message) - def test_get_access(self): + @mock.patch( + 'nova.utils.get_sdk_adapter', side_effect=nova.utils.get_sdk_adapter) + def test_get_access(self, mock_get_sdk_adapter): """Tests that we manage to get an access id based on access_type and access_to parameters. """ access = self.api.get_access(self.context, '1234', 'ip', '0.0.0.0/0') - + mock_get_sdk_adapter.assert_called_once_with( + "shared-file-system", + admin=True, + check_service=True, + context=self.context, + shared_file_system_api_version="2.82", + global_request_id=self.context.global_id, + ) self.assertEqual('a25b2df3-90bd-4add-afa6-5f0dbbd50452', access.id) self.assertEqual('rw', access.access_level) self.assertEqual('active', access.state) @@ -342,10 +386,20 @@ class ManilaTestCase(BaseManilaTestCase, test.NoDBTestCase): self.assertIn("Share nonexisting could not be found.", exc.message) - def test_allow_access(self): + @mock.patch( + 'nova.utils.get_sdk_adapter', side_effect=nova.utils.get_sdk_adapter) + def test_allow_access(self, mock_get_sdk_adapter): """Tests that we manage to allow access to a share. """ access = self.api.allow(self.context, '1234', 'ip', '0.0.0.0/0', 'rw') + mock_get_sdk_adapter.assert_called_once_with( + "shared-file-system", + admin=True, + check_service=True, + context=self.context, + shared_file_system_api_version="2.82", + global_request_id=self.context.global_id, + ) self.assertEqual('a25b2df3-90bd-4add-afa6-5f0dbbd50452', access.id) self.assertEqual('rw', access.access_level) self.assertEqual('active', access.state) @@ -385,7 +439,9 @@ class ManilaTestCase(BaseManilaTestCase, test.NoDBTestCase): self.assertIn("Share nonexisting could not be found.", exc.message) - def test_deny_access(self): + @mock.patch( + 'nova.utils.get_sdk_adapter', side_effect=nova.utils.get_sdk_adapter) + def test_deny_access(self, mock_get_sdk_adapter): """Tests that we manage to deny access to a share. """ self.api.deny( @@ -394,6 +450,41 @@ class ManilaTestCase(BaseManilaTestCase, test.NoDBTestCase): 'ip', '0.0.0.0/0' ) + self.assertEqual(2, mock_get_sdk_adapter.call_count) + + self.assertEqual( + mock_get_sdk_adapter.call_args_list[0].args, + ( + "shared-file-system", + ), + ) + self.assertEqual( + mock_get_sdk_adapter.call_args_list[0].kwargs, + { + "admin": True, + "check_service": True, + "context": self.context, + "shared_file_system_api_version": "2.82", + "global_request_id": self.context.global_id, + }, + ) + + self.assertEqual( + mock_get_sdk_adapter.call_args_list[1].args, + ( + "shared-file-system", + ), + ) + self.assertEqual( + mock_get_sdk_adapter.call_args_list[1].kwargs, + { + "admin": True, + "check_service": True, + "context": self.context, + "shared_file_system_api_version": "2.82", + "global_request_id": self.context.global_id, + }, + ) def test_deny_access_fails_id_missing(self): """Tests that we fail if something wrong happens calling deny method. diff --git a/nova/tests/unit/test_utils.py b/nova/tests/unit/test_utils.py index f6372ce450..1ee8ba9365 100644 --- a/nova/tests/unit/test_utils.py +++ b/nova/tests/unit/test_utils.py @@ -1176,8 +1176,10 @@ class TestGetSDKAdapter(test.NoDBTestCase): self.mock_conf = self.useFixture(fixtures.MockPatch( 'nova.utils.CONF')).mock - def _test_get_sdk_adapter(self, strict=False): - actual = utils.get_sdk_adapter(self.service_type, check_service=strict) + def _test_get_sdk_adapter(self, admin, strict=False): + actual = utils.get_sdk_adapter( + self.service_type, admin, check_service=strict + ) self.mock_get_confgrp.assert_called_once_with(self.service_type) self.mock_get_auth_sess.assert_called_once_with( @@ -1189,24 +1191,26 @@ class TestGetSDKAdapter(test.NoDBTestCase): return actual def test_get_sdk_adapter(self): - self.assertEqual(self._test_get_sdk_adapter(), mock.sentinel.proxy) + self.assertEqual( + self._test_get_sdk_adapter(admin=True), mock.sentinel.proxy + ) def test_get_sdk_adapter_strict(self): self.assertEqual( - self._test_get_sdk_adapter(strict=True), mock.sentinel.proxy) + self._test_get_sdk_adapter(True, strict=True), mock.sentinel.proxy) def test_get_sdk_adapter_strict_fail(self): self.mock_connection.side_effect = sdk_exc.ServiceDiscoveryException() self.assertRaises( exception.ServiceUnavailable, - self._test_get_sdk_adapter, strict=True) + self._test_get_sdk_adapter, True, strict=True) def test_get_sdk_adapter_conf_group_fail(self): self.mock_get_confgrp.side_effect = ( exception.ConfGroupForServiceTypeNotFound(stype=self.service_type)) self.assertRaises(exception.ConfGroupForServiceTypeNotFound, - utils.get_sdk_adapter, self.service_type) + utils.get_sdk_adapter, self.service_type, True) self.mock_get_confgrp.assert_called_once_with(self.service_type) self.mock_connection.assert_not_called() self.mock_get_auth_sess.assert_not_called() diff --git a/nova/utils.py b/nova/utils.py index b85cf0cdf7..b6b3785331 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -966,7 +966,8 @@ def get_ksa_adapter(service_type, ksa_auth=None, ksa_session=None, def get_sdk_adapter( - service_type, check_service=False, conf_group=None, **kwargs + service_type, admin, check_service=False, conf_group=None, + context=None, **kwargs ): """Construct an openstacksdk-brokered Adapter for a given service type. @@ -976,10 +977,13 @@ def get_sdk_adapter( :param service_type: String name of the service type for which the Adapter is to be constructed. + :param admin: If set to true, the service will use Nova's service user + and password; otherwise, it will use the user's token. :param check_service: If True, we will query the endpoint to make sure the service is alive, raising ServiceUnavailable if it is not. :param conf_group: String name of the conf group to use, otherwise the name of the service_type will be used. + :param context: Use to get user's token, if admin is set to False. :param kwargs: Additional arguments to pass to the Adapter constructor. Mainly used to pass microversion to a specific service, e.g. shared_file_system_api_version="2.82". @@ -989,11 +993,37 @@ def get_sdk_adapter( :raise: ServiceUnavailable if check_service is True and the service is down """ confgrp = conf_group or _get_conf_group(service_type) - sess = _get_auth_and_session(confgrp)[1] + try: - conn = connection.Connection( - session=sess, oslo_conf=CONF, service_types={service_type}, - strict_proxies=check_service, **kwargs) + if admin is False: + if context is None: + raise ValueError( + "If admin is set to False then context cannot be None.") + + # NOTE(gibi): this is only needed to make sure + # CONF.service_user.auth_url config is registered + ks_loading.load_auth_from_conf_options( + CONF, nova.conf.service_token.service_user.name) + + # Create a connection using the user's token instead of nova's + # service user/pass. + conn = connection.Connection( + token=context.auth_token, + auth_type="v3token", + project_id=context.project_id, + project_domain_id=context.project_domain_id, + auth_url=CONF.service_user.auth_url, + service_types={service_type}, + strict_proxies=check_service, + **kwargs, + ) + else: + # Create a connection based on nova's service user/pass + sess = _get_auth_and_session(confgrp)[1] + conn = connection.Connection( + session=sess, oslo_conf=CONF, service_types={service_type}, + strict_proxies=check_service, **kwargs) + except sdk_exc.ServiceDiscoveryException as e: raise exception.ServiceUnavailable( _("The %(service_type)s service is unavailable: %(error)s") % diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py index d3c7d191bb..0040beeb1f 100644 --- a/nova/virt/ironic/driver.py +++ b/nova/virt/ironic/driver.py @@ -198,7 +198,7 @@ class IronicDriver(virt_driver.ComputeDriver): # service isn't ready yet. Consumers of ironic_connection are set # up to handle this and raise VirtDriverNotReady as appropriate. self._ironic_connection = utils.get_sdk_adapter( - 'baremetal', check_service=True) + 'baremetal', admin=True, check_service=True) return self._ironic_connection def _get_node(self, node_id):