Merge "Update policies related to user visible extra specs"

cinder/policies/type_extra_specs.py

@@ -31,17 +31,29 @@ GET_POLICY = "volume_extension:types_extra_specs:show"
 READ_SENSITIVE_POLICY = "volume_extension:types_extra_specs:read_sensitive"
 UPDATE_POLICY = "volume_extension:types_extra_specs:update"
 
+deprecated_get_all_policy = base.CinderDeprecatedRule(
+    name=GET_ALL_POLICY,
+    check_str=""
+)
+
+deprecated_get_policy = base.CinderDeprecatedRule(
+    name=GET_POLICY,
+    check_str=""
+)
+
 type_extra_specs_policies = [
     policy.DocumentedRuleDefault(
         name=GET_ALL_POLICY,
-        check_str="",
+        check_str=base.SYSTEM_READER_OR_PROJECT_READER,
         description="List type extra specs.",
         operations=[
             {
                 'method': 'GET',
                 'path': '/types/{type_id}/extra_specs'
             }
-        ]),
+        ],
+        deprecated_rule=deprecated_get_all_policy,
+    ),
     policy.DocumentedRuleDefault(
         name=CREATE_POLICY,
         check_str=base.RULE_ADMIN_API,
@@ -51,17 +63,20 @@ type_extra_specs_policies = [
                 'method': 'POST',
                 'path': '/types/{type_id}/extra_specs'
             }
-        ]),
+        ]
+    ),
     policy.DocumentedRuleDefault(
         name=GET_POLICY,
-        check_str="",
+        check_str=base.SYSTEM_READER_OR_PROJECT_READER,
         description="Show one specified type extra specs.",
         operations=[
             {
                 'method': 'GET',
                 'path': '/types/{type_id}/extra_specs/{extra_spec_key}'
             }
-        ]),
+        ],
+        deprecated_rule=deprecated_get_policy,
+    ),
     policy.DocumentedRuleDefault(
         name=READ_SENSITIVE_POLICY,
         check_str=base.RULE_ADMIN_API,
@@ -87,7 +102,8 @@ type_extra_specs_policies = [
                 'method': 'GET',
                 'path': '/types/{type_id}/extra_specs/{extra_spec_key}'
             }
-        ]),
+        ]
+    ),
     policy.DocumentedRuleDefault(
         name=UPDATE_POLICY,
         check_str=base.RULE_ADMIN_API,
@@ -97,7 +113,8 @@ type_extra_specs_policies = [
                 'method': 'PUT',
                 'path': '/types/{type_id}/extra_specs/{extra_spec_key}'
             }
-        ]),
+        ]
+    ),
     policy.DocumentedRuleDefault(
         name=DELETE_POLICY,
         check_str=base.RULE_ADMIN_API,
@@ -107,7 +124,8 @@ type_extra_specs_policies = [
                 'method': 'DELETE',
                 'path': '/types/{type_id}/extra_specs/{extra_spec_key}'
             }
-        ]),
+        ]
+    ),
 ]
 
 

cinder/policies/volume_type.py

@@ -50,6 +50,10 @@ deprecated_manage_policy = base.CinderDeprecatedRule(
                        'policies that separately govern POST, PUT, and DELETE '
                        'operations.'),
 )
+deprecated_extra_spec_policy = base.CinderDeprecatedRule(
+    name=EXTRA_SPEC_POLICY,
+    check_str=base.RULE_ADMIN_API
+)
 deprecated_encryption_create_policy = base.CinderDeprecatedRule(
     name=CREATE_ENCRYPTION_POLICY,
     # TODO: change to base.RULE_ADMIN_API in Yoga & remove dep_reason
@@ -150,7 +154,7 @@ volume_type_policies = [
     ),
     policy.DocumentedRuleDefault(
         name=EXTRA_SPEC_POLICY,
-        check_str=base.RULE_ADMIN_API,
+        check_str=base.SYSTEM_READER_OR_PROJECT_READER,
         description=(
             "Include the volume type's extra_specs attribute in the volume "
             "type list or show requests.  The ability to make these calls "
@@ -164,7 +168,9 @@ volume_type_policies = [
                 'method': 'GET',
                 'path': '/types'
             }
-        ]),
+        ],
+        deprecated_rule=deprecated_extra_spec_policy,
+    ),
     policy.DocumentedRuleDefault(
         name=QOS_POLICY,
         check_str=base.RULE_ADMIN_API,

cinder/tests/unit/api/contrib/test_types_extra_specs.py

@@ -132,7 +132,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
         self.mock_object(cinder.db, 'volume_type_extra_specs_delete')
 
         self.assertEqual(0, len(self.notifier.notifications))
-        req = fakes.HTTPRequest.blank(self.api_path + '/key5')
+        req = fakes.HTTPRequest.blank(self.api_path + '/key5',
+                                      use_admin_context=True)
         self.controller.delete(req, fake.VOLUME_ID, 'key5')
         self.assertEqual(1, len(self.notifier.notifications))
         self.assertIn('created_at', self.notifier.notifications[0]['payload'])
@@ -144,7 +145,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
                          side_effect=exception.VolumeTypeExtraSpecsNotFound(
                              "Not Found"))
 
