Use client token when talking to manila

When a user asks nova to attach a manila share to the user's instance
nova should use the user's keystone token to talk to manila so that
manila can verify if the user has access to the share.

Manila is the OpenStack Shared Filesystems service.  These series
of patches implement changes required in Nova to allow the shares
provided by Manila to be associated with and attached to instances
using virtiofs.

Implements: blueprint libvirt-virtiofs-attach-manila-shares
Change-Id: I793f0518dcfffd2286e7bc685cab76794aece91d
This commit is contained in:
Balazs Gibizer
2024-07-19 15:26:51 +02:00
committed by René Ribaud
parent 0f9001f06e
commit bee0a5c54a
9 changed files with 164 additions and 35 deletions
@@ -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
+1 -1
View File
@@ -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
+2 -1
View File
@@ -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'}
+8 -6
View File
@@ -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:
+2 -2
View File
@@ -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))
+104 -13
View File
@@ -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.
+10 -6
View File
@@ -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()
+35 -5
View File
@@ -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") %
+1 -1
View File
@@ -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):