From 66b0cf333758b5793208c2a734959aa192bbc39b Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Thu, 20 Apr 2017 16:46:24 -0400 Subject: [PATCH] 2.45: Remove Location header from createImage and createBackup responses This changes the response for the createImage and createBackup server action APIs to no longer return a Location header and instead returns a json dict body with the snapshot image ID. This is done in a new microversion. Implements blueprint remove-create-image-location-header-response Closes-Bug: #1679285 Change-Id: Idc899ee76b8265b1c9e0871b6c7c277424cdd442 --- api-ref/source/parameters.yaml | 12 ++++++ api-ref/source/servers-actions.inc | 16 +++++++- .../v2.45/create-backup-req.json | 7 ++++ .../v2.45/create-backup-resp.json | 3 ++ .../server-action-create-image-resp.json | 3 ++ .../v2.45/server-action-create-image.json | 8 ++++ .../versions/v21-version-get-resp.json | 2 +- .../versions/versions-get-resp.json | 2 +- nova/api/openstack/api_version_request.py | 6 ++- nova/api/openstack/compute/create_backup.py | 6 +++ .../compute/rest_api_version_history.rst | 7 ++++ nova/api/openstack/compute/servers.py | 5 +++ .../v2.45/create-backup-req.json.tpl | 7 ++++ .../v2.45/create-backup-resp.json.tpl | 3 ++ .../server-action-create-image-resp.json.tpl | 4 ++ .../v2.45/server-action-create-image.json.tpl | 9 ++++ .../api_sample_tests/test_create_backup.py | 18 ++++++++ .../api_sample_tests/test_servers.py | 41 +++++++++++++++---- .../openstack/compute/test_create_backup.py | 21 ++++++++++ .../openstack/compute/test_server_actions.py | 15 +++++++ .../microversion-2.45-608ba80a84c8aec8.yaml | 11 +++++ 21 files changed, 193 insertions(+), 13 deletions(-) create mode 100644 doc/api_samples/os-create-backup/v2.45/create-backup-req.json create mode 100644 doc/api_samples/os-create-backup/v2.45/create-backup-resp.json create mode 100644 doc/api_samples/servers/v2.45/server-action-create-image-resp.json create mode 100644 doc/api_samples/servers/v2.45/server-action-create-image.json create mode 100644 nova/tests/functional/api_sample_tests/api_samples/os-create-backup/v2.45/create-backup-req.json.tpl create mode 100644 nova/tests/functional/api_sample_tests/api_samples/os-create-backup/v2.45/create-backup-resp.json.tpl create mode 100644 nova/tests/functional/api_sample_tests/api_samples/servers/v2.45/server-action-create-image-resp.json.tpl create mode 100644 nova/tests/functional/api_sample_tests/api_samples/servers/v2.45/server-action-create-image.json.tpl create mode 100644 releasenotes/notes/microversion-2.45-608ba80a84c8aec8.yaml diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 486e856fe65d..7e41c1cfd7da 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -3,9 +3,14 @@ image_location: description: | The image location URL of the image or backup created, HTTP header "Location: " will be returned. + + .. note:: The URL returned may not be accessible to users and should not + be relied upon. Use microversion 2.45 or simply parse the image ID out + of the URL in the Location response header. in: header required: true type: string + max_version: 2.44 tag_location: description: | The location of the tag. It's individual tag URL which can be used for @@ -4582,6 +4587,13 @@ snapshot_id_optional: in: body required: false type: string +snapshot_id_resp_2_45: + description: | + The UUID for the resulting image snapshot. + in: body + required: true + type: string + min_version: 2.45 snapshot_name: description: | The snapshot name. diff --git a/api-ref/source/servers-actions.inc b/api-ref/source/servers-actions.inc index 77382f2c8fd7..4913849a04e0 100644 --- a/api-ref/source/servers-actions.inc +++ b/api-ref/source/servers-actions.inc @@ -244,7 +244,7 @@ Request - rotation: backup_rotation - metadata: metadata -**Example Create Server Back Up (Createbackup Action)** +**Example Create Server Back Up (createBackup Action)** .. literalinclude:: ../../doc/api_samples/os-create-backup/create-backup-req.json :language: javascript @@ -254,7 +254,13 @@ Response .. rest_parameters:: parameters.yaml - - image_location: image_location + - Location: image_location + - image_id: snapshot_id_resp_2_45 + +**Example Create Server Back Up (v2.45)** + +.. literalinclude:: ../../doc/api_samples/os-create-backup/v2.45/create-backup-resp.json + :language: javascript Create Image (createImage Action) @@ -329,6 +335,12 @@ Response .. rest_parameters:: parameters.yaml - Location: image_location + - image_id: snapshot_id_resp_2_45 + +**Example Create Image (v2.45)** + +.. literalinclude:: ../../doc/api_samples/servers/v2.45/server-action-create-image-resp.json + :language: javascript Lock Server (lock Action) diff --git a/doc/api_samples/os-create-backup/v2.45/create-backup-req.json b/doc/api_samples/os-create-backup/v2.45/create-backup-req.json new file mode 100644 index 000000000000..39f70358480f --- /dev/null +++ b/doc/api_samples/os-create-backup/v2.45/create-backup-req.json @@ -0,0 +1,7 @@ +{ + "createBackup": { + "name": "Backup 1", + "backup_type": "weekly", + "rotation": 1 + } +} diff --git a/doc/api_samples/os-create-backup/v2.45/create-backup-resp.json b/doc/api_samples/os-create-backup/v2.45/create-backup-resp.json new file mode 100644 index 000000000000..05594917ddc4 --- /dev/null +++ b/doc/api_samples/os-create-backup/v2.45/create-backup-resp.json @@ -0,0 +1,3 @@ +{ + "image_id": "0e7761dd-ee98-41f0-ba35-05994e446431" +} \ No newline at end of file diff --git a/doc/api_samples/servers/v2.45/server-action-create-image-resp.json b/doc/api_samples/servers/v2.45/server-action-create-image-resp.json new file mode 100644 index 000000000000..05594917ddc4 --- /dev/null +++ b/doc/api_samples/servers/v2.45/server-action-create-image-resp.json @@ -0,0 +1,3 @@ +{ + "image_id": "0e7761dd-ee98-41f0-ba35-05994e446431" +} \ No newline at end of file diff --git a/doc/api_samples/servers/v2.45/server-action-create-image.json b/doc/api_samples/servers/v2.45/server-action-create-image.json new file mode 100644 index 000000000000..dcb5165e48f7 --- /dev/null +++ b/doc/api_samples/servers/v2.45/server-action-create-image.json @@ -0,0 +1,8 @@ +{ + "createImage" : { + "name" : "foo-image", + "metadata": { + "meta_var": "meta_val" + } + } +} \ No newline at end of file diff --git a/doc/api_samples/versions/v21-version-get-resp.json b/doc/api_samples/versions/v21-version-get-resp.json index 793177e1007c..f935867d3e8d 100644 --- a/doc/api_samples/versions/v21-version-get-resp.json +++ b/doc/api_samples/versions/v21-version-get-resp.json @@ -19,7 +19,7 @@ } ], "status": "CURRENT", - "version": "2.44", + "version": "2.45", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/doc/api_samples/versions/versions-get-resp.json b/doc/api_samples/versions/versions-get-resp.json index e0b3d8a024f2..e00ea77b5ca1 100644 --- a/doc/api_samples/versions/versions-get-resp.json +++ b/doc/api_samples/versions/versions-get-resp.json @@ -22,7 +22,7 @@ } ], "status": "CURRENT", - "version": "2.44", + "version": "2.45", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/nova/api/openstack/api_version_request.py b/nova/api/openstack/api_version_request.py index d1409730dffa..a9b58d887f29 100644 --- a/nova/api/openstack/api_version_request.py +++ b/nova/api/openstack/api_version_request.py @@ -105,6 +105,10 @@ REST_API_VERSION_HISTORY = """REST API Version History: * 2.43 - Deprecate os-hosts API * 2.44 - The servers action addFixedIp, removeFixedIp, addFloatingIp, removeFloatingIp and os-virtual-interfaces APIs are deprecated. + * 2.45 - The createImage and createBackup APIs no longer return a Location + header in the response for the snapshot image, they now return a + json dict in the response body with an image_id key and uuid + value. """ # The minimum and maximum versions of the API supported @@ -113,7 +117,7 @@ REST_API_VERSION_HISTORY = """REST API Version History: # Note(cyeoh): This only applies for the v2.1 API once microversions # support is fully merged. It does not affect the V2 API. _MIN_API_VERSION = "2.1" -_MAX_API_VERSION = "2.44" +_MAX_API_VERSION = "2.45" DEFAULT_API_VERSION = _MIN_API_VERSION # Almost all proxy APIs which related to network, images and baremetal diff --git a/nova/api/openstack/compute/create_backup.py b/nova/api/openstack/compute/create_backup.py index fbe950f7c388..98f0d08f53c8 100644 --- a/nova/api/openstack/compute/create_backup.py +++ b/nova/api/openstack/compute/create_backup.py @@ -33,6 +33,7 @@ class CreateBackupController(wsgi.Controller): super(CreateBackupController, self).__init__(*args, **kwargs) self.compute_api = compute.API() + @wsgi.response(202) @extensions.expected_errors((400, 403, 404, 409)) @wsgi.action('createBackup') @validation.schema(create_backup.create_backup_v20, '2.0', '2.0') @@ -78,6 +79,11 @@ class CreateBackupController(wsgi.Controller): except exception.InvalidRequest as e: raise webob.exc.HTTPBadRequest(explanation=e.format_message()) + # Starting with microversion 2.45 we return a response body containing + # the snapshot image id without the Location header. + if api_version_request.is_supported(req, '2.45'): + return {'image_id': image['id']} + resp = webob.Response(status_int=202) # build location of newly-created image entity if rotation is not zero diff --git a/nova/api/openstack/compute/rest_api_version_history.rst b/nova/api/openstack/compute/rest_api_version_history.rst index b8a6c50f2809..f2055bbea89f 100644 --- a/nova/api/openstack/compute/rest_api_version_history.rst +++ b/nova/api/openstack/compute/rest_api_version_history.rst @@ -529,3 +529,10 @@ user documentation. To query attached neutron interfaces for a specific server, the API `GET /servers/{server_uuid}/os-interface` can be used. + +2.45 +---- + + The ``createImage`` and ``createBackup`` server action APIs no longer return + a ``Location`` header in the response for the snapshot image, they now return + a json dict in the response body with an ``image_id`` key and uuid value. diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index e9a451fdb87d..49ac271b74bd 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -1017,6 +1017,11 @@ class ServersController(wsgi.Controller): except exception.Invalid as err: raise exc.HTTPBadRequest(explanation=err.format_message()) + # Starting with microversion 2.45 we return a response body containing + # the snapshot image id without the Location header. + if api_version_request.is_supported(req, '2.45'): + return {'image_id': image['id']} + # build location of newly-created image entity image_id = str(image['id']) image_ref = glance.generate_image_url(image_id) diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-create-backup/v2.45/create-backup-req.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-create-backup/v2.45/create-backup-req.json.tpl new file mode 100644 index 000000000000..39f70358480f --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/os-create-backup/v2.45/create-backup-req.json.tpl @@ -0,0 +1,7 @@ +{ + "createBackup": { + "name": "Backup 1", + "backup_type": "weekly", + "rotation": 1 + } +} diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-create-backup/v2.45/create-backup-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-create-backup/v2.45/create-backup-resp.json.tpl new file mode 100644 index 000000000000..6239b2b7cbf9 --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/os-create-backup/v2.45/create-backup-resp.json.tpl @@ -0,0 +1,3 @@ +{ + "image_id": "%(uuid)s" +} diff --git a/nova/tests/functional/api_sample_tests/api_samples/servers/v2.45/server-action-create-image-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.45/server-action-create-image-resp.json.tpl new file mode 100644 index 000000000000..6caf39ad7633 --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.45/server-action-create-image-resp.json.tpl @@ -0,0 +1,4 @@ +{ + "image_id": "%(uuid)s" +} + diff --git a/nova/tests/functional/api_sample_tests/api_samples/servers/v2.45/server-action-create-image.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.45/server-action-create-image.json.tpl new file mode 100644 index 000000000000..19c2c489a4d2 --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.45/server-action-create-image.json.tpl @@ -0,0 +1,9 @@ +{ + "createImage" : { + "name" : "%(name)s", + "metadata": { + "meta_var": "meta_val" + } + } +} + diff --git a/nova/tests/functional/api_sample_tests/test_create_backup.py b/nova/tests/functional/api_sample_tests/test_create_backup.py index 5bb4b6c247e8..533897c34c46 100644 --- a/nova/tests/functional/api_sample_tests/test_create_backup.py +++ b/nova/tests/functional/api_sample_tests/test_create_backup.py @@ -36,3 +36,21 @@ class CreateBackupSamplesJsonTest(test_servers.ServersSampleBase): response = self._do_post('servers/%s/action' % self.uuid, 'create-backup-req', {}) self.assertEqual(202, response.status_code) + # we should have gotten a location header back + self.assertIn('location', response.headers) + # we should not have gotten a body back + self.assertEqual(0, len(response.content)) + + +class CreateBackupSamplesJsonTestv2_45(CreateBackupSamplesJsonTest): + """Tests the createBackup server action API with microversion 2.45.""" + microversion = '2.45' + scenarios = [('v2_45', {'api_major_version': 'v2.1'})] + + def test_post_backup_server(self): + # Get api samples to backup server request. + response = self._do_post('servers/%s/action' % self.uuid, + 'create-backup-req', {}) + self._verify_response('create-backup-resp', {}, response, 202) + # assert that no location header was returned + self.assertNotIn('location', response.headers) diff --git a/nova/tests/functional/api_sample_tests/test_servers.py b/nova/tests/functional/api_sample_tests/test_servers.py index f46a9833d7e9..56c83b0ea393 100644 --- a/nova/tests/functional/api_sample_tests/test_servers.py +++ b/nova/tests/functional/api_sample_tests/test_servers.py @@ -225,8 +225,7 @@ class ServerSortKeysJsonTests(ServersSampleBase): 200) -class ServersActionsJsonTest(ServersSampleBase): - +class _ServersActionsJsonTestMixin(object): def _test_server_action(self, uuid, action, req_tpl, subs=None, resp_tpl=None, code=202): subs = subs or {} @@ -240,6 +239,10 @@ class ServersActionsJsonTest(ServersSampleBase): else: self.assertEqual(code, response.status_code) self.assertEqual("", response.text) + return response + + +class ServersActionsJsonTest(ServersSampleBase, _ServersActionsJsonTestMixin): def test_server_reboot_hard(self): uuid = self._post_server() @@ -291,12 +294,6 @@ class ServersActionsJsonTest(ServersSampleBase): 'server-action-confirm-resize', code=204) - def test_server_create_image(self): - uuid = self._post_server() - self._test_server_action(uuid, 'createImage', - 'server-action-create-image', - {'name': 'foo-image'}) - def _wait_for_active_server(self, uuid): """Wait 10 seconds for the server to be ACTIVE, else fail. @@ -370,6 +367,34 @@ class ServersActionsJson219Test(ServersSampleBase): self._verify_response('server-action-rebuild-resp', subs, resp, 202) +class ServersCreateImageJsonTest(ServersSampleBase, + _ServersActionsJsonTestMixin): + """Tests the createImage server action API against 2.1.""" + def test_server_create_image(self): + uuid = self._post_server() + resp = self._test_server_action(uuid, 'createImage', + 'server-action-create-image', + {'name': 'foo-image'}) + # we should have gotten a location header back + self.assertIn('location', resp.headers) + # we should not have gotten a body back + self.assertEqual(0, len(resp.content)) + + +class ServersCreateImageJsonTestv2_45(ServersCreateImageJsonTest): + """Tests the createImage server action API against 2.45.""" + microversion = '2.45' + scenarios = [('v2_45', {'api_major_version': 'v2.1'})] + + def test_server_create_image(self): + uuid = self._post_server() + resp = self._test_server_action( + uuid, 'createImage', 'server-action-create-image', + {'name': 'foo-image'}, 'server-action-create-image-resp') + # assert that no location header was returned + self.assertNotIn('location', resp.headers) + + class ServerStartStopJsonTest(ServersSampleBase): def _test_server_action(self, uuid, action, req_tpl): diff --git a/nova/tests/unit/api/openstack/compute/test_create_backup.py b/nova/tests/unit/api/openstack/compute/test_create_backup.py index 0ebb3cdd576a..2b43eb51da67 100644 --- a/nova/tests/unit/api/openstack/compute/test_create_backup.py +++ b/nova/tests/unit/api/openstack/compute/test_create_backup.py @@ -283,6 +283,27 @@ class CreateBackupTestsV21(admin_only_action_common.CommonMixin, self.assertEqual(202, res.status_int) self.assertIn('fake-image-id', res.headers['Location']) + @mock.patch.object(common, 'check_img_metadata_properties_quota') + @mock.patch.object(api.API, 'backup', return_value=dict( + id='fake-image-id', status='ACTIVE', name='Backup 1', properties={})) + def test_create_backup_v2_45(self, mock_backup, mock_check_image): + """Tests the 2.45 microversion to ensure the Location header is not + in the response. + """ + body = { + 'createBackup': { + 'name': 'Backup 1', + 'backup_type': 'daily', + 'rotation': '1', + }, + } + instance = fake_instance.fake_instance_obj(self.context) + self.mock_get.return_value = instance + req = fakes.HTTPRequest.blank('', version='2.45') + res = self.controller._create_backup(req, instance['uuid'], body=body) + self.assertIsInstance(res, dict) + self.assertEqual('fake-image-id', res['image_id']) + @mock.patch.object(common, 'check_img_metadata_properties_quota') @mock.patch.object(api.API, 'backup') def test_create_backup_raises_conflict_on_invalid_state(self, diff --git a/nova/tests/unit/api/openstack/compute/test_server_actions.py b/nova/tests/unit/api/openstack/compute/test_server_actions.py index 40b0625549da..4bc059ecffb8 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_server_actions.py @@ -901,6 +901,21 @@ class ServerActionsControllerTestV21(test.TestCase): glance.generate_image_url('123'), location) + def test_create_image_v2_45(self): + """Tests the createImage server action API with the 2.45 microversion + where there is a response body but no Location header. + """ + body = { + 'createImage': { + 'name': 'Snapshot 1', + }, + } + req = fakes.HTTPRequest.blank('', version='2.45') + response = self.controller._action_create_image(req, FAKE_UUID, + body=body) + self.assertIsInstance(response, dict) + self.assertEqual('123', response['image_id']) + def test_create_image_name_too_long(self): long_name = 'a' * 260 body = { diff --git a/releasenotes/notes/microversion-2.45-608ba80a84c8aec8.yaml b/releasenotes/notes/microversion-2.45-608ba80a84c8aec8.yaml new file mode 100644 index 000000000000..032fc3cd8cca --- /dev/null +++ b/releasenotes/notes/microversion-2.45-608ba80a84c8aec8.yaml @@ -0,0 +1,11 @@ +--- +other: + - | + The 2.45 microversion is introduced which changes the response for the + ``createImage`` and ``createBackup`` server action APIs to no longer + return a ``Location`` response header. With microversion 2.45 those APIs + now return a json dict in the response body with a single ``image_id`` key + whose value is the snapshot image ID (a uuid). The old ``Location`` header + in the response before microversion 2.45 is most likely broken and + inaccessible by end users since it relies on the internal Glance API + server configuration and does not take into account Glance API versions.