diff --git a/cinder/api/contrib/volume_actions.py b/cinder/api/contrib/volume_actions.py index 23edbe01915..bb4052f5607 100644 --- a/cinder/api/contrib/volume_actions.py +++ b/cinder/api/contrib/volume_actions.py @@ -16,7 +16,6 @@ from castellan import key_manager from oslo_config import cfg import oslo_messaging as messaging -from oslo_utils import encodeutils from oslo_utils import strutils import six from six.moves import http_client @@ -25,11 +24,11 @@ import webob from cinder.api import extensions from cinder.api import microversions as mv from cinder.api.openstack import wsgi +from cinder.api.schemas import volume_actions as volume_action +from cinder.api import validation from cinder import exception from cinder.i18n import _ -from cinder.image import image_utils from cinder.policies import volume_actions as policy -from cinder import utils from cinder import volume @@ -52,6 +51,7 @@ class VolumeActionsController(wsgi.Controller): @wsgi.response(http_client.ACCEPTED) @wsgi.action('os-attach') + @validation.schema(volume_action.attach) def _attach(self, req, id, body): """Add attachment metadata.""" context = req.environ['cinder.context'] @@ -66,23 +66,9 @@ class VolumeActionsController(wsgi.Controller): # Keep API backward compatibility if 'host_name' in body['os-attach']: host_name = body['os-attach']['host_name'] - if 'mountpoint' not in body['os-attach']: - msg = _("Must specify 'mountpoint'") - raise webob.exc.HTTPBadRequest(explanation=msg) mountpoint = body['os-attach']['mountpoint'] - if 'mode' in body['os-attach']: - mode = body['os-attach']['mode'] - else: - mode = 'rw' + mode = body['os-attach'].get('mode', 'rw') - if instance_uuid is None and host_name is None: - msg = _("Invalid request to attach volume to an invalid target") - raise webob.exc.HTTPBadRequest(explanation=msg) - - if mode not in ('rw', 'ro'): - msg = _("Invalid request to attach volume with an invalid mode. " - "Attaching mode should be 'rw' or 'ro'") - raise webob.exc.HTTPBadRequest(explanation=msg) try: self.volume_api.attach(context, volume, instance_uuid, host_name, mountpoint, mode) @@ -101,6 +87,7 @@ class VolumeActionsController(wsgi.Controller): @wsgi.response(http_client.ACCEPTED) @wsgi.action('os-detach') + @validation.schema(volume_action.detach) def _detach(self, req, id, body): """Clear attachment metadata.""" context = req.environ['cinder.context'] @@ -108,8 +95,7 @@ class VolumeActionsController(wsgi.Controller): volume = self.volume_api.get(context, id) attachment_id = None - if body['os-detach']: - attachment_id = body['os-detach'].get('attachment_id', None) + attachment_id = body['os-detach'].get('attachment_id', None) try: self.volume_api.detach(context, volume, attachment_id) @@ -166,16 +152,13 @@ class VolumeActionsController(wsgi.Controller): self.volume_api.roll_detaching(context, volume) @wsgi.action('os-initialize_connection') + @validation.schema(volume_action.initialize_connection) def _initialize_connection(self, req, id, body): """Initialize volume attachment.""" context = req.environ['cinder.context'] # Not found exception will be handled at the wsgi level volume = self.volume_api.get(context, id) - try: - connector = body['os-initialize_connection']['connector'] - except KeyError: - raise webob.exc.HTTPBadRequest( - explanation=_("Must specify 'connector'")) + connector = body['os-initialize_connection']['connector'] try: info = self.volume_api.initialize_connection(context, volume, @@ -195,16 +178,13 @@ class VolumeActionsController(wsgi.Controller): @wsgi.response(http_client.ACCEPTED) @wsgi.action('os-terminate_connection') + @validation.schema(volume_action.terminate_connection) def _terminate_connection(self, req, id, body): """Terminate volume attachment.""" context = req.environ['cinder.context'] # Not found exception will be handled at the wsgi level volume = self.volume_api.get(context, id) - try: - connector = body['os-terminate_connection']['connector'] - except KeyError: - raise webob.exc.HTTPBadRequest( - explanation=_("Must specify 'connector'")) + connector = body['os-terminate_connection']['connector'] try: self.volume_api.terminate_connection(context, volume, connector) except exception.VolumeBackendAPIException: @@ -213,37 +193,23 @@ class VolumeActionsController(wsgi.Controller): @wsgi.response(http_client.ACCEPTED) @wsgi.action('os-volume_upload_image') + @validation.schema(volume_action.volume_upload_image, '2.0', '3.0') + @validation.schema(volume_action.volume_upload_image_v31, '3.1') def _volume_upload_image(self, req, id, body): """Uploads the specified volume to image service.""" context = req.environ['cinder.context'] params = body['os-volume_upload_image'] req_version = req.api_version_request - if not params.get("image_name"): - msg = _("No image_name was specified in request.") - raise webob.exc.HTTPBadRequest(explanation=msg) force = params.get('force', 'False') - try: - force = strutils.bool_from_string(force, strict=True) - except ValueError as error: - err_msg = encodeutils.exception_to_unicode(error) - msg = _("Invalid value for 'force': '%s'") % err_msg - raise webob.exc.HTTPBadRequest(explanation=msg) + force = strutils.bool_from_string(force, strict=True) # Not found exception will be handled at the wsgi level volume = self.volume_api.get(context, id) - context.authorize(policy.UPLOAD_IMAGE_POLICY, target_obj=volume) + context.authorize(policy.UPLOAD_IMAGE_POLICY) # check for valid disk-format disk_format = params.get("disk_format", "raw") - if not image_utils.validate_disk_format(disk_format): - msg = _("Invalid disk-format '%(disk_format)s' is specified. " - "Allowed disk-formats are %(allowed_disk_formats)s.") % { - "disk_format": disk_format, - "allowed_disk_formats": ", ".join( - image_utils.VALID_DISK_FORMATS) - } - raise webob.exc.HTTPBadRequest(explanation=msg) image_metadata = {"container_format": params.get( "container_format", "bare"), @@ -267,14 +233,11 @@ class VolumeActionsController(wsgi.Controller): mv.UPLOAD_IMAGE_PARAMS): image_metadata['visibility'] = params.get('visibility', 'private') - image_metadata['protected'] = params.get('protected', 'False') + image_metadata['protected'] = strutils.bool_from_string( + params.get('protected', 'False'), strict=True) if image_metadata['visibility'] == 'public': - context.authorize(policy.UPLOAD_PUBLIC_POLICY, - target_obj=volume) - - image_metadata['protected'] = ( - utils.get_bool_param('protected', image_metadata)) + context.authorize(policy.UPLOAD_PUBLIC_POLICY) try: response = self.volume_api.copy_volume_to_image(context, @@ -295,6 +258,7 @@ class VolumeActionsController(wsgi.Controller): @wsgi.response(http_client.ACCEPTED) @wsgi.action('os-extend') + @validation.schema(volume_action.extend) def _extend(self, req, id, body): """Extend size of volume.""" context = req.environ['cinder.context'] @@ -302,12 +266,7 @@ class VolumeActionsController(wsgi.Controller): # Not found exception will be handled at the wsgi level volume = self.volume_api.get(context, id) - try: - size = int(body['os-extend']['new_size']) - except (KeyError, ValueError, TypeError): - msg = _("New volume size must be specified as an integer.") - raise webob.exc.HTTPBadRequest(explanation=msg) - + size = int(body['os-extend']['new_size']) try: if (req_version.matches(mv.VOLUME_EXTEND_INUSE) and volume.status in ['in-use']): @@ -319,64 +278,43 @@ class VolumeActionsController(wsgi.Controller): @wsgi.response(http_client.ACCEPTED) @wsgi.action('os-update_readonly_flag') + @validation.schema(volume_action.volume_readonly_update) def _volume_readonly_update(self, req, id, body): """Update volume readonly flag.""" context = req.environ['cinder.context'] # Not found exception will be handled at the wsgi level volume = self.volume_api.get(context, id) - try: - readonly_flag = body['os-update_readonly_flag']['readonly'] - except KeyError: - msg = _("Must specify readonly in request.") - raise webob.exc.HTTPBadRequest(explanation=msg) + readonly_flag = body['os-update_readonly_flag']['readonly'] - try: - readonly_flag = strutils.bool_from_string(readonly_flag, - strict=True) - except ValueError as error: - err_msg = encodeutils.exception_to_unicode(error) - msg = _("Invalid value for 'readonly': '%s'") % err_msg - raise webob.exc.HTTPBadRequest(explanation=msg) + readonly_flag = strutils.bool_from_string(readonly_flag, + strict=True) self.volume_api.update_readonly_flag(context, volume, readonly_flag) @wsgi.response(http_client.ACCEPTED) @wsgi.action('os-retype') + @validation.schema(volume_action.retype) def _retype(self, req, id, body): """Change type of existing volume.""" context = req.environ['cinder.context'] volume = self.volume_api.get(context, id) - try: - new_type = body['os-retype']['new_type'] - except KeyError: - msg = _("New volume type must be specified.") - raise webob.exc.HTTPBadRequest(explanation=msg) + new_type = body['os-retype']['new_type'] policy = body['os-retype'].get('migration_policy') self.volume_api.retype(context, volume, new_type, policy) @wsgi.response(http_client.OK) @wsgi.action('os-set_bootable') + @validation.schema(volume_action.set_bootable) def _set_bootable(self, req, id, body): """Update bootable status of a volume.""" context = req.environ['cinder.context'] # Not found exception will be handled at the wsgi level volume = self.volume_api.get(context, id) - try: - bootable = body['os-set_bootable']['bootable'] - except KeyError: - msg = _("Must specify bootable in request.") - raise webob.exc.HTTPBadRequest(explanation=msg) - - try: - bootable = strutils.bool_from_string(bootable, - strict=True) - except ValueError as error: - err_msg = encodeutils.exception_to_unicode(error) - msg = _("Invalid value for 'bootable': '%s'") % err_msg - raise webob.exc.HTTPBadRequest(explanation=msg) + bootable = strutils.bool_from_string( + body['os-set_bootable']['bootable'], strict=True) update_dict = {'bootable': bootable} diff --git a/cinder/api/schemas/volume_actions.py b/cinder/api/schemas/volume_actions.py new file mode 100644 index 00000000000..31f8f37c0c5 --- /dev/null +++ b/cinder/api/schemas/volume_actions.py @@ -0,0 +1,203 @@ +# Copyright (C) 2018 NTT DATA +# 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. + +""" +Schema for V3 volume_actions API. + +""" + +import copy + +from cinder.api.validation import parameter_types + + +container_format = parameter_types.description + +extend = { + 'type': 'object', + 'properties': { + 'os-extend': { + 'type': 'object', + 'properties': { + 'new_size': parameter_types.volume_size, + }, + 'required': ['new_size'], + 'additionalProperties': False, + }, + }, + 'required': ['os-extend'], + 'additionalProperties': False, +} + + +attach = { + 'type': 'object', + 'properties': { + 'os-attach': { + 'type': 'object', + 'properties': { + 'instance_uuid': parameter_types.uuid, + 'mountpoint': { + 'type': 'string', 'minLength': 1, + 'maxLength': 255 + }, + 'host_name': {'type': 'string', 'maxLength': 255}, + 'mode': {'type': 'string', 'enum': ['rw', 'ro']} + }, + 'required': ['mountpoint'], + 'anyOf': [{'required': ['instance_uuid']}, + {'required': ['host_name']}], + 'additionalProperties': False, + }, + }, + 'required': ['os-attach'], + 'additionalProperties': False, +} + + +detach = { + 'type': 'object', + 'properties': { + 'os-detach': { + 'type': ['object', 'null'], + 'properties': { + 'attachment_id': parameter_types.uuid, + }, + 'additionalProperties': False, + }, + }, + 'required': ['os-detach'], + 'additionalProperties': False, +} + + +retype = { + 'type': 'object', + 'properties': { + 'os-retype': { + 'type': 'object', + 'properties': { + 'new_type': {'type': 'string'}, + 'migration_policy': { + 'type': ['string', 'null'], + 'enum': ['on-demand', 'never']}, + }, + 'required': ['new_type'], + 'additionalProperties': False, + }, + }, + 'required': ['os-retype'], + 'additionalProperties': False, +} + + +set_bootable = { + 'type': 'object', + 'properties': { + 'os-set_bootable': { + 'type': 'object', + 'properties': { + 'bootable': parameter_types.boolean + }, + 'required': ['bootable'], + 'additionalProperties': False, + }, + }, + 'required': ['os-set_bootable'], + 'additionalProperties': False, +} + + +volume_upload_image = { + 'type': 'object', + 'properties': { + 'os-volume_upload_image': { + 'type': 'object', + 'properties': { + 'image_name': { + 'type': 'string', 'minLength': 1, 'maxLength': 255 + }, + 'force': parameter_types.boolean, + 'disk_format': { + 'type': 'string', + 'enum': ['raw', 'vmdk', 'vdi', 'qcow2', + 'vhd', 'vhdx', 'ploop'] + }, + 'container_format': container_format + }, + 'required': ['image_name'], + 'additionalProperties': False, + }, + }, + 'required': ['os-volume_upload_image'], + 'additionalProperties': False, +} + +volume_upload_image_v31 = copy.deepcopy(volume_upload_image) +volume_upload_image_v31['properties']['os-volume_upload_image']['properties'][ + 'visibility'] = {'type': 'string', + 'enum': ['community', 'public', 'private', 'shared']} +volume_upload_image_v31['properties']['os-volume_upload_image']['properties'][ + 'protected'] = parameter_types.boolean + + +initialize_connection = { + 'type': 'object', + 'properties': { + 'os-initialize_connection': { + 'type': 'object', + 'properties': { + 'connector': {'type': ['object', 'string']}, + }, + 'required': ['connector'], + 'additionalProperties': False, + }, + }, + 'required': ['os-initialize_connection'], + 'additionalProperties': False, +} + + +terminate_connection = { + 'type': 'object', + 'properties': { + 'os-terminate_connection': { + 'type': 'object', + 'properties': { + 'connector': {'type': ['string', 'object', 'null']}, + }, + 'required': ['connector'], + 'additionalProperties': False, + }, + }, + 'required': ['os-terminate_connection'], + 'additionalProperties': False, +} + +volume_readonly_update = { + 'type': 'object', + 'properties': { + 'os-update_readonly_flag': { + 'type': 'object', + 'properties': { + 'readonly': parameter_types.boolean + }, + 'required': ['readonly'], + 'additionalProperties': False, + }, + }, + 'required': ['os-update_readonly_flag'], + 'additionalProperties': False, +} diff --git a/cinder/api/validation/parameter_types.py b/cinder/api/validation/parameter_types.py index fc44e6b7be4..b0b895e6830 100644 --- a/cinder/api/validation/parameter_types.py +++ b/cinder/api/validation/parameter_types.py @@ -201,3 +201,10 @@ backup_service = {'type': 'string', 'minLength': 0, 'maxLength': 255} nullable_string = { 'type': ('string', 'null'), 'minLength': 0, 'maxLength': 255 } + + +volume_size = { + 'type': ['integer', 'string'], + 'pattern': '^[0-9]+$', + 'minimum': 1 +} diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py index fd375c20a03..428cbdf6e41 100644 --- a/cinder/image/image_utils.py +++ b/cinder/image/image_utils.py @@ -60,8 +60,6 @@ QEMU_IMG_LIMITS = processutils.ProcessLimits( cpu_time=8, address_space=1 * units.Gi) -VALID_DISK_FORMATS = ('raw', 'vmdk', 'vdi', 'qcow2', - 'vhd', 'vhdx', 'ploop') QEMU_IMG_FORMAT_MAP = { # Convert formats of Glance images to how they are processed with qemu-img. @@ -76,10 +74,6 @@ QEMU_IMG_MIN_FORCE_SHARE_VERSION = [2, 10, 0] QEMU_IMG_MIN_CONVERT_LUKS_VERSION = '2.10' -def validate_disk_format(disk_format): - return disk_format in VALID_DISK_FORMATS - - def fixup_disk_format(disk_format): """Return the format to be provided to qemu-img convert.""" diff --git a/cinder/tests/unit/api/contrib/test_volume_actions.py b/cinder/tests/unit/api/contrib/test_volume_actions.py index b17f2acf72b..65052668900 100644 --- a/cinder/tests/unit/api/contrib/test_volume_actions.py +++ b/cinder/tests/unit/api/contrib/test_volume_actions.py @@ -20,11 +20,13 @@ import mock from oslo_config import cfg import oslo_messaging as messaging from oslo_serialization import jsonutils +import six from six.moves import http_client import webob from cinder.api.contrib import volume_actions from cinder.api import microversions as mv +from cinder.api.openstack import api_version_request as api_version from cinder import context from cinder import db from cinder import exception @@ -55,7 +57,7 @@ class VolumeActionsTest(test.TestCase): super(VolumeActionsTest, self).setUp() self.context = context.RequestContext(fake.USER_ID, fake.PROJECT_ID, is_admin=False) - self.UUID = uuid.uuid4() + self.UUID = six.text_type(uuid.uuid4()) self.controller = volume_actions.VolumeActionsController() self.api_patchers = {} for _meth in self._methods: @@ -101,7 +103,8 @@ class VolumeActionsTest(test.TestCase): with mock.patch.object(volume_api.API, 'initialize_connection') as init_conn: init_conn.return_value = {} - body = {'os-initialize_connection': {'connector': 'fake'}} + body = {'os-initialize_connection': {'connector': { + 'fake': 'fake'}}} req = webob.Request.blank('/v2/%s/volumes/%s/action' % (fake.PROJECT_ID, fake.VOLUME_ID)) req.method = "POST" @@ -147,7 +150,8 @@ class VolumeActionsTest(test.TestCase): 'initialize_connection') as init_conn: init_conn.side_effect = \ exception.VolumeBackendAPIException(data=None) - body = {'os-initialize_connection': {'connector': 'fake'}} + body = {'os-initialize_connection': {'connector': { + 'fake': 'fake'}}} req = webob.Request.blank('/v2/%s/volumes/%s/action' % (fake.PROJECT_ID, fake.VOLUME_ID)) req.method = "POST" @@ -262,7 +266,7 @@ class VolumeActionsTest(test.TestCase): self.controller._attach, req, id, - body) + body=body) def test_volume_attach_to_instance_raises_db_error(self): # In case of DB error 500 error code is returned to user @@ -281,7 +285,7 @@ class VolumeActionsTest(test.TestCase): self.controller._attach, req, id, - body) + body=body) def test_detach(self): body = {'os-detach': {'attachment_id': fake.ATTACHMENT_ID}} @@ -309,7 +313,7 @@ class VolumeActionsTest(test.TestCase): self.controller._detach, req, id, - body) + body=body) def test_volume_detach_raises_db_error(self): # In case of DB error 500 error code is returned to user @@ -326,7 +330,7 @@ class VolumeActionsTest(test.TestCase): self.controller._detach, req, id, - body) + body=body) def test_attach_with_invalid_arguments(self): # Invalid request to attach volume an invalid target @@ -793,7 +797,6 @@ class VolumeImageActionsTest(test.TestCase): vol = { "container_format": 'bare', "disk_format": 'raw', - "updated_at": datetime.datetime(1, 1, 1, 1, 1, 1), "image_name": 'image_name', "force": True} body = {"os-volume_upload_image": vol} @@ -853,7 +856,7 @@ class VolumeImageActionsTest(test.TestCase): body = {"os-volume_upload_image": img} req = fakes.HTTPRequest.blank('/v2/%s/volumes/%s/action' % (fake.PROJECT_ID, id)) - res_dict = self.controller._volume_upload_image(req, id, body) + res_dict = self.controller._volume_upload_image(req, id, body=body) expected = {'os-volume_upload_image': {'id': id, 'updated_at': datetime.datetime(1, 1, 1, 1, 1, 1), @@ -887,7 +890,7 @@ class VolumeImageActionsTest(test.TestCase): self.controller._volume_upload_image, req, id, - body) + body=body) @mock.patch.object(volume_api.API, 'get', fake_volume_get_obj) @mock.patch.object(volume_api.API, 'copy_volume_to_image', @@ -905,7 +908,7 @@ class VolumeImageActionsTest(test.TestCase): self.controller._volume_upload_image, req, id, - body) + body=body) @mock.patch.object(volume_api.API, 'get', fake_volume_get) def test_copy_volume_to_image_invalid_disk_format(self): @@ -917,11 +920,11 @@ class VolumeImageActionsTest(test.TestCase): body = {"os-volume_upload_image": vol} req = fakes.HTTPRequest.blank('/v2/%s/volumes/%s/action' % (fake.PROJECT_ID, id)) - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller._volume_upload_image, req, id, - body) + body=body) @mock.patch.object(volume_api.API, "copy_volume_to_image") def test_copy_volume_to_image_disk_format_ploop(self, @@ -938,7 +941,7 @@ class VolumeImageActionsTest(test.TestCase): image_metadata = {'container_format': 'bare', 'disk_format': 'ploop', 'name': 'image_name'} - self.controller._volume_upload_image(req, volume.id, body) + self.controller._volume_upload_image(req, volume.id, body=body) mock_copy_to_image.assert_called_once_with( req.environ['cinder.context'], volume, image_metadata, False) @@ -959,7 +962,7 @@ class VolumeImageActionsTest(test.TestCase): self.controller._volume_upload_image, req, id, - body) + body=body) @mock.patch.object(volume_api.API, 'get', fake_volume_get_obj) @mock.patch.object(volume_api.API, 'copy_volume_to_image', @@ -977,7 +980,35 @@ class VolumeImageActionsTest(test.TestCase): self.controller._volume_upload_image, req, id, - body) + body=body) + + @mock.patch.object(volume_api.API, 'get', fake_volume_get_obj) + @mock.patch.object(volume_api.API, 'copy_volume_to_image', + side_effect=messaging.RemoteError) + @ddt.data( + ({"image_name": 'image_name', "protected": None}, + exception.ValidationError), + ({"image_name": 'image_name', "protected": ' '}, + exception.ValidationError), + ({"image_name": 'image_name', "protected": 'test'}, + exception.ValidationError), + ({"image_name": 'image_name', "visibility": 'test'}, + exception.ValidationError), + ({"image_name": 'image_name', "visibility": ' '}, + exception.ValidationError), + ({"image_name": 'image_name', "visibility": None}, + exception.ValidationError)) + @ddt.unpack + def test_copy_volume_to_image_invalid_request_body( + self, vol, exception, mock_copy): + id = fake.VOLUME2_ID + body = {"os-volume_upload_image": vol} + req = fakes.HTTPRequest.blank('/v3/%s/volumes/%s/action' % + (fake.PROJECT_ID, id)) + req.api_version_request = api_version.APIVersionRequest("3.1") + self.assertRaises(exception, + self.controller._volume_upload_image, + req, id, body=body) def test_volume_upload_image_typeerror(self): id = fake.VOLUME2_ID @@ -1011,11 +1042,11 @@ class VolumeImageActionsTest(test.TestCase): body = {'os-extend': {'new_size': 'fake'}} req = fakes.HTTPRequest.blank('/v2/%s/volumes/%s/action' % (fake.PROJECT_ID, id)) - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller._extend, req, id, - body) + body=body) @ddt.data({'version': mv.get_prior_version(mv.VOLUME_EXTEND_INUSE), 'status': 'available'}, @@ -1036,7 +1067,7 @@ class VolumeImageActionsTest(test.TestCase): req = fakes.HTTPRequest.blank('/v3/%s/volumes/%s/action' % (fake.PROJECT_ID, vol['id'])) req.api_version_request = mv.get_api_version(version) - self.controller._extend(req, vol['id'], body) + self.controller._extend(req, vol['id'], body=body) if version == mv.VOLUME_EXTEND_INUSE and status == 'in-use': mock_extend.assert_called_with(req.environ['cinder.context'], vol, 2, attached=True) @@ -1053,11 +1084,11 @@ class VolumeImageActionsTest(test.TestCase): body = {"os-volume_upload_image": vol} req = fakes.HTTPRequest.blank('/v2/%s/volumes/%s/action' % (fake.PROJECT_ID, id)) - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller._volume_upload_image, req, id, - body) + body=body) def _create_volume_with_type(self, status='available', display_description='displaydesc', **kwargs): @@ -1104,7 +1135,8 @@ class VolumeImageActionsTest(test.TestCase): use_admin_context=self.context.is_admin) body = self._get_os_volume_upload_image() - res_dict = self.controller._volume_upload_image(req, volume.id, body) + res_dict = self.controller._volume_upload_image(req, volume.id, + body=body) self.assertDictEqual(expected, res_dict) vol_db = objects.Volume.get_by_id(self.context, volume.id) @@ -1123,7 +1155,7 @@ class VolumeImageActionsTest(test.TestCase): body['os-volume_upload_image']['visibility'] = 'public' self.assertRaises(exception.PolicyNotAuthorized, self.controller._volume_upload_image, - req, id, body) + req, id, body=body) @mock.patch.object(volume_api.API, "get_volume_image_metadata") @mock.patch.object(glance.GlanceImageService, "create") @@ -1145,7 +1177,8 @@ class VolumeImageActionsTest(test.TestCase): '/v2/%s/volumes/%s/action' % (fake.PROJECT_ID, volume.id), use_admin_context=self.context.is_admin) body = self._get_os_volume_upload_image() - res_dict = self.controller._volume_upload_image(req, volume.id, body) + res_dict = self.controller._volume_upload_image(req, volume.id, + body=body) self.assertDictEqual(expected, res_dict) vol_db = objects.Volume.get_by_id(self.context, volume.id) @@ -1171,7 +1204,7 @@ class VolumeImageActionsTest(test.TestCase): body = self._get_os_volume_upload_image() self.assertRaises(webob.exc.HTTPBadRequest, self.controller._volume_upload_image, req, volume.id, - body) + body=body) self.assertFalse(mock_copy_to_image.called) vol_db = objects.Volume.get_by_id(self.context, volume.id) @@ -1199,7 +1232,7 @@ class VolumeImageActionsTest(test.TestCase): body['os-volume_upload_image']['force'] = False self.assertRaises(webob.exc.HTTPBadRequest, self.controller._volume_upload_image, req, volume.id, - body) + body=body) self.assertFalse(mock_copy_to_image.called) vol_db = objects.Volume.get_by_id(self.context, volume.id) @@ -1227,7 +1260,7 @@ class VolumeImageActionsTest(test.TestCase): body = self._get_os_volume_upload_image() self.assertRaises(webob.exc.HTTPBadRequest, self.controller._volume_upload_image, req, volume.id, - body) + body=body) self.assertFalse(mock_copy_to_image.called) vol_db = objects.Volume.get_by_id(self.context, volume.id) @@ -1235,7 +1268,8 @@ class VolumeImageActionsTest(test.TestCase): self.assertIsNone(vol_db.previous_status) CONF.set_default('enable_force_upload', True) - res_dict = self.controller._volume_upload_image(req, volume.id, body) + res_dict = self.controller._volume_upload_image(req, volume.id, + body=body) self.assertDictEqual(expected, res_dict) @@ -1259,7 +1293,8 @@ class VolumeImageActionsTest(test.TestCase): use_admin_context=self.context.is_admin) body = self._get_os_volume_upload_image() - res_dict = self.controller._volume_upload_image(req, volume.id, body) + res_dict = self.controller._volume_upload_image(req, volume.id, + body=body) self.assertDictEqual(expected, res_dict) vol_db = objects.Volume.get_by_id(self.context, volume.id) @@ -1281,7 +1316,8 @@ class VolumeImageActionsTest(test.TestCase): use_admin_context=self.context.is_admin) body = self._get_os_volume_upload_image() - res_dict = self.controller._volume_upload_image(req, volume.id, body) + res_dict = self.controller._volume_upload_image(req, volume.id, + body=body) self.assertDictEqual(expected, res_dict) vol_db = objects.Volume.get_by_id(self.context, volume.id) @@ -1305,7 +1341,8 @@ class VolumeImageActionsTest(test.TestCase): '/v2/%s/volumes/%s/action' % (fake.PROJECT_ID, volume.id), use_admin_context=self.context.is_admin) body = self._get_os_volume_upload_image() - res_dict = self.controller._volume_upload_image(req, volume.id, body) + res_dict = self.controller._volume_upload_image(req, volume.id, + body=body) self.assertDictEqual(expected, res_dict) @mock.patch.object(volume_api.API, "get_volume_image_metadata") @@ -1334,11 +1371,12 @@ class VolumeImageActionsTest(test.TestCase): req.headers = mv.get_mv_header(mv.UPLOAD_IMAGE_PARAMS) req.api_version_request = mv.get_api_version(mv.UPLOAD_IMAGE_PARAMS) body = self._get_os_volume_upload_image() + body = self._get_os_volume_upload_image() body['os-volume_upload_image']['visibility'] = 'public' body['os-volume_upload_image']['protected'] = True res_dict = self.controller._volume_upload_image(req, volume.id, - body) + body=body) expected['os-volume_upload_image'].update(visibility='public', protected=True) @@ -1360,7 +1398,8 @@ class VolumeImageActionsTest(test.TestCase): body['os-volume_upload_image']['container_format'] = 'bare' body['os-volume_upload_image']['disk_format'] = 'vhd' - res_dict = self.controller._volume_upload_image(req, volume.id, body) + res_dict = self.controller._volume_upload_image(req, volume.id, + body=body) self.assertDictEqual(expected, res_dict) vol_db = objects.Volume.get_by_id(self.context, volume.id) @@ -1383,7 +1422,8 @@ class VolumeImageActionsTest(test.TestCase): body['os-volume_upload_image']['container_format'] = 'bare' body['os-volume_upload_image']['disk_format'] = 'vhdx' - res_dict = self.controller._volume_upload_image(req, volume.id, body) + res_dict = self.controller._volume_upload_image(req, volume.id, + body=body) self.assertDictEqual(expected, res_dict) vol_db = objects.Volume.get_by_id(self.context, volume.id) diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 821b4625f8f..89ea076b154 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -1614,11 +1614,6 @@ class API(base.Base): def retype(self, context, volume, new_type, migration_policy=None): """Attempt to modify the type associated with an existing volume.""" context.authorize(vol_action_policy.RETYPE_POLICY, target_obj=volume) - if migration_policy and migration_policy not in ('on-demand', 'never'): - msg = _('migration_policy must be \'on-demand\' or \'never\', ' - 'passed: %s') % new_type - LOG.error(msg) - raise exception.InvalidInput(reason=msg) # Support specifying volume type by ID or name try: