openstack/manila
Code Issues Proposed changes

Merge "Fix access rule visibility locks" into stable/2023.2

Browse Source
This commit is contained in:
Zuul
2025-05-01 03:38:17 +00:00
committed by Gerrit Code Review
parent 3f94c85e5a 3f9240a8f5
commit c97f3a1182
5 changed files with 87 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
manila/api/v1/shares.py
View File

@@ -585,12 +585,14 @@ class ShareMixin(object):
unrestrict = access_data.get('unrestrict', False)
search_opts = {
'resource_id': access_id,
'resource_action': constants.RESOURCE_ACTION_DELETE
'resource_action': constants.RESOURCE_ACTION_DELETE,
'all_projects': True,
}
locks, locks_count = (
self.resource_locks_api.get_all(
context, search_opts=search_opts, show_count=True) or []
context.elevated(), search_opts=search_opts,
show_count=True) or []
)
# no locks placed, nothing to do
@@ -616,7 +618,9 @@ class ShareMixin(object):
try:
self.resource_locks_api.ensure_context_can_delete_lock(
context, lock['id'])
except exception.NotAuthorized:
except (exception.NotAuthorized, exception.ResourceLockNotFound):
# If it is not found, then it means that the context doesn't
# have access to this resource and should be denied.
non_deletable_locks.append(lock)
if non_deletable_locks:

5
manila/api/v2/share_accesses.py
View File

