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
This commit is contained in:
Kiran Pawar 2022-01-14 12:01:59 +00:00
parent b3951d06dd
commit 8eb38ac41a
8 changed files with 30 additions and 11 deletions

View File

@ -2451,7 +2451,7 @@ scheduler_hints:
One or more scheduler_hints key and value pairs as a dictionary of One or more scheduler_hints key and value pairs as a dictionary of
strings. Accepted hints are: strings. Accepted hints are:
- ``same_host`` or ``different_host``: values must be a comma separated list of Share IDs - ``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 in: body
required: false required: false
type: object type: object

View File

@ -15,6 +15,7 @@
from manila.api import common from manila.api import common
from manila.common import constants from manila.common import constants
from manila import policy
class ViewBuilder(common.ViewBuilder): class ViewBuilder(common.ViewBuilder):
@ -98,7 +99,7 @@ class ViewBuilder(common.ViewBuilder):
self.update_versioned_resource_dict(request, share_dict, share) 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_dict['share_server_id'] = share_instance.get(
'share_server_id') 'share_server_id')
share_dict['host'] = share_instance.get('host') share_dict['host'] = share_instance.get('host')

View File

@ -86,6 +86,13 @@ rules = [
deprecated_rule=DEPRECATED_CONTEXT_IS_ADMIN, deprecated_rule=DEPRECATED_CONTEXT_IS_ADMIN,
scope_types=['project']), 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*** # # ***Legacy/deprecated unscoped rules*** #
# can be removed after "enforce_scope" defaults to True in oslo.policy # can be removed after "enforce_scope" defaults to True in oslo.policy
policy.RuleDefault( policy.RuleDefault(

View File

@ -223,6 +223,15 @@ def check_is_admin(context):
return authorize(context, 'context_is_admin', target, do_raise=False) 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): def wrap_check_policy(resource):
"""Check policy corresponding to the wrapped methods prior to execution.""" """Check policy corresponding to the wrapped methods prior to execution."""
def check_policy_wraper(func): def check_policy_wraper(func):

View File

@ -13,6 +13,7 @@
# License for the specific language governing permissions and limitations # License for the specific language governing permissions and limitations
# under the License. # under the License.
from manila import policy
from manila.scheduler.filters import base_host from manila.scheduler.filters import base_host
@ -21,7 +22,7 @@ class OnlyHostFilter(base_host.BaseHostFilter):
def host_passes(self, host_state, filter_properties): def host_passes(self, host_state, filter_properties):
context = filter_properties['context'] context = filter_properties['context']
if not context.is_admin: if not policy.check_is_host_admin(context):
return True return True
hints = filter_properties.get('scheduler_hints') hints = filter_properties.get('scheduler_hints')
if hints is None: if hints is None:

View File

@ -967,7 +967,7 @@ class API(base.Base):
export_location_path) export_location_path)
request_spec = self._get_request_spec_dict( 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'], share_proto=share_data['share_proto'],
host=share_data['host']) host=share_data['host'])
@ -979,7 +979,7 @@ class API(base.Base):
return self.db.share_get(context, share['id']) 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: if share is None:
share = {'instance': {}} share = {'instance': {}}
@ -1793,9 +1793,9 @@ class API(base.Base):
raise exception.InvalidShare(reason=msg % payload) raise exception.InvalidShare(reason=msg % payload)
request_spec = self._get_request_spec_dict( request_spec = self._get_request_spec_dict(
context,
share, share,
share_type, share_type,
context,
availability_zone_id=service['availability_zone_id'], availability_zone_id=service['availability_zone_id'],
share_network_id=new_share_network_id) share_network_id=new_share_network_id)
@ -2528,8 +2528,8 @@ class API(base.Base):
else: else:
share_type = share_types.get_share_type( share_type = share_types.get_share_type(
context, share['instance']['share_type_id']) context, share['instance']['share_type_id'])
request_spec = self._get_request_spec_dict(share, share_type, request_spec = self._get_request_spec_dict(context, share,
context) share_type)
request_spec.update({'is_share_extend': True}) request_spec.update({'is_share_extend': True})
self.scheduler_rpcapi.extend_share(context, share['id'], new_size, self.scheduler_rpcapi.extend_share(context, share['id'], new_size,
reservations, request_spec) reservations, request_spec)
@ -2680,8 +2680,8 @@ class API(base.Base):
for share_instance in share_instances: for share_instance in share_instances:
share_type_id = share_instance['share_type_id'] share_type_id = share_instance['share_type_id']
share_type = share_types.get_share_type(context, share_type_id) share_type = share_types.get_share_type(context, share_type_id)
req_spec = self._get_request_spec_dict(share_instance, req_spec = self._get_request_spec_dict(context, share_instance,
share_type, context, share_type,
**kwargs) **kwargs)
shares_req_spec.append(req_spec) shares_req_spec.append(req_spec)

View File

@ -2,6 +2,7 @@
# or extra rules in policy file, it is strongly # or extra rules in policy file, it is strongly
# recommended to switch to new rules. # recommended to switch to new rules.
"context_is_admin": "role:admin" "context_is_admin": "role:admin"
"context_is_host_admin": "role:admin and project_id:%(project_id)s"
"admin_api": "is_admin:True" "admin_api": "is_admin:True"
"admin_or_owner": "is_admin:True or project_id:%(project_id)s" "admin_or_owner": "is_admin:True or project_id:%(project_id)s"
"default": "rule:admin_or_owner" "default": "rule:admin_or_owner"

View File

@ -5305,7 +5305,7 @@ class ShareAPITestCase(test.TestCase):
get_type_calls.append( get_type_calls.append(
mock.call(self.context, instance['share_type_id'])) mock.call(self.context, instance['share_type_id']))
get_request_spec_calls.append( 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( mock_get_type = self.mock_object(
share_types, 'get_share_type', share_types, 'get_share_type',