openstack
/
cinder
Code Issues Proposed changes

Validate bool value using strutils.bool_from_string method 

bool value is validated with different logic scattered all over the places
1. isinstance of six.string_type or bool
2. is_valid_boolstr method

Made changes to use strutils.bool_from_string method to validate bool
value at all places as this method does all things expected above.
Changes are not made in v1 api as it is going to be deprecated in
Liberty release.

Example error message:
Invalid value for force_host_copy: Unrecognized value 'xyz', acceptable
values are: '0', '1', 'f', 'false', 'n', 'no', 'off', 'on', 't', 'true',
'y', 'yes'

ApiImpact: 400 BadRequest response will be returned in case invalid force
value is provided during volume upload to image, earlier it was ignored.

Closes-Bug: 1460575
Change-Id: Ie64d429f5860a950cda9b363a65c5282f128f10b
This commit is contained in:
Rajesh Tailor 2015-05-29 03:26:22 -07:00
parent de42caeb2e
commit 24fd26fc98
4 changed files with 28 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
cinder/api/contrib/admin_actions.py
View File

@ -15,7 +15,6 @@
from oslo_log import log as logging
import oslo_messaging as messaging
from oslo_utils import strutils
import six
import webob
from webob import exc
@ -222,17 +221,13 @@ class VolumeAdminController(AdminController):
host = params['host']
except KeyError:
raise exc.HTTPBadRequest(explanation=_("Must specify 'host'"))
force_host_copy = params.get('force_host_copy', False)
if isinstance(force_host_copy, six.string_types):
try:
force_host_copy = strutils.bool_from_string(force_host_copy,
strict=True)
except ValueError:
raise exc.HTTPBadRequest(
explanation=_("Bad value for 'force_host_copy'"))
elif not isinstance(force_host_copy, bool):
raise exc.HTTPBadRequest(
explanation=_("'force_host_copy' not string or bool"))
force_host_copy = params.get('force_host_copy', 'False')
try:
force_host_copy = strutils.bool_from_string(force_host_copy,
strict=True)
except ValueError as e:
msg = (_("Invalid value for force_host_copy: '%s'") % e.message)
raise exc.HTTPBadRequest(explanation=msg)
self.volume_api.migrate_volume(context, volume, host, force_host_copy)
return webob.Response(status_int=202)

44
cinder/api/contrib/volume_actions.py
View File

@ -265,15 +265,11 @@ class VolumeActionsController(wsgi.Controller):
msg = _("No image_name was specified in request.")
raise webob.exc.HTTPBadRequest(explanation=msg)
force = params.get('force', False)
if isinstance(force, six.string_types):
try:
force = strutils.bool_from_string(force, strict=False)
except ValueError:
msg = _("Bad value for 'force' parameter.")
raise webob.exc.HTTPBadRequest(explanation=msg)
elif not isinstance(force, bool):
msg = _("'force' is not string or bool.")
force = params.get('force', 'False')
try:
force = strutils.bool_from_string(force, strict=True)
except ValueError as error:
msg = _("Invalid value for 'force': '%s'") % error.message
raise webob.exc.HTTPBadRequest(explanation=msg)
try:
@ -337,16 +333,11 @@ class VolumeActionsController(wsgi.Controller):
msg = _("Must specify readonly in request.")
raise webob.exc.HTTPBadRequest(explanation=msg)
if isinstance(readonly_flag, six.string_types):
try:
readonly_flag = strutils.bool_from_string(readonly_flag,
strict=True)
except ValueError:
msg = _("Bad value for 'readonly'")
raise webob.exc.HTTPBadRequest(explanation=msg)
elif not isinstance(readonly_flag, bool):
msg = _("'readonly' not string or bool")
try:
readonly_flag = strutils.bool_from_string(readonly_flag,
strict=True)
except ValueError as error:
msg = _("Invalid value for 'readonly': '%s'") % error.message
raise webob.exc.HTTPBadRequest(explanation=msg)
self.volume_api.update_readonly_flag(context, volume, readonly_flag)
@ -382,16 +373,11 @@ class VolumeActionsController(wsgi.Controller):
msg = _("Must specify bootable in request.")
raise webob.exc.HTTPBadRequest(explanation=msg)
if isinstance(bootable, six.string_types):
try:
bootable = strutils.bool_from_string(bootable,
strict=True)
except ValueError:
msg = _("Bad value for 'bootable'")
raise webob.exc.HTTPBadRequest(explanation=msg)
elif not isinstance(bootable, bool):
msg = _("'bootable' not string or bool")
try:
bootable = strutils.bool_from_string(bootable,
strict=True)
except ValueError as error:
msg = _("Invalid value for 'bootable': '%s'") % error.message
raise webob.exc.HTTPBadRequest(explanation=msg)
update_dict = {'bootable': bootable}

8
cinder/api/v2/snapshots.py
View File

@ -196,11 +196,13 @@ class SnapshotsController(wsgi.Controller):
snapshot['display_name'] = snapshot.get('name')
del snapshot['name']
if not utils.is_valid_boolstr(force):
msg = _("Invalid value '%s' for force. ") % force
try:
force = strutils.bool_from_string(force, strict=True)
except ValueError as error:
msg = _("Invalid value for 'force': '%s'") % error.message
raise exception.InvalidParameterValue(err=msg)
if strutils.bool_from_string(force):
if force:
new_snapshot = self.volume_api.create_snapshot_force(
context,
volume,

10
cinder/tests/unit/api/contrib/test_admin_actions.py
View File

@ -841,7 +841,7 @@ class AdminActionsTest(test.TestCase):
db.snapshot_create(ctx, {'volume_id': volume['id']})
self._migrate_volume_exec(ctx, volume, host, expected_status)
def test_migrate_volume_bad_force_host_copy1(self):
def test_migrate_volume_bad_force_host_copy(self):
expected_status = 400
host = 'test2'
ctx = context.RequestContext('admin', 'fake', True)
@ -849,14 +849,6 @@ class AdminActionsTest(test.TestCase):
self._migrate_volume_exec(ctx, volume, host, expected_status,
force_host_copy='foo')
def test_migrate_volume_bad_force_host_copy2(self):
expected_status = 400
host = 'test2'
ctx = context.RequestContext('admin', 'fake', True)
volume = self._migrate_volume_prep()
self._migrate_volume_exec(ctx, volume, host, expected_status,
force_host_copy=1)
def _migrate_volume_comp_exec(self, ctx, volume, new_volume, error,
expected_status, expected_id, no_body=False):
req = webob.Request.blank('/v2/fake/volumes/%s/action' % volume['id'])