From 8eb38ac41a776f0878368efa2a083830eded9589 Mon Sep 17 00:00:00 2001 From: Kiran Pawar Date: Fri, 14 Jan 2022 12:01:59 +0000 Subject: [PATCH] onlyHostFilter: Fix follow-up suggestions. 1. Change context as first argument to function. 2. Fix spelling mistake in version history 3. Add new host_admin RBAC policy which is applied in onlyHostFilter since non-admin user as well needs to create share on specific host. Change-Id: Id2c09ebab874ec983da7f26370932d46a0447801 --- api-ref/source/parameters.yaml | 2 +- manila/api/views/shares.py | 3 ++- manila/policies/base.py | 7 +++++++ manila/policy.py | 9 +++++++++ manila/scheduler/filters/host.py | 3 ++- manila/share/api.py | 14 +++++++------- manila/tests/policy.yaml | 1 + manila/tests/share/test_api.py | 2 +- 8 files changed, 30 insertions(+), 11 deletions(-) diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 97679c8747..6f2f519793 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -2451,7 +2451,7 @@ scheduler_hints: One or more scheduler_hints key and value pairs as a dictionary of strings. Accepted hints are: - ``same_host`` or ``different_host``: values must be a comma separated list of Share IDs - - ``only_host``: value must be a manage-share service host in ``host@backend#POOL`` format (admin only) + - ``only_host``: value must be a manage-share service host in ``host@backend#POOL`` format (admin only). Only available in and beyond API version 2.67 in: body required: false type: object diff --git a/manila/api/views/shares.py b/manila/api/views/shares.py index 83b0aa0977..30c399ad7c 100644 --- a/manila/api/views/shares.py +++ b/manila/api/views/shares.py @@ -15,6 +15,7 @@ from manila.api import common from manila.common import constants +from manila import policy class ViewBuilder(common.ViewBuilder): @@ -98,7 +99,7 @@ class ViewBuilder(common.ViewBuilder): self.update_versioned_resource_dict(request, share_dict, share) - if context.is_admin: + if policy.check_is_host_admin(context): share_dict['share_server_id'] = share_instance.get( 'share_server_id') share_dict['host'] = share_instance.get('host') diff --git a/manila/policies/base.py b/manila/policies/base.py index 564c89c976..149b26a7e0 100644 --- a/manila/policies/base.py +++ b/manila/policies/base.py @@ -86,6 +86,13 @@ rules = [ deprecated_rule=DEPRECATED_CONTEXT_IS_ADMIN, scope_types=['project']), + policy.RuleDefault( + name='context_is_host_admin', + check_str='role:admin and ' + 'project_id:%(project_id)s', + description='Privileged user who can select host during scheduling', + scope_types=['project']), + # ***Legacy/deprecated unscoped rules*** # # can be removed after "enforce_scope" defaults to True in oslo.policy policy.RuleDefault( diff --git a/manila/policy.py b/manila/policy.py index 5a33bc1f10..16c90a7435 100644 --- a/manila/policy.py +++ b/manila/policy.py @@ -223,6 +223,15 @@ def check_is_admin(context): return authorize(context, 'context_is_admin', target, do_raise=False) +def check_is_host_admin(context): + """Whether or not user is host admin according to policy setting. + + """ + # the target is user-self + target = default_target(context) + return authorize(context, 'context_is_host_admin', target, do_raise=False) + + def wrap_check_policy(resource): """Check policy corresponding to the wrapped methods prior to execution.""" def check_policy_wraper(func): diff --git a/manila/scheduler/filters/host.py b/manila/scheduler/filters/host.py index ba54e90d96..6e20df7c85 100644 --- a/manila/scheduler/filters/host.py +++ b/manila/scheduler/filters/host.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +from manila import policy from manila.scheduler.filters import base_host @@ -21,7 +22,7 @@ class OnlyHostFilter(base_host.BaseHostFilter): def host_passes(self, host_state, filter_properties): context = filter_properties['context'] - if not context.is_admin: + if not policy.check_is_host_admin(context): return True hints = filter_properties.get('scheduler_hints') if hints is None: diff --git a/manila/share/api.py b/manila/share/api.py index 76285026ca..c29a6ceaba 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -967,7 +967,7 @@ class API(base.Base): export_location_path) request_spec = self._get_request_spec_dict( - share, share_type, context, size=0, + context, share, share_type, size=0, share_proto=share_data['share_proto'], host=share_data['host']) @@ -979,7 +979,7 @@ class API(base.Base): return self.db.share_get(context, share['id']) - def _get_request_spec_dict(self, share, share_type, context, **kwargs): + def _get_request_spec_dict(self, context, share, share_type, **kwargs): if share is None: share = {'instance': {}} @@ -1793,9 +1793,9 @@ class API(base.Base): raise exception.InvalidShare(reason=msg % payload) request_spec = self._get_request_spec_dict( + context, share, share_type, - context, availability_zone_id=service['availability_zone_id'], share_network_id=new_share_network_id) @@ -2528,8 +2528,8 @@ class API(base.Base): else: share_type = share_types.get_share_type( context, share['instance']['share_type_id']) - request_spec = self._get_request_spec_dict(share, share_type, - context) + request_spec = self._get_request_spec_dict(context, share, + share_type) request_spec.update({'is_share_extend': True}) self.scheduler_rpcapi.extend_share(context, share['id'], new_size, reservations, request_spec) @@ -2680,8 +2680,8 @@ class API(base.Base): for share_instance in share_instances: share_type_id = share_instance['share_type_id'] share_type = share_types.get_share_type(context, share_type_id) - req_spec = self._get_request_spec_dict(share_instance, - share_type, context, + req_spec = self._get_request_spec_dict(context, share_instance, + share_type, **kwargs) shares_req_spec.append(req_spec) diff --git a/manila/tests/policy.yaml b/manila/tests/policy.yaml index 964deaedd5..47f63a5c2d 100644 --- a/manila/tests/policy.yaml +++ b/manila/tests/policy.yaml @@ -2,6 +2,7 @@ # or extra rules in policy file, it is strongly # recommended to switch to new rules. "context_is_admin": "role:admin" +"context_is_host_admin": "role:admin and project_id:%(project_id)s" "admin_api": "is_admin:True" "admin_or_owner": "is_admin:True or project_id:%(project_id)s" "default": "rule:admin_or_owner" diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index cc1065193e..419d8f04aa 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -5305,7 +5305,7 @@ class ShareAPITestCase(test.TestCase): get_type_calls.append( mock.call(self.context, instance['share_type_id'])) get_request_spec_calls.append( - mock.call(instance, fake_share_type, self.context)) + mock.call(self.context, instance, fake_share_type)) mock_get_type = self.mock_object( share_types, 'get_share_type',