From 6eac65a8e13d3040c0c6561709f59b3d9bab5435 Mon Sep 17 00:00:00 2001
From: Édouard Thuleau <edouard.thuleau@orange.com>
Date: Thu, 25 Oct 2012 10:32:48 +0200
Subject: 'BackupCreate' rotation parameter >= 0

In the extension 'admin-actions' of OpenStack Compute API, the action
create backups should only accept rotation parameter greater than or
equal to zero.

If the 'rotation' parameter is negative, OS API returns a badRequest
400 HTTP error.

Fixes LP bug #1071168

Change-Id: I6ef08fbecb0f88942cdb16193e9d2c3831866fe9
---
 .../api/openstack/compute/contrib/admin_actions.py | 16 +++++++---
 nova/compute/manager.py                            |  2 +-
 .../compute/contrib/test_admin_actions.py          | 37 ++++++++++++++++++++--
 3 files changed, 47 insertions(+), 8 deletions(-)

(limited to 'nova')

diff --git a/nova/api/openstack/compute/contrib/admin_actions.py b/nova/api/openstack/compute/contrib/admin_actions.py
index 1bac0851d..02a588ffa 100644
--- a/nova/api/openstack/compute/contrib/admin_actions.py
+++ b/nova/api/openstack/compute/contrib/admin_actions.py
@@ -229,6 +229,10 @@ class AdminActionsController(wsgi.Controller):
         except ValueError:
             msg = _("createBackup attribute 'rotation' must be an integer")
             raise exc.HTTPBadRequest(explanation=msg)
+        if rotation < 0:
+            msg = _("createBackup attribute 'rotation' must be greater "
+                    "than or equal to zero")
+            raise exc.HTTPBadRequest(explanation=msg)
 
         props = {}
         metadata = entity.get('metadata', {})
@@ -251,12 +255,14 @@ class AdminActionsController(wsgi.Controller):
             common.raise_http_conflict_for_instance_invalid_state(state_error,
                     'createBackup')
 
-        # build location of newly-created image entity
-        image_id = str(image['id'])
-        image_ref = os.path.join(req.application_url, 'images', image_id)
-
         resp = webob.Response(status_int=202)
-        resp.headers['Location'] = image_ref
+
+        # build location of newly-created image entity if rotation is not zero
+        if rotation > 0:
+            image_id = str(image['id'])
+            image_ref = os.path.join(req.application_url, 'images', image_id)
+            resp.headers['Location'] = image_ref
+
         return resp
 
     @wsgi.action('os-migrateLive')
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index de848abdd..d7c1d7367 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -1302,7 +1302,7 @@ class ComputeManager(manager.SchedulerDependentManager):
         if image_type == 'snapshot' and rotation:
             raise exception.ImageRotationNotAllowed()
 
-        elif image_type == 'backup' and rotation:
+        elif image_type == 'backup' and rotation >= 0:
             self._rotate_backups(context, instance, backup_type, rotation)
 
         elif image_type == 'backup':
diff --git a/nova/tests/api/openstack/compute/contrib/test_admin_actions.py b/nova/tests/api/openstack/compute/contrib/test_admin_actions.py
index c412b2c5c..175c0cd9f 100644
--- a/nova/tests/api/openstack/compute/contrib/test_admin_actions.py
+++ b/nova/tests/api/openstack/compute/contrib/test_admin_actions.py
@@ -249,6 +249,22 @@ class CreateBackupTests(test.TestCase):
         response = request.get_response(self.app)
         self.assertEqual(response.status_int, 400)
 
+    def test_create_backup_negative_rotation(self):
+        """Rotation must be greater than or equal to zero
+        for backup requests
+        """
+        body = {
+            'createBackup': {
+                'name': 'Backup 1',
+                'backup_type': 'daily',
+                'rotation': -1,
+            },
+        }
+
+        request = self._get_request(body)
+        response = request.get_response(self.app)
+        self.assertEqual(response.status_int, 400)
+
     def test_create_backup_no_backup_type(self):
         """Backup Type (daily or weekly) is required for backup requests"""
         body = {
@@ -269,8 +285,24 @@ class CreateBackupTests(test.TestCase):
         response = request.get_response(self.app)
         self.assertEqual(response.status_int, 400)
 
-    def test_create_backup(self):
-        """The happy path for creating backups"""
+    def test_create_backup_rotation_is_zero(self):
+        """The happy path for creating backups if rotation is zero"""
+        body = {
+            'createBackup': {
+                'name': 'Backup 1',
+                'backup_type': 'daily',
+                'rotation': 0,
+            },
+        }
+
+        request = self._get_request(body)
+        response = request.get_response(self.app)
+
+        self.assertEqual(response.status_int, 202)
+        self.assertFalse('Location' in response.headers)
+
+    def test_create_backup_rotation_is_positive(self):
+        """The happy path for creating backups if rotation is positive"""
         body = {
             'createBackup': {
                 'name': 'Backup 1',
@@ -282,6 +314,7 @@ class CreateBackupTests(test.TestCase):
         request = self._get_request(body)
         response = request.get_response(self.app)
 
+        self.assertEqual(response.status_int, 202)
         self.assertTrue(response.headers['Location'])
 
     def test_create_backup_raises_conflict_on_invalid_state(self):
-- 
cgit