-        req = fakes.HTTPRequest.blank(self.api_path + '/key6')
+        req = fakes.HTTPRequest.blank(self.api_path + '/key6',
+                                      use_admin_context=True)
         self.assertRaises(exception.VolumeTypeExtraSpecsNotFound,
                           self.controller.delete, req, fake.VOLUME_ID, 'key6')
 
@@ -155,7 +157,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
         body = {"extra_specs": {"key1": "value1"}}
 
         self.assertEqual(0, len(self.notifier.notifications))
-        req = fakes.HTTPRequest.blank(self.api_path)
+        req = fakes.HTTPRequest.blank(self.api_path,
+                                      use_admin_context=True)
         res_dict = self.controller.create(req, fake.VOLUME_ID, body=body)
         self.assertEqual(1, len(self.notifier.notifications))
         self.assertIn('created_at', self.notifier.notifications[0]['payload'])
@@ -179,7 +182,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
         body = {"extra_specs": {"image_service:store_id": "cheap"}}
 
         self.assertEqual(0, len(self.notifier.notifications))
-        req = fakes.HTTPRequest.blank(self.api_path)
+        req = fakes.HTTPRequest.blank(self.api_path,
+                                      use_admin_context=True)
         res_dict = self.controller.create(req, fake.VOLUME_ID, body=body)
         self.assertEqual(1, len(self.notifier.notifications))
         self.assertIn('created_at', self.notifier.notifications[0]['payload'])
@@ -199,7 +203,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
             }]
         }
         body = {"extra_specs": {"image_service:store_id": "fast"}}
-        req = fakes.HTTPRequest.blank(self.api_path)
+        req = fakes.HTTPRequest.blank(self.api_path,
+                                      use_admin_context=True)
         self.assertRaises(cinder.exception.GlanceStoreNotFound,
                           self.controller.create,
                           req, fake.VOLUME_ID, body=body)
@@ -216,7 +221,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
             }]
         }
         body = {"extra_specs": {"image_service:store_id": "read_only_store"}}
-        req = fakes.HTTPRequest.blank(self.api_path)
+        req = fakes.HTTPRequest.blank(self.api_path,
+                                      use_admin_context=True)
         self.assertRaises(cinder.exception.GlanceStoreReadOnly,
                           self.controller.create,
                           req, fake.VOLUME_ID, body=body)
@@ -236,7 +242,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
 
         self.assertEqual(0, len(self.notifier.notifications))
 
-        req = fakes.HTTPRequest.blank(self.api_path)
+        req = fakes.HTTPRequest.blank(self.api_path,
+                                      use_admin_context=True)
         res_dict = self.controller.create(req, fake.VOLUME_ID, body=body)
         self.assertEqual(1, len(self.notifier.notifications))
         self.assertEqual('value1',
@@ -259,7 +266,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
 
         self.assertEqual(0, len(self.notifier.notifications))
 
-        req = fakes.HTTPRequest.blank(self.api_path)
+        req = fakes.HTTPRequest.blank(self.api_path,
+                                      use_admin_context=True)
         res_dict = self.controller.create(req, fake.VOLUME_ID, body=body)
         self.assertEqual(1, len(self.notifier.notifications))
         self.assertEqual('value1',
@@ -290,7 +298,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
 
         self.assertEqual(0, len(self.notifier.notifications))
         req = fakes.HTTPRequest.blank(
-            self.api_path + "/image_service:store_id")
+            self.api_path + "/image_service:store_id",
+            use_admin_context=True)
         res_dict = self.controller.update(req, fake.VOLUME_ID,
                                           "image_service:store_id",
                                           body=body)
@@ -321,7 +330,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
 
         self.assertEqual(0, len(self.notifier.notifications))
         req = fakes.HTTPRequest.blank(
-            self.api_path + "/image_service:store_id")
+            self.api_path + "/image_service:store_id",
+            use_admin_context=True)
         self.assertRaises(cinder.exception.GlanceStoreNotFound,
                           self.controller.update,
                           req, fake.VOLUME_ID,
@@ -349,7 +359,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
 
         self.assertEqual(0, len(self.notifier.notifications))
         req = fakes.HTTPRequest.blank(
-            self.api_path + "/image_service:store_id")
+            self.api_path + "/image_service:store_id",
+            use_admin_context=True)
         self.assertRaises(cinder.exception.GlanceStoreReadOnly,
                           self.controller.update,
                           req, fake.VOLUME_ID,
@@ -363,7 +374,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
         body = {"key1": "value1"}
 
         self.assertEqual(0, len(self.notifier.notifications))
-        req = fakes.HTTPRequest.blank(self.api_path + '/key1')
+        req = fakes.HTTPRequest.blank(self.api_path + '/key1',
+                                      use_admin_context=True)
         res_dict = self.controller.update(req, fake.VOLUME_ID, 'key1',
                                           body=body)
         self.assertEqual(1, len(self.notifier.notifications))
@@ -378,7 +390,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
                          return_create_volume_type_extra_specs)
         body = {"key1": "value1", "key2": "value2"}
 
