Add admin only keys to share metadata

In this change, we add admin policy check for metadata keys
"__affinity_same_host" and "__affinity_different_host".
These keys are added an constants, allowing for the addition of other
admin only keys if necessary in future changes.

This change allows for the affinity filter spec[1] implementation without
the additional changes necessary in the metadata spec[2].

Bumps microversion to 2.68.

[1] https://specs.openstack.org/openstack/manila-specs/specs/xena/affinity-antiaffinity-filter.html
[2] https://specs.openstack.org/openstack/manila-specs/specs/wallaby/metadata-for-share-resources.html

Partially-Implements: metadata-for-share-resources
Change-Id: I6ee02cb66727a3c2b425fad1d9d1245d4e9c1b24

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
 
 

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

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,
+            if set(metadata).intersection(set(ignore_keys)):
-                                                        share,
+                try:
-                                                        metadata,
+                    policy.check_policy(
-                                                        delete)
+                        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)
 
 

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
+    )

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
+    ),
 ]
 
 

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'],

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'

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.