Fix error status code on update-volume API
As the following part of API-WG guidline[1], If a request contains a reference to a nonexistent resource in the body (not URI), the code should be 400 Bad Request. Do not use 404 NotFound because :rfc:`7231#section-6.5.4` (section 6.5.4) mentions the origin server did not find a current representation for the target resource for 404 and representation for the target resource means a URI Nova should return a BadRequest response(400) in this case, because new_volume_id is specified in a request body. old_volume_id is not necessary to be changed because the value is specified with URI. So it is valid to return NotFound response if that is not existent. [1]: https://github.com/openstack/api-wg/blob/master/guidelines/http.rst#failure-code-clarifications Close-Bug: #1629110 Change-Id: Ib781b116f5af713d64b5880858cc4f81c3da3977 (cherry picked from commitedd86d9dac
) (cherry picked from commit4e6958551c
)
This commit is contained in:
parent
090bff767b
commit
a967cfe37a
|
@ -355,8 +355,19 @@ class VolumeAttachmentController(wsgi.Controller):
|
|||
except KeyError:
|
||||
msg = _("volumeId must be specified.")
|
||||
raise exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
self._validate_volume_id(new_volume_id)
|
||||
new_volume = self.volume_api.get(context, new_volume_id)
|
||||
try:
|
||||
new_volume = self.volume_api.get(context, new_volume_id)
|
||||
except exception.VolumeNotFound as e:
|
||||
# NOTE: This BadRequest is different from the above NotFound even
|
||||
# though the same VolumeNotFound exception. This is intentional
|
||||
# because new_volume_id is specified in a request body and if a
|
||||
# nonexistent resource in the body (not URI) the code should be
|
||||
# 400 Bad Request as API-WG guideline. On the other hand,
|
||||
# old_volume_id is specified with URI. So it is valid to return
|
||||
# NotFound response if that is not existent.
|
||||
raise exc.HTTPBadRequest(explanation=e.format_message())
|
||||
|
||||
instance = common.get_instance(self.compute_api, context, server_id)
|
||||
|
||||
|
|
|
@ -325,12 +325,22 @@ class VolumeAttachmentController(wsgi.Controller):
|
|||
old_volume_id = id
|
||||
try:
|
||||
old_volume = self.volume_api.get(context, old_volume_id)
|
||||
|
||||
new_volume_id = body['volumeAttachment']['volumeId']
|
||||
new_volume = self.volume_api.get(context, new_volume_id)
|
||||
except exception.VolumeNotFound as e:
|
||||
raise exc.HTTPNotFound(explanation=e.format_message())
|
||||
|
||||
new_volume_id = body['volumeAttachment']['volumeId']
|
||||
try:
|
||||
new_volume = self.volume_api.get(context, new_volume_id)
|
||||
except exception.VolumeNotFound as e:
|
||||
# NOTE: This BadRequest is different from the above NotFound even
|
||||
# though the same VolumeNotFound exception. This is intentional
|
||||
# because new_volume_id is specified in a request body and if a
|
||||
# nonexistent resource in the body (not URI) the code should be
|
||||
# 400 Bad Request as API-WG guideline. On the other hand,
|
||||
# old_volume_id is specified with URI. So it is valid to return
|
||||
# NotFound response if that is not existent.
|
||||
raise exc.HTTPBadRequest(explanation=e.format_message())
|
||||
|
||||
instance = common.get_instance(self.compute_api, context, server_id)
|
||||
|
||||
bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
|
||||
|
|
|
@ -650,9 +650,17 @@ class VolumeAttachTestsV21(test.NoDBTestCase):
|
|||
status_int = result.status_int
|
||||
self.assertEqual(202, status_int)
|
||||
|
||||
def test_swap_volume_no_attachment(self):
|
||||
def test_swap_volume_with_nonexistent_uri(self):
|
||||
self.assertRaises(exc.HTTPNotFound, self._test_swap,
|
||||
self.attachments, FAKE_UUID_C)
|
||||
self.attachments, uuid=FAKE_UUID_C)
|
||||
|
||||
@mock.patch.object(cinder.API, 'get')
|
||||
def test_swap_volume_with_nonexistent_dest_in_body(self, mock_update):
|
||||
mock_update.side_effect = [
|
||||
None, exception.VolumeNotFound(volume_id=FAKE_UUID_C)]
|
||||
body = {'volumeAttachment': {'volumeId': FAKE_UUID_C}}
|
||||
self.assertRaises(exc.HTTPBadRequest, self._test_swap,
|
||||
self.attachments, body=body)
|
||||
|
||||
def test_swap_volume_without_volumeId(self):
|
||||
body = {'volumeAttachment': {'device': '/dev/fake'}}
|
||||
|
|
Loading…
Reference in New Issue