-        req = fakes.HTTPRequest.blank(self.api_path + '/key1')
+        req = fakes.HTTPRequest.blank(self.api_path + '/key1',
+                                      use_admin_context=True)
         self.assertRaises(exception.ValidationError, self.controller.update,
                           req, fake.VOLUME_ID, 'key1', body=body)
 
@@ -388,13 +401,15 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
                          return_create_volume_type_extra_specs)
         body = {"key1": "value1"}
 
-        req = fakes.HTTPRequest.blank(self.api_path + '/bad')
+        req = fakes.HTTPRequest.blank(self.api_path + '/bad',
+                                      use_admin_context=True)
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
                           req, fake.VOLUME_ID, 'bad', body=body)
 
     def _extra_specs_empty_update(self, body):
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s/extra_specs' % (
-            fake.PROJECT_ID, fake.VOLUME_TYPE_ID))
+            fake.PROJECT_ID, fake.VOLUME_TYPE_ID),
+            use_admin_context=True)
         req.method = 'POST'
 
         self.assertRaises(exception.ValidationError,
@@ -409,7 +424,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
 
     def _extra_specs_create_bad_body(self, body):
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s/extra_specs' % (
-            fake.PROJECT_ID, fake.VOLUME_TYPE_ID))
+            fake.PROJECT_ID, fake.VOLUME_TYPE_ID),
+            use_admin_context=True)
         req.method = 'POST'
 
         self.assertRaises(exception.ValidationError,
@@ -440,17 +456,20 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
                          'volume_type_extra_specs_update_or_create',
                          return_create_volume_type_extra_specs)
         body = {"extra_specs": {"key1": "value1"}}
-        req = fakes.HTTPRequest.blank(self.api_path)
+        req = fakes.HTTPRequest.blank(self.api_path,
+                                      use_admin_context=True)
         with mock.patch.object(
                 cinder.db,
                 'volume_get_all',
                 return_value=['a']):
             req = fakes.HTTPRequest.blank('/v3/%s/types/%s/extra_specs' % (
-                fake.PROJECT_ID, fake.VOLUME_TYPE_ID))
+                fake.PROJECT_ID, fake.VOLUME_TYPE_ID),
+                use_admin_context=True)
             req.method = 'POST'
 
             body = {"extra_specs": {"key1": "value1"}}
-            req = fakes.HTTPRequest.blank(self.api_path)
+            req = fakes.HTTPRequest.blank(self.api_path,
+                                          use_admin_context=True)
             self.assertRaises(webob.exc.HTTPBadRequest,
                               self.controller.create,
                               req,
@@ -462,7 +481,8 @@ class VolumeTypesExtraSpecsTest(test.TestCase):
               {'extra_specs': {'     ': 'a'}})
     def test_create_with_invalid_extra_specs(self, body):
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s/extra_specs' % (
-            fake.PROJECT_ID, fake.VOLUME_TYPE_ID))
+            fake.PROJECT_ID, fake.VOLUME_TYPE_ID),
+            use_admin_context=True)
         req.method = 'POST'
 
         self.assertRaises(exception.ValidationError,

cinder/tests/unit/api/contrib/test_types_manage.py

@@ -151,7 +151,8 @@ class VolumeTypesManageApiTest(test.TestCase):
                          return_volume_types_destroy)
 
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
+            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE),
+            use_admin_context=True)
         self.assertEqual(0, len(self.notifier.notifications))
         self.controller._delete(req, DEFAULT_VOLUME_TYPE)
         self.assertEqual(1, len(self.notifier.notifications))
@@ -164,7 +165,8 @@ class VolumeTypesManageApiTest(test.TestCase):
 
         self.assertEqual(0, len(self.notifier.notifications))
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, NOT_FOUND_VOLUME_TYPE))
+            fake.PROJECT_ID, NOT_FOUND_VOLUME_TYPE),
+            use_admin_context=True)
         self.assertRaises(exception.VolumeTypeNotFound,
                           self.controller._delete, req, NOT_FOUND_VOLUME_TYPE)
         self.assertEqual(1, len(self.notifier.notifications))
