openstack/manila
Code Issues Proposed changes

Merge "Non-admin user can perform 'extra-specs-list'"

Browse Source
This commit is contained in:
Jenkins 2015-09-17 13:53:28 +00:00 committed by Gerrit Code Review
commit fb7e91ae31
8 changed files with 79 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
etc/manila/policy.json
View File

@ -27,6 +27,10 @@
"share:update_share_metadata": "rule:default",
"share:migrate": "rule:admin_api",
"share_type:index": "rule:default",
"share_type:show": "rule:default",
"share_type:default": "rule:default",
"share_instance:index": "rule:admin_api",
"share_instance:show": "rule:admin_api",

8
manila/api/v1/share_types.py
View File

@ -22,8 +22,11 @@ from manila.api.openstack import wsgi
from manila.api.views import types as views_types
from manila import exception
from manila.i18n import _
from manila import policy
from manila.share import share_types
RESOURCE_NAME = 'share_type'
class ShareTypesController(wsgi.Controller):
"""The share types API controller for the OpenStack API."""
@ -32,6 +35,9 @@ class ShareTypesController(wsgi.Controller):
def index(self, req):
"""Returns the list of share types."""
context = req.environ['manila.context']
policy.check_policy(context, RESOURCE_NAME, 'index')
limited_types = self._get_share_types(req)
req.cache_db_share_types(limited_types)
return self._view_builder.index(req, limited_types)
@ -39,6 +45,7 @@ class ShareTypesController(wsgi.Controller):
def show(self, req, id):
"""Return a single share type item."""
context = req.environ['manila.context']
policy.check_policy(context, RESOURCE_NAME, 'show')
try:
share_type = share_types.get_share_type(context, id)
@ -53,6 +60,7 @@ class ShareTypesController(wsgi.Controller):
def default(self, req):
"""Return default volume type."""
context = req.environ['manila.context']
policy.check_policy(context, RESOURCE_NAME, 'default')
try:
share_type = share_types.get_default_share_type(context)

22
manila/api/views/types.py
View File

@ -14,6 +14,7 @@
# under the License.
from manila.api import common
from manila.share import share_types
class ViewBuilder(common.ViewBuilder):
@ -22,11 +23,22 @@ class ViewBuilder(common.ViewBuilder):
def show(self, request, share_type, brief=False):
"""Trim away extraneous share type attributes."""
extra_specs = share_type.get('extra_specs', {})
required_extra_specs = share_type.get('required_extra_specs', {})
# Remove non-tenant-visible extra specs in a non-admin context
if not request.environ['manila.context'].is_admin:
extra_spec_names = share_types.get_tenant_visible_extra_specs()
extra_specs = self._filter_extra_specs(extra_specs,
extra_spec_names)
required_extra_specs = self._filter_extra_specs(
required_extra_specs, extra_spec_names)
trimmed = dict(id=share_type.get('id'),
name=share_type.get('name'),
extra_specs=share_type.get('extra_specs'),
required_extra_specs=share_type.get(
'required_extra_specs'))
extra_specs=extra_specs,
required_extra_specs=required_extra_specs)
if brief:
return trimmed
else:
@ -38,3 +50,7 @@ class ViewBuilder(common.ViewBuilder):
for share_type in share_types]
return dict(volume_types=share_types_list,
share_types=share_types_list)
def _filter_extra_specs(self, extra_specs, valid_keys):
return {key: value for key, value in extra_specs.items()
if key in valid_keys}

4
manila/common/constants.py
View File

