From 22ccfe999cfd52d41013cd29cb85ec2f3c44ee4a Mon Sep 17 00:00:00 2001
From: Kiran Pawar <kinpaa@gmail.com>
Date: Tue, 15 Oct 2024 13:38:52 +0000
Subject: [PATCH] Add new policy `list_all_projects` for share/share-snapshot
Administrator can now configure new policy `list_all_projects` for share
and share snapshot. This policy is applicable for listing of
respective resources in all projects and helps to remove hardcoded admin
checks.
Closes-bug: #2084532
Change-Id: I43cca075bb5c5d0a5061fbd6b4064ccdfc2a23bb
---
manila/policies/share_snapshot.py | 23 ++++++++++++++++++
manila/policies/shares.py | 24 +++++++++++++++++++
manila/share/api.py | 17 ++++++++++---
manila/tests/share/test_api.py | 14 +++++++++--
...shares-and-snapshots-0b02bea6e121c6a2.yaml | 7 ++++++
5 files changed, 80 insertions(+), 5 deletions(-)
create mode 100644 releasenotes/notes/bug-2084532-add-new-policy-list-all-project-for-shares-and-snapshots-0b02bea6e121c6a2.yaml
diff --git a/manila/policies/share_snapshot.py b/manila/policies/share_snapshot.py
index d074e489ab..d4d20bc7d7 100644
--- a/manila/policies/share_snapshot.py
+++ b/manila/policies/share_snapshot.py
@@ -100,6 +100,12 @@ deprecated_list_snapshots_in_deferred_deletion_states = policy.DeprecatedRule(
deprecated_reason=DEPRECATED_REASON,
deprecated_since='2024.1/Caracal'
)
+deprecated_list_all_projects = policy.DeprecatedRule(
+ name=BASE_POLICY_NAME % 'list_all_projects',
+ check_str=base.RULE_ADMIN_API,
+ deprecated_reason=DEPRECATED_REASON,
+ deprecated_since='2025.1/Epoxy'
+)
share_snapshot_policies = [
policy.DocumentedRuleDefault(
@@ -132,6 +138,23 @@ share_snapshot_policies = [
],
deprecated_rule=deprecated_snapshot_get_all
),
+ policy.DocumentedRuleDefault(
+ name=BASE_POLICY_NAME % 'list_all_projects',
+ check_str=base.ADMIN,
+ scope_types=['project'],
+ description="List share snapshots by all projects.",
+ operations=[
+ {
+ 'method': 'GET',
+ 'path': '/snapshots?all_tenants=1',
+ },
+ {
+ 'method': 'GET',
+ 'path': '/snapshots/detail?all_tenants=1',
+ }
+ ],
+ deprecated_rule=deprecated_list_all_projects
+ ),
policy.DocumentedRuleDefault(
name=BASE_POLICY_NAME % 'force_delete',
check_str=base.ADMIN,
diff --git a/manila/policies/shares.py b/manila/policies/shares.py
index 90bff62399..d80be76ebe 100644
--- a/manila/policies/shares.py
+++ b/manila/policies/shares.py
@@ -227,6 +227,13 @@ deprecated_list_shares_in_deferred_deletion_states = policy.DeprecatedRule(
deprecated_since='2024.1/Caracal'
)
+deprecated_list_all_projects = policy.DeprecatedRule(
+ name=BASE_POLICY_NAME % 'list_all_projects',
+ check_str=base.RULE_ADMIN_API,
+ deprecated_reason=DEPRECATED_REASON,
+ deprecated_since='2025.1/Epoxy'
+)
+
shares_policies = [
policy.DocumentedRuleDefault(
name=BASE_POLICY_NAME % 'create',
@@ -682,6 +689,23 @@ shares_policies = [
],
deprecated_rule=deprecated_list_shares_in_deferred_deletion_states
),
+ policy.DocumentedRuleDefault(
+ name=BASE_POLICY_NAME % 'list_all_projects',
+ check_str=base.ADMIN,
+ scope_types=['project'],
+ description="List share by all projects.",
+ operations=[
+ {
+ 'method': 'GET',
+ 'path': '/shares?all_tenants=1',
+ },
+ {
+ 'method': 'GET',
+ 'path': '/shares/detail?all_tenants=1',
+ }
+ ],
+ deprecated_rule=deprecated_list_all_projects
+ ),
]
diff --git a/manila/share/api.py b/manila/share/api.py
index 7566b81ed9..362910081f 100644
--- a/manila/share/api.py
+++ b/manila/share/api.py
@@ -2356,6 +2356,12 @@ class API(base.Base):
if show_deferred_deleted:
filters['list_deferred_delete'] = True
+ list_all_projects = False
+ all_tenants = utils.is_all_tenants(search_opts)
+ if all_tenants:
+ list_all_projects = policy.check_policy(
+ context, 'share', 'list_all_projects', do_raise=False)
+
# Get filtered list of shares
if 'host' in filters:
policy.check_policy(context, 'share', 'list_by_host')
@@ -2365,7 +2371,7 @@ class API(base.Base):
result = get_methods['get_by_share_server'](
context, search_opts.pop('share_server_id'), filters=filters,
sort_key=sort_key, sort_dir=sort_dir)
- elif context.is_admin and utils.is_all_tenants(search_opts):
+ elif list_all_projects:
result = get_methods['get_all'](
context, filters=filters, sort_key=sort_key, sort_dir=sort_dir)
else:
@@ -2420,7 +2426,12 @@ class API(base.Base):
LOG.debug("Searching for snapshots by: %s", search_opts)
# Read and remove key 'all_tenants' if was provided
- all_tenants = search_opts.pop('all_tenants', None)
+ list_all_projects = False
+ all_tenants = utils.is_all_tenants(search_opts)
+ if all_tenants:
+ search_opts.pop('all_tenants', None)
+ list_all_projects = policy.check_policy(
+ context, 'share_snapshot', 'list_all_projects', do_raise=False)
string_args = {'sort_key': sort_key, 'sort_dir': sort_dir}
string_args.update(search_opts)
@@ -2448,7 +2459,7 @@ class API(base.Base):
self.db.share_snapshot_get_all_by_project_with_count
if show_count else self.db.share_snapshot_get_all_by_project)}
- if context.is_admin and all_tenants:
+ if list_all_projects:
result = get_methods['get_all'](
context, filters=search_opts, limit=limit, offset=offset,
sort_key=sort_key, sort_dir=sort_dir)
diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py
index d664051bfc..dfbc1fa845 100644
--- a/manila/tests/share/test_api.py
+++ b/manila/tests/share/test_api.py
@@ -264,6 +264,8 @@ class ShareAPITestCase(test.TestCase):
ctx = context.RequestContext('fake_uid', 'fake_pid_1', is_admin=True)
self.mock_object(db_api, 'share_get_all',
mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES))
+ self.mock_object(share_api.policy, 'check_policy',
+ mock.Mock(return_value=True))
shares = self.api.get_all(ctx, {'all_tenants': 1})
@@ -276,6 +278,8 @@ class ShareAPITestCase(test.TestCase):
ctx = context.RequestContext('fake_uid', 'fake_pid_1', is_admin=True)
self.mock_object(db_api, 'share_get_all',
mock.Mock(return_value=_FAKE_LIST_OF_ALL_SHARES))
+ self.mock_object(share_api.policy, 'check_policy',
+ mock.Mock(return_value=True))
shares = self.api.get_all(ctx, {'all_tenants': ''})
@@ -348,6 +352,7 @@ class ShareAPITestCase(test.TestCase):
ctx, 'share',
'list_shares_in_deferred_deletion_states',
do_raise=False),
+ mock.call(ctx, 'share', 'list_all_projects', do_raise=False),
mock.call(ctx, 'share', 'list_by_share_server_id')])
db_api.share_get_all_by_share_server.assert_called_once_with(
@@ -3050,12 +3055,17 @@ class ShareAPITestCase(test.TestCase):
@mock.patch.object(db_api, 'share_snapshot_get_all', mock.Mock())
def test_get_all_snapshots_admin_all_tenants(self):
- mock_policy = self.mock_object(share_api.policy, 'check_policy',
- mock.Mock(return_value=False))
+ mock_policy = self.mock_object(
+ share_api.policy, 'check_policy',
+ mock.Mock(side_effect=[False, True, False]))
self.api.get_all_snapshots(self.context,
search_opts={'all_tenants': 1})
mock_policy.assert_has_calls([
mock.call(self.context, 'share_snapshot', 'get_all_snapshots'),
+ mock.call(
+ self.context, 'share_snapshot',
+ 'list_all_projects',
+ do_raise=False),
mock.call(
self.context, 'share_snapshot',
'list_snapshots_in_deferred_deletion_states',
diff --git a/releasenotes/notes/bug-2084532-add-new-policy-list-all-project-for-shares-and-snapshots-0b02bea6e121c6a2.yaml b/releasenotes/notes/bug-2084532-add-new-policy-list-all-project-for-shares-and-snapshots-0b02bea6e121c6a2.yaml
new file mode 100644
index 0000000000..b9aec2d880
--- /dev/null
+++ b/releasenotes/notes/bug-2084532-add-new-policy-list-all-project-for-shares-and-snapshots-0b02bea6e121c6a2.yaml
@@ -0,0 +1,7 @@
+---
+upgrade:
+ - |
+ Administrator can now configure new policy `list_all_projects` for share
+ and share snapshot. This policy is applicable for listing of respective
+ resources in all projects. Please check `launchpad bug 2084532
+ <https://bugs.launchpad.net/manila/+bug/2084532>`_ for more details.