openstack
/
cinder
Code Issues Proposed changes

Fix combination of parameters for update APIs 

For group_type and volume_type update APIs, If user passes
description or is_public parameter in the request body then name
can be null. But after schema validation [1][2], it is failing with
BadRequest.

This patch allows users to pass 'name' as None if "description"
or is_public" parameter is passed in the request body.

[1]: https://review.openstack.org/#/c/520561/
[2]: https://review.openstack.org/#/c/519643/

Change-Id: Id5d5f5ed8b449c9f1059528de241417f231f90b5
Closes-Bug: #1742940
This commit is contained in:
Neha Alhat 2018-01-14 15:57:06 +05:30
parent 5d761327da
commit 2b2ed02fe5
6 changed files with 99 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
cinder/api/contrib/types_manage.py
View File

@ -99,6 +99,18 @@ class VolumeTypesManageController(wsgi.Controller):
if is_public is not None:
is_public = strutils.bool_from_string(is_public, strict=True)
# If name specified, name can not be empty.
if name and len(name.strip()) == 0:
msg = _("Volume type name can not be empty.")
raise webob.exc.HTTPBadRequest(explanation=msg)
# Name, description and is_public can not be None.
# Specify one of them, or a combination thereof.
if name is None and description is None and is_public is None:
msg = _("Specify volume type name, description, is_public or "
"a combination thereof.")
raise webob.exc.HTTPBadRequest(explanation=msg)
try:
volume_types.update(context, id, name, description,
is_public=is_public)

7
cinder/api/schemas/group_types.py
View File

@ -48,16 +48,11 @@ update = {
'group_type': {
'type': 'object',
'properties': {
'name': parameter_types.name,
'name': parameter_types.name_allow_zero_min_length,
'description': parameter_types.description,
'is_public': parameter_types.boolean,
},
'additionalProperties': False,
'anyOf': [
{'required': ['name']},
{'required': ['description']},
{'required': ['is_public']},
]
},
},
'required': ['group_type'],

7
cinder/api/schemas/volume_types.py
View File

@ -44,16 +44,11 @@ update = {
'volume_type': {
'type': 'object',
'properties': {
'name': parameter_types.name,
'name': parameter_types.name_allow_zero_min_length,
'description': parameter_types.description,
'is_public': parameter_types.boolean,
},
'additionalProperties': False,
'anyOf': [
{'required': ['name']},
{'required': ['description']},
{'required': ['is_public']}
]
},
},
'required': ['volume_type'],

11
cinder/api/v3/group_types.py
View File

@ -102,6 +102,17 @@ class GroupTypesController(wsgi.Controller):
if is_public is not None:
is_public = strutils.bool_from_string(is_public, strict=True)
# If name specified, name can not be empty.
if name and len(name.strip()) == 0:
msg = _("Group type name can not be empty.")
raise webob.exc.HTTPBadRequest(explanation=msg)
# Name, description and is_public can not be None.
# Specify one of them, or a combination thereof.
if name is None and description is None and is_public is None:
msg = _("Specify group type name, description or "
"a combination thereof.")
raise webob.exc.HTTPBadRequest(explanation=msg)
try:
group_types.update(context, id, name, description,
is_public=is_public)

39
cinder/tests/unit/api/contrib/test_types_manage.py
View File

@ -502,7 +502,7 @@ class VolumeTypesManageApiTest(test.TestCase):
fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
req.method = 'PUT'
self.assertRaises(exception.ValidationError,
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller._update, req,
DEFAULT_VOLUME_TYPE, body=body)
@ -513,7 +513,7 @@ class VolumeTypesManageApiTest(test.TestCase):
fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
req.method = 'PUT'
self.assertRaises(exception.ValidationError,
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller._update, req,
DEFAULT_VOLUME_TYPE, body=body)
@ -699,3 +699,38 @@ class VolumeTypesManageApiTest(test.TestCase):
if expected_results.get('is_public') is not None:
self.assertEqual(expected_results['is_public'],
results['volume_type']['is_public'])
def test_update_with_name_null(self):
body = {"volume_type": {"name": None}}
req = fakes.HTTPRequest.blank('/v2/%s/types/%s' % (
fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
req.method = 'PUT'
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller._update, req,
DEFAULT_VOLUME_TYPE, body=body)
@ddt.data({"volume_type": {"name": None, "description": "description"}},
{"volume_type": {"name": None, "is_public": True}},
{"volume_type": {"description": "description",
"is_public": True}})
def test_update_volume_type(self, body):
req = fakes.HTTPRequest.blank('/v2/%s/types/%s' % (
fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
req.method = 'PUT'
ctxt = context.RequestContext(fake.USER_ID, fake.PROJECT_ID, True)
req.environ['cinder.context'] = ctxt
volume_type_1 = volume_types.create(ctxt, 'volume_type')
res = self.controller._update(req, volume_type_1.get('id'), body=body)
expected_name = body['volume_type'].get('name')
if expected_name is not None:
self.assertEqual(expected_name, res['volume_type']['name'])
expected_is_public = body['volume_type'].get('is_public')
if expected_is_public is not None:
self.assertEqual(expected_is_public,
res['volume_type']['is_public'])
self.assertEqual(body['volume_type'].get('description'),
res['volume_type']['description'])

37
cinder/tests/unit/api/v3/test_group_types.py
View File

@ -357,6 +357,43 @@ class GroupTypesApiTest(test.TestCase):
self.ctxt, type_id, 'group_type1', None,
is_public=boolean_is_public)
def test_update_group_type_with_name_null(self):
req = fakes.HTTPRequest.blank(
'/v3/%s/types/%s' % (fake.PROJECT_ID, fake.GROUP_TYPE_ID),
version=mv.GROUP_TYPE)
req.environ['cinder.context'] = self.ctxt
body = {"group_type": {"name": None}}
self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
req, fake.GROUP_TYPE_ID, body=body)
@ddt.data({"group_type": {"name": None,
"description": "description"}},
{"group_type": {"name": "test",
"is_public": True}},
{"group_type": {"description": None,
"is_public": True}})
def test_update_group_type(self, body):
req = fakes.HTTPRequest.blank(
'/v3/%s/types/%s' % (fake.PROJECT_ID, fake.GROUP_TYPE_ID),
version=mv.GROUP_TYPE)
group_type_1 = group_types.create(self.ctxt, 'group_type')
req.environ['cinder.context'] = self.ctxt
res = self.controller.update(req, group_type_1.get('id'), body=body)
expected_name = body['group_type'].get('name')
if expected_name is not None:
self.assertEqual(expected_name, res['group_type']['name'])
expected_is_public = body['group_type'].get('is_public')
if expected_is_public is not None:
self.assertEqual(expected_is_public,
res['group_type']['is_public'])
self.assertEqual(body['group_type'].get('description'),
res['group_type']['description'])
def test_group_types_show(self):
self.mock_object(group_types, 'get_group_type',
return_group_types_get_group_type)