Show qos_specs_id based on policy

Right now qos_specs_id is only shown to an admin user
when showing a volume type. This patch changes that to
be based on policy to allow for more flexibility. It
also adds unit tests for showing a volume type
with policy permissions for qos_specs_id as well as
extra_specs.

APIImpact

Change-Id: I4e6e99b8992b6941ba247bee90493cc2adba7f0b
Closes-bug: #1512876

diff --git a/cinder/api/common.py b/cinder/api/common.py
index 17e7dddb4e619e467826638b20afce552614ce16..eb1f1cae305a16d98937ca33005e368c295ba213 100644 (file)
--- a/cinder/api/common.py
+++ b/cinder/api/common.py
@@ -25,7 +25,9 @@ import webob
 
 from cinder.api.openstack import wsgi
 from cinder.api import xmlutil
+from cinder import exception
 from cinder.i18n import _
+import cinder.policy
 from cinder import utils
 
 
@@ -78,6 +80,14 @@ def validate_key_names(key_names_list):
     return True
 
 
+def validate_policy(context, action):
+    try:
+        cinder.policy.enforce_action(context, action)
+        return True
+    except exception.PolicyNotAuthorized:
+        return False
+
+
 def get_pagination_params(params, max_limit=None):
     """Return marker, limit, offset tuple from request.
 

diff --git a/cinder/api/v2/types.py b/cinder/api/v2/types.py
index d6e8ceaeb194d7bc767022a3606ca5ebf14074a4..8a334eba4fbbd60f0f909e29aac87851fe21ea09 100644 (file)
--- a/cinder/api/v2/types.py
+++ b/cinder/api/v2/types.py
@@ -22,14 +22,11 @@ from cinder.api import common
 from cinder.api.openstack import wsgi
 from cinder.api.v2.views import types as views_types
 from cinder.api import xmlutil
-from cinder import context as ctx
 from cinder import exception
 from cinder.i18n import _
 from cinder import utils
 from cinder.volume import volume_types
 
-import cinder.policy
-
 
 def make_voltype(elem):
     elem.set('id')
@@ -61,18 +58,6 @@ class VolumeTypesController(wsgi.Controller):
 
     _view_builder_class = views_types.ViewBuilder
 
-    def _validate_policy(self, context):
-        target = {
-            'project_id': context.project_id,
-            'user_id': context.user_id,
-        }
-        try:
-            action = 'volume_extension:access_types_extra_specs'
-            cinder.policy.enforce(context, action, target)
-            return True
-        except Exception:
-            return False
-
     @wsgi.serializers(xml=VolumeTypesTemplate)
     def index(self, req):
         """Returns the list of volume types."""
@@ -85,9 +70,6 @@ class VolumeTypesController(wsgi.Controller):
         """Return a single volume type item."""
         context = req.environ['cinder.context']
 
-        if not context.is_admin and self._validate_policy(context):
-            context = ctx.get_admin_context()
-
         # get default volume type
         if id is not None and id == 'default':
             vol_type = volume_types.get_default_volume_type()
@@ -134,8 +116,6 @@ class VolumeTypesController(wsgi.Controller):
         # to filters.
         filters = {}
         context = req.environ['cinder.context']
-        if not context.is_admin and self._validate_policy(context):
-            context = ctx.get_admin_context()
         if context.is_admin:
             # Only admin has query access to all volume types
             filters['is_public'] = self._parse_is_public(

diff --git a/cinder/api/v2/views/types.py b/cinder/api/v2/views/types.py
index 00591ae9a6e552aa9895cc9651e57f1fced0dd28..fbf0631110e29da8ddc974ef5effaf928200269d 100644 (file)
--- a/cinder/api/v2/views/types.py
+++ b/cinder/api/v2/views/types.py
@@ -26,9 +26,14 @@ class ViewBuilder(common.ViewBuilder):
                        name=volume_type.get('name'),
                        is_public=volume_type.get('is_public'),
                        description=volume_type.get('description'))
-        if context.is_admin:
-            trimmed['qos_specs_id'] = volume_type.get('qos_specs_id')
+        if common.validate_policy(
+           context,
+           'volume_extension:access_types_extra_specs'):
             trimmed['extra_specs'] = volume_type.get('extra_specs')
+        if common.validate_policy(
+           context,
+           'volume_extension:access_types_qos_specs_id'):
+            trimmed['qos_specs_id'] = volume_type.get('qos_specs_id')
         return trimmed if brief else dict(volume_type=trimmed)
 
     def index(self, request, volume_types):

diff --git a/cinder/tests/unit/api/v2/test_types.py b/cinder/tests/unit/api/v2/test_types.py
index ddec2d39c9430f23b9afed7c62598eb890cbc17a..981ace0273c74d00d992d355fbb938ce1f2b8748 100644 (file)
--- a/cinder/tests/unit/api/v2/test_types.py
+++ b/cinder/tests/unit/api/v2/test_types.py
@@ -16,10 +16,12 @@
 import uuid
 
 from lxml import etree
+import mock
 from oslo_utils import timeutils
 import six
 import webob
 
+import cinder.api.common as common
 from cinder.api.v2 import types
 from cinder.api.v2.views import types as views_types
 from cinder import context
@@ -322,6 +324,103 @@ class VolumeTypesApiTest(test.TestCase):
         )
         self.assertDictMatch(expected_volume_type, output['volume_type'])
 
+    def test_view_builder_show_qos_specs_id_policy(self):
+        with mock.patch.object(common,
+                               'validate_policy',
+                               side_effect=[False, True]):
+            view_builder = views_types.ViewBuilder()
+            now = timeutils.utcnow().isoformat()
+            raw_volume_type = dict(
+                name='new_type',
+                description='new_type_desc',
+                qos_specs_id='new_id',
+                is_public=True,
+                deleted=False,
+                created_at=now,
+                updated_at=now,
+                extra_specs={},
+                deleted_at=None,
+                id=42,
+            )
+
+            request = fakes.HTTPRequest.blank("/v2")
+            output = view_builder.show(request, raw_volume_type)
+
+            self.assertIn('volume_type', output)
+            expected_volume_type = dict(
+                name='new_type',
+                description='new_type_desc',
+                qos_specs_id='new_id',
+                is_public=True,
+                id=42,
+            )
+            self.assertDictMatch(expected_volume_type, output['volume_type'])
+
+    def test_view_builder_show_extra_specs_policy(self):
+        with mock.patch.object(common,
+                               'validate_policy',
+                               side_effect=[True, False]):
+            view_builder = views_types.ViewBuilder()
+            now = timeutils.utcnow().isoformat()
+            raw_volume_type = dict(
+                name='new_type',
+                description='new_type_desc',
+                qos_specs_id='new_id',
+                is_public=True,
+                deleted=False,
+                created_at=now,
+                updated_at=now,
+                extra_specs={},
+                deleted_at=None,
+                id=42,
+            )
+
+            request = fakes.HTTPRequest.blank("/v2")
+            output = view_builder.show(request, raw_volume_type)
+
+            self.assertIn('volume_type', output)
+            expected_volume_type = dict(
+                name='new_type',
+                description='new_type_desc',
+                extra_specs={},
+                is_public=True,
+                id=42,
+            )
+            self.assertDictMatch(expected_volume_type, output['volume_type'])
+
+    def test_view_builder_show_pass_all_policy(self):
+        with mock.patch.object(common,
+                               'validate_policy',
+                               side_effect=[True, True]):
+            view_builder = views_types.ViewBuilder()
+            now = timeutils.utcnow().isoformat()
+            raw_volume_type = dict(
+                name='new_type',
+                description='new_type_desc',
+                qos_specs_id='new_id',
+                is_public=True,
+                deleted=False,
+                created_at=now,
+                updated_at=now,
+                extra_specs={},
+                deleted_at=None,
+                id=42,
+            )
+
+            request = fakes.HTTPRequest.blank("/v2")
+            output = view_builder.show(request, raw_volume_type)
+
+            self.assertIn('volume_type', output)
+            expected_volume_type = dict(
+                name='new_type',
+                description='new_type_desc',
+                qos_specs_id='new_id',
+                extra_specs={},
+                is_public=True,
+                id=42,
+            )
+            self.assertDictMatch(expected_volume_type, output['volume_type'])
+
     def test_view_builder_list(self):
         view_builder = views_types.ViewBuilder()
 

diff --git a/cinder/tests/unit/policy.json b/cinder/tests/unit/policy.json
index 07c1363f5e9b254e8cb21ea511352a2a6497d98a..2ff533cbe3d8d2f971998c8a5db65421116e33ee 100644 (file)
--- a/cinder/tests/unit/policy.json
+++ b/cinder/tests/unit/policy.json
@@ -49,6 +49,8 @@
     "volume_extension:volume_actions:upload_image": "",
     "volume_extension:types_manage": "",
     "volume_extension:types_extra_specs": "",
+    "volume_extension:access_types_qos_specs_id": "rule:admin_api",
+    "volume_extension:access_types_extra_specs": "rule:admin_api",
     "volume_extension:volume_type_access": "",
     "volume_extension:volume_type_access:addProjectAccess": "rule:admin_api",
     "volume_extension:volume_type_access:removeProjectAccess": "rule:admin_api",

diff --git a/etc/cinder/policy.json b/etc/cinder/policy.json
index a7686a579bc095464d75e59246fc4ee91caa595c..02af88bdfef96720c84f9aa227822694edc032d4 100644 (file)
--- a/etc/cinder/policy.json
+++ b/etc/cinder/policy.json
@@ -26,6 +26,7 @@
 
     "volume_extension:types_manage": "rule:admin_api",
     "volume_extension:types_extra_specs": "rule:admin_api",
+    "volume_extension:access_types_qos_specs_id": "rule:admin_api",
     "volume_extension:access_types_extra_specs": "rule:admin_api",
     "volume_extension:volume_type_access": "rule:admin_or_owner",
     "volume_extension:volume_type_access:addProjectAccess": "rule:admin_api",