From f5ebb33f9a0ebc3b8f6d0e0d97e3d4b673106907 Mon Sep 17 00:00:00 2001
From: Eoghan Glynn <eglynn@redhat.com>
Date: Wed, 15 Aug 2012 14:10:03 +0100
Subject: [PATCH] Move volume size validation to api layer.

As per the discussion on this patch:

  ihttps://review.openstack.org/10540

Change-Id: I5d3031a08aa1e2d17b5fb233ebaec938c66fa88b
---
 cinder/api/openstack/volume/volumes.py        | 10 +----
 .../api/openstack/volume/test_volumes.py      | 12 ++----
 cinder/tests/test_volume.py                   | 38 +++++++++++++++++++
 cinder/volume/api.py                          | 13 ++++++-
 4 files changed, 54 insertions(+), 19 deletions(-)

diff --git a/cinder/api/openstack/volume/volumes.py b/cinder/api/openstack/volume/volumes.py
index 82dbe461b..0525a4f35 100644
--- a/cinder/api/openstack/volume/volumes.py
+++ b/cinder/api/openstack/volume/volumes.py
@@ -242,15 +242,7 @@ class VolumeController(object):
 
         volume = body['volume']
 
-        def as_int(s):
-            try:
-                return int(s)
-            except ValueError:
-                return s
-
-        # NOTE(eglynn): we're tolerant of non-int sizes here, as type
-        # integrity is enforced later in the creation codepath
-        size = as_int(volume['size'])
+        size = volume['size']
 
         LOG.audit(_("Create volume of %s GB"), size, context=context)
 
diff --git a/cinder/tests/api/openstack/volume/test_volumes.py b/cinder/tests/api/openstack/volume/test_volumes.py
index 2a63b561e..2c69ea553 100644
--- a/cinder/tests/api/openstack/volume/test_volumes.py
+++ b/cinder/tests/api/openstack/volume/test_volumes.py
@@ -64,10 +64,10 @@ class VolumeApiTest(test.TestCase):
         self.stubs.Set(volume_api.API, 'get', fakes.stub_volume_get)
         self.stubs.Set(volume_api.API, 'delete', fakes.stub_volume_delete)
 
-    def _do_test_volume_create(self, size):
+    def test_volume_create(self):
         self.stubs.Set(volume_api.API, "create", fakes.stub_volume_create)
 
-        vol = {"size": size,
+        vol = {"size": 100,
                "display_name": "Volume Test Name",
                "display_description": "Volume Test Desc",
                "availability_zone": "zone1:host1"}
@@ -91,12 +91,6 @@ class VolumeApiTest(test.TestCase):
                                'size': 100}}
         self.assertEqual(res_dict, expected)
 
-    def test_volume_create_int_size(self):
-        self._do_test_volume_create(100)
-
-    def test_volume_create_str_size(self):
-        self._do_test_volume_create('100')
-
     def test_volume_creation_fails_with_bad_size(self):
         vol = {"size": '',
                "display_name": "Volume Test Name",
@@ -139,7 +133,7 @@ class VolumeApiTest(test.TestCase):
                             'metadata': {},
                             'id': '1',
                             'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
-                            'size': 1}
+                            'size': '1'}
                     }
         body = {"volume": vol}
         req = fakes.HTTPRequest.blank('/v1/volumes')
diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index 0cdd3596f..7ab934bbc 100644
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -38,6 +38,7 @@ from cinder.openstack.common import log as os_logging
 from cinder.openstack.common import importutils
 from cinder.openstack.common import rpc
 import cinder.policy
+from cinder import quota
 from cinder import test
 import cinder.volume.api
 
@@ -549,6 +550,43 @@ class VolumeTestCase(test.TestCase):
             db.volume_destroy(self.context, volume_id)
             os.unlink(dst_path)
 
+    def _do_test_create_volume_with_size(self, size):
+        def fake_allowed_volumes(context, requested_volumes, size):
+            return requested_volumes
+
+        self.stubs.Set(quota, 'allowed_volumes', fake_allowed_volumes)
+
+        volume_api = cinder.volume.api.API()
+
+        volume = volume_api.create(self.context,
+                                   size,
+                                   'name',
+                                   'description')
+        self.assertEquals(volume['size'], int(size))
+
+    def test_create_volume_int_size(self):
+        """Test volume creation with int size."""
+        self._do_test_create_volume_with_size(2)
+
+    def test_create_volume_string_size(self):
+        """Test volume creation with string size."""
+        self._do_test_create_volume_with_size('2')
+
+    def test_create_volume_with_bad_size(self):
+        def fake_allowed_volumes(context, requested_volumes, size):
+            return requested_volumes
+
+        self.stubs.Set(quota, 'allowed_volumes', fake_allowed_volumes)
+
+        volume_api = cinder.volume.api.API()
+
+        self.assertRaises(exception.InvalidInput,
+                          volume_api.create,
+                          self.context,
+                          '2Gb',
+                          'name',
+                          'description')
+
 
 class DriverTestCase(test.TestCase):
     """Base Test class for Drivers."""
diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index 003b3cf2b..a562c95ec 100644
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -93,8 +93,19 @@ class API(base.Base):
             snapshot_id = snapshot['id']
         else:
             snapshot_id = None
+
+        def as_int(s):
+            try:
+                return int(s)
+            except ValueError:
+                return s
+
+        # tolerate size as stringified int
+        size = as_int(size)
+
         if not isinstance(size, int) or size <= 0:
-            msg = _('Volume size must be an integer and greater than 0')
+            msg = (_("Volume size '%s' must be an integer and greater than 0")
+                   % size)
             raise exception.InvalidInput(reason=msg)
         if quota.allowed_volumes(context, 1, size) < 1:
             pid = context.project_id
-- 
2.45.2