@@ -55,10 +55,11 @@ class ShareAccessesController(wsgi.Controller, wsgi.AdminActionsMixin):
search_opts = {
'resource_id': id,
'resource_action': constants.RESOURCE_ACTION_SHOW,
'resource_type': 'access_rule'
'resource_type': 'access_rule',
'all_projects': True,
}
locks, count = self.resource_locks_api.get_all(
context, search_opts, show_count=True)
context.elevated(), search_opts, show_count=True)
if count:
return self.resource_locks_api.access_is_restricted(context,

34
manila/tests/api/v1/test_shares.py
View File

@@ -1282,6 +1282,7 @@ class ShareActionsTest(test.TestCase):
self.mock_object(share_api.API, "deny_access", _stub_deny_access)
self.mock_object(share_api.API, "access_get", _fake_access_get)
self.mock_object(self.controller, '_check_for_access_rule_locks')
id = 'fake_share_id'
body = {"os-deny_access": {"access_id": 'fake_acces_id'}}
@@ -1299,14 +1300,20 @@ class ShareActionsTest(test.TestCase):
share_network = db_utils.create_share_network()
share = db_utils.create_share(share_network_id=share_network['id'])
self.mock_object(share_api.API, 'get', mock.Mock(return_value=share))
self.mock_object(self.controller, '_check_for_access_rule_locks')
id = 'fake_share_id'
body = {"os-deny_access": {"access_id": 'fake_acces_id'}}
access_data = {"access_id": 'fake_acces_id'}
body = {"os-deny_access": access_data}
req = fakes.HTTPRequest.blank('/v1/tenant1/shares/%s/action' % id)
res = self.controller._deny_access(req, id, body)
self.assertEqual(202, res.status_int)
self.controller._check_for_access_rule_locks.assert_called_once_with(
req.environ['manila.context'], access_data,
access_data['access_id'], id
)
self.mock_policy_check.assert_called_once_with(
req.environ['manila.context'], 'share', 'deny_access')
@@ -1316,6 +1323,7 @@ class ShareActionsTest(test.TestCase):
self.mock_object(share_api.API, "deny_access", _stub_deny_access)
self.mock_object(share_api.API, "access_get", _fake_access_get)
self.mock_object(self.controller, '_check_for_access_rule_locks')
id = 'super_fake_share_id'
body = {"os-deny_access": {"access_id": 'fake_acces_id'}}
@@ -1363,12 +1371,14 @@ class ShareActionsTest(test.TestCase):
access_id = 'fake_access_id'
share_id = 'fake_share_id'
self.mock_object(context, 'elevated', mock.Mock(return_value=context))
self.controller._check_for_access_rule_locks(
context, {}, access_id, share_id)
delete_search_opts = {
'resource_id': access_id,
'resource_action': constants.RESOURCE_ACTION_DELETE
'resource_action': constants.RESOURCE_ACTION_DELETE,
'all_projects': True,
}
resource_locks.API.get_all.assert_called_once_with(
@@ -1387,6 +1397,7 @@ class ShareActionsTest(test.TestCase):
access_id = 'fake_access_id'
share_id = 'fake_share_id'
self.mock_object(context, 'elevated', mock.Mock(return_value=context))
self.assertRaises(
webob.exc.HTTPForbidden,
self.controller._check_for_access_rule_locks,
@@ -1394,7 +1405,8 @@ class ShareActionsTest(test.TestCase):
delete_search_opts = {
'resource_id': access_id,
'resource_action': constants.RESOURCE_ACTION_DELETE
'resource_action': constants.RESOURCE_ACTION_DELETE,
'all_projects': True,
}
resource_locks.API.get_all.assert_called_once_with(
@@ -1419,6 +1431,7 @@ class ShareActionsTest(test.TestCase):
access_id = 'fake_access_id'
share_id = 'fake_share_id'
self.mock_object(context, 'elevated', mock.Mock(return_value=context))
self.assertRaises(
webob.exc.HTTPForbidden,
self.controller._check_for_access_rule_locks,
@@ -1426,7 +1439,8 @@ class ShareActionsTest(test.TestCase):
delete_search_opts = {
'resource_id': access_id,
'resource_action': constants.RESOURCE_ACTION_DELETE
'resource_action': constants.RESOURCE_ACTION_DELETE,
'all_projects': True,
}
resource_locks.API.get_all.assert_called_once_with(
@@ -1457,6 +1471,7 @@ class ShareActionsTest(test.TestCase):
access_id = 'fake_access_id'
share_id = 'fake_share_id'
self.mock_object(context, 'elevated', mock.Mock(return_value=context))
self.assertRaises(
webob.exc.HTTPForbidden,
self.controller._check_for_access_rule_locks,
@@ -1464,7 +1479,8 @@ class ShareActionsTest(test.TestCase):
)
delete_search_opts = {
'resource_id': access_id,
'resource_action': constants.RESOURCE_ACTION_DELETE
'resource_action': constants.RESOURCE_ACTION_DELETE,
'all_projects': True,
}
resource_locks.API.get_all.assert_called_once_with(
context, search_opts=delete_search_opts, show_count=True
@@ -1491,15 +1507,19 @@ class ShareActionsTest(test.TestCase):
access_id = 'fake_access_id'
share_id = 'fake_share_id'
self.mock_object(context, 'elevated', mock.Mock(return_value=context))
self.controller._check_for_access_rule_locks(
context, {'unrestrict': True}, access_id, share_id)
delete_search_opts = {
'resource_id': access_id,
'resource_action': constants.RESOURCE_ACTION_DELETE
'resource_action': constants.RESOURCE_ACTION_DELETE,
'all_projects': True,
}
resource_locks.API.get_all.assert_called_once_with(
context, search_opts=delete_search_opts, show_count=True)
context.elevated(), search_opts=delete_search_opts,
show_count=True
)
(resource_locks.API.ensure_context_can_delete_lock
.assert_called_once_with(
context, locks[0]['id']))

42
manila/tests/lock/test_api.py
View File

@@ -517,3 +517,45 @@ class ResourceLockApiTest(test.TestCase):
utils.IsAMatcher(context.RequestContext),
'd767d3cd-1187-404a-a91f-8b172e0e768e'
)
def test_ensure_context_can_delete_lock_policy_fails(self):
lock = {'id': 'd767d3cd-1187-404a-a91f-8b172e0e768e'}
self.mock_object(
self.lock_api.db, 'resource_lock_get', mock.Mock(return_value=lock)
)
self.mock_object(
policy,
'check_policy',
mock.Mock(side_effect=exception.PolicyNotAuthorized(
action="resource_lock:delete")),
)
self.assertRaises(
exception.NotAuthorized,
self.lock_api.ensure_context_can_delete_lock,
self.ctxt,
'd767d3cd-1187-404a-a91f-8b172e0e768e')
self.lock_api.db.resource_lock_get.assert_called_once_with(
self.ctxt, 'd767d3cd-1187-404a-a91f-8b172e0e768e'
)
policy.check_policy.assert_called_once_with(
self.ctxt, 'resource_lock', 'delete', lock)
def test_ensure_context_can_delete_lock(self):
lock = {'id': 'd767d3cd-1187-404a-a91f-8b172e0e768e'}
self.mock_object(
self.lock_api.db, 'resource_lock_get', mock.Mock(return_value=lock)
)
self.mock_object(policy, 'check_policy')
self.mock_object(self.lock_api, '_check_allow_lock_manipulation')
self.lock_api.ensure_context_can_delete_lock(
self.ctxt,
'd767d3cd-1187-404a-a91f-8b172e0e768e')
self.lock_api.db.resource_lock_get.assert_called_once_with(
self.ctxt, 'd767d3cd-1187-404a-a91f-8b172e0e768e'
)
policy.check_policy.assert_called_once_with(
self.ctxt, 'resource_lock', 'delete', lock)

8
releasenotes/notes/bug-2089061-fix-access-rules-locks-lookup-b5efbd41397acba3.yaml Normal file
View File

@@ -0,0 +1,8 @@
---
fixes:
- |
While displaying and deleting access rules, manila was limiting the search
for locks to the context of the request. Now, manila will search within
all of the projects for locks and properly apply visibility and deletion
restrictions. For more details, please refer to
`launchpad bug #2089061 <https://bugs.launchpad.net/manila/+bug/2089061>`_.