From 266c8012e7a54da6380cf82ee6880351e2752c42 Mon Sep 17 00:00:00 2001 From: Ashley Rodriguez Date: Thu, 13 Jan 2022 16:06:04 -0500 Subject: [PATCH] Metadata for Share Resource This change adds metadata controller for Shares resource APIImpact Partially-implements: bp/metadata-for-share-resources Change-Id: I76d26f4ddce7570463efd896b571b1e1a9222ca5 --- manila/api/common.py | 25 +++ manila/api/v1/share_metadata.py | 35 +++- manila/api/v2/metadata.py | 218 +++++++++++++++++++++++++ manila/api/v2/router.py | 56 +++++-- manila/api/v2/shares.py | 94 ++++++++++- manila/db/api.py | 10 +- manila/db/sqlalchemy/api.py | 9 +- manila/exception.py | 2 +- manila/policies/shares.py | 20 ++- manila/share/api.py | 94 ++--------- manila/tests/api/v2/test_metadata.py | 137 ++++++++++++++++ manila/tests/db/sqlalchemy/test_api.py | 69 ++++++++ manila/tests/db_utils.py | 2 +- manila/tests/test_exception.py | 6 +- 14 files changed, 653 insertions(+), 124 deletions(-) create mode 100644 manila/api/v2/metadata.py create mode 100644 manila/tests/api/v2/test_metadata.py diff --git a/manila/api/common.py b/manila/api/common.py index a19a379e0e..605243157f 100644 --- a/manila/api/common.py +++ b/manila/api/common.py @@ -612,3 +612,28 @@ def validate_subnet_create(context, share_network_id, data, raise exc.HTTPConflict(explanation=msg) return share_network, existing_subnets + + +def _check_metadata_properties(metadata=None): + if not metadata: + metadata = {} + + for k, v in metadata.items(): + if not k: + msg = _("Metadata property key is blank.") + LOG.warning(msg) + raise exception.InvalidMetadata(message=msg) + if len(k) > 255: + msg = _("Metadata property key is " + "greater than 255 characters.") + LOG.warning(msg) + raise exception.InvalidMetadataSize(message=msg) + if not v: + msg = _("Metadata property value is blank.") + LOG.warning(msg) + raise exception.InvalidMetadata(message=msg) + if len(v) > 1023: + msg = _("Metadata property value is " + "greater than 1023 characters.") + LOG.warning(msg) + raise exception.InvalidMetadataSize(message=msg) diff --git a/manila/api/v1/share_metadata.py b/manila/api/v1/share_metadata.py index 493494b1af..589a4470aa 100644 --- a/manila/api/v1/share_metadata.py +++ b/manila/api/v1/share_metadata.py @@ -18,8 +18,10 @@ from oslo_log import log import webob from webob import exc +from manila.api import common as api_common from manila.api.openstack import wsgi from manila.common import constants +from manila import db from manila import exception from manila.i18n import _ from manila import policy @@ -39,7 +41,8 @@ class ShareMetadataController(object): def _get_metadata(self, context, share_id): try: share = self.share_api.get(context, share_id) - meta = self.share_api.get_share_metadata(context, share) + rv = db.share_metadata_get(context, share['id']) + meta = dict(rv.items()) except exception.NotFound: msg = _('share does not exist') raise exc.HTTPNotFound(explanation=msg) @@ -115,13 +118,27 @@ class ShareMetadataController(object): msg = _("Cannot set or update admin only metadata.") LOG.exception(msg) raise exc.HTTPForbidden(explanation=msg) - ignore_keys = None - return self.share_api.update_share_metadata( - context, - share, - metadata, - ignore_keys=ignore_keys, - delete=delete) + ignore_keys = [] + + rv = db.share_metadata_get(context, share['id']) + orig_meta = dict(rv.items()) + if delete: + _metadata = metadata + for key in ignore_keys: + if key in orig_meta: + _metadata[key] = orig_meta[key] + else: + metadata_copy = metadata.copy() + for key in ignore_keys: + metadata_copy.pop(key, None) + _metadata = orig_meta.copy() + _metadata.update(metadata_copy) + + api_common._check_metadata_properties(_metadata) + db.share_metadata_update(context, share['id'], + _metadata, delete) + + return _metadata except exception.NotFound: msg = _('share does not exist') raise exc.HTTPNotFound(explanation=msg) @@ -162,7 +179,7 @@ class ShareMetadataController(object): if id in constants.AdminOnlyMetadata.SCHEDULER_FILTERS: policy.check_policy(context, 'share', 'update_admin_only_metadata') - self.share_api.delete_share_metadata(context, share, id) + db.share_metadata_delete(context, share['id'], id) except exception.NotFound: msg = _('share does not exist') raise exc.HTTPNotFound(explanation=msg) diff --git a/manila/api/v2/metadata.py b/manila/api/v2/metadata.py new file mode 100644 index 0000000000..9a9b85b9a0 --- /dev/null +++ b/manila/api/v2/metadata.py @@ -0,0 +1,218 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from webob import exc + +from manila.api import common +from manila.api.openstack import wsgi +from manila import db +from manila import exception +from manila.i18n import _ +from manila import policy + + +class MetadataController(object): + """An abstract metadata controller resource.""" + + # From db, ensure it exists + resource_get = { + "share": "share_get", + } + + resource_metadata_get = { + "share": "share_metadata_get", + } + + resource_metadata_get_item = { + "share": "share_metadata_get_item", + } + + resource_metadata_update = { + "share": "share_metadata_update", + } + + resource_metadata_update_item = { + "share": "share_metadata_update_item", + } + + resource_metadata_delete = { + "share": "share_metadata_delete", + } + + resource_policy_get = { + 'share': 'get', + } + + def __init__(self): + super(MetadataController, self).__init__() + self.resource_name = None + + def _get_resource(self, context, resource_id, + for_modification=False, parent_id=None): + if self.resource_name in ['share']: + # we would allow retrieving some "public" resources + # across project namespaces + kwargs = {} + else: + kwargs = {'project_only': True} + try: + get_res_method = getattr( + db, self.resource_get[self.resource_name]) + if parent_id is not None: + kwargs["parent_id"] = parent_id + res = get_res_method(context, resource_id, **kwargs) + + get_policy = self.resource_policy_get[self.resource_name] + if res.get('is_public') is False or for_modification: + policy.check_policy(context, self.resource_name, + get_policy, res) + + except exception.NotFound: + msg = _('%s not found.' % self.resource_name.capitalize()) + raise exc.HTTPNotFound(explanation=msg) + return res + + def _get_metadata(self, context, resource_id, parent_id=None): + + self._get_resource(context, resource_id, parent_id=parent_id) + get_metadata_method = getattr( + db, self.resource_metadata_get[self.resource_name]) + + result = get_metadata_method(context, resource_id) + + return result + + @wsgi.response(200) + def _index_metadata(self, req, resource_id, parent_id=None): + context = req.environ['manila.context'] + metadata = self._get_metadata(context, resource_id, + parent_id=parent_id) + + return {'metadata': metadata} + + @wsgi.response(200) + def _create_metadata(self, req, resource_id, body, parent_id=None): + """Returns the new metadata item created.""" + + context = req.environ['manila.context'] + try: + metadata = body['metadata'] + common._check_metadata_properties(metadata) + except (KeyError, TypeError): + msg = _("Malformed request body") + raise exc.HTTPBadRequest(explanation=msg) + except exception.InvalidMetadata as error: + raise exc.HTTPBadRequest(explanation=error.msg) + except exception.InvalidMetadataSize as error: + raise exc.HTTPBadRequest(explanation=error.msg) + + self._get_resource(context, resource_id, + for_modification=True, parent_id=parent_id) + + create_metadata_method = getattr( + db, self.resource_metadata_update[self.resource_name]) + result = create_metadata_method(context, resource_id, metadata, + delete='False') + + return {'metadata': result} + + def _update_metadata_item(self, req, resource_id, body, key, + parent_id=None): + """Updates the specified metadata item.""" + + context = req.environ['manila.context'] + try: + meta_item = body['metadata'] + common._check_metadata_properties(meta_item) + except (TypeError, KeyError): + expl = _('Malformed request body') + raise exc.HTTPBadRequest(explanation=expl) + except exception.InvalidMetadata as error: + raise exc.HTTPBadRequest(explanation=error.msg) + except exception.InvalidMetadataSize as error: + raise exc.HTTPBadRequest(explanation=error.msg) + + if key not in meta_item: + expl = _('Request body and URI mismatch') + raise exc.HTTPBadRequest(explanation=expl) + if len(meta_item) > 1: + expl = _('Request body contains too many items') + raise exc.HTTPBadRequest(explanation=expl) + self._get_resource(context, resource_id, + for_modification=True, parent_id=parent_id) + + update_metadata_item_method = getattr( + db, self.resource_metadata_update_item[self.resource_name]) + result = update_metadata_item_method(context, resource_id, meta_item) + + return {'metadata': result} + + @wsgi.response(200) + def _update_all_metadata(self, req, resource_id, body, parent_id=None): + """Deletes existing metadata, and returns the updated metadata.""" + + context = req.environ['manila.context'] + try: + metadata = body['metadata'] + common._check_metadata_properties(metadata) + except (TypeError, KeyError): + expl = _('Malformed request body') + raise exc.HTTPBadRequest(explanation=expl) + except exception.InvalidMetadata as error: + raise exc.HTTPBadRequest(explanation=error.msg) + except exception.InvalidMetadataSize as error: + raise exc.HTTPBadRequest(explanation=error.msg) + + self._get_resource(context, resource_id, + for_modification=True, parent_id=parent_id) + meta_ref = self._get_metadata(context, resource_id, + parent_id=parent_id) + + for key in meta_ref: + delete_metadata_method = getattr( + db, self.resource_metadata_delete[self.resource_name]) + delete_metadata_method(context, resource_id, key) + + update_metadata_method = getattr( + db, self.resource_metadata_update[self.resource_name]) + new_metadata = update_metadata_method(context, resource_id, + metadata, delete='False') + return {'metadata': new_metadata} + + @wsgi.response(200) + def _show_metadata(self, req, resource_id, key, parent_id=None): + """Return metadata item.""" + + context = req.environ['manila.context'] + self._get_resource(context, resource_id, + for_modification=False, parent_id=parent_id) + get_metadata_item_method = getattr( + db, self.resource_metadata_get_item[self.resource_name]) + item = get_metadata_item_method(context, resource_id, key) + + return {'metadata': {key: item}} + + @wsgi.response(200) + def _delete_metadata(self, req, resource_id, key, parent_id=None): + """Deletes existing metadata item.""" + + context = req.environ['manila.context'] + self._get_resource(context, resource_id, + for_modification=True, parent_id=parent_id) + + get_metadata_item_method = getattr( + db, self.resource_metadata_get_item[self.resource_name]) + get_metadata_item_method(context, resource_id, key) + + delete_metadata_method = getattr( + db, self.resource_metadata_delete[self.resource_name]) + delete_metadata_method(context, resource_id, key) diff --git a/manila/api/v2/router.py b/manila/api/v2/router.py index 4292d0a81d..9162e17550 100644 --- a/manila/api/v2/router.py +++ b/manila/api/v2/router.py @@ -26,7 +26,6 @@ from manila.api.v1 import limits from manila.api.v1 import scheduler_stats from manila.api.v1 import security_service from manila.api.v1 import share_manage -from manila.api.v1 import share_metadata from manila.api.v1 import share_types_extra_specs from manila.api.v1 import share_unmanage from manila.api.v2 import availability_zones @@ -163,6 +162,45 @@ class APIRouter(manila.api.openstack.APIRouter): action="manage", conditions={"method": ["POST"]}) + for path_prefix in ['/{project_id}', '']: + # project_id is optional + mapper.connect("share_metadata", + "%s/shares/{resource_id}/metadata" + % path_prefix, + controller=self.resources["shares"], + action="create_metadata", + conditions={"method": ["POST"]}) + mapper.connect("share_metadata", + "%s/shares/{resource_id}/metadata" + % path_prefix, + controller=self.resources["shares"], + action="update_all_metadata", + conditions={"method": ["PUT"]}) + mapper.connect("share_metadata", + "%s/shares/{resource_id}/metadata/{key}" + % path_prefix, + controller=self.resources["shares"], + action="update_metadata_item", + conditions={"method": ["POST"]}) + mapper.connect("share_metadata", + "%s/shares/{resource_id}/metadata" + % path_prefix, + controller=self.resources["shares"], + action="index_metadata", + conditions={"method": ["GET"]}) + mapper.connect("share_metadata", + "%s/shares/{resource_id}/metadata/{key}" + % path_prefix, + controller=self.resources["shares"], + action="show_metadata", + conditions={"method": ["GET"]}) + mapper.connect("share_metadata", + "%s/shares/{resource_id}/metadata/{key}" + % path_prefix, + controller=self.resources["shares"], + action="delete_metadata", + conditions={"method": ["DELETE"]}) + self.resources["share_instances"] = share_instances.create_resource() mapper.resource("share_instance", "share_instances", controller=self.resources["share_instances"], @@ -280,22 +318,6 @@ class APIRouter(manila.api.openstack.APIRouter): action="show", conditions={"method": ["GET"]}) - self.resources["share_metadata"] = share_metadata.create_resource() - share_metadata_controller = self.resources["share_metadata"] - - mapper.resource("share_metadata", "metadata", - controller=share_metadata_controller, - parent_resource=dict(member_name="share", - collection_name="shares")) - - for path_prefix in ['/{project_id}', '']: - # project_id is optional - mapper.connect("metadata", - "%s/shares/{share_id}/metadata" % path_prefix, - controller=share_metadata_controller, - action="update_all", - conditions={"method": ["PUT"]}) - self.resources["limits"] = limits.create_resource() mapper.resource("limit", "limits", controller=self.resources["limits"]) diff --git a/manila/api/v2/shares.py b/manila/api/v2/shares.py index b4651b7bd0..04b2f54231 100644 --- a/manila/api/v2/shares.py +++ b/manila/api/v2/shares.py @@ -25,6 +25,7 @@ from manila.api.openstack import wsgi from manila.api.v1 import share_manage from manila.api.v1 import share_unmanage from manila.api.v1 import shares +from manila.api.v2 import metadata from manila.api.views import share_accesses as share_access_views from manila.api.views import share_migration as share_migration_views from manila.api.views import shares as share_views @@ -32,16 +33,18 @@ from manila.common import constants from manila import db from manila import exception from manila.i18n import _ +from manila import policy from manila import share from manila import utils LOG = log.getLogger(__name__) -class ShareController(shares.ShareMixin, +class ShareController(wsgi.Controller, + shares.ShareMixin, share_manage.ShareManageMixin, share_unmanage.ShareUnmanageMixin, - wsgi.Controller, + metadata.MetadataController, wsgi.AdminActionsMixin): """The Shares API v2 controller for the OpenStack API.""" resource_name = 'share' @@ -596,6 +599,93 @@ class ShareController(shares.ShareMixin, return self._get_shares(req, is_detail=True) + def _validate_metadata_for_update(self, req, share_id, metadata, + delete=True): + admin_metadata_ignore_keys = ( + constants.AdminOnlyMetadata.SCHEDULER_FILTERS + ) + context = req.environ['manila.context'] + if set(metadata).intersection(set(admin_metadata_ignore_keys)): + try: + policy.check_policy( + context, 'share', 'update_admin_only_metadata') + except exception.PolicyNotAuthorized: + msg = _("Cannot set or update admin only metadata.") + LOG.exception(msg) + raise exc.HTTPForbidden(explanation=msg) + admin_metadata_ignore_keys = [] + + current_share_metadata = db.share_metadata_get(context, share_id) + if delete: + _metadata = metadata + for key in admin_metadata_ignore_keys: + if key in current_share_metadata: + _metadata[key] = current_share_metadata[key] + else: + metadata_copy = metadata.copy() + for key in admin_metadata_ignore_keys: + metadata_copy.pop(key, None) + _metadata = current_share_metadata.copy() + _metadata.update(metadata_copy) + + return _metadata + + # NOTE: (ashrod98) original metadata method and policy overrides + @wsgi.Controller.api_version("2.0") + @wsgi.Controller.authorize("get_share_metadata") + def index_metadata(self, req, resource_id): + """Returns the list of metadata for a given share.""" + return self._index_metadata(req, resource_id) + + @wsgi.Controller.api_version("2.0") + @wsgi.Controller.authorize("update_share_metadata") + def create_metadata(self, req, resource_id, body): + if not self.is_valid_body(body, 'metadata'): + expl = _('Malformed request body') + raise exc.HTTPBadRequest(explanation=expl) + _metadata = self._validate_metadata_for_update(req, resource_id, + body['metadata'], + delete=False) + body['metadata'] = _metadata + return self._create_metadata(req, resource_id, body) + + @wsgi.Controller.api_version("2.0") + @wsgi.Controller.authorize("update_share_metadata") + def update_all_metadata(self, req, resource_id, body): + if not self.is_valid_body(body, 'metadata'): + expl = _('Malformed request body') + raise exc.HTTPBadRequest(explanation=expl) + _metadata = self._validate_metadata_for_update(req, resource_id, + body['metadata']) + body['metadata'] = _metadata + return self._update_all_metadata(req, resource_id, body) + + @wsgi.Controller.api_version("2.0") + @wsgi.Controller.authorize("update_share_metadata") + def update_metadata_item(self, req, resource_id, body, key): + if not self.is_valid_body(body, 'meta'): + expl = _('Malformed request body') + raise exc.HTTPBadRequest(explanation=expl) + _metadata = self._validate_metadata_for_update(req, resource_id, + body['metadata'], + delete=False) + body['metadata'] = _metadata + return self._update_metadata_item(req, resource_id, body, key) + + @wsgi.Controller.api_version("2.0") + @wsgi.Controller.authorize("get_share_metadata") + def show_metadata(self, req, resource_id, key): + return self._show_metadata(req, resource_id, key) + + @wsgi.Controller.api_version("2.0") + @wsgi.Controller.authorize("delete_share_metadata") + def delete_metadata(self, req, resource_id, key): + context = req.environ['manila.context'] + if key in constants.AdminOnlyMetadata.SCHEDULER_FILTERS: + policy.check_policy(context, 'share', + 'update_admin_only_metadata') + return self._delete_metadata(req, resource_id, key) + def create_resource(): return wsgi.Resource(ShareController()) diff --git a/manila/db/api.py b/manila/db/api.py index b6878edbda..55c0bc8fbf 100644 --- a/manila/db/api.py +++ b/manila/db/api.py @@ -389,9 +389,9 @@ def share_update(context, share_id, values): return IMPL.share_update(context, share_id, values) -def share_get(context, share_id): +def share_get(context, share_id, **kwargs): """Get share by id.""" - return IMPL.share_get(context, share_id) + return IMPL.share_get(context, share_id, **kwargs) def share_get_all(context, filters=None, sort_key=None, sort_dir=None): @@ -805,17 +805,17 @@ def share_metadata_get_item(context, share_id, key): def share_metadata_delete(context, share_id, key): """Delete the given metadata item.""" - IMPL.share_metadata_delete(context, share_id, key) + return IMPL.share_metadata_delete(context, share_id, key) def share_metadata_update(context, share, metadata, delete): """Update metadata if it exists, otherwise create it.""" - IMPL.share_metadata_update(context, share, metadata, delete) + return IMPL.share_metadata_update(context, share, metadata, delete) def share_metadata_update_item(context, share_id, item): """update meta item containing key and value for given share.""" - IMPL.share_metadata_update_item(context, share_id, item) + return IMPL.share_metadata_update_item(context, share_id, item) ################### diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index 32e5a2e1d0..04fd2abd65 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -3518,8 +3518,8 @@ def share_metadata_get_item(context, share_id, key, session=None): try: row = _share_metadata_get_item(context, share_id, key, session=session) - except exception.ShareMetadataNotFound: - raise exception.ShareMetadataNotFound() + except exception.MetadataItemNotFound: + raise exception.MetadataItemNotFound() result = {} result[row['key']] = row['value'] @@ -3593,7 +3593,7 @@ def _share_metadata_update(context, share_id, metadata, delete, session=None): meta_ref = _share_metadata_get_item(context, share_id, meta_key, session=session) - except exception.ShareMetadataNotFound: + except exception.MetadataItemNotFound: meta_ref = models.ShareMetadata() item.update({"key": meta_key, "share_id": share_id}) @@ -3609,8 +3609,7 @@ def _share_metadata_get_item(context, share_id, key, session=None): first()) if not result: - raise exception.ShareMetadataNotFound(metadata_key=key, - share_id=share_id) + raise exception.MetadataItemNotFound() return result diff --git a/manila/exception.py b/manila/exception.py index f936c81839..204e85124a 100644 --- a/manila/exception.py +++ b/manila/exception.py @@ -598,7 +598,7 @@ class UnmanageInvalidShareSnapshot(InvalidShareSnapshot): "invalid share snapshot: %(reason)s.") -class ShareMetadataNotFound(NotFound): +class MetadataItemNotFound(NotFound): message = _("Metadata item is not found.") diff --git a/manila/policies/shares.py b/manila/policies/shares.py index 345407e1f0..d7d1e9d4cf 100644 --- a/manila/policies/shares.py +++ b/manila/policies/shares.py @@ -598,12 +598,20 @@ shares_policies = [ name=BASE_POLICY_NAME % 'update_share_metadata', check_str=base.SYSTEM_ADMIN_OR_PROJECT_MEMBER, scope_types=['system', 'project'], - description=("Update share metadata."), + description="Update share metadata.", operations=[ { 'method': 'PUT', 'path': '/shares/{share_id}/metadata', - } + }, + { + 'method': 'POST', + 'path': '/shares/{share_id}/metadata/{key}', + }, + { + 'method': 'POST', + 'path': '/shares/{share_id}/metadata', + }, ], deprecated_rule=deprecated_share_update_metadata ), @@ -611,7 +619,7 @@ shares_policies = [ name=BASE_POLICY_NAME % 'delete_share_metadata', check_str=base.SYSTEM_ADMIN_OR_PROJECT_MEMBER, scope_types=['system', 'project'], - description=("Delete share metadata."), + description="Delete share metadata.", operations=[ { 'method': 'DELETE', @@ -624,11 +632,15 @@ shares_policies = [ name=BASE_POLICY_NAME % 'get_share_metadata', check_str=base.SYSTEM_OR_PROJECT_READER, scope_types=['system', 'project'], - description=("Get share metadata."), + description="Get share metadata.", operations=[ { 'method': 'GET', 'path': '/shares/{share_id}/metadata', + }, + { + 'method': 'GET', + 'path': '/shares/{share_id}/metadata/{key}', } ], deprecated_rule=deprecated_share_get_metadata diff --git a/manila/share/api.py b/manila/share/api.py index 41d1ffc36d..34e4cebffc 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -221,7 +221,10 @@ class API(base.Base): az_request_multiple_subnet_support_map=None): """Create new share.""" - self._check_metadata_properties(metadata) + try: + api_common._check_metadata_properties(metadata) + except (exception.InvalidMetadata, exception.InvalidMetadataSize): + raise if snapshot_id is not None: snapshot = self.get_snapshot(context, snapshot_id) @@ -2088,7 +2091,10 @@ class API(base.Base): msg = _("Invalid share access level: %s.") % access_level raise exception.InvalidShareAccess(reason=msg) - self._check_metadata_properties(metadata) + try: + api_common._check_metadata_properties(metadata) + except (exception.InvalidMetadata, exception.InvalidMetadataSize): + raise access_exists = self.db.share_access_check_for_existing_access( ctx, share['id'], access_type, access_to) @@ -2173,17 +2179,6 @@ class API(base.Base): return rule - @policy.wrap_check_policy('share') - def get_share_metadata(self, context, share): - """Get all metadata associated with a share.""" - rv = self.db.share_metadata_get(context, share['id']) - return dict(rv.items()) - - @policy.wrap_check_policy('share') - def delete_share_metadata(self, context, share, key): - """Delete the given metadata item from a share.""" - self.db.share_metadata_delete(context, share['id'], key) - def _validate_scheduler_hints(self, context, share, share_uuids): for uuid in share_uuids: if not uuidutils.is_uuid_like(uuid): @@ -2201,7 +2196,7 @@ class API(base.Base): for uuid in share_uuids: try: result = self.db.share_metadata_get_item(context, uuid, key) - except exception.ShareMetadataNotFound: + except exception.MetadataItemNotFound: item = {key: share['id']} else: existing_uuids = result.get(key, "") @@ -2235,14 +2230,14 @@ class API(base.Base): try: result = self.db.share_metadata_get_item(context, share['id'], key) - except exception.ShareMetadataNotFound: + except exception.MetadataItemNotFound: return share_uuids = result.get(key, "").split(",") for uuid in share_uuids: try: result = self.db.share_metadata_get_item(context, uuid, key) - except exception.ShareMetadataNotFound: + except exception.MetadataItemNotFound: continue new_val_uuids = [val_uuid for val_uuid @@ -2280,72 +2275,17 @@ class API(base.Base): raise exception.ShareSizeExceedsLimit( size=size, limit=quotas['per_share_gigabytes']) - def _check_metadata_properties(self, metadata=None): - if not metadata: - metadata = {} - - for k, v in metadata.items(): - if not k: - msg = _("Metadata property key is blank.") - LOG.warning(msg) - raise exception.InvalidMetadata(message=msg) - if len(k) > 255: - msg = _("Metadata property key is " - "greater than 255 characters.") - LOG.warning(msg) - raise exception.InvalidMetadataSize(message=msg) - if not v: - msg = _("Metadata property value is blank.") - LOG.warning(msg) - raise exception.InvalidMetadata(message=msg) - if len(v) > 1023: - msg = _("Metadata property value is " - "greater than 1023 characters.") - LOG.warning(msg) - raise exception.InvalidMetadataSize(message=msg) - def update_share_access_metadata(self, context, access_id, metadata): """Updates share access metadata.""" - self._check_metadata_properties(metadata) + try: + api_common._check_metadata_properties(metadata) + except exception.InvalidMetadata: + raise exception.InvalidMetadata() + except exception.InvalidMetadataSize: + raise exception.InvalidMetadataSize() return self.db.share_access_metadata_update( context, access_id, metadata) - @policy.wrap_check_policy('share') - def update_share_metadata(self, - context, share, - metadata, ignore_keys=None, - delete=False): - """Updates or creates share metadata. - - If delete is True, metadata items that are not specified in the - `metadata` argument will be deleted. - - Non-admin user may not attempt to create or update admin-only keys. - For example: "__affinity_same_host" or "__affinity_different_host". - These keys will be ignored in update-all method, preserving their - values, unless RBAC policy allows manipluation of this data. - - """ - ignore_keys = ignore_keys or [] - orig_meta = self.get_share_metadata(context, share) - if delete: - _metadata = metadata - for key in ignore_keys: - if key in orig_meta: - _metadata[key] = orig_meta[key] - else: - metadata_copy = metadata.copy() - for key in ignore_keys: - metadata_copy.pop(key, None) - _metadata = orig_meta.copy() - _metadata.update(metadata_copy) - - self._check_metadata_properties(_metadata) - self.db.share_metadata_update(context, share['id'], - _metadata, delete) - - return _metadata - def get_share_network(self, context, share_net_id): return self.db.share_network_get(context, share_net_id) diff --git a/manila/tests/api/v2/test_metadata.py b/manila/tests/api/v2/test_metadata.py new file mode 100644 index 0000000000..4a32c26039 --- /dev/null +++ b/manila/tests/api/v2/test_metadata.py @@ -0,0 +1,137 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from unittest import mock + +import ddt +import webob + +from manila.api.v2 import metadata +from manila import context +from manila import exception +from manila import policy +from manila import test +from manila.tests.api import fakes +from manila.tests import db_utils + + +@ddt.ddt +class MetadataAPITest(test.TestCase): + + def _get_request(self, version="2.65", use_admin_context=True): + req = fakes.HTTPRequest.blank( + '/v2/shares/{resource_id}/metadata', + version=version, use_admin_context=use_admin_context) + return req + + def setUp(self): + super(MetadataAPITest, self).setUp() + self.controller = ( + metadata.MetadataController()) + self.controller.resource_name = 'share' + self.admin_context = context.RequestContext('admin', 'fake', True) + self.member_context = context.RequestContext('fake', 'fake') + self.mock_policy_check = self.mock_object( + policy, 'check_policy', mock.Mock(return_value=True)) + self.resource = db_utils.create_share(size=1) + + def test_create_index_metadata(self): + url = self._get_request() + body = {'metadata': {'test_key1': 'test_v1', 'test_key2': 'test_v2'}} + update = self.controller._create_metadata( + url, self.resource['id'], body=body) + + get = self.controller._index_metadata(url, self.resource['id']) + + self.assertEqual(2, len(get['metadata'])) + self.assertEqual(update['metadata'], get['metadata']) + + @ddt.data(({'metadata': {'key1': 'v1'}}, 'key1'), + ({'metadata': {'test_key1': 'test_v1'}}, 'test_key1'), + ({'metadata': {'key1': 'v2'}}, 'key1')) + @ddt.unpack + def test_update_metadata_item(self, body, key): + url = self._get_request() + update = self.controller._update_metadata_item( + url, self.resource['id'], body=body, key=key) + self.assertEqual(body, update) + + get = self.controller._index_metadata(url, self.resource['id']) + + self.assertEqual(1, len(get)) + self.assertEqual(body['metadata'], get['metadata']) + + @ddt.data({'metadata': {'key1': 'v1', 'key2': 'v2'}}, + {'metadata': {'test_key1': 'test_v1'}}, + {'metadata': {'key1': 'v2'}}) + def test_update_all_metadata(self, body): + url = self._get_request() + update = self.controller._update_all_metadata( + url, self.resource['id'], body=body) + self.assertEqual(body, update) + + get = self.controller._index_metadata(url, self.resource['id']) + + self.assertEqual(len(body['metadata']), len(get['metadata'])) + self.assertEqual(body['metadata'], get['metadata']) + + def test_delete_metadata(self): + body = {'metadata': {'test_key3': 'test_v3', 'testkey': 'testval'}} + url = self._get_request() + self.controller._create_metadata(url, self.resource['id'], body=body) + + self.controller._delete_metadata(url, self.resource['id'], + 'test_key3') + show_result = self.controller._index_metadata(url, self.resource[ + 'id']) + + self.assertEqual(1, len(show_result['metadata'])) + self.assertNotIn('test_key3', show_result['metadata']) + + def test_update_metadata_with_resource_id_not_found(self): + url = self._get_request() + id = 'invalid_id' + body = {'metadata': {'key1': 'v1'}} + + self.assertRaises( + webob.exc.HTTPNotFound, + self.controller._create_metadata, + url, id, body) + + def test_update_metadata_with_body_error(self): + self.assertRaises( + webob.exc.HTTPBadRequest, + self.controller._create_metadata, + self._get_request(), self.resource['id'], + {'metadata_error': {'key1': 'v1'}}) + + @ddt.data({'metadata': {'key1': 'v1', 'key2': None}}, + {'metadata': {None: 'v1', 'key2': 'v2'}}, + {'metadata': {'k' * 256: 'v2'}}, + {'metadata': {'key1': 'v' * 1024}}) + @ddt.unpack + def test_update_metadata_with_invalid_metadata(self, metadata): + self.assertRaises( + webob.exc.HTTPBadRequest, + self.controller._create_metadata, + self._get_request(), self.resource['id'], + {'metadata': metadata}) + + def test_delete_metadata_not_found(self): + body = {'metadata': {'test_key_exist': 'test_v_exist'}} + update = self.controller._update_all_metadata( + self._get_request(), self.resource['id'], body=body) + self.assertEqual(body, update) + self.assertRaises( + exception.MetadataItemNotFound, + self.controller._delete_metadata, + self._get_request(), self.resource['id'], 'key1') diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py index e55f524c02..93abbcb620 100644 --- a/manila/tests/db/sqlalchemy/test_api.py +++ b/manila/tests/db/sqlalchemy/test_api.py @@ -1119,6 +1119,75 @@ class ShareDatabaseAPITestCase(test.TestCase): self.assertEqual(share['is_soft_deleted'], False) + def test_share_metadata_get(self): + metadata = {'a': 'b', 'c': 'd'} + + self.share_1 = db_utils.create_share(size=1) + db_api.share_metadata_update( + self.ctxt, share_id=self.share_1['id'], + metadata=metadata, delete=False) + self.assertEqual( + metadata, db_api.share_metadata_get( + self.ctxt, share_id=self.share_1['id'])) + + def test_share_metadata_get_item(self): + metadata = {'a': 'b', 'c': 'd'} + key = 'a' + shouldbe = {'a': 'b'} + self.share_1 = db_utils.create_share(size=1) + db_api.share_metadata_update( + self.ctxt, share_id=self.share_1['id'], + metadata=metadata, delete=False) + self.assertEqual( + shouldbe, db_api.share_metadata_get_item( + self.ctxt, share_id=self.share_1['id'], + key=key)) + + def test_share_metadata_update(self): + metadata1 = {'a': '1', 'c': '2'} + metadata2 = {'a': '3', 'd': '5'} + should_be = {'a': '3', 'c': '2', 'd': '5'} + self.share_1 = db_utils.create_share(size=1) + db_api.share_metadata_update( + self.ctxt, share_id=self.share_1['id'], + metadata=metadata1, delete=False) + db_api.share_metadata_update( + self.ctxt, share_id=self.share_1['id'], + metadata=metadata2, delete=False) + self.assertEqual( + should_be, db_api.share_metadata_get( + self.ctxt, share_id=self.share_1['id'])) + + def test_share_metadata_update_item(self): + metadata1 = {'a': '1', 'c': '2'} + metadata2 = {'a': '3'} + should_be = {'a': '3', 'c': '2'} + self.share_1 = db_utils.create_share(size=1) + db_api.share_metadata_update( + self.ctxt, share_id=self.share_1['id'], + metadata=metadata1, delete=False) + db_api.share_metadata_update_item( + self.ctxt, share_id=self.share_1['id'], + item=metadata2) + self.assertEqual( + should_be, db_api.share_metadata_get( + self.ctxt, share_id=self.share_1['id'])) + + def test_share_metadata_delete(self): + key = 'a' + metadata = {'a': '1', 'c': '2'} + should_be = {'c': '2'} + self.share_1 = db_utils.create_share(size=1) + db_api.share_metadata_update( + self.ctxt, share_id=self.share_1['id'], + metadata=metadata, delete=False) + db_api.share_metadata_delete( + self.ctxt, share_id=self.share_1['id'], + key=key) + self.assertEqual( + should_be, db_api.share_metadata_get( + self.ctxt, share_id=self.share_1['id'])) + @ddt.ddt class ShareGroupDatabaseAPITestCase(test.TestCase): diff --git a/manila/tests/db_utils.py b/manila/tests/db_utils.py index a5429a46cd..1d6d133f35 100644 --- a/manila/tests/db_utils.py +++ b/manila/tests/db_utils.py @@ -88,7 +88,7 @@ def create_share(**kwargs): 'share_server_id': None, 'user_id': 'fake', 'project_id': 'fake', - 'metadata': {'fake_key': 'fake_value'}, + 'metadata': {}, 'availability_zone': 'fake_availability_zone', 'status': constants.STATUS_CREATING, 'host': 'fake_host', diff --git a/manila/tests/test_exception.py b/manila/tests/test_exception.py index 2da8ae9a27..8c11eadae2 100644 --- a/manila/tests/test_exception.py +++ b/manila/tests/test_exception.py @@ -444,9 +444,9 @@ class ManilaExceptionResponseCode404(test.TestCase): self.assertEqual(404, e.code) self.assertIn(snapshot_id, e.msg) - def test_share_metadata_not_found(self): - # verify response code for exception.ShareMetadataNotFound - e = exception.ShareMetadataNotFound() + def test_metadata_item_not_found(self): + # verify response code for exception.MetadataItemNotFound + e = exception.MetadataItemNotFound() self.assertEqual(404, e.code) def test_security_service_not_found(self):