@@ -175,7 +177,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         self.mock_object(volume_types, 'destroy',
                          return_volume_types_with_volumes_destroy)
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
+            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE),
+            use_admin_context=True)
         self.assertEqual(0, len(self.notifier.notifications))
         self.controller._delete(req, DEFAULT_VOLUME_TYPE)
         self.assertEqual(1, len(self.notifier.notifications))
@@ -217,7 +220,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         body = {"volume_type": {"name": "vol_type_1",
                                 "os-volume-type-access:is_public": True,
                                 "extra_specs": {"key1": "value1"}}}
-        req = fakes.HTTPRequest.blank('/v3/%s/types' % fake.PROJECT_ID)
+        req = fakes.HTTPRequest.blank('/v3/%s/types' % fake.PROJECT_ID,
+                                      use_admin_context=True)
 
         self.assertEqual(0, len(self.notifier.notifications))
         res_dict = self.controller._create(req, body=body)
@@ -242,7 +246,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         body = {"volume_type": {"name": "vol_type_1",
                                 "description": type_description,
                                 "extra_specs": {"key1": "value1"}}}
-        req = fakes.HTTPRequest.blank('/v3/%s/types' % fake.PROJECT_ID)
+        req = fakes.HTTPRequest.blank('/v3/%s/types' % fake.PROJECT_ID,
+                                      use_admin_context=True)
 
         res_dict = self.controller._create(req, body=body)
 
@@ -274,7 +279,8 @@ class VolumeTypesManageApiTest(test.TestCase):
 
         body = {"volume_type": {"name": "vol_type_1",
                                 "extra_specs": {"key1": "value1"}}}
-        req = fakes.HTTPRequest.blank('/v3/%s/types' % fake.PROJECT_ID)
+        req = fakes.HTTPRequest.blank('/v3/%s/types' % fake.PROJECT_ID,
+                                      use_admin_context=True)
         self.assertRaises(webob.exc.HTTPConflict,
                           self.controller._create, req, body=body)
 
@@ -382,7 +388,8 @@ class VolumeTypesManageApiTest(test.TestCase):
             DEFAULT_VOLUME_TYPE, is_public=False)
         body = {"volume_type": {"is_public": False}}
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
+            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE),
+            use_admin_context=True)
         req.method = 'PUT'
 
         self.assertEqual(0, len(self.notifier.notifications))
@@ -407,7 +414,8 @@ class VolumeTypesManageApiTest(test.TestCase):
             mock_get, mock_update):
         body = {"volume_type": {"is_public": is_public}}
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
+            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE),
+            use_admin_context=True)
         req.method = 'PUT'
         ctxt = context.RequestContext(fake.USER_ID, fake.PROJECT_ID, True)
         req.environ['cinder.context'] = ctxt
@@ -430,7 +438,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         type_description = ""
         body = {"volume_type": {"description": type_description}}
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
+            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE),
+            use_admin_context=True)
         req.method = 'PUT'
         resp = self.controller._update(req, DEFAULT_VOLUME_TYPE, body=body)
         self._check_test_results(resp,
@@ -466,7 +475,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         body = {"volume_type": {"name": "vol_type_1_1",
                                 "description": "vol_type_desc_1_1"}}
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, NOT_FOUND_VOLUME_TYPE))
+            fake.PROJECT_ID, NOT_FOUND_VOLUME_TYPE),
+            use_admin_context=True)
         req.method = 'PUT'
 
         self.assertEqual(0, len(self.notifier.notifications))
@@ -486,7 +496,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         body = {"volume_type": {"name": "vol_type_1_1",
                                 "description": "vol_type_desc_1_1"}}
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
+            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE),
+            use_admin_context=True)
         req.method = 'PUT'
 
         self.assertEqual(0, len(self.notifier.notifications))
@@ -498,7 +509,8 @@ class VolumeTypesManageApiTest(test.TestCase):
     def test_update_no_name_no_description(self):
         body = {"volume_type": {}}
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
+            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE),
+            use_admin_context=True)
         req.method = 'PUT'
 
         self.assertRaises(webob.exc.HTTPBadRequest,
@@ -509,7 +521,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         body = {"volume_type": {"name": "  ",
                                 "description": "something"}}
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
+            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE),
+            use_admin_context=True)
         req.method = 'PUT'
 
         self.assertRaises(webob.exc.HTTPBadRequest,
@@ -554,7 +567,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         updated_desc = "%s_%s" % (desc, UPDATE_DESC_ONLY_TYPE)
         body = {"volume_type": {"description": updated_desc}}
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, UPDATE_DESC_ONLY_TYPE))
+            fake.PROJECT_ID, UPDATE_DESC_ONLY_TYPE),
+            use_admin_context=True)
         req.method = 'PUT'
 
         self.assertEqual(0, len(self.notifier.notifications))
@@ -577,7 +591,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         updated_desc = "%s_%s" % (desc, DEFAULT_VOLUME_TYPE)
         body = {"volume_type": {"is_public": is_public}}
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
+            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE),
+            use_admin_context=True)
         req.method = 'PUT'
 
         self.assertEqual(0, len(self.notifier.notifications))
@@ -614,7 +629,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         # first attempt fail
         body = {"volume_type": {"name": name}}
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, UPDATE_NAME_AFTER_DELETE_TYPE))
+            fake.PROJECT_ID, UPDATE_NAME_AFTER_DELETE_TYPE),
+            use_admin_context=True)
         req.method = 'PUT'
 
         self.assertEqual(0, len(self.notifier.notifications))
@@ -629,7 +645,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         self.mock_object(volume_types, 'destroy',
                          return_volume_types_destroy)
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, UPDATE_NAME_AFTER_DELETE_TYPE))
+            fake.PROJECT_ID, UPDATE_NAME_AFTER_DELETE_TYPE),
+            use_admin_context=True)
         self.assertEqual(0, len(self.notifier.notifications))
         self.controller._delete(req, UPDATE_NAME_AFTER_DELETE_TYPE)
         self.assertEqual(1, len(self.notifier.notifications))
@@ -638,7 +655,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         mock_update.side_effect = mock.MagicMock()
         body = {"volume_type": {"name": updated_name}}
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, UPDATE_NAME_AFTER_DELETE_TYPE))
+            fake.PROJECT_ID, UPDATE_NAME_AFTER_DELETE_TYPE),
+            use_admin_context=True)
         req.method = 'PUT'
 
         self.notifier.reset()