@ -118,6 +118,10 @@ class ExtraSpecs(object):
DRIVER_HANDLES_SHARE_SERVERS,
SNAPSHOT_SUPPORT,
)
# NOTE(cknight): Some extra specs are necessary parts of the Manila API and
# should be visible to non-admin users. This list matches the UNDELETABLE
# list today, but that may not always remain true.
TENANT_VISIBLE = UNDELETABLE
BOOLEAN = (
DRIVER_HANDLES_SHARE_SERVERS,
SNAPSHOT_SUPPORT,

4
manila/share/share_types.py
View File

@ -203,6 +203,10 @@ def get_undeletable_extra_specs():
return constants.ExtraSpecs.UNDELETABLE
def get_tenant_visible_extra_specs():
return constants.ExtraSpecs.TENANT_VISIBLE
def get_boolean_extra_specs():
return constants.ExtraSpecs.BOOLEAN

27
manila/tests/api/v1/test_share_types.py
View File

@ -22,6 +22,7 @@ from manila.api.v1 import share_types as types
from manila.api.views import types as views_types
from manila.common import constants
from manila import exception
from manila import policy
from manila.share import share_types
from manila import test
from manila.tests.api import fakes
@ -75,12 +76,17 @@ class ShareTypesApiTest(test.TestCase):
def setUp(self):
super(ShareTypesApiTest, self).setUp()
self.controller = types.ShareTypesController()
self.mock_object(policy, 'check_policy',
mock.Mock(return_value=True))
def test_share_types_index(self):
@ddt.data(True, False)
def test_share_types_index(self, admin):
self.mock_object(share_types, 'get_all_types',
return_share_types_get_all_types)
req = fakes.HTTPRequest.blank('/v2/fake/types')
req = fakes.HTTPRequest.blank('/v2/fake/types',
use_admin_context=admin)
res_dict = self.controller.index(req)
self.assertEqual(3, len(res_dict['share_types']))
@ -89,11 +95,16 @@ class ShareTypesApiTest(test.TestCase):
actual_names = map(lambda e: e['name'], res_dict['share_types'])
self.assertEqual(set(actual_names), set(expected_names))
for entry in res_dict['share_types']:
self.assertEqual('value1', entry['extra_specs']['key1'])
if admin:
self.assertEqual('value1', entry['extra_specs'].get('key1'))
else:
self.assertIsNone(entry['extra_specs'].get('key1'))
self.assertTrue('required_extra_specs' in entry)
required_extra_spec = entry['required_extra_specs'].get(
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS, '')
self.assertEqual('true', required_extra_spec)
policy.check_policy.assert_called_once_with(
req.environ['manila.context'], types.RESOURCE_NAME, 'index')
def test_share_types_index_no_data(self):
self.mock_object(share_types, 'get_all_types',
@ -103,6 +114,8 @@ class ShareTypesApiTest(test.TestCase):
res_dict = self.controller.index(req)
self.assertEqual(0, len(res_dict['share_types']))
policy.check_policy.assert_called_once_with(
req.environ['manila.context'], types.RESOURCE_NAME, 'index')
def test_share_types_show(self):
self.mock_object(share_types, 'get_share_type',
@ -114,6 +127,8 @@ class ShareTypesApiTest(test.TestCase):
self.assertEqual(2, len(res_dict))
self.assertEqual('1', res_dict['share_type']['id'])
self.assertEqual('share_type_1', res_dict['share_type']['name'])
policy.check_policy.assert_called_once_with(
req.environ['manila.context'], types.RESOURCE_NAME, 'show')
def test_share_types_show_not_found(self):
self.mock_object(share_types, 'get_share_type',
@ -122,6 +137,8 @@ class ShareTypesApiTest(test.TestCase):
req = fakes.HTTPRequest.blank('/v2/fake/types/777')
self.assertRaises(webob.exc.HTTPNotFound, self.controller.show,
req, '777')
policy.check_policy.assert_called_once_with(
req.environ['manila.context'], types.RESOURCE_NAME, 'show')
def test_share_types_default(self):
self.mock_object(share_types, 'get_default_share_type',
@ -133,6 +150,8 @@ class ShareTypesApiTest(test.TestCase):
self.assertEqual(2, len(res_dict))
self.assertEqual('1', res_dict['share_type']['id'])
self.assertEqual('share_type_1', res_dict['share_type']['name'])
policy.check_policy.assert_called_once_with(
req.environ['manila.context'], types.RESOURCE_NAME, 'default')
def test_share_types_default_not_found(self):
self.mock_object(share_types, 'get_default_share_type',
@ -141,6 +160,8 @@ class ShareTypesApiTest(test.TestCase):
req = fakes.HTTPRequest.blank('/v2/fake/types/default')
self.assertRaises(webob.exc.HTTPNotFound, self.controller.default, req)
policy.check_policy.assert_called_once_with(
req.environ['manila.context'], types.RESOURCE_NAME, 'default')
def test_view_builder_show(self):
view_builder = views_types.ViewBuilder()

4
manila/tests/policy.json
View File

@ -18,6 +18,10 @@
"share:extend": "",
"share:shrink": "",
"share_type:index": "rule:default",
"share_type:show": "rule:default",
"share_type:default": "rule:default",
"share_instance:index": "rule:admin_api",
"share_instance:show": "rule:admin_api",

12
manila_tempest_tests/tests/api/admin/test_share_types_extra_specs_negative.py
View File

@ -66,6 +66,18 @@ class ExtraSpecsAdminNegativeTest(base.BaseSharesAdminTest):
self.member_shares_client.get_share_type_extra_specs,
st["share_type"]["id"])
@test.attr(type=["gate", "smoke", ])
def test_try_read_extra_specs_on_share_type_with_user(self):
st = self._create_share_type()
share_type = self.member_shares_client.get_share_type(
st['share_type']['id'])
# Verify a non-admin can only read the required extra-specs
expected_keys = ['driver_handles_share_servers', 'snapshot_support']
actual_keys = share_type['share_type']['extra_specs'].keys()
self.assertEqual(sorted(expected_keys), sorted(actual_keys),
'Incorrect extra specs visible to non-admin user; '
'expected %s, got %s' % (expected_keys, actual_keys))
@test.attr(type=["gate", "smoke", ])
def test_try_update_extra_spec_with_user(self):
st = self._create_share_type()