From 22ccfe999cfd52d41013cd29cb85ec2f3c44ee4a Mon Sep 17 00:00:00 2001 From: Kiran Pawar 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 + `_ for more details.