From 38434795445270efdbec9c0471e36498fc1045ca Mon Sep 17 00:00:00 2001
From: wangxiyuan <wangxiyuan@huawei.com>
Date: Fri, 9 Dec 2016 15:40:38 +0800
Subject: [PATCH] Don't drop the volume type's extra spec at DB layer
Now the volume type's extra spec will be always dropped at
DB layer if the request is non admin. But this behavior is
controled by policy. We should return the extra specs to API and
let the policy check to decide whether drop it or not.
Change-Id: I1130a53a02da7aa4b8eb0587186331166d2b9bc1
Closes-bug: #1648717
---
cinder/db/sqlalchemy/api.py | 11 +++++-----
cinder/tests/unit/api/v2/test_types.py | 30 ++++++++++++++++++++++++++
cinder/tests/unit/test_volume_types.py | 6 +++---
3 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index dde7d0e99e7..846584af8b6 100644
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -754,12 +754,11 @@ def _dict_with_extra_specs_if_authorized(context, inst_type_query):
"""
inst_type_dict = dict(inst_type_query)
- if not is_admin_context(context):
- del(inst_type_dict['extra_specs'])
- else:
- extra_specs = {x['key']: x['value']
- for x in inst_type_query['extra_specs']}
- inst_type_dict['extra_specs'] = extra_specs
+
+ extra_specs = {x['key']: x['value']
+ for x in inst_type_query['extra_specs']}
+ inst_type_dict['extra_specs'] = extra_specs
+
return inst_type_dict
diff --git a/cinder/tests/unit/api/v2/test_types.py b/cinder/tests/unit/api/v2/test_types.py
index 58aeb8b0c52..df50b835fae 100644
--- a/cinder/tests/unit/api/v2/test_types.py
+++ b/cinder/tests/unit/api/v2/test_types.py
@@ -397,6 +397,36 @@ class VolumeTypesApiTest(test.TestCase):
)
self.assertDictEqual(expected_volume_type, output['volume_type'])
+ with mock.patch.object(common,
+ 'validate_policy',
+ side_effect=[False, 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',
+ is_public=True,
+ id=42,
+ )
+ self.assertDictEqual(expected_volume_type, output['volume_type'])
+
def test_view_builder_show_pass_all_policy(self):
with mock.patch.object(common,
'validate_policy',
diff --git a/cinder/tests/unit/test_volume_types.py b/cinder/tests/unit/test_volume_types.py
index 34e022c1f7b..d51a6aaf668 100644
--- a/cinder/tests/unit/test_volume_types.py
+++ b/cinder/tests/unit/test_volume_types.py
@@ -496,15 +496,15 @@ class VolumeTypeTestCase(test.TestCase):
self.assertFalse(volume_types.is_public_volume_type(self.ctxt,
volume_type_id))
- def test_ensure_no_extra_specs_for_non_admin(self):
- # non-admin users shouldn't get extra-specs back in type-get/list etc
+ def test_ensure__extra_specs_for_non_admin(self):
+ # non-admin users get extra-specs back in type-get/list etc at DB layer
ctxt = context.RequestContext('average-joe',
'd802f078-0af1-4e6b-8c02-7fac8d4339aa',
auth_token='token',
is_admin=False)
volume_types.create(self.ctxt, "type-test", is_public=False)
vtype = volume_types.get_volume_type_by_name(ctxt, 'type-test')
- self.assertIsNone(vtype.get('extra_specs', None))
+ self.assertIsNotNone(vtype.get('extra_specs', None))
def test_ensure_extra_specs_for_admin(self):
# admin users should get extra-specs back in type-get/list etc