From 094f3c606dab9e9cc72a9d246a7fd6ad99b1b9a9 Mon Sep 17 00:00:00 2001 From: Kiran Pawar Date: Thu, 3 Oct 2024 13:20:51 +0000 Subject: [PATCH] update access_rule Change-Id: I5127bf64bfa39ed20e0dac58b6be31924c5faaba --- manila/api/v2/share_accesses.py | 36 +++++++++++++ manila/common/constants.py | 4 ++ manila/db/api.py | 5 ++ manila/db/sqlalchemy/api.py | 9 ++++ manila/share/access.py | 30 ++++++++--- manila/share/api.py | 24 +++++++++ manila/share/driver.py | 2 +- manila/share/drivers/cephfs/driver.py | 4 +- manila/share/drivers/container/driver.py | 2 +- manila/share/drivers/dell_emc/driver.py | 2 +- manila/share/drivers/ganesha/__init__.py | 6 +-- manila/share/drivers/generic.py | 5 +- manila/share/drivers/glusterfs/layout.py | 2 +- manila/share/drivers/hitachi/hnas/driver.py | 2 +- manila/share/drivers/hitachi/hsp/driver.py | 2 +- manila/share/drivers/hpe/hpe_3par_driver.py | 2 +- manila/share/drivers/huawei/base.py | 2 +- manila/share/drivers/huawei/huawei_nas.py | 6 +-- manila/share/drivers/huawei/v3/connection.py | 4 +- manila/share/drivers/ibm/gpfs.py | 2 +- manila/share/drivers/infinidat/infinibox.py | 2 +- manila/share/drivers/infortrend/driver.py | 2 +- .../drivers/infortrend/infortrend_nas.py | 2 +- .../drivers/inspur/as13000/as13000_nas.py | 2 +- .../drivers/inspur/instorage/instorage.py | 4 +- manila/share/drivers/lvm.py | 2 +- manila/share/drivers/macrosan/macrosan_nas.py | 2 +- manila/share/drivers/maprfs/maprfs_native.py | 2 +- .../dataontap/cluster_mode/drv_multi_svm.py | 4 +- .../dataontap/cluster_mode/drv_single_svm.py | 4 +- .../netapp/dataontap/cluster_mode/lib_base.py | 2 +- .../share/drivers/nexenta/ns4/nexenta_nas.py | 2 +- .../share/drivers/nexenta/ns5/nexenta_nas.py | 2 +- .../share/drivers/purestorage/flashblade.py | 1 + manila/share/drivers/qnap/qnap.py | 2 +- manila/share/drivers/quobyte/quobyte.py | 2 +- manila/share/drivers/tegile/tegile.py | 2 +- manila/share/drivers/veritas/veritas_isa.py | 2 +- .../drivers/windows/windows_smb_helper.py | 2 +- manila/share/drivers/zadara/zadara.py | 2 +- manila/share/drivers/zfsonlinux/driver.py | 2 +- manila/tests/api/v2/test_share_accesses.py | 37 ++++++++++++++ .../tests/share/drivers/cephfs/test_driver.py | 6 ++- .../share/drivers/container/test_driver.py | 2 +- manila/tests/share/drivers/dummy.py | 2 +- .../share/drivers/glusterfs/test_layout.py | 3 +- .../share/drivers/hitachi/hnas/test_driver.py | 21 ++++---- .../share/drivers/hitachi/hsp/test_driver.py | 16 +++--- manila/tests/share/drivers/ibm/test_gpfs.py | 4 ++ .../share/drivers/infinidat/test_infinidat.py | 10 ++-- .../inspur/as13000/test_as13000_nas.py | 4 +- .../inspur/instorage/test_instorage.py | 6 ++- .../drivers/macrosan/test_macrosan_nas.py | 8 +-- .../tests/share/drivers/maprfs/test_maprfs.py | 14 ++--- .../dataontap/cluster_mode/test_lib_base.py | 5 ++ .../drivers/nexenta/ns4/test_nexenta_nas.py | 11 ++-- .../drivers/nexenta/ns5/test_nexenta_nas.py | 6 +-- .../drivers/purestorage/test_flashblade.py | 2 +- manila/tests/share/drivers/qnap/test_qnap.py | 6 ++- .../share/drivers/quobyte/test_quobyte.py | 12 +++-- .../tests/share/drivers/tegile/test_tegile.py | 3 +- manila/tests/share/drivers/test_ganesha.py | 26 ++++++---- manila/tests/share/drivers/test_generic.py | 5 +- manila/tests/share/drivers/test_lvm.py | 1 + .../share/drivers/veritas/test_veritas_isa.py | 18 +++---- .../windows/test_windows_smb_helper.py | 10 ++-- .../share/drivers/zfsonlinux/test_driver.py | 2 +- manila/tests/share/test_access.py | 51 +++++++++++++++---- manila/tests/share/test_driver.py | 3 +- ...evel-for-access-rule-741f8fc3cc190701.yaml | 7 +++ 70 files changed, 352 insertions(+), 147 deletions(-) create mode 100644 releasenotes/notes/bug-2066871-allow-to-update-access-level-for-access-rule-741f8fc3cc190701.yaml diff --git a/manila/api/v2/share_accesses.py b/manila/api/v2/share_accesses.py index 2735cab068..539ef5f494 100644 --- a/manila/api/v2/share_accesses.py +++ b/manila/api/v2/share_accesses.py @@ -21,6 +21,7 @@ from manila.api.openstack import wsgi from manila.api.views import share_accesses as share_access_views +from manila.common import constants from manila import exception from manila.i18n import _ from manila import share @@ -76,6 +77,41 @@ def index(self, req): return self._view_builder.list_view(req, access_rules) + def update(self, req, id, body): + """Update access level of the given share access rule.""" + context = req.environ['manila.context'] + if not self.is_valid_body(body, 'update_access'): + raise webob.exc.HTTPBadRequest() + + access_data = body['update_access'] + share_access = self._get_share_access(context, id) + if share_access['access_type'] != 'ip': + msg = _("Invalid access_type. Only allowed to " + "update 'ip' access_type.") + raise webob.exc.HTTPBadRequest(explanation=msg) + + access_level = access_data.get('access_level', None) + if not access_level: + msg = _("Invalid input. Missing 'access_level' in " + "update request.") + raise webob.exc.HTTPBadRequest(explanation=msg) + + if access_level not in constants.ACCESS_LEVELS: + msg = _("Invalid or unsupported share access " + "level: %s.") % access_level + raise webob.exc.HTTPBadRequest(explanation=msg) + + if access_level == share_access.access_level: + return self._view_builder.view(req, share_access) + + share = self.share_api.get(context, share_access.share_id) + values = { + 'access_level': access_level, + } + access = self.share_api.update_access( + context, share, share_access, values) + return self._view_builder.view(req, access) + def create_resource(): return wsgi.Resource(ShareAccessesController()) diff --git a/manila/common/constants.py b/manila/common/constants.py index d74881dc18..e0dec9b225 100644 --- a/manila/common/constants.py +++ b/manila/common/constants.py @@ -52,8 +52,10 @@ # Access rule states ACCESS_STATE_QUEUED_TO_APPLY = 'queued_to_apply' ACCESS_STATE_QUEUED_TO_DENY = 'queued_to_deny' +ACCESS_STATE_QUEUED_TO_UPDATE = 'queued_to_update' ACCESS_STATE_APPLYING = 'applying' ACCESS_STATE_DENYING = 'denying' +ACCESS_STATE_UPDATING = 'updating' ACCESS_STATE_ACTIVE = 'active' ACCESS_STATE_ERROR = 'error' ACCESS_STATE_DELETED = 'deleted' @@ -82,8 +84,10 @@ ACCESS_RULES_STATES = ( ACCESS_STATE_QUEUED_TO_APPLY, ACCESS_STATE_QUEUED_TO_DENY, + ACCESS_STATE_QUEUED_TO_UPDATE, ACCESS_STATE_APPLYING, ACCESS_STATE_DENYING, + ACCESS_STATE_UPDATING, ACCESS_STATE_ACTIVE, ACCESS_STATE_ERROR, ACCESS_STATE_DELETED, diff --git a/manila/db/api.py b/manila/db/api.py index fea3795944..3e79f0e4ff 100644 --- a/manila/db/api.py +++ b/manila/db/api.py @@ -552,6 +552,11 @@ def share_access_create(context, values): return IMPL.share_access_create(context, values) +def share_access_update(context, access_id, values): + """Update access to share.""" + return IMPL.share_access_update(context, access_id, values) + + def share_access_get(context, access_id): """Get share access rule.""" return IMPL.share_access_get(context, access_id) diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index f06635e28c..0585e710e4 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -2804,6 +2804,15 @@ def share_access_create(context, values): return share_access_get(context, access_ref['id']) +@require_context +@context_manager.writer +def share_access_update(context, access_id, values): + access_ref = share_access_get(context, access_id) + access_ref.update(values) + access_ref.save(session=context.session) + return access_ref + + @require_context def share_instance_access_create(context, values, share_instance_id): values = ensure_model_dict_has_id(values) diff --git a/manila/share/access.py b/manila/share/access.py index 6711df96cc..112bd26fb6 100644 --- a/manila/share/access.py +++ b/manila/share/access.py @@ -289,7 +289,8 @@ def update_access_rules(self, context, share_instance_id, # Is there a sync in progress? If yes, ignore the incoming request. rule_filter = { 'state': (constants.ACCESS_STATE_APPLYING, - constants.ACCESS_STATE_DENYING), + constants.ACCESS_STATE_DENYING, + constants.ACCESS_STATE_UPDATING), } syncing_rules = self.get_and_update_share_instance_access_rules( context, filters=rule_filter, share_instance_id=share_instance_id) @@ -332,7 +333,8 @@ def _update_access_rules(self, context, share_instance_id, rules_to_be_removed_from_db = [] # Populate rules to send to the driver - (access_rules_to_be_on_share, add_rules, delete_rules) = ( + (access_rules_to_be_on_share, add_rules, + delete_rules, update_rules) = ( self._get_rules_to_send_to_driver(context, share_instance) ) @@ -343,11 +345,13 @@ def _update_access_rules(self, context, share_instance_id, add_rules = [] rules_to_be_removed_from_db = delete_rules delete_rules = [] + update_rules = [] try: driver_rule_updates = self._update_rules_through_share_driver( context, share_instance, access_rules_to_be_on_share, - add_rules, delete_rules, rules_to_be_removed_from_db, + add_rules, delete_rules, update_rules, + rules_to_be_removed_from_db, share_server) self.process_driver_rule_updates( @@ -356,6 +360,7 @@ def _update_access_rules(self, context, share_instance_id, # Update access rules that are still in 'applying' state conditionally_change = { constants.ACCESS_STATE_APPLYING: constants.ACCESS_STATE_ACTIVE, + constants.ACCESS_STATE_UPDATING: constants.ACCESS_STATE_ACTIVE } self.get_and_update_share_instance_access_rules( context, share_instance_id=share_instance_id, @@ -365,6 +370,7 @@ def _update_access_rules(self, context, share_instance_id, conditionally_change_rule_state = { constants.ACCESS_STATE_APPLYING: constants.ACCESS_STATE_ERROR, constants.ACCESS_STATE_DENYING: constants.ACCESS_STATE_ERROR, + constants.ACCESS_STATE_UPDATING: constants.ACCESS_STATE_ERROR, } self.get_and_update_share_instance_access_rules( context, share_instance_id=share_instance_id, @@ -399,6 +405,7 @@ def _update_access_rules(self, context, share_instance_id, def _update_rules_through_share_driver(self, context, share_instance, access_rules_to_be_on_share, add_rules, delete_rules, + update_rules, rules_to_be_removed_from_db, share_server): driver_rule_updates = {} @@ -407,6 +414,7 @@ def _update_rules_through_share_driver(self, context, share_instance, share_protocol == 'nfs'): add_rules = self._filter_ipv6_rules(add_rules) delete_rules = self._filter_ipv6_rules(delete_rules) + update_rules = self._filter_ipv6_rules(update_rules) access_rules_to_be_on_share = self._filter_ipv6_rules( access_rules_to_be_on_share) try: @@ -416,6 +424,7 @@ def _update_rules_through_share_driver(self, context, share_instance, access_rules_to_be_on_share, add_rules=add_rules, delete_rules=delete_rules, + update_rules=update_rules, share_server=share_server ) or {} except NotImplementedError: @@ -476,6 +485,7 @@ def process_driver_rule_updates(self, context, driver_rule_updates, conditional_state_updates = { constants.ACCESS_STATE_APPLYING: state, constants.ACCESS_STATE_DENYING: state, + constants.ACCESS_STATE_UPDATING: state, constants.ACCESS_STATE_ACTIVE: state, } else: @@ -513,10 +523,12 @@ def _filter_ipv6_rules(rules): def _get_rules_to_send_to_driver(self, context, share_instance): add_rules = [] delete_rules = [] + update_rules = [] access_filters = { 'state': (constants.ACCESS_STATE_APPLYING, constants.ACCESS_STATE_ACTIVE, - constants.ACCESS_STATE_DENYING), + constants.ACCESS_STATE_DENYING, + constants.ACCESS_STATE_UPDATING), } existing_rules_in_db = self.get_and_update_share_instance_access_rules( context, filters=access_filters, @@ -528,11 +540,14 @@ def _get_rules_to_send_to_driver(self, context, share_instance): add_rules.append(rule) elif rule['state'] == constants.ACCESS_STATE_DENYING: delete_rules.append(rule) + elif rule['state'] == constants.ACCESS_STATE_UPDATING: + update_rules.append(rule) delete_rule_ids = [r['id'] for r in delete_rules] access_rules_to_be_on_share = [ r for r in existing_rules_in_db if r['id'] not in delete_rule_ids ] - return access_rules_to_be_on_share, add_rules, delete_rules + return (access_rules_to_be_on_share, add_rules, + delete_rules, update_rules) def _check_needs_refresh(self, context, share_instance_id): rules_to_apply_or_deny = ( @@ -579,13 +594,16 @@ def _update_and_get_unsynced_access_rules_from_db(self, context, share_instance_id): rule_filter = { 'state': (constants.ACCESS_STATE_QUEUED_TO_APPLY, - constants.ACCESS_STATE_QUEUED_TO_DENY), + constants.ACCESS_STATE_QUEUED_TO_DENY, + constants.ACCESS_STATE_QUEUED_TO_UPDATE), } conditionally_change = { constants.ACCESS_STATE_QUEUED_TO_APPLY: constants.ACCESS_STATE_APPLYING, constants.ACCESS_STATE_QUEUED_TO_DENY: constants.ACCESS_STATE_DENYING, + constants.ACCESS_STATE_QUEUED_TO_UPDATE: + constants.ACCESS_STATE_UPDATING, } rules_to_apply_or_deny = ( self.get_and_update_share_instance_access_rules( diff --git a/manila/share/api.py b/manila/share/api.py index 92e2d2fee8..03b09869f4 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -2325,6 +2325,30 @@ def _conditionally_transition_share_instance_access_rules_status( context, conditionally_change=conditionally_change, share_instance_id=share_instance['id']) + def update_access(self, ctx, share, access, values): + + if self._any_invalid_share_instance(share, allow_on_error_state=True): + msg = _("Access rules cannot be updated while the share, " + "any of its replicas or migration copies lacks a valid " + "host or is in an invalid state.") + raise exception.InvalidShare(message=msg) + + access = self.db.share_access_update(ctx, access['id'], values) + for share_instance in share.instances: + self.update_access_to_instance(ctx, share_instance, access) + + return access + + def update_access_to_instance(self, context, share_instance, access): + self._conditionally_transition_share_instance_access_rules_status( + context, share_instance) + updates = {'state': constants.ACCESS_STATE_QUEUED_TO_UPDATE} + self.access_helper.get_and_update_share_instance_access_rule( + context, access['id'], updates=updates, + share_instance_id=share_instance['id']) + + self.share_rpcapi.update_access(context, share_instance) + def deny_access(self, ctx, share, access, allow_on_error_state=False): """Deny access to share.""" diff --git a/manila/share/driver.py b/manila/share/driver.py index 3a106e1af4..f6120778b1 100644 --- a/manila/share/driver.py +++ b/manila/share/driver.py @@ -801,7 +801,7 @@ def deny_access(self, context, share, access, share_server=None): raise NotImplementedError() def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules for given share. ``access_rules`` contains all access_rules that need to be on the diff --git a/manila/share/drivers/cephfs/driver.py b/manila/share/drivers/cephfs/driver.py index 0e24407e2e..4e9bf15878 100644 --- a/manila/share/drivers/cephfs/driver.py +++ b/manila/share/drivers/cephfs/driver.py @@ -535,7 +535,7 @@ def delete_share(self, context, share, share_server=None): rados_command(self.rados_client, "fs subvolume rm", argdict) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_access, share_server=None): return self.protocol_helper.update_access( context, share, access_rules, add_rules, delete_rules, share_server=share_server) @@ -895,7 +895,7 @@ def _deny_access(self, context, share, access, share_server=None): rados_command(self.rados_client, "fs subvolume evict", argdict) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): access_updates = {} argdict = { diff --git a/manila/share/drivers/container/driver.py b/manila/share/drivers/container/driver.py index efdfa963e6..b720a56217 100644 --- a/manila/share/drivers/container/driver.py +++ b/manila/share/drivers/container/driver.py @@ -189,7 +189,7 @@ def ensure_share(self, context, share, share_server=None): pass def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): server_id = self._get_container_name(share_server["id"]) share_name = self._get_share_name(share) LOG.debug("Updating access to share %(share)s at " diff --git a/manila/share/drivers/dell_emc/driver.py b/manila/share/drivers/dell_emc/driver.py index f786eadc94..38bd5167b8 100644 --- a/manila/share/drivers/dell_emc/driver.py +++ b/manila/share/drivers/dell_emc/driver.py @@ -256,7 +256,7 @@ def deny_access(self, context, share, access, share_server=None): self.plugin.deny_access(context, share, access, share_server) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access to the share.""" self.plugin.update_access(context, share, access_rules, add_rules, delete_rules, share_server) diff --git a/manila/share/drivers/ganesha/__init__.py b/manila/share/drivers/ganesha/__init__.py index b079472e79..bfc302eca9 100644 --- a/manila/share/drivers/ganesha/__init__.py +++ b/manila/share/drivers/ganesha/__init__.py @@ -50,7 +50,7 @@ def init_helper(self): @abc.abstractmethod def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules of share.""" @@ -155,7 +155,7 @@ def _deny_access(self, base_path, share, access): self.ganesha.remove_export("%s--%s" % (share['name'], access['id'])) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules of share.""" rule_state_map = {} if not (add_rules or delete_rules): @@ -226,7 +226,7 @@ def _get_export_pseudo_path(self, share): raise NotImplementedError() def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules of share. Creates an export per share. Modifies access rules of shares by diff --git a/manila/share/drivers/generic.py b/manila/share/drivers/generic.py index ff8ff83caf..eaacf2c6bc 100644 --- a/manila/share/drivers/generic.py +++ b/manila/share/drivers/generic.py @@ -851,7 +851,7 @@ def ensure_share(self, context, share, share_server=None): @ensure_server def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules for given share. This driver has two different behaviors according to parameters: @@ -877,7 +877,8 @@ def update_access(self, context, share, access_rules, add_rules, self._get_helper(share).update_access(share_server['backend_details'], share['name'], access_rules, add_rules=add_rules, - delete_rules=delete_rules) + delete_rules=delete_rules, + update_rules=update_rules) def _get_helper(self, share): helper = self._helpers.get(share['share_proto']) diff --git a/manila/share/drivers/glusterfs/layout.py b/manila/share/drivers/glusterfs/layout.py index f340d5dc5f..70fe29723a 100644 --- a/manila/share/drivers/glusterfs/layout.py +++ b/manila/share/drivers/glusterfs/layout.py @@ -115,7 +115,7 @@ def validator(rule): return validator def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules for given share. Driver supports 2 different cases in this method: diff --git a/manila/share/drivers/hitachi/hnas/driver.py b/manila/share/drivers/hitachi/hnas/driver.py index 193fc7d61d..f12e74c819 100644 --- a/manila/share/drivers/hitachi/hnas/driver.py +++ b/manila/share/drivers/hitachi/hnas/driver.py @@ -157,7 +157,7 @@ def __init__(self, *args, **kwargs): job_timeout) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules for given share. :param context: The `context.RequestContext` object for the request diff --git a/manila/share/drivers/hitachi/hsp/driver.py b/manila/share/drivers/hitachi/hsp/driver.py index bd227b7f0c..a96f7574d6 100644 --- a/manila/share/drivers/hitachi/hsp/driver.py +++ b/manila/share/drivers/hitachi/hsp/driver.py @@ -167,7 +167,7 @@ def delete_share(self, context, share, share_server=None): {'shr': share['id']}) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): LOG.debug("Updating access rules for share: %(shr)s.", {'shr': share['id']}) diff --git a/manila/share/drivers/hpe/hpe_3par_driver.py b/manila/share/drivers/hpe/hpe_3par_driver.py index 46b2a5533a..999ff1ffd6 100644 --- a/manila/share/drivers/hpe/hpe_3par_driver.py +++ b/manila/share/drivers/hpe/hpe_3par_driver.py @@ -552,7 +552,7 @@ def ensure_share(self, context, share, share_server=None): pass def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access to the share.""" extra_specs = None if 'NFS' == share['share_proto']: # Avoiding DB call otherwise diff --git a/manila/share/drivers/huawei/base.py b/manila/share/drivers/huawei/base.py index 79fac2a71b..2c5342bd8c 100644 --- a/manila/share/drivers/huawei/base.py +++ b/manila/share/drivers/huawei/base.py @@ -54,7 +54,7 @@ def ensure_share(self, share, share_server=None): @abc.abstractmethod def update_access(self, share, access_rules, add_rules, - delete_rules, share_server): + delete_rules, update_rules, share_server): """Update access rules list.""" @abc.abstractmethod diff --git a/manila/share/drivers/huawei/huawei_nas.py b/manila/share/drivers/huawei/huawei_nas.py index 9218fac7f8..70bae8d8a3 100644 --- a/manila/share/drivers/huawei/huawei_nas.py +++ b/manila/share/drivers/huawei/huawei_nas.py @@ -163,11 +163,11 @@ def deny_access(self, context, share, access, share_server=None): self.plugin.deny_access(share, access, share_server) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules list.""" LOG.debug("Update access.") - self.plugin.update_access(share, access_rules, - add_rules, delete_rules, share_server) + self.plugin.update_access(share, access_rules, add_rules, + delete_rules, update_rules, share_server) def get_pool(self, share): """Return pool name where the share resides on.""" diff --git a/manila/share/drivers/huawei/v3/connection.py b/manila/share/drivers/huawei/v3/connection.py index fb3c7b1a8e..6cf9ee77d1 100644 --- a/manila/share/drivers/huawei/v3/connection.py +++ b/manila/share/drivers/huawei/v3/connection.py @@ -809,7 +809,7 @@ def clear_access(self, share, share_server=None): share_proto) def update_access(self, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules list.""" if not (add_rules or delete_rules): self.clear_access(share, share_server) @@ -1789,7 +1789,7 @@ def promote_replica(self, context, replica_list, replica, access_rules, cleared_old_active_access = True try: - self.update_access(replica, access_rules, [], [], share_server) + self.update_access(replica, access_rules, [], [], [], share_server) except Exception: LOG.warning('Failed to set access rules to ' 'new active replica %s.', diff --git a/manila/share/drivers/ibm/gpfs.py b/manila/share/drivers/ibm/gpfs.py index 53f20063b4..8c34acf6cc 100644 --- a/manila/share/drivers/ibm/gpfs.py +++ b/manila/share/drivers/ibm/gpfs.py @@ -511,7 +511,7 @@ def ensure_share(self, ctx, share, share_server=None): """Ensure that storage are mounted and exported.""" def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules for given share.""" helper = self._get_helper(share) location = self._get_share_path(share) diff --git a/manila/share/drivers/infinidat/infinibox.py b/manila/share/drivers/infinidat/infinibox.py index 81eed719ea..d0afa4c5fb 100644 --- a/manila/share/drivers/infinidat/infinibox.py +++ b/manila/share/drivers/infinidat/infinibox.py @@ -543,7 +543,7 @@ def get_backend_info(self, context): } def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): # As the Infinibox API can bulk update export access rules, we will try # to use the access_rules list self._verify_share_protocol(share) diff --git a/manila/share/drivers/infortrend/driver.py b/manila/share/drivers/infortrend/driver.py index cc25b4ceb6..259113c648 100644 --- a/manila/share/drivers/infortrend/driver.py +++ b/manila/share/drivers/infortrend/driver.py @@ -132,7 +132,7 @@ def _update_share_stats(self): super(InfortrendNASDriver, self)._update_share_stats(data) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules for given share. :param context: Current context diff --git a/manila/share/drivers/infortrend/infortrend_nas.py b/manila/share/drivers/infortrend/infortrend_nas.py index 5b3ce169cf..4f98e8c3fa 100644 --- a/manila/share/drivers/infortrend/infortrend_nas.py +++ b/manila/share/drivers/infortrend/infortrend_nas.py @@ -348,7 +348,7 @@ def _check_share_exist(self, pool_name, share_name): return any(subfolder['name'] == share_name for subfolder in subfolders) def update_access(self, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): self._evict_unauthorized_clients(share, access_rules, share_server) access_dict = {} for access in access_rules: diff --git a/manila/share/drivers/inspur/as13000/as13000_nas.py b/manila/share/drivers/inspur/as13000/as13000_nas.py index e135503b78..fdea65709e 100644 --- a/manila/share/drivers/inspur/as13000/as13000_nas.py +++ b/manila/share/drivers/inspur/as13000/as13000_nas.py @@ -472,7 +472,7 @@ def transfer_rule_to_client(proto, rule): @inspur_driver_debug_trace def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """update access of share""" pool, share_name, _, proto = self._get_share_instance_pnsp(share) share_path = self._generate_share_path(pool, share_name) diff --git a/manila/share/drivers/inspur/instorage/instorage.py b/manila/share/drivers/inspur/instorage/instorage.py index 3a1918063e..4f9d87a143 100644 --- a/manila/share/drivers/inspur/instorage/instorage.py +++ b/manila/share/drivers/inspur/instorage/instorage.py @@ -222,7 +222,7 @@ def ensure_share(self, context, share, share_server=None): return self.assistant.get_export_locations(share_name, share_proto) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update the share instance's access rule.""" share_name = self.generate_share_name(share) share_proto = share['share_proto'] @@ -608,7 +608,7 @@ def check_access_type(access_type, *rules): return False def update_access(self, share_name, share_proto, - access_rules, add_rules, delete_rules): + access_rules, add_rules, delete_rules, update_rules): if share_proto == 'CIFS': if self.check_access_type('user', access_rules, add_rules, delete_rules): diff --git a/manila/share/drivers/lvm.py b/manila/share/drivers/lvm.py index 27bc602436..f69fd5f42a 100644 --- a/manila/share/drivers/lvm.py +++ b/manila/share/drivers/lvm.py @@ -356,7 +356,7 @@ def _delete_share(self, ctx, share): LOG.warning(exc) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules for given share. This driver has two different behaviors according to parameters: diff --git a/manila/share/drivers/macrosan/macrosan_nas.py b/manila/share/drivers/macrosan/macrosan_nas.py index 7b36849e7f..c59340eb6a 100644 --- a/manila/share/drivers/macrosan/macrosan_nas.py +++ b/manila/share/drivers/macrosan/macrosan_nas.py @@ -146,7 +146,7 @@ def ensure_share(self, context, share, share_server=None): @debug_trace def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules list. :param context: Current context diff --git a/manila/share/drivers/maprfs/maprfs_native.py b/manila/share/drivers/maprfs/maprfs_native.py index 7c67b3b23a..274ec43141 100644 --- a/manila/share/drivers/maprfs/maprfs_native.py +++ b/manila/share/drivers/maprfs/maprfs_native.py @@ -282,7 +282,7 @@ def delete_snapshot(self, context, snapshot, share_server=None): raise exception.MapRFSException(msg=msg) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules for given share.""" for access in access_rules: if access['access_type'].lower() != 'user': diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py index a418f70a3f..e3714af7a0 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py @@ -111,9 +111,9 @@ def unmanage_snapshot_with_server(self, snapshot, share_server=None): self.library.unmanage_snapshot(snapshot, share_server=share_server) def update_access(self, context, share, access_rules, add_rules, - delete_rules, **kwargs): + delete_rules, update_rules, **kwargs): self.library.update_access(context, share, access_rules, add_rules, - delete_rules, **kwargs) + delete_rules, update_rules, **kwargs) def _update_share_stats(self, data=None): data = self.library.get_share_stats( diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py index b7f7fe7f8e..b3498a4d2a 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py @@ -99,9 +99,9 @@ def unmanage_snapshot_with_server(self, snapshot, share_server=None): raise NotImplementedError def update_access(self, context, share, access_rules, add_rules, - delete_rules, **kwargs): + delete_rules, update_rules, **kwargs): self.library.update_access(context, share, access_rules, add_rules, - delete_rules, **kwargs) + delete_rules, update_rules, **kwargs) def _update_share_stats(self, data=None): data = self.library.get_share_stats( diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py index a4804ab603..8faf425599 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -2470,7 +2470,7 @@ def _update_access(self, helper, share, share_name, access_rules): @na_utils.trace def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Updates access rules for a share.""" # NOTE(felipe_rodrigues): do not add export rules to a non-active diff --git a/manila/share/drivers/nexenta/ns4/nexenta_nas.py b/manila/share/drivers/nexenta/ns4/nexenta_nas.py index 650489e15e..c3b4637dde 100644 --- a/manila/share/drivers/nexenta/ns4/nexenta_nas.py +++ b/manila/share/drivers/nexenta/ns4/nexenta_nas.py @@ -108,7 +108,7 @@ def delete_snapshot(self, context, snapshot, share_server=None): self.helper.delete_snapshot(snapshot['share_name'], snapshot['name']) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules for given share. :param context: The `context.RequestContext` object for the request diff --git a/manila/share/drivers/nexenta/ns5/nexenta_nas.py b/manila/share/drivers/nexenta/ns5/nexenta_nas.py index fcb05aaaf0..ce02cf6d9e 100644 --- a/manila/share/drivers/nexenta/ns5/nexenta_nas.py +++ b/manila/share/drivers/nexenta/ns5/nexenta_nas.py @@ -392,7 +392,7 @@ def manage_existing(self, share, driver_options): }]} def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules for given share. Using access_rules list for both adding and deleting rules. diff --git a/manila/share/drivers/purestorage/flashblade.py b/manila/share/drivers/purestorage/flashblade.py index a615551595..7ddfdae201 100644 --- a/manila/share/drivers/purestorage/flashblade.py +++ b/manila/share/drivers/purestorage/flashblade.py @@ -448,6 +448,7 @@ def update_access( access_rules, add_rules, delete_rules, + update_rules, share_server=None, ): """Update access of share""" diff --git a/manila/share/drivers/qnap/qnap.py b/manila/share/drivers/qnap/qnap.py index 20c8626613..b96c0181b2 100644 --- a/manila/share/drivers/qnap/qnap.py +++ b/manila/share/drivers/qnap/qnap.py @@ -627,7 +627,7 @@ def _get_vol_host(self, host_list, vol_name_timestamp): @utils.synchronized('qnap-update_access') def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): if not (add_rules or delete_rules): volName = self.private_storage.get(share['id'], 'volName') LOG.debug('volName: %s', volName) diff --git a/manila/share/drivers/quobyte/quobyte.py b/manila/share/drivers/quobyte/quobyte.py index e2c2ec071a..f3b7d88482 100644 --- a/manila/share/drivers/quobyte/quobyte.py +++ b/manila/share/drivers/quobyte/quobyte.py @@ -365,7 +365,7 @@ def shrink_share(self, shrink_share, shrink_size, share_server=None): self._resize_share(share=shrink_share, new_size=shrink_size) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules for given share. Two different cases are supported in here: diff --git a/manila/share/drivers/tegile/tegile.py b/manila/share/drivers/tegile/tegile.py index cacb08e4fe..03516a9b9f 100644 --- a/manila/share/drivers/tegile/tegile.py +++ b/manila/share/drivers/tegile/tegile.py @@ -398,7 +398,7 @@ def _check_share_access(self, share_proto, access_type): @debugger def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): if not (add_rules or delete_rules): # Recovery mode pool, project, share_name = ( diff --git a/manila/share/drivers/veritas/veritas_isa.py b/manila/share/drivers/veritas/veritas_isa.py index 8483235dff..8041b3cbb5 100644 --- a/manila/share/drivers/veritas/veritas_isa.py +++ b/manila/share/drivers/veritas/veritas_isa.py @@ -421,7 +421,7 @@ def _deny_access(self, context, share, access, share_server=None): json.dumps(data2), 'DELETE') def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access to the share.""" if (add_rules or delete_rules): diff --git a/manila/share/drivers/windows/windows_smb_helper.py b/manila/share/drivers/windows/windows_smb_helper.py index b96830f2fe..be2d47c12d 100644 --- a/manila/share/drivers/windows/windows_smb_helper.py +++ b/manila/share/drivers/windows/windows_smb_helper.py @@ -179,7 +179,7 @@ def _revoke_share_access(self, server, share_name, access_to): 'share_name': share_name}) def update_access(self, server, share_name, access_rules, add_rules, - delete_rules): + delete_rules, update_rules): self.validate_access_rules( access_rules + add_rules, self._SUPPORTED_ACCESS_TYPES, diff --git a/manila/share/drivers/zadara/zadara.py b/manila/share/drivers/zadara/zadara.py index 37ab6015b9..66c62bcffb 100644 --- a/manila/share/drivers/zadara/zadara.py +++ b/manila/share/drivers/zadara/zadara.py @@ -384,7 +384,7 @@ def _deny_access(self, context, share, access, share_server=None): vpsa_srv=vpsa_srv) def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): access_updates = {} if not (add_rules or delete_rules): # add_rules and delete_rules can be empty lists, in cases diff --git a/manila/share/drivers/zfsonlinux/driver.py b/manila/share/drivers/zfsonlinux/driver.py index 799f70ace9..5bcef88317 100644 --- a/manila/share/drivers/zfsonlinux/driver.py +++ b/manila/share/drivers/zfsonlinux/driver.py @@ -704,7 +704,7 @@ def shrink_share(self, share, new_size, share_server=None): @ensure_share_server_not_provided def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Updates access rules for given share.""" dataset_name = self._get_dataset_name(share) executor = self._get_shell_executor_by_host(share['host']) diff --git a/manila/tests/api/v2/test_share_accesses.py b/manila/tests/api/v2/test_share_accesses.py index ab0586a9ee..a4778e1f92 100644 --- a/manila/tests/api/v2/test_share_accesses.py +++ b/manila/tests/api/v2/test_share_accesses.py @@ -15,6 +15,7 @@ from unittest import mock +import copy import ddt from webob import exc @@ -164,3 +165,39 @@ def test_show_with_unsupported_version(self, version): self.controller.show, self._get_show_request(version=version), self.access['id']) + + def _get_update_request(self, access_id=None): + access_id = access_id or self.access['id'] + req = fakes.HTTPRequest.blank( + '/v2/share-access-rules/%s' % access_id, version="2.88", + experimental=True) + return req + + def test_update_metadata(self): + update_share_access = copy.deepcopy(self.access) + update_share_access.update({'access_level': 'ro'}) + self.mock_object( + self.controller.share_api, 'update_access', + mock.Mock(return_value=update_share_access)) + + body = {'access': {'access_level': 'ro'}} + url = self._get_update_request() + ret = self.controller.update(url, self.access['id'], body=body) + self.assertEqual(update_share_access['access_level'], + ret['access']['access_level']) + + def test_update_metadata_invalid_access_level(self): + body = {'access': {'access_level': 'fake_access'}} + self.assertRaises( + exc.HTTPBadRequest, + self.controller.update, + self._get_update_request(), self.access['id'], + body=body) + + def test_update_metadata_invalid_update_request(self): + body = {'access': {'access_key': 'xxxx'}} + self.assertRaises( + exc.HTTPBadRequest, + self.controller.update, + self._get_update_request(), self.access['id'], + body=body) diff --git a/manila/tests/share/drivers/cephfs/test_driver.py b/manila/tests/share/drivers/cephfs/test_driver.py index d9ba75218d..f45be0058c 100644 --- a/manila/tests/share/drivers/cephfs/test_driver.py +++ b/manila/tests/share/drivers/cephfs/test_driver.py @@ -226,10 +226,11 @@ def test_update_access(self): } add_rules = access_rules = [alice, ] delete_rules = [] + update_rules = [] self._driver.update_access( self._context, self._share, access_rules, add_rules, delete_rules, - None) + update_rules, None) self._driver.protocol_helper.update_access.assert_called_once_with( self._context, self._share, access_rules, add_rules, delete_rules, @@ -833,6 +834,7 @@ def test_update_access_add_rm(self): self._share, access_rules=[alice, manila, admin, dabo], add_rules=[alice, manila, admin, dabo], + update_rules=[], delete_rules=[bob]) expected_access_updates = { @@ -901,7 +903,7 @@ def test_update_access_all(self): access_updates = self._native_protocol_helper.update_access( self._context, self._share, access_rules=[alice], add_rules=[], - delete_rules=[]) + delete_rules=[], update_rules=[]) self.assertEqual( {'accessid1': {'access_key': 'abc123'}}, access_updates) diff --git a/manila/tests/share/drivers/container/test_driver.py b/manila/tests/share/drivers/container/test_driver.py index b162fc9a2f..06044b167a 100644 --- a/manila/tests/share/drivers/container/test_driver.py +++ b/manila/tests/share/drivers/container/test_driver.py @@ -265,7 +265,7 @@ def test_update_access_access_rules_ok(self): self._driver.update_access(self._context, self.share, [{'access_level': const.ACCESS_LEVEL_RW}], - [], [], {"id": "fake"}) + [], [], [], {"id": "fake"}) helper.update_access.assert_called_with('manila_fake', fake_share_name, [{'access_level': 'rw'}], diff --git a/manila/tests/share/drivers/dummy.py b/manila/tests/share/drivers/dummy.py index 904f894f16..a3c5a4590f 100644 --- a/manila/tests/share/drivers/dummy.py +++ b/manila/tests/share/drivers/dummy.py @@ -319,7 +319,7 @@ def ensure_share(self, context, share, share_server=None): @slow_me_down def update_access(self, context, share, access_rules, add_rules, - delete_rules, share_server=None): + delete_rules, update_rules, share_server=None): """Update access rules for given share.""" for rule in add_rules + access_rules: share_proto = share["share_proto"].lower() diff --git a/manila/tests/share/drivers/glusterfs/test_layout.py b/manila/tests/share/drivers/glusterfs/test_layout.py index a82b830ca0..5ec6b16b9a 100644 --- a/manila/tests/share/drivers/glusterfs/test_layout.py +++ b/manila/tests/share/drivers/glusterfs/test_layout.py @@ -159,7 +159,8 @@ def test_update_access(self, inset, outset, recovery): ] for r in rs ] for rs in (inset, outset)) - _driver.update_access(self.fake_context, self.fake_share, *in_rules) + _driver.update_access( + self.fake_context, self.fake_share, *in_rules, []) _layout._share_manager.assert_called_once_with(self.fake_share) _driver._update_access_via_manager.assert_called_once_with( diff --git a/manila/tests/share/drivers/hitachi/hnas/test_driver.py b/manila/tests/share/drivers/hitachi/hnas/test_driver.py index e9d627a984..7819f99d30 100644 --- a/manila/tests/share/drivers/hitachi/hnas/test_driver.py +++ b/manila/tests/share/drivers/hitachi/hnas/test_driver.py @@ -280,7 +280,8 @@ def test_update_access_nfs(self, empty_rules): self.mock_object(ssh.HNASSSHBackend, "update_nfs_access_rule", mock.Mock()) - self._driver.update_access('context', share_nfs, access_list, [], []) + self._driver.update_access('context', share_nfs, access_list, + [], [], []) ssh.HNASSSHBackend.update_nfs_access_rule.assert_called_once_with( access_list_updated, share_id=share_nfs['id']) @@ -301,7 +302,7 @@ def test_update_access_ip_exception(self): self.assertRaises(exception.InvalidShareAccess, self._driver.update_access, 'context', share_nfs, - access_list, [], []) + access_list, [], [], []) def test_update_access_not_found_exception(self): access1 = { @@ -321,7 +322,8 @@ def test_update_access_not_found_exception(self): self.assertRaises(exception.ShareResourceNotFound, self._driver.update_access, 'context', share_nfs, - access_list, add_rules=[], delete_rules=[]) + access_list, add_rules=[], delete_rules=[], + update_rules=[]) @ddt.data([access_cifs_rw, 'acr'], [access_cifs_ro, 'ar']) @ddt.unpack @@ -331,7 +333,7 @@ def test_allow_access_cifs(self, access_cifs, permission): self.mock_object(ssh.HNASSSHBackend, 'cifs_allow_access') self._driver.update_access('context', share_cifs, [], - access_list_allow, []) + access_list_allow, [], []) ssh.HNASSSHBackend.cifs_allow_access.assert_called_once_with( share_cifs['id'], 'fake_user', permission, is_snapshot=False) @@ -349,7 +351,7 @@ def test_allow_access_cifs_invalid_type(self): self.assertRaises(exception.InvalidShareAccess, self._driver.update_access, 'context', share_cifs, - [], access_list_allow, []) + [], access_list_allow, [], []) def test_deny_access_cifs(self): access_list_deny = [access_cifs_rw] @@ -357,7 +359,7 @@ def test_deny_access_cifs(self): self.mock_object(ssh.HNASSSHBackend, 'cifs_deny_access') self._driver.update_access('context', share_cifs, [], [], - access_list_deny) + access_list_deny, []) ssh.HNASSSHBackend.cifs_deny_access.assert_called_once_with( share_cifs['id'], 'fake_user', is_snapshot=False) @@ -376,14 +378,14 @@ def test_deny_access_cifs_unsupported_type(self): self.mock_object(ssh.HNASSSHBackend, 'cifs_deny_access') self._driver.update_access('context', share_cifs, [], [], - access_list_deny) + access_list_deny, []) self.assertTrue(self.mock_log.warning.called) def test_update_access_invalid_share_protocol(self): self.mock_object(self._driver, '_ensure_share') ex = self.assertRaises(exception.ShareBackendException, self._driver.update_access, 'context', - invalid_share, [], [], []) + invalid_share, [], [], [], []) self.assertEqual(invalid_protocol_msg, ex.msg) def test_update_access_cifs_recovery_mode(self): @@ -395,7 +397,8 @@ def test_update_access_cifs_recovery_mode(self): self.mock_object(ssh.HNASSSHBackend, 'cifs_deny_access') self.mock_object(ssh.HNASSSHBackend, 'cifs_allow_access') - self._driver.update_access('context', share_cifs, access_list, [], []) + self._driver.update_access('context', share_cifs, access_list, + [], [], []) ssh.HNASSSHBackend.list_cifs_permissions.assert_called_once_with( share_cifs['id']) diff --git a/manila/tests/share/drivers/hitachi/hsp/test_driver.py b/manila/tests/share/drivers/hitachi/hsp/test_driver.py index 1df887ab18..c1f5f27335 100644 --- a/manila/tests/share/drivers/hitachi/hsp/test_driver.py +++ b/manila/tests/share/drivers/hitachi/hsp/test_driver.py @@ -81,7 +81,7 @@ def test_update_access_add(self, add_rule): side_effect=add_rule)) self._driver.update_access('context', self.fake_share_instance, [], - access_list, []) + access_list, [], []) self.assertTrue(self.mock_log.debug.called) @@ -113,7 +113,7 @@ def test_update_access_add_exception(self): self.assertRaises(exception.HSPBackendException, self._driver.update_access, 'context', - self.fake_share_instance, [], access_list, []) + self.fake_share_instance, [], access_list, [], []) rest.HSPRestBackend.get_file_system.assert_called_once_with( self.fake_share_instance['id']) @@ -147,7 +147,7 @@ def test_update_access_recovery(self): self.mock_object(rest.HSPRestBackend, "add_access_rule") self._driver.update_access('context', self.fake_share_instance, - access_list, [], []) + access_list, [], [], []) self.assertTrue(self.mock_log.debug.called) @@ -191,7 +191,7 @@ def test_update_access_delete(self, delete_rule): mock.Mock(return_value=fakes.hsp_rules)) self._driver.update_access('context', self.fake_share_instance, [], [], - delete_rules) + delete_rules, []) self.assertTrue(self.mock_log.debug.called) @@ -231,7 +231,7 @@ def test_update_access_delete_exception(self): self.assertRaises(exception.HSPBackendException, self._driver.update_access, 'context', - self.fake_share_instance, [], [], delete_rules) + self.fake_share_instance, [], [], delete_rules, []) self.assertTrue(self.mock_log.debug.called) @@ -262,9 +262,9 @@ def test_update_access_ip_exception(self, is_recovery): mock.Mock(return_value=fakes.hsp_rules)) if is_recovery: - access_args = [access_list, [], []] + access_args = [access_list, [], [], []] else: - access_args = [[], access_list, []] + access_args = [[], access_list, [], []] self.assertRaises(exception.InvalidShareAccess, self._driver.update_access, 'context', @@ -287,7 +287,7 @@ def test_update_access_not_found_exception(self): self.assertRaises(exception.ShareResourceNotFound, self._driver.update_access, 'context', - self.fake_share_instance, access_list, [], []) + self.fake_share_instance, access_list, [], [], []) rest.HSPRestBackend.get_file_system.assert_called_once_with( self.fake_share_instance['id']) diff --git a/manila/tests/share/drivers/ibm/test_gpfs.py b/manila/tests/share/drivers/ibm/test_gpfs.py index 57fdd1a36d..2597ce2672 100644 --- a/manila/tests/share/drivers/ibm/test_gpfs.py +++ b/manila/tests/share/drivers/ibm/test_gpfs.py @@ -455,6 +455,7 @@ def test_update_access_allow(self): ["ignored"], [self.access], [], + [], share_server=None) self._helper_fake.allow_access.assert_called_once_with( @@ -473,6 +474,7 @@ def test_update_access_deny(self): ["ignored"], [], [self.access], + [], share_server=None) self._helper_fake.deny_access.assert_called_once_with( @@ -495,6 +497,7 @@ def test_update_access_both(self): ["ignore"], [access_1], [access_2], + [], share_server=None) self.assertFalse(self._helper_fake.resync_access.called) @@ -519,6 +522,7 @@ def test_update_access_resync(self): [access_1, access_2], [], [], + [], share_server=None) self._helper_fake.resync_access.assert_called_once_with( diff --git a/manila/tests/share/drivers/infinidat/test_infinidat.py b/manila/tests/share/drivers/infinidat/test_infinidat.py index b0408a3cc4..3049389a2a 100644 --- a/manila/tests/share/drivers/infinidat/test_infinidat.py +++ b/manila/tests/share/drivers/infinidat/test_infinidat.py @@ -724,7 +724,7 @@ def test_update_access(self): {'access_level': constants.ACCESS_LEVEL_RO, 'access_to': '5.6.7.8/28', 'access_type': 'ip'}] - self.driver.update_access(None, test_share, access_rules, [], []) + self.driver.update_access(None, test_share, access_rules, [], [], []) permissions = self._mock_filesystem.get_exports()[0].get_permissions() # now we are supposed to have three permissions: @@ -763,7 +763,7 @@ def test_update_access_share_doesnt_exist(self): 'access_type': 'ip'}] self.assertRaises(exception.ShareResourceNotFound, self.driver.update_access, None, test_share, - access_rules, [], []) + access_rules, [], [], []) def test_update_access_api_fail(self): self._mock_filesystem.get_exports.side_effect = self._raise_infinisdk @@ -779,7 +779,7 @@ def test_update_access_api_fail(self): 'access_type': 'ip'}] self.assertRaises(exception.ShareBackendException, self.driver.update_access, None, test_share, - access_rules, [], []) + access_rules, [], [], []) def test_update_access_fails_non_ip_access_type(self): access_rules = [ @@ -788,7 +788,7 @@ def test_update_access_fails_non_ip_access_type(self): 'access_type': 'user'}] self.assertRaises(exception.InvalidShareAccess, self.driver.update_access, None, test_share, - access_rules, [], []) + access_rules, [], [], []) def test_update_access_fails_invalid_ip(self): access_rules = [ @@ -797,7 +797,7 @@ def test_update_access_fails_invalid_ip(self): 'access_type': 'ip'}] self.assertRaises(ValueError, self.driver.update_access, None, test_share, - access_rules, [], []) + access_rules, [], [], []) def test_snapshot_update_access(self): access_rules = [ diff --git a/manila/tests/share/drivers/inspur/as13000/test_as13000_nas.py b/manila/tests/share/drivers/inspur/as13000/test_as13000_nas.py index 7f15100f0e..27d8ff0d76 100644 --- a/manila/tests/share/drivers/inspur/as13000/test_as13000_nas.py +++ b/manila/tests/share/drivers/inspur/as13000/test_as13000_nas.py @@ -692,10 +692,10 @@ def test_update_access(self, share_proto, use_access): 'send_rest_api') if use_access: self.driver.update_access(self._ctxt, share_instance, - access_rules, [], []) + access_rules, [], [], []) else: self.driver.update_access(self._ctxt, share_instance, - [], add_rules, del_rules) + [], add_rules, del_rules, []) access_clients = [{'name': rule['access_to'], 'type': 0 if share_proto == 'nfs' else 1, diff --git a/manila/tests/share/drivers/inspur/instorage/test_instorage.py b/manila/tests/share/drivers/inspur/instorage/test_instorage.py index 601a1b7587..40852a6131 100644 --- a/manila/tests/share/drivers/inspur/instorage/test_instorage.py +++ b/manila/tests/share/drivers/inspur/instorage/test_instorage.py @@ -242,7 +242,8 @@ def test_update_access(self): instorage.InStorageAssistant, 'update_access' ) - self.driver.update_access(self._ctxt, self.share_instance, [], [], []) + self.driver.update_access( + self._ctxt, self.share_instance, [], [], [], []) mock_ua.assert_called_once_with( 'fakeinstanceid', 'fake_proto', [], [], [] @@ -1512,6 +1513,7 @@ def test_update_access(self, proto, ret): proto, [], [], + [], [] ) cat_mock.assert_not_called() @@ -1523,6 +1525,7 @@ def test_update_access(self, proto, ret): proto, [], [], + [], [] ) cat_mock.assert_called_once() @@ -1532,6 +1535,7 @@ def test_update_access(self, proto, ret): proto, [], [], + [], [] ) if proto == 'CIFS': diff --git a/manila/tests/share/drivers/macrosan/test_macrosan_nas.py b/manila/tests/share/drivers/macrosan/test_macrosan_nas.py index 2fcc9a9ceb..b1daa93df8 100644 --- a/manila/tests/share/drivers/macrosan/test_macrosan_nas.py +++ b/manila/tests/share/drivers/macrosan/test_macrosan_nas.py @@ -917,7 +917,7 @@ def test_update_access_add_delete(self): self.mock_object(macrosan_helper.MacrosanHelper, '_deny_access') self.driver.update_access(self._context, share, - None, add_rules, delete_rules) + None, add_rules, delete_rules, None) @ddt.data('nfs', 'cifs') def test_update_access_nfs(self, proto): @@ -942,7 +942,7 @@ def test_update_access_nfs(self, proto): self.mock_object(macrosan_helper.MacrosanHelper, '_allow_access') self.driver.update_access(self._context, share, - access_rules, {}, {}) + access_rules, {}, {}, {}) mock_ca.assert_called_once_with(share, None) def test_update_access_fail(self): @@ -959,7 +959,7 @@ def test_update_access_fail(self): mock.Mock(side_effect=exception.InvalidShareAccess( reason='fake_exception'))) result = self.driver.update_access(self._context, share, - access_rules, None, None) + access_rules, None, None, None) expect = { 'fakeid': { 'state': 'error', @@ -983,7 +983,7 @@ def test_update_access_add_fail(self): self.mock_object(macrosan_helper.MacrosanHelper, '_deny_access') result = self.driver.update_access(self._context, share, - None, add_rules, delete_rules) + None, add_rules, delete_rules, None) expect = { 'fakeid': { 'state': 'error' diff --git a/manila/tests/share/drivers/maprfs/test_maprfs.py b/manila/tests/share/drivers/maprfs/test_maprfs.py index 9cb5773a44..1970569ccd 100644 --- a/manila/tests/share/drivers/maprfs/test_maprfs.py +++ b/manila/tests/share/drivers/maprfs/test_maprfs.py @@ -298,7 +298,7 @@ def test_update_access_add(self): self._driver._maprfs_util._execute = mock.Mock(return_value=('', 0)) self._driver.update_access(self._context, self.share, [self.access], - [self.access], []) + [self.access], [], []) self._driver._maprfs_util._execute.assert_any_call( self.maprcli_bin, 'volume', 'modify', '-name', volume, '-readAce', @@ -320,7 +320,7 @@ def test_update_access_add_no_user_no_group_exists(self): self._driver._maprfs_util._execute = mock.Mock(return_value=('', 0)) self._driver.update_access(self._context, self.share, [self.access], - [self.access], []) + [self.access], [], []) self._driver._maprfs_util._execute.assert_any_call( self.maprcli_bin, 'volume', 'modify', '-name', volume, '-readAce', @@ -341,7 +341,7 @@ def test_update_access_delete(self): self._driver._maprfs_util._execute = mock.Mock(return_value=('', 0)) self._driver.update_access(self._context, self.share, [], [], - [self.access]) + [self.access], []) self._driver._maprfs_util._execute.assert_any_call( self.maprcli_bin, 'volume', 'modify', '-name', volume, '-readAce', @@ -363,7 +363,7 @@ def test_update_access_recover(self): self._driver._maprfs_util._execute = mock.Mock(return_value=('', 0)) self._driver.update_access(self._context, self.share, [self.access], - [], []) + [], [], []) self._driver._maprfs_util._execute.assert_any_call( self.maprcli_bin, 'volume', 'modify', '-name', volume, '-readAce', @@ -377,7 +377,7 @@ def test_update_access_share_not_exists(self): self._driver._maprfs_util._execute = mock.Mock(return_value=('', 0)) self._driver.update_access(self._context, self.share, [self.access], - [], []) + [], [], []) self._driver._maprfs_util._execute.assert_not_called() @@ -396,7 +396,7 @@ def test_update_access_exception(self): self.assertRaises(exception.MapRFSException, self._driver.update_access, self._context, - self.share, [self.access], [], []) + self.share, [self.access], [], [], []) def test_update_access_invalid_access(self): access = fake_share.fake_access(access_type='ip', access_to='fake', @@ -404,7 +404,7 @@ def test_update_access_invalid_access(self): self.assertRaises(exception.InvalidShareAccess, self._driver.update_access, self._context, - self.share, [access], [], []) + self.share, [access], [], [], []) def test_ensure_share(self): self._driver._maprfs_util.volume_exists = mock.Mock( diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py index 64e2529022..dfcdb1a3d2 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py @@ -3511,6 +3511,7 @@ def test_update_access(self): [fake.SHARE_ACCESS], [], [], + [], share_server=fake.SHARE_SERVER) mock_get_vserver.assert_called_once_with( @@ -3544,6 +3545,7 @@ def test_update_access_no_share_server(self, get_vserver_exception): [fake.SHARE_ACCESS], [], [], + [], share_server=fake.SHARE_SERVER) mock_get_vserver.assert_called_once_with( @@ -3577,6 +3579,7 @@ def test_update_access_share_not_found(self): [fake.SHARE_ACCESS], [], [], + [], share_server=fake.SHARE_SERVER) mock_get_vserver.assert_called_once_with( @@ -3610,6 +3613,7 @@ def test_update_access_to_active_replica(self): [fake.SHARE_ACCESS], [], [], + [], share_server=fake.SHARE_SERVER) mock_get_vserver.assert_called_once_with( @@ -3645,6 +3649,7 @@ def test_update_access_to_in_sync_replica(self, is_readable): [fake.SHARE_ACCESS], [], [], + [], share_server=fake.SHARE_SERVER) if is_readable: diff --git a/manila/tests/share/drivers/nexenta/ns4/test_nexenta_nas.py b/manila/tests/share/drivers/nexenta/ns4/test_nexenta_nas.py index a7b3834770..33990220cf 100644 --- a/manila/tests/share/drivers/nexenta/ns4/test_nexenta_nas.py +++ b/manila/tests/share/drivers/nexenta/ns4/test_nexenta_nas.py @@ -442,6 +442,7 @@ def test_update_access__unsupported_access_type(self, post): share, [access], None, + None, None) @mock.patch(PATH_TO_RPC) @@ -485,7 +486,7 @@ def my_side_effect(*args, **kwargs): post.return_value = FakeResponse() post.side_effect = my_side_effect - self.drv.update_access(self.ctx, share, access_rules, None, None) + self.drv.update_access(self.ctx, share, access_rules, None, None, None) post.assert_called_with( self.request_params.url, data=self.request_params.build_post_args( @@ -498,7 +499,7 @@ def my_side_effect(*args, **kwargs): [access1, {'access_type': 'ip', 'access_to': '2.2.2.2', 'access_level': 'rw'}], - None, None) + None, None, None) @mock.patch(PATH_TO_RPC) def test_update_access__add_one_ip_to_empty_access_list(self, post): @@ -536,7 +537,7 @@ def my_side_effect(*args, **kwargs): raise exception.ManilaException('Unexpected request') post.return_value = FakeResponse() - self.drv.update_access(self.ctx, share, [access], None, None) + self.drv.update_access(self.ctx, share, [access], None, None, None) post.assert_called_with( self.request_params.url, data=self.request_params.build_post_args( @@ -552,7 +553,7 @@ def my_side_effect(*args, **kwargs): [{'access_type': 'ip', 'access_to': '1111', 'access_level': 'rw'}], - None, None) + None, None, None) @mock.patch(PATH_TO_RPC) def test_deny_access__unsupported_access_type(self, post): @@ -565,7 +566,7 @@ def test_deny_access__unsupported_access_type(self, post): } self.assertRaises(exception.InvalidShareAccess, self.drv.update_access, - self.ctx, share, [access], None, None) + self.ctx, share, [access], None, None, None) def test_share_backend_name(self): self.assertEqual('NexentaStor', self.drv.share_backend_name) diff --git a/manila/tests/share/drivers/nexenta/ns5/test_nexenta_nas.py b/manila/tests/share/drivers/nexenta/ns5/test_nexenta_nas.py index 74a55cf816..e692ac64ed 100644 --- a/manila/tests/share/drivers/nexenta/ns5/test_nexenta_nas.py +++ b/manila/tests/share/drivers/nexenta/ns5/test_nexenta_nas.py @@ -341,7 +341,7 @@ def test_update_access__ip_rw(self, update_nfs_access): self.assertEqual( {'fake_id': {'state': 'active'}}, self.drv.update_access( - self.ctx, SHARE, [access], None, None)) + self.ctx, SHARE, [access], None, None, None)) self.drv._update_nfs_access.assert_called_with(SHARE, ['1.1.1.1'], []) @mock.patch('%s._update_nfs_access' % DRV_PATH) @@ -356,7 +356,7 @@ def test_update_access__ip_ro(self, update_nfs_access): expected = {'fake_id': {'state': 'active'}} self.assertEqual( expected, self.drv.update_access( - self.ctx, SHARE, [access], None, None)) + self.ctx, SHARE, [access], None, None, None)) self.drv._update_nfs_access.assert_called_with(SHARE, [], ['1.1.1.1']) @ddt.data('rw', 'ro') @@ -369,7 +369,7 @@ def test_update_access__not_ip(self, access_level): } expected = {'fake_id': {'state': 'error'}} self.assertEqual(expected, self.drv.update_access( - self.ctx, SHARE, [access], None, None)) + self.ctx, SHARE, [access], None, None, None)) @mock.patch('%s._get_capacity_info' % DRV_PATH) @mock.patch('manila.share.driver.ShareDriver._update_share_stats') diff --git a/manila/tests/share/drivers/purestorage/test_flashblade.py b/manila/tests/share/drivers/purestorage/test_flashblade.py index e6d44f7ebf..0d07597b5f 100644 --- a/manila/tests/share/drivers/purestorage/test_flashblade.py +++ b/manila/tests/share/drivers/purestorage/test_flashblade.py @@ -331,7 +331,7 @@ def test_update_access_share(self): } rule_map = self.driver.update_access( - None, test_nfs_share, access_rules, [], [] + None, test_nfs_share, access_rules, [], [], [] ) self.assertEqual(expected_rule_map, rule_map) diff --git a/manila/tests/share/drivers/qnap/test_qnap.py b/manila/tests/share/drivers/qnap/test_qnap.py index 884d65cda6..529f6b2e35 100644 --- a/manila/tests/share/drivers/qnap/test_qnap.py +++ b/manila/tests/share/drivers/qnap/test_qnap.py @@ -858,7 +858,7 @@ def test_update_access_allow_access( private_storage=mock_private_storage) self.driver.update_access( 'context', self.share, 'access_rules', - None, None, share_server=None) + None, None, None, share_server=None) mock_api_executor.return_value.set_nfs_access.assert_called_once_with( 'fakeVolName', 2, 'all') @@ -882,9 +882,10 @@ def test_update_access_deny_and_allow_access( delete_rules.append('access1') add_rules = [] add_rules.append('access1') + update_rules = [] self.driver.update_access( 'context', self.share, None, - add_rules, delete_rules, share_server=None) + add_rules, delete_rules, update_rules, share_server=None) mock_deny_access.assert_called_once_with( 'context', self.share, 'access1', None) @@ -907,6 +908,7 @@ def test_update_access_without_volname(self): access_rules='access_rules', add_rules=None, delete_rules=None, + update_rules=None, share_server=None) @mock.patch.object(qnap.QnapShareDriver, '_get_location_path') diff --git a/manila/tests/share/drivers/quobyte/test_quobyte.py b/manila/tests/share/drivers/quobyte/test_quobyte.py index 469962a409..f8d71da353 100644 --- a/manila/tests/share/drivers/quobyte/test_quobyte.py +++ b/manila/tests/share/drivers/quobyte/test_quobyte.py @@ -564,7 +564,8 @@ def test_update_access_add_delete(self, qb_deny_mock, qb_allow_mock): self.share, access_rules=None, add_rules=[access_1], - delete_rules=[access_2, access_3]) + delete_rules=[access_2, access_3], + update_rules=[]) qb_allow_mock.assert_called_once_with(self._context, self.share, access_1) @@ -575,7 +576,8 @@ def test_update_access_add_delete(self, qb_deny_mock, qb_allow_mock): @mock.patch.object(quobyte.LOG, "warning") def test_update_access_no_rules(self, qb_log_mock): self._driver.update_access(context=None, share=None, access_rules=[], - add_rules=[], delete_rules=[]) + add_rules=[], delete_rules=[], + update_rules=[]) qb_log_mock.assert_has_calls([mock.ANY]) @@ -600,7 +602,7 @@ def test_update_access_recovery_additionals(self, self._driver.update_access(self._context, self.share, access_rules=add_access_rules, add_rules=[], - delete_rules=[]) + delete_rules=[], update_rules=[]) assert_calls = [mock.call(self._context, self.share, new_access_1), mock.call(self._context, self.share, new_access_2)] @@ -627,7 +629,7 @@ def test_update_access_recovery_superfluous(self, self._driver.update_access(self._context, self.share, access_rules=old_access_rules, add_rules=[], - delete_rules=[]) + delete_rules=[], update_rules=[]) qb_deny_mock.assert_called_once_with(self._context, self.share, @@ -665,7 +667,7 @@ def test_update_access_recovery_add_superfluous(self, self._driver.update_access(self._context, self.share, new_access_rules, add_rules=[], - delete_rules=[]) + delete_rules=[], update_rules=[]) a_calls = [mock.call(self._context, self.share, new_access_1), mock.call(self._context, self.share, new_access_2)] diff --git a/manila/tests/share/drivers/tegile/test_tegile.py b/manila/tests/share/drivers/tegile/test_tegile.py index 0979b70a68..8d59ce6dd3 100644 --- a/manila/tests/share/drivers/tegile/test_tegile.py +++ b/manila/tests/share/drivers/tegile/test_tegile.py @@ -730,7 +730,8 @@ def test_update_access(self, access_rules, add_rules, test_share, access_rules=access_rules, add_rules=add_rules, - delete_rules=delete_rules) + delete_rules=delete_rules, + update_rules=None) allow_params = ( '%s/%s/%s/%s' % ( diff --git a/manila/tests/share/drivers/test_ganesha.py b/manila/tests/share/drivers/test_ganesha.py index 63fd985e55..d3d6a3bae4 100644 --- a/manila/tests/share/drivers/test_ganesha.py +++ b/manila/tests/share/drivers/test_ganesha.py @@ -271,7 +271,7 @@ def test_update_access_for_allow(self): self._helper.update_access( self._context, self.share, access_rules=[self.access], - add_rules=[self.access], delete_rules=[]) + add_rules=[self.access], delete_rules=[], update_rules=[]) self._helper._allow_access.assert_called_once_with( '/', self.share, self.access) @@ -286,7 +286,7 @@ def test_update_access_for_deny(self): self._helper.update_access( self._context, self.share, access_rules=[], - add_rules=[], delete_rules=[self.access]) + add_rules=[], delete_rules=[self.access], update_rules=[]) self._helper._deny_access.assert_called_once_with( '/', self.share, self.access) @@ -301,7 +301,7 @@ def test_update_access_recovery(self): self._helper.update_access( self._context, self.share, access_rules=[self.access], - add_rules=[], delete_rules=[]) + add_rules=[], delete_rules=[], update_rules=[]) self._helper._allow_access.assert_called_once_with( '/', self.share, self.access) @@ -316,7 +316,8 @@ def test_update_access_invalid_share_access_type(self): result = self._helper.update_access(self._context, self.share, access_rules=[bad_rule], - add_rules=[], delete_rules=[]) + add_rules=[], delete_rules=[], + update_rules=[]) self.assertEqual(expected, result) @@ -326,7 +327,8 @@ def test_update_access_invalid_share_access_level(self): result = self._helper.update_access(self._context, self.share, access_rules=[bad_rule], - add_rules=[], delete_rules=[]) + add_rules=[], delete_rules=[], + update_rules=[]) self.assertEqual(expected, result) @@ -484,7 +486,7 @@ def test_update_access_add_export(self): self._helper.update_access( self._context, self.share, access_rules=[self.rule1], - add_rules=[], delete_rules=[]) + add_rules=[], delete_rules=[], update_rules=[]) mock_gh.check_export_exists.assert_called_once_with('fakename') mock_gh.get_export_id.assert_called_once_with() @@ -520,7 +522,7 @@ def test_update_access_update_export(self, client): self._helper.update_access( self._context, self.share, access_rules=[self.rule1, self.rule2], - add_rules=[self.rule2], delete_rules=[]) + add_rules=[self.rule2], delete_rules=[], update_rules=[]) mock_gh.check_export_exists.assert_called_once_with('fakename') mock_gh.update_export.assert_called_once_with('fakename', @@ -541,7 +543,7 @@ def test_update_access_remove_export(self): self._helper.update_access( self._context, self.share, access_rules=[], - add_rules=[], delete_rules=[self.rule1]) + add_rules=[], delete_rules=[self.rule1], update_rules=[]) mock_gh.check_export_exists.assert_called_once_with('fakename') mock_gh.remove_export.assert_called_once_with('fakename') @@ -559,7 +561,7 @@ def test_update_access_export_file_already_removed(self): self._helper.update_access( self._context, self.share, access_rules=[], - add_rules=[], delete_rules=[self.rule1]) + add_rules=[], delete_rules=[self.rule1], update_rules=[]) mock_gh.check_export_exists.assert_called_once_with('fakename') ganesha.LOG.warning.assert_called_once_with(mock.ANY, mock.ANY) @@ -577,7 +579,8 @@ def test_update_access_invalid_share_access_type(self): result = self._helper.update_access(self._context, self.share, access_rules=[bad_rule], - add_rules=[], delete_rules=[]) + add_rules=[], delete_rules=[], + update_rules=[]) self.assertEqual(expected, result) @@ -591,6 +594,7 @@ def test_update_access_invalid_share_access_level(self): result = self._helper.update_access(self._context, self.share, access_rules=[bad_rule], - add_rules=[], delete_rules=[]) + add_rules=[], delete_rules=[], + update_rules=[]) self.assertEqual(expected, result) diff --git a/manila/tests/share/drivers/test_generic.py b/manila/tests/share/drivers/test_generic.py index 77cbee727d..21298b636a 100644 --- a/manila/tests/share/drivers/test_generic.py +++ b/manila/tests/share/drivers/test_generic.py @@ -1160,18 +1160,21 @@ def test_update_access(self, access_level): get_fake_access_rule('2.2.2.2', access_level)] add_rules = [get_fake_access_rule('2.2.2.2', access_level), ] delete_rules = [get_fake_access_rule('3.3.3.3', access_level), ] + update_rules = [] # run self._driver.update_access(self._context, self.share, access_rules, add_rules=add_rules, delete_rules=delete_rules, + update_rules=update_rules, share_server=self.server) # asserts (self._driver._helpers[self.share['share_proto']]. update_access.assert_called_once_with( self.server['backend_details'], self.share['name'], - access_rules, add_rules=add_rules, delete_rules=delete_rules)) + access_rules, add_rules=add_rules, delete_rules=delete_rules, + update_rules=update_rules)) @ddt.data(fake_share.fake_share(), fake_share.fake_share(share_proto='NFSBOGUS'), diff --git a/manila/tests/share/drivers/test_lvm.py b/manila/tests/share/drivers/test_lvm.py index 2501221518..1d072ab3fc 100644 --- a/manila/tests/share/drivers/test_lvm.py +++ b/manila/tests/share/drivers/test_lvm.py @@ -467,6 +467,7 @@ def test_update_access(self, access_level): self._driver.update_access(self._context, self.share, access_rules, add_rules=add_rules, delete_rules=delete_rules, + update_rules=None, share_server=self.server) (self._driver._helpers[self.share['share_proto']]. update_access.assert_called_once_with( diff --git a/manila/tests/share/drivers/veritas/test_veritas_isa.py b/manila/tests/share/drivers/veritas/test_veritas_isa.py index 4b4134874c..5a0393b971 100644 --- a/manila/tests/share/drivers/veritas/test_veritas_isa.py +++ b/manila/tests/share/drivers/veritas/test_veritas_isa.py @@ -283,7 +283,7 @@ def test_delete_snapshot_if_not_present_at_backend(self): def test_update_access_for_allow(self): self.mock_object(self._driver, '_access_api') self._driver.update_access(self._context, self.share, [], - [self.access], []) + [self.access], [], []) self.assertEqual(2, self._driver._access_api.call_count) def test_update_access_for_allow_negative(self): @@ -292,22 +292,22 @@ def test_update_access_for_allow_negative(self): self.assertRaises(exception.ShareBackendException, self._driver.update_access, self._context, - self.share, [], [self.access], []) + self.share, [], [self.access], [], []) self.assertRaises(exception.InvalidShareAccess, self._driver.update_access, self._context, - self.share, [], [self.access2], []) + self.share, [], [self.access2], [], []) self.assertRaises(exception.InvalidShareAccessLevel, self._driver.update_access, self._context, - self.share, [], [self.access3], []) + self.share, [], [self.access3], [], []) def test_update_access_for_deny(self): self.mock_object(self._driver, '_access_api') self._driver.update_access(self._context, self.share, - [], [], [self.access]) + [], [], [self.access], []) self.assertEqual(2, self._driver._access_api.call_count) def test_update_access_for_deny_negative(self): @@ -316,19 +316,19 @@ def test_update_access_for_deny_negative(self): self.assertRaises(exception.ShareBackendException, self._driver.update_access, self._context, - self.share, [], [], [self.access]) + self.share, [], [], [self.access], []) def test_update_access_for_deny_for_invalid_access_type(self): self.mock_object(self._driver, '_access_api') self._driver.update_access(self._context, self.share, - [], [], [self.access2]) + [], [], [self.access2], []) self.assertEqual(0, self._driver._access_api.call_count) def test_update_access_for_empty_rule_list(self): self.mock_object(self._driver, '_allow_access') self.mock_object(self._driver, '_deny_access') self._driver.update_access(self._context, self.share, - [], [], []) + [], [], [], []) self.assertEqual(0, self._driver._allow_access.call_count) self.assertEqual(0, self._driver._deny_access.call_count) @@ -351,7 +351,7 @@ def test_update_access_for_access_rules(self): a_rule = self._driver._return_access_lists_difference([self.access4], existing_a_rules) self._driver.update_access(self._context, self.share, - [self.access4], [], []) + [self.access4], [], [], []) self.assertEqual(d_rule, existing_a_rules) self.assertEqual(a_rule, [self.access4]) diff --git a/manila/tests/share/drivers/windows/test_windows_smb_helper.py b/manila/tests/share/drivers/windows/test_windows_smb_helper.py index 87a005352b..4174e4a52f 100644 --- a/manila/tests/share/drivers/windows/test_windows_smb_helper.py +++ b/manila/tests/share/drivers/windows/test_windows_smb_helper.py @@ -214,7 +214,7 @@ def test_update_access_invalid_type(self): exception.InvalidShareAccess, self._win_smb_helper.update_access, mock.sentinel.server, mock.sentinel.share_name, - [invalid_access_rule], [], []) + [invalid_access_rule], [], [], []) def test_update_access_invalid_level(self): invalid_access_rule = dict(self._FAKE_RW_ACC_RULE, @@ -223,7 +223,7 @@ def test_update_access_invalid_level(self): exception.InvalidShareAccessLevel, self._win_smb_helper.update_access, mock.sentinel.server, mock.sentinel.share_name, - [], [invalid_access_rule], []) + [], [invalid_access_rule], [], []) @mock.patch.object(windows_smb_helper.WindowsSMBHelper, '_revoke_share_access') @@ -235,7 +235,7 @@ def test_update_access_deleting_invalid_rule(self, mock_revoke): self._win_smb_helper.update_access( mock.sentinel.server, mock.sentinel.share_name, - [], [], delete_rules) + [], [], delete_rules, []) mock_revoke.assert_called_once_with( mock.sentinel.server, mock.sentinel.share_name, @@ -256,7 +256,7 @@ def test_update_access(self, mock_revoke, mock_grant, self._win_smb_helper.update_access( mock.sentinel.server, mock.sentinel.share_name, - [], added_rules, deleted_rules) + [], added_rules, deleted_rules, []) mock_revoke.assert_has_calls( [mock.call(mock.sentinel.server, mock.sentinel.share_name, @@ -291,7 +291,7 @@ def test_update_access_maintenance( self._win_smb_helper.update_access( mock.sentinel.server, mock.sentinel.share_name, - all_rules, [], []) + all_rules, [], [], []) mock_get_access_rules.assert_called_once_with( mock.sentinel.server, mock.sentinel.share_name) diff --git a/manila/tests/share/drivers/zfsonlinux/test_driver.py b/manila/tests/share/drivers/zfsonlinux/test_driver.py index c48c8f615f..f38ab87064 100644 --- a/manila/tests/share/drivers/zfsonlinux/test_driver.py +++ b/manila/tests/share/drivers/zfsonlinux/test_driver.py @@ -1151,7 +1151,7 @@ def test_update_access(self): } result = self.driver.update_access( - 'fake_context', share, [1], [2], [3]) + 'fake_context', share, [1], [2], [3], []) self.driver._get_dataset_name.assert_called_once_with(share) mock_shell_executor.assert_called_once_with(share['host']) diff --git a/manila/tests/share/test_access.py b/manila/tests/share/test_access.py index 75789b5b5a..b6c8c68e65 100644 --- a/manila/tests/share/test_access.py +++ b/manila/tests/share/test_access.py @@ -203,6 +203,8 @@ def test_get_and_update_access_rule_updates_conditionally_changed( constants.ACCESS_STATE_QUEUED_TO_DENY, constants.ACCESS_STATE_DENYING: constants.ACCESS_STATE_QUEUED_TO_DENY, + constants.ACCESS_STATE_QUEUED_TO_UPDATE: + constants.ACCESS_STATE_UPDATING, } actual_rule = ( @@ -247,7 +249,8 @@ def test_update_access_rules_an_update_is_in_progress(self, initial_state): expected_filters = { 'state': (constants.ACCESS_STATE_APPLYING, - constants.ACCESS_STATE_DENYING), + constants.ACCESS_STATE_DENYING, + constants.ACCESS_STATE_UPDATING), } self.assertIsNone(retval) mock_debug_log.assert_called_once() @@ -273,17 +276,21 @@ def test_update_access_rules_nothing_to_update(self): expected_rule_filter_1 = { 'state': (constants.ACCESS_STATE_APPLYING, - constants.ACCESS_STATE_DENYING), + constants.ACCESS_STATE_DENYING, + constants.ACCESS_STATE_UPDATING), } expected_rule_filter_2 = { 'state': (constants.ACCESS_STATE_QUEUED_TO_APPLY, - constants.ACCESS_STATE_QUEUED_TO_DENY), + constants.ACCESS_STATE_QUEUED_TO_DENY, + constants.ACCESS_STATE_QUEUED_TO_UPDATE), } expected_conditionally_change = { constants.ACCESS_STATE_QUEUED_TO_APPLY: constants.ACCESS_STATE_APPLYING, constants.ACCESS_STATE_QUEUED_TO_DENY: constants.ACCESS_STATE_DENYING, + constants.ACCESS_STATE_QUEUED_TO_UPDATE: + constants.ACCESS_STATE_UPDATING } self.assertIsNone(retval) mock_debug_log.assert_called_once() @@ -320,17 +327,21 @@ def test_update_access_rules_delete_all_rules(self, delete_all_rules): expected_rule_filter_1 = { 'state': (constants.ACCESS_STATE_APPLYING, - constants.ACCESS_STATE_DENYING), + constants.ACCESS_STATE_DENYING, + constants.ACCESS_STATE_UPDATING), } expected_rule_filter_2 = { 'state': (constants.ACCESS_STATE_QUEUED_TO_APPLY, - constants.ACCESS_STATE_QUEUED_TO_DENY), + constants.ACCESS_STATE_QUEUED_TO_DENY, + constants.ACCESS_STATE_QUEUED_TO_UPDATE), } expected_conditionally_change = { constants.ACCESS_STATE_QUEUED_TO_APPLY: constants.ACCESS_STATE_APPLYING, constants.ACCESS_STATE_QUEUED_TO_DENY: constants.ACCESS_STATE_DENYING, + constants.ACCESS_STATE_QUEUED_TO_UPDATE: + constants.ACCESS_STATE_UPDATING } expected_get_and_update_calls = [] if delete_all_rules: @@ -422,7 +433,8 @@ def test__update_access_rules_with_driver_updates( expected_filters_1 = { 'state': (constants.ACCESS_STATE_APPLYING, constants.ACCESS_STATE_ACTIVE, - constants.ACCESS_STATE_DENYING), + constants.ACCESS_STATE_DENYING, + constants.ACCESS_STATE_UPDATING), } expected_filters_2 = {'state': constants.STATUS_ERROR} expected_get_and_update_calls = [ @@ -479,6 +491,7 @@ def test__update_access_rules_with_driver_updates( constants.ACCESS_STATE_APPLYING: access_state, constants.ACCESS_STATE_DENYING: access_state, constants.ACCESS_STATE_ACTIVE: access_state, + constants.ACCESS_STATE_UPDATING: access_state } expected_access_rule_update_calls = [ mock.call( @@ -497,6 +510,7 @@ def test__update_access_rules_with_driver_updates( self.assertFalse(one_access_rule_update_call.called) expected_conditionally_change = { constants.ACCESS_STATE_APPLYING: constants.ACCESS_STATE_ACTIVE, + constants.ACCESS_STATE_UPDATING: constants.ACCESS_STATE_ACTIVE, } expected_get_and_update_calls.append( mock.call(self.context, share_instance_id=share_instance_id, @@ -563,24 +577,30 @@ def _driver_side_effect(*args, **kwargs): expected_filters_1 = { 'state': (constants.ACCESS_STATE_APPLYING, constants.ACCESS_STATE_ACTIVE, - constants.ACCESS_STATE_DENYING), + constants.ACCESS_STATE_DENYING, + constants.ACCESS_STATE_UPDATING), } conditionally_change_2 = { constants.ACCESS_STATE_APPLYING: constants.ACCESS_STATE_ACTIVE, + constants.ACCESS_STATE_UPDATING: constants.ACCESS_STATE_ACTIVE, } expected_filters_3 = { 'state': (constants.ACCESS_STATE_QUEUED_TO_APPLY, - constants.ACCESS_STATE_QUEUED_TO_DENY), + constants.ACCESS_STATE_QUEUED_TO_DENY, + constants.ACCESS_STATE_QUEUED_TO_UPDATE), } expected_conditionally_change_3 = { constants.ACCESS_STATE_QUEUED_TO_APPLY: constants.ACCESS_STATE_APPLYING, constants.ACCESS_STATE_QUEUED_TO_DENY: constants.ACCESS_STATE_DENYING, + constants.ACCESS_STATE_QUEUED_TO_UPDATE: + constants.ACCESS_STATE_UPDATING, } expected_conditionally_change_4 = { constants.ACCESS_STATE_APPLYING: constants.ACCESS_STATE_ERROR, constants.ACCESS_STATE_DENYING: constants.ACCESS_STATE_ERROR, + constants.ACCESS_STATE_UPDATING: constants.ACCESS_STATE_ERROR, } expected_get_and_update_calls = [ mock.call(self.context, filters=expected_filters_1, @@ -701,13 +721,16 @@ def test__check_needs_refresh(self, expected_needs_refresh): expected_filter = { 'state': (constants.ACCESS_STATE_QUEUED_TO_APPLY, - constants.ACCESS_STATE_QUEUED_TO_DENY), + constants.ACCESS_STATE_QUEUED_TO_DENY, + constants.ACCESS_STATE_QUEUED_TO_UPDATE), } expected_conditionally_change = { constants.ACCESS_STATE_QUEUED_TO_APPLY: constants.ACCESS_STATE_APPLYING, constants.ACCESS_STATE_QUEUED_TO_DENY: constants.ACCESS_STATE_DENYING, + constants.ACCESS_STATE_QUEUED_TO_UPDATE: + constants.ACCESS_STATE_UPDATING, } self.assertEqual(expected_needs_refresh, needs_refresh) @@ -740,9 +763,12 @@ def test__update_rules_through_share_driver(self, proto, pass_add_rules, fail_add_rules = self._get_pass_rules_and_fail_rules() pass_delete_rules, fail_delete_rules = ( self._get_pass_rules_and_fail_rules()) + pass_update_rules, fail_update_rules = ( + self._get_pass_rules_and_fail_rules()) test_rules = pass_rules + fail_rules test_add_rules = pass_add_rules + fail_add_rules test_delete_rules = pass_delete_rules + fail_delete_rules + test_update_rules = pass_update_rules + fail_update_rules fake_expect_driver_update_rules = pass_rules update_access_call = self.mock_object( @@ -754,6 +780,7 @@ def test__update_rules_through_share_driver(self, proto, access_rules_to_be_on_share=test_rules, add_rules=test_add_rules, delete_rules=test_delete_rules, + update_rules=test_update_rules, rules_to_be_removed_from_db=test_rules, share_server=None)) @@ -761,12 +788,14 @@ def test__update_rules_through_share_driver(self, proto, update_access_call.assert_called_once_with( self.context, share_instance, pass_rules, add_rules=pass_add_rules, - delete_rules=pass_delete_rules, share_server=None) + delete_rules=pass_delete_rules, + update_rules=pass_update_rules, + share_server=None) else: update_access_call.assert_called_once_with( self.context, share_instance, test_rules, add_rules=test_add_rules, delete_rules=test_delete_rules, - share_server=None) + update_rules=test_update_rules, share_server=None) self.assertEqual(fake_expect_driver_update_rules, driver_update_rules) def _get_pass_rules_and_fail_rules(self): diff --git a/manila/tests/share/test_driver.py b/manila/tests/share/test_driver.py index 2713001471..1c757cdceb 100644 --- a/manila/tests/share/test_driver.py +++ b/manila/tests/share/test_driver.py @@ -697,7 +697,8 @@ def test_update_access(self): 'fake_share', 'fake_access_rules', 'fake_add_rules', - 'fake_delete_rules' + 'fake_delete_rules', + 'fake_update_rules' ) def test_create_replica(self): diff --git a/releasenotes/notes/bug-2066871-allow-to-update-access-level-for-access-rule-741f8fc3cc190701.yaml b/releasenotes/notes/bug-2066871-allow-to-update-access-level-for-access-rule-741f8fc3cc190701.yaml new file mode 100644 index 0000000000..f536c73fda --- /dev/null +++ b/releasenotes/notes/bug-2066871-allow-to-update-access-level-for-access-rule-741f8fc3cc190701.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Since microversion 2.88, Manila will allow to update access_level of access + rule of type `ip` using `openstack share access update` API. Currently this + is supported only for NetApp ONTAP backend. For more details, please check + `Launchpad bug #2066871 `_