diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py index 5ffd8e96afd3..d43340e104de 100644 --- a/nova/api/openstack/images.py +++ b/nova/api/openstack/images.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import os.path + import webob.exc from nova import compute @@ -99,21 +101,27 @@ class Controller(object): raise webob.exc.HTTPBadRequest() try: - server_id = self._server_id_from_req_data(body) + server_id = self._server_id_from_req(req, body) image_name = body["image"]["name"] except KeyError: raise webob.exc.HTTPBadRequest() - image = self._compute_service.snapshot(context, server_id, image_name) + props = self._get_extra_properties(req, body) + + image = self._compute_service.snapshot(context, server_id, + image_name, props) return dict(image=self.get_builder(req).build(image, detail=True)) def get_builder(self, request): """Indicates that you must use a Controller subclass.""" raise NotImplementedError - def _server_id_from_req_data(self, data): + def _server_id_from_req(self, req, data): raise NotImplementedError() + def _get_extra_properties(self, req, data): + return {} + class ControllerV10(Controller): """Version 1.0 specific controller logic.""" @@ -149,8 +157,12 @@ class ControllerV10(Controller): builder = self.get_builder(req).build return dict(images=[builder(image, detail=True) for image in images]) - def _server_id_from_req_data(self, data): - return data['image']['serverId'] + def _server_id_from_req(self, req, data): + try: + return data['image']['serverId'] + except KeyError: + msg = _("Expected serverId attribute on server entity.") + raise webob.exc.HTTPBadRequest(explanation=msg) class ControllerV11(Controller): @@ -189,8 +201,27 @@ class ControllerV11(Controller): builder = self.get_builder(req).build return dict(images=[builder(image, detail=True) for image in images]) - def _server_id_from_req_data(self, data): - return data['image']['serverRef'] + def _server_id_from_req(self, req, data): + try: + server_ref = data['image']['serverRef'] + except KeyError: + msg = _("Expected serverRef attribute on server entity.") + raise webob.exc.HTTPBadRequest(explanation=msg) + + head, tail = os.path.split(server_ref) + + if head and head != os.path.join(req.application_url, 'servers'): + msg = _("serverRef must match request url") + raise webob.exc.HTTPBadRequest(explanation=msg) + + return tail + + def _get_extra_properties(self, req, data): + server_ref = data['image']['serverRef'] + if not server_ref.startswith('http'): + server_ref = os.path.join(req.application_url, 'servers', + server_ref) + return {'instance_ref': server_ref} def create_resource(version='1.0'): diff --git a/nova/api/openstack/views/images.py b/nova/api/openstack/views/images.py index 2773c9c135fb..d6a0541020f9 100644 --- a/nova/api/openstack/views/images.py +++ b/nova/api/openstack/views/images.py @@ -46,13 +46,9 @@ class ViewBuilder(object): except KeyError: image['status'] = image['status'].upper() - def _build_server(self, image, instance_id): + def _build_server(self, image, image_obj): """Indicates that you must use a ViewBuilder subclass.""" - raise NotImplementedError - - def generate_server_ref(self, server_id): - """Return an href string pointing to this server.""" - return os.path.join(self._url, "servers", str(server_id)) + raise NotImplementedError() def generate_href(self, image_id): """Return an href string pointing to this object.""" @@ -60,8 +56,6 @@ class ViewBuilder(object): def build(self, image_obj, detail=False): """Return a standardized image structure for display by the API.""" - properties = image_obj.get("properties", {}) - self._format_dates(image_obj) if "status" in image_obj: @@ -72,11 +66,7 @@ class ViewBuilder(object): "name": image_obj.get("name"), } - if "instance_id" in properties: - try: - self._build_server(image, int(properties["instance_id"])) - except ValueError: - pass + self._build_server(image, image_obj) if detail: image.update({ @@ -94,15 +84,21 @@ class ViewBuilder(object): class ViewBuilderV10(ViewBuilder): """OpenStack API v1.0 Image Builder""" - def _build_server(self, image, instance_id): - image["serverId"] = instance_id + def _build_server(self, image, image_obj): + try: + image['serverId'] = int(image_obj['properties']['instance_id']) + except (KeyError, ValueError): + pass class ViewBuilderV11(ViewBuilder): """OpenStack API v1.1 Image Builder""" - def _build_server(self, image, instance_id): - image["serverRef"] = self.generate_server_ref(instance_id) + def _build_server(self, image, image_obj): + try: + image['serverRef'] = image_obj['properties']['instance_ref'] + except KeyError: + return def build(self, image_obj, detail=False): """Return a standardized image structure for display by the API.""" diff --git a/nova/compute/api.py b/nova/compute/api.py index a7ea88d51a63..593a16a1f6ab 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -701,7 +701,7 @@ class API(base.Base): raise exception.Error(_("Unable to find host for Instance %s") % instance_id) - def snapshot(self, context, instance_id, name): + def snapshot(self, context, instance_id, name, extra_properties=None): """Snapshot the given instance. :returns: A dict containing image metadata @@ -709,6 +709,7 @@ class API(base.Base): properties = {'instance_id': str(instance_id), 'user_id': str(context.user_id), 'image_state': 'creating'} + properties.update(extra_properties or {}) sent_meta = {'name': name, 'is_public': False, 'status': 'creating', 'properties': properties} recv_meta = self.image_service.create(context, sent_meta) diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py index f8d158ddd750..c74974b16211 100644 --- a/nova/tests/api/openstack/fakes.py +++ b/nova/tests/api/openstack/fakes.py @@ -140,9 +140,10 @@ def stub_out_networking(stubs): def stub_out_compute_api_snapshot(stubs): - def snapshot(self, context, instance_id, name): - return dict(id='123', status='ACTIVE', - properties=dict(instance_id='123')) + def snapshot(self, context, instance_id, name, extra_properties=None): + props = dict(instance_id=instance_id, instance_ref=instance_id) + props.update(extra_properties or {}) + return dict(id='123', status='ACTIVE', name=name, properties=props) stubs.Set(nova.compute.API, 'snapshot', snapshot) diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index e4204809fef8..446d68e9edbd 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -618,7 +618,6 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): { 'id': 124, 'name': 'queued backup', - 'serverId': 42, 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, 'status': 'QUEUED', @@ -626,7 +625,6 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): { 'id': 125, 'name': 'saving backup', - 'serverId': 42, 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, 'status': 'SAVING', @@ -635,7 +633,6 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): { 'id': 126, 'name': 'active backup', - 'serverId': 42, 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, 'status': 'ACTIVE' @@ -643,7 +640,6 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): { 'id': 127, 'name': 'killed backup', - 'serverId': 42, 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, 'status': 'FAILED', @@ -689,7 +685,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): { 'id': 124, 'name': 'queued backup', - 'serverRef': "http://localhost/v1.1/servers/42", + 'serverRef': "http://localhost:8774/v1.1/servers/42", 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, 'status': 'QUEUED', @@ -711,7 +707,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): { 'id': 125, 'name': 'saving backup', - 'serverRef': "http://localhost/v1.1/servers/42", + 'serverRef': "http://localhost:8774/v1.1/servers/42", 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, 'status': 'SAVING', @@ -734,7 +730,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): { 'id': 126, 'name': 'active backup', - 'serverRef': "http://localhost/v1.1/servers/42", + 'serverRef': "http://localhost:8774/v1.1/servers/42", 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, 'status': 'ACTIVE', @@ -756,7 +752,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): { 'id': 127, 'name': 'killed backup', - 'serverRef': "http://localhost/v1.1/servers/42", + 'serverRef': "http://localhost:8774/v1.1/servers/42", 'updated': self.NOW_API_FORMAT, 'created': self.NOW_API_FORMAT, 'status': 'FAILED', @@ -1002,6 +998,30 @@ class ImageControllerWithGlanceServiceTest(test.TestCase): response = req.get_response(fakes.wsgi_app()) self.assertEqual(200, response.status_int) + def test_create_image_v1_1_actual_server_ref(self): + + serverRef = 'http://localhost/v1.1/servers/1' + body = dict(image=dict(serverRef=serverRef, name='Backup 1')) + req = webob.Request.blank('/v1.1/images') + req.method = 'POST' + req.body = json.dumps(body) + req.headers["content-type"] = "application/json" + response = req.get_response(fakes.wsgi_app()) + self.assertEqual(200, response.status_int) + result = json.loads(response.body) + self.assertEqual(result['image']['serverRef'], serverRef) + + def test_create_image_v1_1_server_ref_bad_hostname(self): + + serverRef = 'http://asdf/v1.1/servers/1' + body = dict(image=dict(serverRef=serverRef, name='Backup 1')) + req = webob.Request.blank('/v1.1/images') + req.method = 'POST' + req.body = json.dumps(body) + req.headers["content-type"] = "application/json" + response = req.get_response(fakes.wsgi_app()) + self.assertEqual(400, response.status_int) + def test_create_image_v1_1_xml_serialization(self): body = dict(image=dict(serverRef='123', name='Backup 1')) @@ -1018,7 +1038,7 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):