From cbd3ec476f769c42e5b2a0ef8c996b60935e7f6c Mon Sep 17 00:00:00 2001 From: ghanshyam Date: Thu, 28 Jul 2016 16:11:47 +0900 Subject: [PATCH] Strict ImageRef validation to UUID only Currently imageRef in server create, rebuild and rescue operation can be accepted as random url which contains image UUID and fetch the UUID from that. As /images proxy APIs are deprecated, and ImageRef in server creation etc are UUID only and valid against glance. This patch makes imageRef handling as UUID only and return 400 if non UUID are requested. NOTE- Previously nova use to allow the empty string which was ok in case of boot from volume. We will keep the same behavior of allowing empty string in case of boot from volume only and 400 in all other case. Closes-Bug: #1607229 Change-Id: I49f4da62c1b5b3fd8c5f67039ae113f76722b26c --- api-ref/source/parameters.yaml | 6 +- .../servers/v2.19/server-create-req.json | 2 +- nova/api/openstack/common.py | 39 ----- nova/api/openstack/compute/rescue.py | 5 +- nova/api/openstack/compute/schemas/rescue.py | 2 +- nova/api/openstack/compute/schemas/servers.py | 9 +- nova/api/openstack/compute/servers.py | 4 +- nova/api/validation/parameter_types.py | 13 +- .../servers/v2.19/server-create-req.json.tpl | 2 +- nova/tests/functional/integrated_helpers.py | 38 +---- .../compute/test_block_device_mapping.py | 38 +++++ .../compute/test_block_device_mapping_v1.py | 45 ++++++ .../unit/api/openstack/compute/test_rescue.py | 24 +++- .../openstack/compute/test_server_actions.py | 16 +-- .../api/openstack/compute/test_serversV21.py | 136 ++++++------------ ...mageRef-as-uuid-only-0164c04206a42683.yaml | 17 +++ 16 files changed, 197 insertions(+), 199 deletions(-) create mode 100644 releasenotes/notes/imageRef-as-uuid-only-0164c04206a42683.yaml diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index a116de49c09d..89c092da9256 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -2262,8 +2262,10 @@ image_status: type: string imageRef: description: | - The image reference, as a UUID or full URL, for the image to use for your server - instance. + The UUID of the image to use for your server instance. + This is not required in case of boot from volume. + In all other cases it is required and must be a valid UUID + otherwise API will return 400. in: body required: true type: string diff --git a/doc/api_samples/servers/v2.19/server-create-req.json b/doc/api_samples/servers/v2.19/server-create-req.json index 24cdb9c2e5c5..2a14738ed837 100644 --- a/doc/api_samples/servers/v2.19/server-create-req.json +++ b/doc/api_samples/servers/v2.19/server-create-req.json @@ -4,7 +4,7 @@ "accessIPv6": "80fe::", "name" : "new-server-test", "description" : "new-server-description", - "imageRef" : "http://glance.openstack.example.com/images/70a599e0-31e7-49b7-b260-868f441e862b", + "imageRef" : "70a599e0-31e7-49b7-b260-868f441e862b", "flavorRef" : "http://openstack.example.com/flavors/1", "metadata" : { "My Server Name" : "Apache1" diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index 3fcb4bbdbb63..56ccb9d6149e 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -20,7 +20,6 @@ import re from oslo_log import log as logging from oslo_utils import strutils -from oslo_utils import uuidutils import six import six.moves.urllib.parse as urlparse import webob @@ -548,41 +547,3 @@ def is_all_tenants(search_opts): # The empty string is considered enabling all_tenants all_tenants = 'all_tenants' in search_opts return all_tenants - - -def image_uuid_from_href(image_href, key): - """If the image href was generated by nova api, strip image_href down to an - id and use the default glance connection params - - :param image_href: URL or UUID for an image. - :param key: The request body key for the image_href value. This is used in - error messages. - :returns: The parsed image UUID. - :raises: webob.exc.HTTPBadRequest if the image_href does not have a valid - image UUID in it. - """ - if not image_href: - # NOTE(mriedem): This error message is copied from how our jsonschema - # validation error looks. - msg = _("Invalid input for field/attribute %(path)s. " - "Value: %(value)s. %(message)s") % { - 'path': key, 'value': image_href, - 'message': ( - 'Invalid image reference format. Specify the image ' - 'reference by UUID or full URL.') - } - raise exc.HTTPBadRequest(explanation=msg) - - image_uuid = image_href.split('/').pop() - - if not uuidutils.is_uuid_like(image_uuid): - msg = _("Invalid input for field/attribute %(path)s. " - "Value: %(value)s. %(message)s") % { - 'path': key, 'value': image_href, - 'message': ( - 'Invalid image reference format. Specify the image ' - 'reference by UUID or full URL.') - } - raise exc.HTTPBadRequest(explanation=msg) - - return image_uuid diff --git a/nova/api/openstack/compute/rescue.py b/nova/api/openstack/compute/rescue.py index 5cc74d665659..a4d63cb6f856 100644 --- a/nova/api/openstack/compute/rescue.py +++ b/nova/api/openstack/compute/rescue.py @@ -55,9 +55,8 @@ class RescueController(wsgi.Controller): instance = common.get_instance(self.compute_api, context, id) rescue_image_ref = None - if body['rescue'] and 'rescue_image_ref' in body['rescue']: - rescue_image_ref = common.image_uuid_from_href( - body['rescue']['rescue_image_ref'], 'rescue_image_ref') + if body['rescue']: + rescue_image_ref = body['rescue'].get('rescue_image_ref') try: self.compute_api.rescue(context, instance, diff --git a/nova/api/openstack/compute/schemas/rescue.py b/nova/api/openstack/compute/schemas/rescue.py index 92b6247ff343..f60bdf886426 100644 --- a/nova/api/openstack/compute/schemas/rescue.py +++ b/nova/api/openstack/compute/schemas/rescue.py @@ -22,7 +22,7 @@ rescue = { 'type': ['object', 'null'], 'properties': { 'adminPass': parameter_types.admin_password, - 'rescue_image_ref': parameter_types.image_ref, + 'rescue_image_ref': parameter_types.image_id, }, 'additionalProperties': False, }, diff --git a/nova/api/openstack/compute/schemas/servers.py b/nova/api/openstack/compute/schemas/servers.py index 924363f59253..30293285d792 100644 --- a/nova/api/openstack/compute/schemas/servers.py +++ b/nova/api/openstack/compute/schemas/servers.py @@ -25,7 +25,12 @@ base_create = { 'type': 'object', 'properties': { 'name': parameter_types.name, - 'imageRef': parameter_types.image_ref, + # NOTE(gmann): In case of boot from volume, imageRef was + # allowed as the empty string also So keeping the same + # behavior and allow empty string in case of boot from + # volume only. Python code make sure empty string is + # not alowed for other cases. + 'imageRef': parameter_types.image_id_or_empty_string, 'flavorRef': parameter_types.flavor_ref, 'adminPass': parameter_types.admin_password, 'metadata': parameter_types.metadata, @@ -108,7 +113,7 @@ base_rebuild = { 'type': 'object', 'properties': { 'name': parameter_types.name, - 'imageRef': parameter_types.image_ref, + 'imageRef': parameter_types.image_id, 'adminPass': parameter_types.admin_password, 'metadata': parameter_types.metadata, 'preserve_ephemeral': parameter_types.boolean, diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 07e9116bd32a..4be1eea78937 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -864,8 +864,7 @@ class ServersController(wsgi.Controller): if not image_href and create_kwargs.get('block_device_mapping'): return '' elif image_href: - return common.image_uuid_from_href(six.text_type(image_href), - 'imageRef') + return image_href else: msg = _("Missing imageRef attribute") raise exc.HTTPBadRequest(explanation=msg) @@ -899,7 +898,6 @@ class ServersController(wsgi.Controller): rebuild_dict = body['rebuild'] image_href = rebuild_dict["imageRef"] - image_href = common.image_uuid_from_href(image_href, 'imageRef') password = self._get_server_admin_password(rebuild_dict) diff --git a/nova/api/validation/parameter_types.py b/nova/api/validation/parameter_types.py index 4110a5e0f017..c3e548709a91 100644 --- a/nova/api/validation/parameter_types.py +++ b/nova/api/validation/parameter_types.py @@ -269,6 +269,14 @@ image_id = { } +image_id_or_empty_string = { + 'oneOf': [ + {'type': 'string', 'format': 'uuid'}, + {'type': 'string', 'maxLength': 0} + ] +} + + volume_id = { 'type': 'string', 'format': 'uuid' } @@ -294,11 +302,6 @@ admin_password = { } -image_ref = { - 'type': 'string', -} - - flavor_ref = { 'type': ['string', 'integer'], 'minLength': 1 } diff --git a/nova/tests/functional/api_sample_tests/api_samples/servers/v2.19/server-create-req.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.19/server-create-req.json.tpl index b1306fea57db..4ccb45357ed2 100644 --- a/nova/tests/functional/api_sample_tests/api_samples/servers/v2.19/server-create-req.json.tpl +++ b/nova/tests/functional/api_sample_tests/api_samples/servers/v2.19/server-create-req.json.tpl @@ -4,7 +4,7 @@ "accessIPv6": "%(access_ip_v6)s", "name" : "new-server-test", "description" : "new-server-description", - "imageRef" : "%(glance_host)s/images/%(image_id)s", + "imageRef" : "%(image_id)s", "flavorRef" : "%(host)s/flavors/1", "metadata" : { "My Server Name" : "Apache1" diff --git a/nova/tests/functional/integrated_helpers.py b/nova/tests/functional/integrated_helpers.py index 38f21b63a365..f01993ba9a50 100644 --- a/nova/tests/functional/integrated_helpers.py +++ b/nova/tests/functional/integrated_helpers.py @@ -120,24 +120,11 @@ class _IntegratedTestBase(test.TestCase): def get_invalid_image(self): return str(uuid.uuid4()) - def _get_any_image_href(self): - image = self.api.get_images()[0] - LOG.debug("Image: %s" % image) - - if self._image_ref_parameter in image: - image_href = image[self._image_ref_parameter] - else: - image_href = image['id'] - image_href = 'http://fake.server/%s' % image_href - return image_href - def _build_minimal_create_server_request(self): server = {} - image_href = self._get_any_image_href() - # We now have a valid imageId - server[self._image_ref_parameter] = image_href + server[self._image_ref_parameter] = self.api.get_images()[0]['id'] # Set a valid flavorId flavor = self.api.get_flavors()[0] @@ -181,19 +168,11 @@ class _IntegratedTestBase(test.TestCase): def _build_server(self, flavor_id): server = {} - - image_href = self._get_any_image_href() image = self.api.get_images()[0] LOG.debug("Image: %s" % image) - if self._image_ref_parameter in image: - image_href = image[self._image_ref_parameter] - else: - image_href = image['id'] - image_href = 'http://fake.server/%s' % image_href - # We now have a valid imageId - server[self._image_ref_parameter] = image_href + server[self._image_ref_parameter] = image['id'] # Set a valid flavorId flavor = self.api.get_flavor(flavor_id) @@ -243,19 +222,8 @@ class InstanceHelperMixin(object): flavor_id=None): server = {} - if image_uuid: - image_href = 'http://fake.server/%s' % image_uuid - else: - image = api.get_images()[0] - - if 'imageRef' in image: - image_href = image['imageRef'] - else: - image_href = image['id'] - image_href = 'http://fake.server/%s' % image_href - # We now have a valid imageId - server['imageRef'] = image_href + server['imageRef'] = image_uuid or api.get_images()[0]['id'] if not flavor_id: # Set a valid flavorId diff --git a/nova/tests/unit/api/openstack/compute/test_block_device_mapping.py b/nova/tests/unit/api/openstack/compute/test_block_device_mapping.py index 0408cd5e2d24..79a8a47d066c 100644 --- a/nova/tests/unit/api/openstack/compute/test_block_device_mapping.py +++ b/nova/tests/unit/api/openstack/compute/test_block_device_mapping.py @@ -148,6 +148,44 @@ class BlockDeviceMappingTestV21(test.TestCase): params = {block_device_mapping.ATTRIBUTE_NAME: self.bdm} self._test_create(params, no_image=True) + @mock.patch.object(compute_api.API, '_validate_bdm') + @mock.patch.object(compute_api.API, '_get_bdm_image_metadata') + def test_create_instance_with_bdms_and_empty_imageRef( + self, mock_bdm_image_metadata, mock_validate_bdm): + mock_bdm_image_metadata.return_value = {} + mock_validate_bdm.return_value = True + old_create = compute_api.API.create + + def create(*args, **kwargs): + self.assertThat( + block_device.BlockDeviceDict(self.bdm[0]), + matchers.DictMatches(kwargs['block_device_mapping'][0]) + ) + return old_create(*args, **kwargs) + + self.stub_out('nova.compute.api.API.create', create) + + params = {block_device_mapping.ATTRIBUTE_NAME: self.bdm, + 'imageRef': ''} + self._test_create(params) + + def test_create_instance_with_imageRef_as_full_url(self): + bdm = [{'device_name': 'foo'}] + image_href = ('http://localhost/v2/fake/images/' + '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6') + params = {block_device_mapping.ATTRIBUTE_NAME: bdm, + 'imageRef': image_href} + self.assertRaises(exception.ValidationError, + self._test_create, params) + + def test_create_instance_with_non_uuid_imageRef(self): + bdm = [{'device_name': 'foo'}] + + params = {block_device_mapping.ATTRIBUTE_NAME: bdm, + 'imageRef': '123123abcd'} + self.assertRaises(exception.ValidationError, + self._test_create, params) + def test_create_instance_with_device_name_not_string(self): self.bdm[0]['device_name'] = 123 old_create = compute_api.API.create diff --git a/nova/tests/unit/api/openstack/compute/test_block_device_mapping_v1.py b/nova/tests/unit/api/openstack/compute/test_block_device_mapping_v1.py index 74bae5387c91..7de77077d04c 100644 --- a/nova/tests/unit/api/openstack/compute/test_block_device_mapping_v1.py +++ b/nova/tests/unit/api/openstack/compute/test_block_device_mapping_v1.py @@ -140,6 +140,51 @@ class BlockDeviceMappingTestV21(test.TestCase): self.mox.ReplayAll() self._test_create(params, no_image=True) + @mock.patch.object(compute_api.API, '_validate_bdm') + @mock.patch.object(compute_api.API, '_get_bdm_image_metadata') + def test_create_instance_with_imageRef_as_empty_string( + self, mock_bdm_image_metadata, mock_validate_bdm): + volume = { + 'id': uuids.volume_id, + 'status': 'active', + 'volume_image_metadata': + {'test_key': 'test_value'} + } + mock_bdm_image_metadata.return_value = volume + mock_validate_bdm.return_value = True + params = {'block_device_mapping': self.bdm, + 'imageRef': ''} + old_create = compute_api.API.create + + def create(*args, **kwargs): + self.assertEqual(kwargs['block_device_mapping'], self.bdm) + return old_create(*args, **kwargs) + + self.stub_out('nova.compute.api.API.create', create) + self._test_create(params) + + def test_create_instance_with_imageRef_as_full_url(self): + bdm = [{ + 'volume_id': self.volume_id, + 'device_name': 'vda' + }] + image_href = ('http://localhost/v2/fake/images/' + '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6') + params = {'block_device_mapping': bdm, + 'imageRef': image_href} + self.assertRaises(exception.ValidationError, + self._test_create, params) + + def test_create_instance_with_non_uuid_imageRef(self): + bdm = [{ + 'volume_id': self.volume_id, + 'device_name': 'vda' + }] + params = {'block_device_mapping': bdm, + 'imageRef': 'bad-format'} + self.assertRaises(exception.ValidationError, + self._test_create, params) + def test_create_instance_with_volumes_disabled(self): bdm = [{'device_name': 'foo'}] params = {'block_device_mapping': bdm} diff --git a/nova/tests/unit/api/openstack/compute/test_rescue.py b/nova/tests/unit/api/openstack/compute/test_rescue.py index bd1a3dccae0b..ee036c467fad 100644 --- a/nova/tests/unit/api/openstack/compute/test_rescue.py +++ b/nova/tests/unit/api/openstack/compute/test_rescue.py @@ -43,7 +43,6 @@ def fake_compute_get(*args, **kwargs): class RescueTestV21(test.NoDBTestCase): image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' - image_href = 'http://localhost/v2/fake/images/%s' % image_uuid def setUp(self): super(RescueTestV21, self).setUp() @@ -139,11 +138,26 @@ class RescueTestV21(test.NoDBTestCase): self.controller._rescue, self.fake_req, UUID, body=body) - @mock.patch('nova.compute.api.API.rescue') - def test_rescue_with_bad_image_specified(self, mock_compute_api_rescue): + def test_rescue_with_bad_image_specified(self): body = {"rescue": {"adminPass": "ABC123", "rescue_image_ref": "img-id"}} - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, + self.controller._rescue, + self.fake_req, UUID, body=body) + + def test_rescue_with_imageRef_as_full_url(self): + image_href = ('http://localhost/v2/fake/images/' + '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6') + body = {"rescue": {"adminPass": "ABC123", + "rescue_image_ref": image_href}} + self.assertRaises(exception.ValidationError, + self.controller._rescue, + self.fake_req, UUID, body=body) + + def test_rescue_with_imageRef_as_empty_string(self): + body = {"rescue": {"adminPass": "ABC123", + "rescue_image_ref": ''}} + self.assertRaises(exception.ValidationError, self.controller._rescue, self.fake_req, UUID, body=body) @@ -151,7 +165,7 @@ class RescueTestV21(test.NoDBTestCase): def test_rescue_with_image_specified(self, mock_compute_api_rescue): instance = fake_compute_get() body = {"rescue": {"adminPass": "ABC123", - "rescue_image_ref": self.image_href}} + "rescue_image_ref": self.image_uuid}} resp_json = self.controller._rescue(self.fake_req, UUID, body=body) self.assertEqual("ABC123", resp_json['adminPass']) 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 8606dca7b721..7ea764fbb444 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_server_actions.py @@ -338,15 +338,6 @@ class ServerActionsControllerTestV21(test.TestCase): self.assertEqual(info['image_href_in_call'], self.image_uuid) def test_rebuild_instance_with_image_href_uses_uuid(self): - info = dict(image_href_in_call=None) - - def rebuild(self2, context, instance, image_href, *args, **kwargs): - info['image_href_in_call'] = image_href - - self.stub_out('nova.db.instance_get', - fakes.fake_instance_get(vm_state=vm_states.ACTIVE)) - self.stubs.Set(compute_api.API, 'rebuild', rebuild) - # proper local hrefs must start with 'http://localhost/v2/' body = { 'rebuild': { @@ -354,8 +345,9 @@ class ServerActionsControllerTestV21(test.TestCase): }, } - self.controller._action_rebuild(self.req, FAKE_UUID, body=body) - self.assertEqual(info['image_href_in_call'], self.image_uuid) + self.assertRaises(exception.ValidationError, + self.controller._action_rebuild, + self.req, FAKE_UUID, body=body) def test_rebuild_accepted_minimum_pass_disabled(self): # run with enable_instance_password disabled to verify adminPass @@ -517,7 +509,7 @@ class ServerActionsControllerTestV21(test.TestCase): "imageRef": "foo", }, } - self.assertRaises(webob.exc.HTTPBadRequest, + self.assertRaises(exception.ValidationError, self.controller._action_rebuild, self.req, FAKE_UUID, body=body) diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index 608aecb9e496..391e08f7640e 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -1571,7 +1571,6 @@ class ServersControllerDeleteTest(ControllerTest): class ServersControllerRebuildInstanceTest(ControllerTest): image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' - image_href = 'http://localhost/v2/fake/images/%s' % image_uuid def setUp(self): super(ServersControllerRebuildInstanceTest, self).setUp() @@ -1594,7 +1593,7 @@ class ServersControllerRebuildInstanceTest(ControllerTest): self.body = { 'rebuild': { 'name': 'new_name', - 'imageRef': self.image_href, + 'imageRef': self.image_uuid, 'metadata': { 'open': 'stack', }, @@ -1604,6 +1603,29 @@ class ServersControllerRebuildInstanceTest(ControllerTest): self.req.method = 'POST' self.req.headers["content-type"] = "application/json" + def test_rebuild_server_with_image_not_uuid(self): + self.body['rebuild']['imageRef'] = 'not-uuid' + self.assertRaises(exception.ValidationError, + self.controller._action_rebuild, + self.req, FAKE_UUID, + body=self.body) + + def test_rebuild_server_with_image_as_full_url(self): + image_href = ('http://localhost/v2/fake/images/' + '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6') + self.body['rebuild']['imageRef'] = image_href + self.assertRaises(exception.ValidationError, + self.controller._action_rebuild, + self.req, FAKE_UUID, + body=self.body) + + def test_rebuild_server_with_image_as_empty_string(self): + self.body['rebuild']['imageRef'] = '' + self.assertRaises(exception.ValidationError, + self.controller._action_rebuild, + self.req, FAKE_UUID, + body=self.body) + def test_rebuild_instance_name_with_spaces_in_the_middle(self): self.body['rebuild']['name'] = 'abc def' self.req.body = jsonutils.dump_as_bytes(self.body) @@ -1751,7 +1773,7 @@ class ServersControllerRebuildInstanceTest(ControllerTest): def test_rebuild_bad_personality(self): body = { "rebuild": { - "imageRef": self.image_href, + "imageRef": self.image_uuid, "personality": [{ "path": "/path/to/file", "contents": "INVALID b64", @@ -1766,7 +1788,7 @@ class ServersControllerRebuildInstanceTest(ControllerTest): def test_rebuild_personality(self): body = { "rebuild": { - "imageRef": self.image_href, + "imageRef": self.image_uuid, "personality": [{ "path": "/path/to/file", "contents": base64.b64encode("Test String"), @@ -2437,10 +2459,9 @@ class ServersControllerCreateTest(test.TestCase): self.assertRaises(webob.exc.HTTPBadRequest, self._test_create_instance, flavor=1324) - def test_create_server_bad_image_href(self): - image_href = 1 + def test_create_server_bad_image_uuid(self): self.body['server']['min_count'] = 1 - self.body['server']['imageRef'] = image_href, + self.body['server']['imageRef'] = 1, self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(exception.ValidationError, self.controller.create, @@ -2505,23 +2526,24 @@ class ServersControllerCreateTest(test.TestCase): "Flavor's disk is too small for requested image."): self.controller.create(self.req, body=self.body) - def test_create_instance_image_ref_is_bookmark(self): - image_href = 'http://localhost/fake/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href - self.req.body = jsonutils.dump_as_bytes(self.body) - res = self.controller.create(self.req, body=self.body).obj + def test_create_instance_with_image_non_uuid(self): + self.body['server']['imageRef'] = 'not-uuid' + self.assertRaises(exception.ValidationError, + self.controller.create, + self.req, body=self.body) - server = res['server'] - self.assertEqual(FAKE_UUID, server['id']) - - def test_create_instance_image_ref_is_invalid(self): - image_uuid = 'this_is_not_a_valid_uuid' - image_href = 'http://localhost/fake/images/%s' % image_uuid - flavor_ref = 'http://localhost/fake/flavors/3' + def test_create_instance_with_image_as_full_url(self): + image_href = ('http://localhost/v2/fake/images/' + '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6') self.body['server']['imageRef'] = image_href - self.body['server']['flavorRef'] = flavor_ref - self.req.body = jsonutils.dump_as_bytes(self.body) - self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, + self.assertRaises(exception.ValidationError, + self.controller.create, + self.req, body=self.body) + + def test_create_instance_with_image_as_empty_string(self): + self.body['server']['imageRef'] = '' + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.create, self.req, body=self.body) def test_create_instance_no_key_pair(self): @@ -2682,10 +2704,7 @@ class ServersControllerCreateTest(test.TestCase): # test with admin passwords disabled See lp bug 921814 self.flags(enable_instance_password=False) - # proper local hrefs must start with 'http://localhost/v2/' self.flags(enable_instance_password=False) - image_href = 'http://localhost/v2/fake/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href self.req.body = jsonutils.dump_as_bytes(self.body) res = self.controller.create(self.req, body=self.body).obj @@ -2694,50 +2713,36 @@ class ServersControllerCreateTest(test.TestCase): self.assertEqual(FAKE_UUID, server['id']) def test_create_instance_name_too_long(self): - # proper local hrefs must start with 'http://localhost/v2/' - image_href = 'http://localhost/v2/images/%s' % self.image_uuid self.body['server']['name'] = 'X' * 256 - self.body['server']['imageRef'] = image_href self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(exception.ValidationError, self.controller.create, self.req, body=self.body) def test_create_instance_name_with_spaces_in_the_middle(self): - # proper local hrefs must start with 'http://localhost/v2/' - image_href = 'http://localhost/v2/images/%s' % self.image_uuid self.body['server']['name'] = 'abc def' - self.body['server']['imageRef'] = image_href self.req.body = jsonutils.dump_as_bytes(self.body) self.controller.create(self.req, body=self.body) def test_create_instance_name_with_leading_trailing_spaces(self): - # proper local hrefs must start with 'http://localhost/v2/' - image_href = 'http://localhost/v2/images/%s' % self.image_uuid self.body['server']['name'] = ' abc def ' - self.body['server']['imageRef'] = image_href self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(exception.ValidationError, self.controller.create, self.req, body=self.body) def test_create_instance_name_with_leading_trailing_spaces_in_compat_mode( self): - # proper local hrefs must start with 'http://localhost/v2/' - image_href = 'http://localhost/v2/images/%s' % self.image_uuid self.body['server']['name'] = ' abc def ' - self.body['server']['imageRef'] = image_href self.req.body = jsonutils.dump_as_bytes(self.body) self.req.set_legacy_v2() self.controller.create(self.req, body=self.body) def test_create_instance_name_all_blank_spaces(self): - # proper local hrefs must start with 'http://localhost/v2/' image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' - image_href = 'http://localhost/v2/images/%s' % image_uuid flavor_ref = 'http://localhost/fake/flavors/3' body = { 'server': { 'name': ' ' * 64, - 'imageRef': image_href, + 'imageRef': image_uuid, 'flavorRef': flavor_ref, 'metadata': { 'hello': 'world', @@ -2754,9 +2759,6 @@ class ServersControllerCreateTest(test.TestCase): self.controller.create, req, body=body) def test_create_az_with_leading_trailing_spaces(self): - # proper local hrefs must start with 'http://localhost/v2/' - image_href = 'http://localhost/v2/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href self.body['server']['availability_zone'] = ' zone1 ' self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(exception.ValidationError, @@ -2764,10 +2766,7 @@ class ServersControllerCreateTest(test.TestCase): def test_create_az_with_leading_trailing_spaces_in_compat_mode( self): - # proper local hrefs must start with 'http://localhost/v2/' - image_href = 'http://localhost/v2/images/%s' % self.image_uuid self.body['server']['name'] = ' abc def ' - self.body['server']['imageRef'] = image_href self.body['server']['availability_zones'] = ' zone1 ' self.req.body = jsonutils.dump_as_bytes(self.body) self.req.set_legacy_v2() @@ -2776,9 +2775,6 @@ class ServersControllerCreateTest(test.TestCase): self.controller.create(self.req, body=self.body) def test_create_instance(self): - # proper local hrefs must start with 'http://localhost/v2/' - image_href = 'http://localhost/v2/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href self.req.body = jsonutils.dump_as_bytes(self.body) res = self.controller.create(self.req, body=self.body).obj @@ -2793,14 +2789,12 @@ class ServersControllerCreateTest(test.TestCase): self.stubs.Set(keypairs.Keypairs, 'server_create', fake_keypair_server_create) - # proper local hrefs must start with 'http://localhost/v2/' image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' - image_href = 'http://localhost/v2/images/%s' % image_uuid flavor_ref = 'http://localhost/123/flavors/3' body = { 'server': { 'name': 'server_test', - 'imageRef': image_href, + 'imageRef': image_uuid, 'flavorRef': flavor_ref, 'metadata': { 'hello': 'world', @@ -2818,9 +2812,6 @@ class ServersControllerCreateTest(test.TestCase): def test_create_instance_pass_disabled(self): self.flags(enable_instance_password=False) - # proper local hrefs must start with 'http://localhost/v2/' - image_href = 'http://localhost/v2/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href self.req.body = jsonutils.dump_as_bytes(self.body) res = self.controller.create(self.req, body=self.body).obj @@ -2837,8 +2828,6 @@ class ServersControllerCreateTest(test.TestCase): 'cpuset': None, 'memsize': 0, 'memtotal': 0}) - image_href = 'http://localhost/v2/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, self.req, body=self.body) @@ -2855,8 +2844,6 @@ class ServersControllerCreateTest(test.TestCase): def test_create_instance_too_much_metadata(self): self.flags(quota_metadata_items=1) - image_href = 'http://localhost/v2/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href self.body['server']['metadata']['vote'] = 'fiddletown' self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(webob.exc.HTTPForbidden, @@ -2864,8 +2851,6 @@ class ServersControllerCreateTest(test.TestCase): def test_create_instance_metadata_key_too_long(self): self.flags(quota_metadata_items=1) - image_href = 'http://localhost/v2/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href self.body['server']['metadata'] = {('a' * 260): '12345'} self.req.body = jsonutils.dump_as_bytes(self.body) @@ -2874,8 +2859,6 @@ class ServersControllerCreateTest(test.TestCase): def test_create_instance_metadata_value_too_long(self): self.flags(quota_metadata_items=1) - image_href = 'http://localhost/v2/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href self.body['server']['metadata'] = {'key1': ('a' * 260)} self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(exception.ValidationError, @@ -2883,8 +2866,6 @@ class ServersControllerCreateTest(test.TestCase): def test_create_instance_metadata_key_blank(self): self.flags(quota_metadata_items=1) - image_href = 'http://localhost/v2/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href self.body['server']['metadata'] = {'': 'abcd'} self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(exception.ValidationError, @@ -2892,8 +2873,6 @@ class ServersControllerCreateTest(test.TestCase): def test_create_instance_metadata_not_dict(self): self.flags(quota_metadata_items=1) - image_href = 'http://localhost/v2/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href self.body['server']['metadata'] = 'string' self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(exception.ValidationError, @@ -2901,8 +2880,6 @@ class ServersControllerCreateTest(test.TestCase): def test_create_instance_metadata_key_not_string(self): self.flags(quota_metadata_items=1) - image_href = 'http://localhost/v2/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href self.body['server']['metadata'] = {1: 'test'} self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(exception.ValidationError, @@ -2910,8 +2887,6 @@ class ServersControllerCreateTest(test.TestCase): def test_create_instance_metadata_value_not_string(self): self.flags(quota_metadata_items=1) - image_href = 'http://localhost/v2/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href self.body['server']['metadata'] = {'test': ['a', 'list']} self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(exception.ValidationError, @@ -2923,8 +2898,6 @@ class ServersControllerCreateTest(test.TestCase): self._test_create_extra, params) def test_create_instance_invalid_key_name(self): - image_href = 'http://localhost/v2/images/2' - self.body['server']['imageRef'] = image_href self.body['server']['key_name'] = 'nonexistentkey' self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(webob.exc.HTTPBadRequest, @@ -2939,18 +2912,14 @@ class ServersControllerCreateTest(test.TestCase): self._check_admin_password_len(res["server"]) def test_create_instance_invalid_flavor_href(self): - image_href = 'http://localhost/v2/images/2' flavor_ref = 'http://localhost/v2/flavors/asdf' - self.body['server']['imageRef'] = image_href self.body['server']['flavorRef'] = flavor_ref self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, self.req, body=self.body) def test_create_instance_invalid_flavor_id_int(self): - image_href = 'http://localhost/v2/images/2' flavor_ref = -1 - self.body['server']['imageRef'] = image_href self.body['server']['flavorRef'] = flavor_ref self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(webob.exc.HTTPBadRequest, @@ -2964,22 +2933,12 @@ class ServersControllerCreateTest(test.TestCase): self.controller.create, self.req, body=self.body) def test_create_instance_bad_flavor_href(self): - image_href = 'http://localhost/v2/images/2' flavor_ref = 'http://localhost/v2/flavors/17' - self.body['server']['imageRef'] = image_href self.body['server']['flavorRef'] = flavor_ref self.req.body = jsonutils.dump_as_bytes(self.body) self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create, self.req, body=self.body) - def test_create_instance_bad_href(self): - image_href = 'asdf' - self.body['server']['imageRef'] = image_href - self.req.body = jsonutils.dump_as_bytes(self.body) - - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, self.req, body=self.body) - def test_create_instance_local_href(self): self.req.body = jsonutils.dump_as_bytes(self.body) res = self.controller.create(self.req, body=self.body).obj @@ -3340,9 +3299,6 @@ class ServersControllerCreateTest(test.TestCase): class ServersControllerCreateTestV219(ServersControllerCreateTest): def _create_instance_req(self, set_desc, desc=None): - # proper local hrefs must start with 'http://localhost/v2/' - image_href = 'http://localhost/v2/images/%s' % self.image_uuid - self.body['server']['imageRef'] = image_href if set_desc: self.body['server']['description'] = desc self.req.body = jsonutils.dump_as_bytes(self.body) diff --git a/releasenotes/notes/imageRef-as-uuid-only-0164c04206a42683.yaml b/releasenotes/notes/imageRef-as-uuid-only-0164c04206a42683.yaml new file mode 100644 index 000000000000..b9232cdee788 --- /dev/null +++ b/releasenotes/notes/imageRef-as-uuid-only-0164c04206a42683.yaml @@ -0,0 +1,17 @@ +--- +upgrade: + - imageRef input to the REST API is now restricted + to be UUID or an empty string only. imageRef + input while create, rebuild and rescue server etc + must be a valid UUID now. Previously, a random + image ref url containing image UUID was accepted. + But now all the reference of imageRef must be a + valid UUID (with below exception) otherwise API + will return 400. + + Exception- In case boot server from volume. + Previously empty string was allowed in imageRef + and which is ok in case of boot from volume. + Nova will keep the same behavior and allow empty + string in case of boot from volume only and 400 + in all other case.