@@ -702,7 +720,8 @@ class VolumeTypesManageApiTest(test.TestCase):
     def test_update_with_name_null(self):
         body = {"volume_type": {"name": None}}
         req = fakes.HTTPRequest.blank('/v3/%s/types/%s' % (
-            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
+            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE),
+            use_admin_context=True)
         req.method = 'PUT'
 
         self.assertRaises(webob.exc.HTTPBadRequest,

cinder/tests/unit/policies/test_type_extra_specs.py

@@ -0,0 +1,285 @@
+# Copyright (c) 2021 Red Hat, Inc.
+# All Rights Reserved.
+#
+#    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.
+
+import ddt
+
+from cinder.api.contrib import types_extra_specs
+from cinder.api import microversions as mv
+from cinder.api.v3 import types
+from cinder.policies import type_extra_specs as policy
+from cinder.policies import volume_type as type_policy
+from cinder.tests.unit.api import fakes as fake_api
+from cinder.tests.unit.policies import base
+from cinder.tests.unit import utils as test_utils
+
+
+@ddt.ddt
+class TypeExtraSpecsPolicyTest(base.BasePolicyTest):
+    """Verify extra specs policy settings for the types API"""
+
+    # Deprecated check_str="" allows anyone to read extra specs
+    authorized_readers = [
+        'legacy_admin',
+        'legacy_owner',
+        'system_admin',
+        'project_admin',
+        'project_member',
+        'project_reader',
+        'project_foo',
+        'system_member',
+        'system_reader',
+        'system_foo',
+        'other_project_member',
+        'other_project_reader',
+    ]
+
+    unauthorized_readers = [
+    ]
+
+    authorized_admins = [
+        'legacy_admin',
+        'system_admin',
+        'project_admin',
+    ]
+
+    unauthorized_admins = [
+        'legacy_owner',
+        'system_member',
+        'system_reader',
+        'system_foo',
+        'project_member',
+        'project_reader',
+        'project_foo',
+        'other_project_member',
+        'other_project_reader',
+    ]
+
+    unauthorized_exceptions = []
+
+    # Basic policy test is without enforcing scope (which cinder doesn't
+    # yet support) and deprecated rules enabled.
+    def setUp(self, enforce_scope=False, enforce_new_defaults=False,
+              *args, **kwargs):
+        super().setUp(enforce_scope, enforce_new_defaults, *args, **kwargs)
+        self.controller = types_extra_specs.VolumeTypeExtraSpecsController()
+        self.api_path = '/v3/%s/types' % (self.project_id)
+        self.api_version = mv.BASE_VERSION
+
+    @ddt.data(*base.all_users)
+    def test_get_all_policy(self, user_id):
+        vol_type = test_utils.create_volume_type(self.project_admin_context,
+                                                 testcase_instance=self,
+                                                 name='fake_vol_type',
+                                                 extra_specs={'foo': 'bar'})
+        rule_name = policy.GET_ALL_POLICY
+        url = '%s/%s/extra_specs' % (self.api_path, vol_type.id)
+        req = fake_api.HTTPRequest.blank(url, version=self.api_version)
+        self.common_policy_check(user_id,
+                                 self.authorized_readers,
+                                 self.unauthorized_readers,
+                                 self.unauthorized_exceptions,
+                                 rule_name,
+                                 self.controller.index,
+                                 req,
+                                 type_id=vol_type.id)
+
+    @ddt.data(*base.all_users)
+    def test_get_policy(self, user_id):
+        vol_type = test_utils.create_volume_type(self.project_admin_context,
+                                                 testcase_instance=self,
+                                                 name='fake_vol_type',
+                                                 extra_specs={'foo': 'bar'})
+        rule_name = policy.GET_POLICY
+        url = '%s/%s/extra_specs/foo' % (self.api_path, vol_type.id)
+        req = fake_api.HTTPRequest.blank(url, version=self.api_version)
+
+        # Relax the READ_SENSITIVE_POLICY policy so that any user is able
+        # to "see" the spec.
+        self.policy.set_rules({policy.READ_SENSITIVE_POLICY: ""},
+                              overwrite=False)
+
+        self.common_policy_check(user_id,
+                                 self.authorized_readers,
+                                 self.unauthorized_readers,
+                                 self.unauthorized_exceptions,
+                                 rule_name,
+                                 self.controller.show,
+                                 req,
+                                 type_id=vol_type.id,
+                                 id='foo')
+
+    @ddt.data(*base.all_users)
+    def test_create_policy(self, user_id):
+        vol_type = test_utils.create_volume_type(self.project_admin_context,
+                                                 testcase_instance=self,
+                                                 name='fake_vol_type')
+        rule_name = policy.CREATE_POLICY
+        url = '%s/%s/extra_specs' % (self.api_path, vol_type.id)
+        req = fake_api.HTTPRequest.blank(url, version=self.api_version)
+        req.method = 'POST'
+        body = {
+            "extra_specs": {
+                "foo": "bar",
+            }
+        }
+
+        self.common_policy_check(user_id,
+                                 self.authorized_admins,
+                                 self.unauthorized_admins,
+                                 self.unauthorized_exceptions,
+                                 rule_name,
+                                 self.controller.create,
+                                 req,
+                                 type_id=vol_type.id,
+                                 body=body)
+
+    @ddt.data(*base.all_users)
+    def test_update_policy(self, user_id):
+        vol_type = test_utils.create_volume_type(self.project_admin_context,
+                                                 testcase_instance=self,
+                                                 name='fake_vol_type',
+                                                 extra_specs={'foo': 'bar'})
+        rule_name = policy.UPDATE_POLICY
+        url = '%s/%s/extra_specs/foo' % (self.api_path, vol_type.id)
+        req = fake_api.HTTPRequest.blank(url, version=self.api_version)
+        req.method = 'PUT'
+        body = {"foo": "zap"}
+
+        self.common_policy_check(user_id,
+                                 self.authorized_admins,
+                                 self.unauthorized_admins,
+                                 self.unauthorized_exceptions,
+                                 rule_name,
+                                 self.controller.update,
+                                 req,
+                                 type_id=vol_type.id,
+                                 id='foo',
+                                 body=body)
+
+    @ddt.data(*base.all_users)
+    def test_delete_policy(self, user_id):
+        vol_type = test_utils.create_volume_type(self.project_admin_context,
+                                                 testcase_instance=self,
+                                                 name='fake_vol_type',
+                                                 extra_specs={'foo': 'bar'})
+        rule_name = policy.DELETE_POLICY
+        url = '%s/%s/extra_specs/foo' % (self.api_path, vol_type.id)
+        req = fake_api.HTTPRequest.blank(url, version=self.api_version)
+        req.method = 'DELETE'
+
+        self.common_policy_check(user_id,
+                                 self.authorized_admins,
+                                 self.unauthorized_admins,
+                                 self.unauthorized_exceptions,
+                                 rule_name,
+                                 self.controller.delete,
+                                 req,
+                                 type_id=vol_type.id,
+                                 id='foo')
+
+    @ddt.data(*base.all_users)
+    def test_read_sensitive_policy(self, user_id):
+        # The 'multiattach' extra spec is user visible, and the
+        # 'sensitive' extra spec should not be user visible.
+        extra_specs = {
+            'multiattach': '<is> True',
+            'sensitive': 'secret',
+        }
+        vol_type = test_utils.create_volume_type(self.project_admin_context,
+                                                 testcase_instance=self,
+                                                 name='fake_vol_type',
+                                                 extra_specs=extra_specs)
+        rule_name = policy.READ_SENSITIVE_POLICY
+        url = '%s/%s' % (self.api_path, vol_type.id)
+        req = fake_api.HTTPRequest.blank(url, version=self.api_version)
+
+        # Relax these policies in order to get past those checks.
+        self.policy.set_rules({type_policy.GET_POLICY: ""},
+                              overwrite=False)
+        self.policy.set_rules({type_policy.EXTRA_SPEC_POLICY: ""},
+                              overwrite=False)
+
+        # With the relaxed policies, all users are authorized because
+        # failing the READ_SENSITIVE_POLICY policy check is not fatal.
+        authorized_users = [user_id]
+        unauthorized_users = []
+
+        controller = types.VolumeTypesController()
+        response = self.common_policy_check(user_id,
+                                            authorized_users,
+                                            unauthorized_users,
+                                            self.unauthorized_exceptions,
+                                            rule_name,
+                                            controller.show,
+                                            req,
+                                            id=vol_type.id)
+
+        if user_id in self.authorized_admins:
+            # Admins should see all extra specs
+            expected = extra_specs
+        else:
+            # Non-admins should only see user visible extra specs
+            expected = {'multiattach': '<is> True'}
+        self.assertDictEqual(expected, response['volume_type']['extra_specs'])
+
+
+class TypeExtraSpecsPolicySecureRbacTest(TypeExtraSpecsPolicyTest):
+    authorized_readers = [
+        'legacy_admin',
+        'system_admin',
+        'project_admin',
+        'project_member',
+        'project_reader',
+        'system_member',
+        'system_reader',
+        'other_project_member',
+        'other_project_reader',
+    ]
+
+    unauthorized_readers = [
+        # These are unauthorized because they don't have the reader role
+        'legacy_owner',
+        'project_foo',
+        'system_foo',
+    ]
+
+    # NOTE(Xena): The authorized_admins and unauthorized_admins are the same
+    # as the TypeExtraSpecsPolicyTest. This is because in Xena the "admin only"
+    # rules are the legacy RULE_ADMIN_API. This will change in Yoga, when
+    # RULE_ADMIN_API will be deprecated in favor of the SYSTEM_ADMIN rule that
+    # is scope based.
+    authorized_admins = [
+        'legacy_admin',
+        'system_admin',
+        'project_admin',
+    ]
+
+    unauthorized_admins = [
+        'legacy_owner',
+        'system_member',
+        'system_reader',
+        'system_foo',
+        'project_member',
+        'project_reader',
+        'project_foo',
+        'other_project_member',
+        'other_project_reader',
+    ]
+
+    def setUp(self, *args, **kwargs):
+        # Test secure RBAC by disabling deprecated policy rules (scope
+        # is still not enabled).
+        super().setUp(enforce_scope=False, enforce_new_defaults=True,
+                      *args, **kwargs)

cinder/tests/unit/policies/test_volume_type.py

@@ -25,6 +25,7 @@ from cinder.tests.unit.api import fakes as fake_api
 from cinder.tests.unit import fake_constants
 from cinder.tests.unit.policies import base
 from cinder.tests.unit.policies import test_base
+from cinder.tests.unit import utils as test_utils
 
 
 @ddt.ddt
@@ -75,9 +76,11 @@ class VolumeTypePolicyTest(base.BasePolicyTest):
 
     @ddt.data(*base.all_users)
     def test_type_get_policy(self, user_id):
+        vol_type = test_utils.create_volume_type(self.project_admin_context,
+                                                 testcase_instance=self,
+                                                 name='fake_vol_type')
         rule_name = type_policy.GET_POLICY
-        # the default type is guaranteed to exist
+        url = '%s/%s' % (self.api_path, vol_type.id)
-        url = self.api_path + '/default'
         req = fake_api.HTTPRequest.blank(url, version=self.api_version)
         self.common_policy_check(user_id,
                                  self.authorized_readers,
@@ -86,7 +89,48 @@ class VolumeTypePolicyTest(base.BasePolicyTest):
                                  rule_name,
                                  self.controller.show,
                                  req,
-                                 'default')
+                                 id=vol_type.id)
+
+    @ddt.data(*base.all_users)
+    def test_extra_spec_policy(self, user_id):
+        vol_type = test_utils.create_volume_type(
+            self.project_admin_context,
+            testcase_instance=self,
+            name='fake_vol_type',
+            extra_specs={'multiattach': '<is> True'})
+        rule_name = type_policy.EXTRA_SPEC_POLICY
+        url = '%s/%s' % (self.api_path, vol_type.id)
+        req = fake_api.HTTPRequest.blank(url, version=self.api_version)
+
+        # Relax the GET_POLICY in order to get past that check.
+        self.policy.set_rules({type_policy.GET_POLICY: ""},
+                              overwrite=False)
+
+        # With the relaxed GET_POLICY, all users are authorized because
+        # failing the policy check is not fatal.
+        authorized_readers = [user_id]
+        unauthorized_readers = []
+
+        response = self.common_policy_check(user_id,
+                                            authorized_readers,
+                                            unauthorized_readers,
+                                            self.unauthorized_exceptions,
+                                            rule_name,
+                                            self.controller.show,
+                                            req,
+                                            id=vol_type.id)
+
+        # Check whether the response should contain extra_specs. The logic
+        # is a little unusual:
+        #   - The new rule is SYSTEM_READER_OR_PROJECT_READER (i.e. users
+        #     with the 'reader' role)
+        #   - The deprecated rule is RULE_ADMIN_API (i.e. users with the
+        #     'admin' role)
+        context = self.create_context(user_id)
+        if 'reader' in context.roles or 'admin' in context.roles:
+            self.assertIn('extra_specs', response['volume_type'])
+        else:
+            self.assertNotIn('extra_specs', response['volume_type'])
 
 
 class VolumeTypePolicySecureRbacTest(VolumeTypePolicyTest):

