diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index cdf64cf5de..6b76b69f01 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -175,13 +175,14 @@ REST_API_VERSION_HISTORY = """ * 2.66 - Added filter search by group spec for share group type list. * 2.67 - Added ability to set 'only_host' scheduler hint via the share create API. + * 2.68 - Added admin only capabilities to share metadata API """ # The minimum and maximum versions of the API supported # The default api version request is defined to be the # minimum version of the API supported. _MIN_API_VERSION = "2.0" -_MAX_API_VERSION = "2.67" +_MAX_API_VERSION = "2.68" DEFAULT_API_VERSION = _MIN_API_VERSION diff --git a/manila/api/openstack/rest_api_version_history.rst b/manila/api/openstack/rest_api_version_history.rst index eec55b98b6..8c5fc7eee9 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -370,6 +370,10 @@ user documentation. 2.67 ____ - Added supprot for 'only_host' key in "scheduler_hints" in the request body + Added support for 'only_host' key in "scheduler_hints" in the request body of the POST/shares request. This hint will invoke OnlyHost scheduler filter during share creation. + +2.68 +---- + Added admin only capabilities to share metadata API diff --git a/manila/api/v1/share_metadata.py b/manila/api/v1/share_metadata.py index c74c8b1fd7..493494b1af 100644 --- a/manila/api/v1/share_metadata.py +++ b/manila/api/v1/share_metadata.py @@ -14,16 +14,21 @@ # under the License. from http import client as http_client - +from oslo_log import log import webob from webob import exc from manila.api.openstack import wsgi +from manila.common import constants from manila import exception from manila.i18n import _ +from manila import policy from manila import share +LOG = log.getLogger(__name__) + + class ShareMetadataController(object): """The share metadata API controller for the OpenStack API.""" @@ -58,7 +63,6 @@ class ShareMetadataController(object): share_id, metadata, delete=False) - return {'metadata': new_metadata} def update(self, req, share_id, id, body): @@ -92,19 +96,32 @@ class ShareMetadataController(object): raise exc.HTTPBadRequest(explanation=expl) context = req.environ['manila.context'] - new_metadata = self._update_share_metadata(context, share_id, - metadata, delete=True) + + new_metadata = self._update_share_metadata( + context, share_id, metadata, delete=True) return {'metadata': new_metadata} def _update_share_metadata(self, context, share_id, metadata, delete=False): + ignore_keys = constants.AdminOnlyMetadata.SCHEDULER_FILTERS try: share = self.share_api.get(context, share_id) - return self.share_api.update_share_metadata(context, - share, - metadata, - delete) + if set(metadata).intersection(set(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) + ignore_keys = None + return self.share_api.update_share_metadata( + context, + share, + metadata, + ignore_keys=ignore_keys, + delete=delete) except exception.NotFound: msg = _('share does not exist') raise exc.HTTPNotFound(explanation=msg) @@ -142,10 +159,17 @@ class ShareMetadataController(object): try: share = self.share_api.get(context, share_id) + 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) except exception.NotFound: msg = _('share does not exist') raise exc.HTTPNotFound(explanation=msg) + except exception.PolicyNotAuthorized: + msg = _("Cannot delete admin only metadata.") + LOG.exception(msg) + raise exc.HTTPForbidden(explanation=msg) return webob.Response(status_int=http_client.OK) diff --git a/manila/common/constants.py b/manila/common/constants.py index dcf9502fe4..f14bdd1d38 100644 --- a/manila/common/constants.py +++ b/manila/common/constants.py @@ -294,3 +294,13 @@ class ExtraSpecs(object): } REPLICATION_TYPES = ('writable', 'readable', 'dr') + + +class AdminOnlyMetadata(object): + AFFINITY_KEY = "__affinity_same_host" + ANTI_AFFINITY_KEY = "__affinity_different_host" + + SCHEDULER_FILTERS = ( + AFFINITY_KEY, + ANTI_AFFINITY_KEY + ) diff --git a/manila/policies/shares.py b/manila/policies/shares.py index 833df70eb5..3bd02dd905 100644 --- a/manila/policies/shares.py +++ b/manila/policies/shares.py @@ -212,6 +212,12 @@ deprecated_share_snapshot_update = policy.DeprecatedRule( deprecated_since=versionutils.deprecated.WALLABY ) +deprecated_update_admin_only_metadata = policy.DeprecatedRule( + name=BASE_POLICY_NAME % 'update_admin_only_metadata', + check_str=base.RULE_ADMIN_API, + deprecated_reason=DEPRECATED_REASON, + deprecated_since="YOGA" +) shares_policies = [ policy.DocumentedRuleDefault( @@ -648,6 +654,21 @@ base_snapshot_policies = [ ], deprecated_rule=deprecated_share_snapshot_update ), + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'update_admin_only_metadata', + check_str=base.SYSTEM_ADMIN_OR_PROJECT_ADMIN, + scope_types=['system', 'project'], + description=( + "Update metadata items that are considered \"admin only\" " + "by the service."), + operations=[ + { + 'method': 'PUT', + 'path': '/shares/{share_id}/metadata', + } + ], + deprecated_rule=deprecated_update_admin_only_metadata + ), ] diff --git a/manila/share/api.py b/manila/share/api.py index 8726b0a1b2..4d2b45b860 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -2200,19 +2200,34 @@ class API(base.Base): context, access_id, metadata) @policy.wrap_check_policy('share') - def update_share_metadata(self, context, share, metadata, delete=False): + 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) + _metadata.update(metadata_copy) self._check_metadata_properties(_metadata) self.db.share_metadata_update(context, share['id'], diff --git a/manila/tests/api/v1/test_share_metadata.py b/manila/tests/api/v1/test_share_metadata.py index 1ec64ea9fa..15f45c43b5 100644 --- a/manila/tests/api/v1/test_share_metadata.py +++ b/manila/tests/api/v1/test_share_metadata.py @@ -20,6 +20,7 @@ import webob from manila.api.v1 import share_metadata from manila.api.v1 import shares +from manila.common import constants from manila import context from manila import db from manila.share import api @@ -27,6 +28,8 @@ from manila import test from manila.tests.api import fakes CONF = cfg.CONF +AFFINITY_KEY = constants.AdminOnlyMetadata.AFFINITY_KEY +ANTI_AFFINITY_KEY = constants.AdminOnlyMetadata.ANTI_AFFINITY_KEY @ddt.ddt @@ -60,6 +63,7 @@ class ShareMetaDataTest(test.TestCase): 'key3': 'value3', }, } + self.assertEqual(expected, res_dict) def test_index_nonexistent_share(self): @@ -116,6 +120,51 @@ class ShareMetaDataTest(test.TestCase): self.assertRaises(webob.exc.HTTPNotFound, self.controller.delete, req, self.share_id, 'key6') + @ddt.data((AFFINITY_KEY, '/' + AFFINITY_KEY), + (ANTI_AFFINITY_KEY, '/' + ANTI_AFFINITY_KEY)) + @ddt.unpack + def test_delete_affinities_user(self, key, path): + self.userctxt = context.RequestContext('demo', 'fake', False) + req = fakes.HTTPRequest.blank(self.url + path) + req.method = 'DELETE' + req.content_type = "application/json" + req.environ['manila.context'] = self.userctxt + establish = {key: 'share1'} + db.share_metadata_update( + self.ctxt, self.share_id, establish, delete=False) + + self.assertRaises( + webob.exc.HTTPForbidden, + self.controller.delete, + req, self.share_id, key) + + # test that nothing was deleted + data = db.share_metadata_get(self.userctxt, self.share_id) + if key in data: + res_dict = {'meta': {key: data[key]}} + self.assertEqual(res_dict, {'meta': establish}) + + @ddt.data((AFFINITY_KEY, '/' + AFFINITY_KEY), + (ANTI_AFFINITY_KEY, '/' + ANTI_AFFINITY_KEY)) + @ddt.unpack + def test_delete_affinities_admin(self, key, path): + req = fakes.HTTPRequest.blank(self.url + path) + req.method = 'DELETE' + req.content_type = "application/json" + admin_context = req.environ['manila.context'].elevated() + req.environ['manila.context'] = admin_context + establish = {key: 'share1'} + db.share_metadata_update( + self.ctxt, self.share_id, establish, delete=False) + + self.controller.delete( + req, self.share_id, key) + + # test that key was deleted + data = db.share_metadata_get(self.ctxt, self.share_id) + res_dict = {'meta': data} + self.assertEqual(res_dict, {'meta': self.origin_metadata}) + def test_create(self): req = fakes.HTTPRequest.blank('/v1/share_metadata') req.method = 'POST' @@ -228,6 +277,59 @@ class ShareMetaDataTest(test.TestCase): self.assertRaises(webob.exc.HTTPNotFound, self.controller.update_all, req, '100', body) + @ddt.data({AFFINITY_KEY: 'foo'}, + {ANTI_AFFINITY_KEY: 'foo'}, + {AFFINITY_KEY: 'foo', + ANTI_AFFINITY_KEY: 'bar'}, + {AFFINITY_KEY: 'foo', + ANTI_AFFINITY_KEY: 'bar', + 'foo': 'bar'}) + def test_update_all_affinities_user(self, metadata): + body = {'metadata': metadata} + self.userctxt = context.RequestContext('demo', 'fake', False) + req = fakes.HTTPRequest.blank(self.url) + req.method = 'PUT' + req.content_type = "application/json" + req.environ['manila.context'] = self.userctxt + establish = {AFFINITY_KEY: 'share1'} + db.share_metadata_update( + self.ctxt, self.share_id, establish, delete=False) + before_update_all = db.share_metadata_get(self.userctxt, self.share_id) + + body = {'metadata': metadata} + req.body = jsonutils.dumps(body).encode("utf-8") + self.assertRaises( + webob.exc.HTTPForbidden, + self.controller.update_all, + req, self.share_id, body) + + # test nothing was deleted or updated + after_update_all = db.share_metadata_get(self.userctxt, self.share_id) + self.assertEqual(after_update_all, before_update_all) + + @ddt.data({AFFINITY_KEY: 'foo'}, + {ANTI_AFFINITY_KEY: 'foo'}, + {AFFINITY_KEY: 'foo', + ANTI_AFFINITY_KEY: 'bar'}, + {AFFINITY_KEY: 'foo', + ANTI_AFFINITY_KEY: 'bar', + 'foo': 'bar'}) + def test_update_all_affinities_admin(self, metadata): + req = fakes.HTTPRequest.blank(self.url) + req.method = 'PUT' + req.content_type = "application/json" + admin_context = req.environ['manila.context'].elevated() + req.environ['manila.context'] = admin_context + establish = {AFFINITY_KEY: 'share1'} + db.share_metadata_update( + self.ctxt, self.share_id, establish, delete=False) + + body = {'metadata': metadata} + req.body = jsonutils.dumps(body).encode("utf-8") + res_dict = self.controller.update_all(req, self.share_id, body) + expected = body + self.assertEqual(res_dict, expected) + def test_update_item(self): req = fakes.HTTPRequest.blank(self.url + '/key1') req.method = 'PUT' @@ -313,6 +415,52 @@ class ShareMetaDataTest(test.TestCase): self.controller.update, req, self.share_id, 'bad', body) + @ddt.data((AFFINITY_KEY, '/' + AFFINITY_KEY), + (ANTI_AFFINITY_KEY, '/' + ANTI_AFFINITY_KEY)) + @ddt.unpack + def test_update_item_affinities_user(self, key, path): + self.userctxt = context.RequestContext('demo', 'fake', False) + req = fakes.HTTPRequest.blank(self.url + path) + req.method = 'PUT' + req.content_type = "application/json" + req.environ['manila.context'] = self.userctxt + establish = {AFFINITY_KEY: 'share1'} + db.share_metadata_update( + self.ctxt, self.share_id, establish, delete=False) + + body = {'meta': {key: 'share1,share2'}} + req.body = jsonutils.dumps(body).encode("utf-8") + self.assertRaises( + webob.exc.HTTPForbidden, + self.controller.update, + req, self.share_id, key, body) + + # test that nothing was updated + data = db.share_metadata_get(self.ctxt, self.share_id) + if AFFINITY_KEY in data: + res_dict = {'meta': {AFFINITY_KEY: data[AFFINITY_KEY]}} + self.assertEqual(res_dict, {'meta': establish}) + + @ddt.data((AFFINITY_KEY, '/' + AFFINITY_KEY), + (ANTI_AFFINITY_KEY, '/' + ANTI_AFFINITY_KEY)) + @ddt.unpack + def test_update_item_affinities_admin(self, key, path): + req = fakes.HTTPRequest.blank(self.url + path) + req.method = 'PUT' + req.content_type = "application/json" + admin_context = req.environ['manila.context'].elevated() + req.environ['manila.context'] = admin_context + establish = {AFFINITY_KEY: 'share1'} + db.share_metadata_update( + self.ctxt, self.share_id, establish, delete=False) + + body = {'meta': {key: 'share1,share2'}} + req.body = jsonutils.dumps(body).encode("utf-8") + res_dict = self.controller.update( + req, self.share_id, key, body) + expected = body + self.assertEqual(res_dict, expected) + def test_invalid_metadata_items_on_create(self): req = fakes.HTTPRequest.blank(self.url) req.method = 'POST' diff --git a/releasenotes/notes/add-admin-only-keys-to-share-metadata-5301424ccd9edf8a.yaml b/releasenotes/notes/add-admin-only-keys-to-share-metadata-5301424ccd9edf8a.yaml new file mode 100644 index 0000000000..0b870f7d82 --- /dev/null +++ b/releasenotes/notes/add-admin-only-keys-to-share-metadata-5301424ccd9edf8a.yaml @@ -0,0 +1,8 @@ +--- + +fixes: + - | + User specified scheduler hints such as "affinity_same_host" and + "affinity_different_host" are stored as share metadata. These are + stored as admin-only metadata keys that cannot be deleted or + manipulated by nonadmin users.