Accept a full serverRef to OSAPI POST /images (snapshot)

5 files changed, 88 insertions, 38 deletions

diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py
index 5ffd8e96a..d43340e10 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 2773c9c13..d6a054102 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 a7ea88d51..593a16a1f 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 f8d158ddd..c74974b16 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 e4204809f..446d68e9e 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):
             <image
                    created="None"
                    id="123"
-                   name="None"
+                   name="Backup 1"
                    serverRef="http://localhost/v1.1/servers/123"
                    status="ACTIVE"
                    updated="None"
@@ -1065,7 +1085,8 @@ class ImageControllerWithGlanceServiceTest(test.TestCase):
         image_id += 1
 
         # Backup for User 1
-        backup_properties = {'instance_id': '42', 'user_id': '1'}
+        server_ref = 'http://localhost:8774/v1.1/servers/42'
+        backup_properties = {'instance_ref': server_ref, 'user_id': '1'}
         for status in ('queued', 'saving', 'active', 'killed'):
             add_fixture(id=image_id, name='%s backup' % status,
                         is_public=False, status=status,