cinder/tests/unit/policy.yaml

@@ -16,30 +16,3 @@
 # enable, set-log and get-log actions.
 # PUT  /os-services/{action}
 #"volume_extension:services:update": "rule:admin_api"
-
-# Create, update and delete volume type.
-# POST  /types
-# PUT  /types
-# DELETE  /types
-"volume_extension:types_manage": ""
-
-# List type extra specs.
-# GET  /types/{type_id}/extra_specs
-"volume_extension:types_extra_specs:index": ""
-
-# Create type extra specs.
-# POST  /types/{type_id}/extra_specs
-"volume_extension:types_extra_specs:create": ""
-
-# Show one specified type extra specs.
-# GET  /types/{type_id}/extra_specs/{extra_spec_key}
-"volume_extension:types_extra_specs:show": ""
-
-# Update type extra specs.
-# PUT  /types/{type_id}/extra_specs/{extra_spec_key}
-"volume_extension:types_extra_specs:update": ""
-
-# Delete type extra specs.
-# DELETE  /types/{type_id}/extra_specs/{extra_spec_key}
-"volume_extension:types_extra_specs:delete": ""
-

doc/source/configuration/block-storage/policy-personas.rst

@@ -1693,13 +1693,13 @@ matrix is validated by human beings.
        | ``GET  /types``
        | The ability to make these API calls is governed by other policies.
      - volume_extension:access_types_extra_specs
-     - rule:admin_api
+     - empty
-     - no
+     - yes
-     - no
+     - yes
-     - no
+     - yes
-     - no
+     - yes
+     - yes
      - yes
-     - no
      - yes
    * - List or show volume type with access type qos specs id attribute
      - | Adds ``qos_specs_id`` to the following responses:
@@ -2237,13 +2237,13 @@ matrix is validated by human beings.
    * - List type extra specs
      - ``GET  /types/{type_id}/extra_specs``
      - volume_extension:types_extra_specs:index
-     - rule:admin_api
+     - empty
-     - no
+     - yes
-     - no
+     - yes
-     - no
+     - yes
-     - no
+     - yes
+     - yes
      - yes
-     - no
      - yes
    * - Create type extra specs
      - ``POST  /types/{type_id}/extra_specs``
@@ -2259,13 +2259,13 @@ matrix is validated by human beings.
    * - Show one specified type extra specs
      - ``GET  /types/{type_id}/extra_specs/{extra_spec_key}``
      - volume_extension:types_extra_specs:show
-     - rule:admin_api
+     - empty
-     - no
+     - yes
-     - no
+     - yes
-     - no
+     - yes
-     - no
+     - yes
+     - yes
      - yes
-     - no
      - yes
    * - Update type extra specs
      - ``PUT  /types/{type_id}/extra_specs/{extra_spec_key}``
@@ -2289,6 +2289,23 @@ matrix is validated by human beings.
      - yes
      - no
      - yes
+   * - Include extra_specs fields that may reveal sensitive information about
+       the deployment that should not be exposed to end users in various
+       volume-type responses that show extra_specs.
+     - | ``GET  /types``
+       | ``GET  /types/{type_id}``
+       | ``GET  /types/{type_id}/extra_specs``
+       | ``GET  /types/{type_id}/extra_specs/{extra_spec_key}``
+       | The ability to make these API calls is governed by other policies.
+     - volume_extension:types_extra_specs:read_sensitive
+     - rule:admin_api
+     - no
+     - no
+     - no
+     - no
+     - yes
+     - no
+     - yes
 
 .. list-table:: Volumes
    :header-rows: 1