Merge "Fix postgresql failures related to Data type"

5 files changed, 136 insertions, 40 deletions

diff --git a/nova/api/openstack/compute/contrib/fixed_ips.py b/nova/api/openstack/compute/contrib/fixed_ips.py
index 8c7e4f7d3..b474b685d 100644
--- a/nova/api/openstack/compute/contrib/fixed_ips.py
+++ b/nova/api/openstack/compute/contrib/fixed_ips.py
@@ -31,7 +31,8 @@ class FixedIPController(object):
 
         try:
             fixed_ip = db.fixed_ip_get_by_address_detailed(context, id)
-        except exception.FixedIpNotFoundForAddress as ex:
+        except (exception.FixedIpNotFoundForAddress,
+                exception.FixedIpInvalid) as ex:
             raise webob.exc.HTTPNotFound(explanation=ex.format_message())
 
         fixed_ip_info = {"fixed_ip": {}}
@@ -54,6 +55,7 @@ class FixedIPController(object):
     def action(self, req, id, body):
         context = req.environ['nova.context']
         authorize(context)
+
         if 'reserve' in body:
             return self._set_reserved(context, id, True)
         elif 'unreserve' in body:
@@ -67,7 +69,8 @@ class FixedIPController(object):
             fixed_ip = db.fixed_ip_get_by_address(context, address)
             db.fixed_ip_update(context, fixed_ip['address'],
                                {'reserved': reserved})
-        except exception.FixedIpNotFoundForAddress:
+        except (exception.FixedIpNotFoundForAddress,
+                exception.FixedIpInvalid) as ex:
             msg = _("Fixed IP %s not found") % address
             raise webob.exc.HTTPNotFound(explanation=msg)
 
diff --git a/nova/api/openstack/compute/contrib/floating_ips.py b/nova/api/openstack/compute/contrib/floating_ips.py
index a3d03e8de..284a211cd 100644
--- a/nova/api/openstack/compute/contrib/floating_ips.py
+++ b/nova/api/openstack/compute/contrib/floating_ips.py
@@ -125,7 +125,7 @@ class FloatingIPController(object):
 
         try:
             floating_ip = self.network_api.get_floating_ip(context, id)
-        except exception.NotFound:
+        except (exception.NotFound, exception.InvalidID):
             msg = _("Floating ip not found for id %s") % id
             raise webob.exc.HTTPNotFound(explanation=msg)
 
@@ -173,7 +173,7 @@ class FloatingIPController(object):
         # get the floating ip object
         try:
             floating_ip = self.network_api.get_floating_ip(context, id)
-        except exception.NotFound:
+        except (exception.NotFound, exception.InvalidID):
             msg = _("Floating ip not found for id %s") % id
             raise webob.exc.HTTPNotFound(explanation=msg)
         address = floating_ip['address']
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 18aa185ab..3b1d0b3a4 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -649,14 +649,18 @@ def certificate_get_all_by_user_and_project(context, user_id, project_id):
 
 @require_context
 def floating_ip_get(context, id):
-    result = model_query(context, models.FloatingIp, project_only=True).\
-                 filter_by(id=id).\
-                 options(joinedload_all('fixed_ip.instance')).\
-                 first()
-
-    if not result:
-        raise exception.FloatingIpNotFound(id=id)
+    try:
+        result = model_query(context, models.FloatingIp, project_only=True).\
+                     filter_by(id=id).\
+                     options(joinedload_all('fixed_ip.instance')).\
+                     first()
 
+        if not result:
+            raise exception.FloatingIpNotFound(id=id)
+    except DataError:
+        msg = _("Invalid floating ip id %s in request") % id
+        LOG.warn(msg)
+        raise exception.InvalidID(id=id)
     return result
 
 
@@ -866,14 +870,18 @@ def _floating_ip_get_by_address(context, address, session=None):
     # if address string is empty explicitly set it to None
     if not address:
         address = None
+    try:
+        result = model_query(context, models.FloatingIp, session=session).\
+                    filter_by(address=address).\
+                    options(joinedload_all('fixed_ip.instance')).\
+                    first()
 
-    result = model_query(context, models.FloatingIp, session=session).\
-                filter_by(address=address).\
-                options(joinedload_all('fixed_ip.instance')).\
-                first()
-
-    if not result:
-        raise exception.FloatingIpNotFoundForAddress(address=address)
+        if not result:
+            raise exception.FloatingIpNotFoundForAddress(address=address)
+    except DataError:
+        msg = _("Invalid floating IP %s in request") % address
+        LOG.warn(msg)
+        raise exception.InvalidIpAddressError(msg)
 
     # If the floating IP has a project ID set, check to make sure
     # the non-admin user has access.
@@ -1149,11 +1157,16 @@ def _fixed_ip_get_by_address(context, address, session=None):
         session = get_session()
 
     with session.begin(subtransactions=True):
-        result = model_query(context, models.FixedIp, session=session).\
-                         filter_by(address=address).\
-                         first()
-        if not result:
-            raise exception.FixedIpNotFoundForAddress(address=address)
+        try:
+            result = model_query(context, models.FixedIp, session=session).\
+                                 filter_by(address=address).\
+                                 first()
+            if not result:
+                raise exception.FixedIpNotFoundForAddress(address=address)
+        except DataError:
+            msg = _("Invalid fixed IP Address %s in request") % address
+            LOG.warn(msg)
+            raise exception.FixedIpInvalid(msg)
 
         # NOTE(sirp): shouldn't we just use project_only here to restrict the
         # results?
@@ -1175,19 +1188,25 @@ def fixed_ip_get_by_address_detailed(context, address):
     """
     :returns: a tuple of (models.FixedIp, models.Network, models.Instance)
     """
-    result = model_query(context, models.FixedIp,
-                         models.Network, models.Instance).\
-                         filter_by(address=address).\
-                         outerjoin((models.Network,
-                                    models.Network.id ==
-                                    models.FixedIp.network_id)).\
-                         outerjoin((models.Instance,
-                                    models.Instance.uuid ==
-                                    models.FixedIp.instance_uuid)).\
-        first()
+    try:
+        result = model_query(context, models.FixedIp,
+                             models.Network, models.Instance).\
+                             filter_by(address=address).\
+                             outerjoin((models.Network,
+                                        models.Network.id ==
+                                        models.FixedIp.network_id)).\
+                             outerjoin((models.Instance,
+                                        models.Instance.uuid ==
+                                        models.FixedIp.instance_uuid)).\
+            first()
 
-    if not result:
-        raise exception.FixedIpNotFoundForAddress(address=address)
+        if not result:
+            raise exception.FixedIpNotFoundForAddress(address=address)
+
+    except DataError:
+        msg = _("Invalid fixed IP Address %s in request") % address
+        LOG.warn(msg)
+        raise exception.FixedIpInvalid(msg)
 
     return result
 
@@ -1317,9 +1336,14 @@ def virtual_interface_get_by_address(context, address):
 
     :param address: = the address of the interface you're looking to get
     """
-    vif_ref = _virtual_interface_query(context).\
-                      filter_by(address=address).\
-                      first()
+    try:
+        vif_ref = _virtual_interface_query(context).\
+                          filter_by(address=address).\
+                          first()
+    except DataError:
+        msg = _("Invalid virtual interface address %s in request") % address
+        LOG.warn(msg)
+        raise exception.InvalidIpAddressError(msg)
     return vif_ref
 
 
diff --git a/nova/tests/api/openstack/compute/contrib/test_fixed_ips.py b/nova/tests/api/openstack/compute/contrib/test_fixed_ips.py
index 67417e60e..2f9f6c5bc 100644
--- a/nova/tests/api/openstack/compute/contrib/test_fixed_ips.py
+++ b/nova/tests/api/openstack/compute/contrib/test_fixed_ips.py
@@ -132,6 +132,11 @@ class FixedIpTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPNotFound, self.controller.show, req,
                           '10.0.0.1')
 
+    def test_fixed_ips_get_invalid_ip_address(self):
+        req = fakes.HTTPRequest.blank('/v2/fake/os-fixed-ips/inv.ali.d.ip')
+        self.assertRaises(webob.exc.HTTPNotFound, self.controller.show, req,
+                          'inv.ali.d.ip')
+
     def test_fixed_ips_get_deleted_ip_fail(self):
         req = fakes.HTTPRequest.blank('/v2/fake/os-fixed-ips/10.0.0.2')
         self.assertRaises(webob.exc.HTTPNotFound, self.controller.show, req,
@@ -154,6 +159,13 @@ class FixedIpTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPNotFound, self.controller.action, req,
                           '10.0.0.1', body)
 
+    def test_fixed_ip_reserve_invalid_ip_address(self):
+        body = {'reserve': None}
+        req = fakes.HTTPRequest.blank(
+            '/v2/fake/os-fixed-ips/inv.ali.d.ip/action')
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.action, req, 'inv.ali.d.ip', body)
+
     def test_fixed_ip_reserve_deleted_ip(self):
         body = {'reserve': None}
         req = fakes.HTTPRequest.blank(
@@ -178,6 +190,13 @@ class FixedIpTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPNotFound, self.controller.action, req,
                           '10.0.0.1', body)
 
+    def test_fixed_ip_unreserve_invalid_ip_address(self):
+        body = {'unreserve': None}
+        req = fakes.HTTPRequest.blank(
+            '/v2/fake/os-fixed-ips/inv.ali.d.ip/action')
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          self.controller.action, req, 'inv.ali.d.ip', body)
+
     def test_fixed_ip_unreserve_deleted_ip(self):
         body = {'unreserve': None}
         req = fakes.HTTPRequest.blank(
diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py
index ca542790c..efe243d1b 100644
--- a/nova/tests/test_db_api.py
+++ b/nova/tests/test_db_api.py
@@ -27,8 +27,10 @@ import uuid as stdlib_uuid
 import mox
 from oslo.config import cfg
 from sqlalchemy.dialects import sqlite
+from sqlalchemy import exc
 from sqlalchemy.exc import IntegrityError
 from sqlalchemy import MetaData
+from sqlalchemy.orm import query
 from sqlalchemy.sql.expression import select
 
 from nova.compute import vm_states
@@ -2782,6 +2784,13 @@ class FixedIPTestCase(BaseInstanceTypeTestCase):
                                       network_id=None,
                                       updated_at=new))
 
+    def mock_db_query_first_to_raise_data_error_exception(self):
+        self.mox.StubOutWithMock(query.Query, 'first')
+        query.Query.first().AndRaise(exc.DataError(mox.IgnoreArg(),
+                                                   mox.IgnoreArg(),
+                                                   mox.IgnoreArg()))
+        self.mox.ReplayAll()
+
     def test_fixed_ip_disassociate_all_by_timeout_single_host(self):
         now = timeutils.utcnow()
         self._timeout_test(self.ctxt, now, False)
@@ -3118,7 +3127,7 @@ class FixedIPTestCase(BaseInstanceTypeTestCase):
         self.assertRaises(exception.FixedIpNotFound,
                           db.fixed_ip_get, self.ctxt, 0)
 
-    def test_fixed_ip_get_sucsess2(self):
+    def test_fixed_ip_get_success2(self):
         adress = 'fixed_ip_adress'
         instance_uuid = self._create_instance()
         network_id = db.network_create_safe(self.ctxt, {})['id']
@@ -3139,7 +3148,7 @@ class FixedIPTestCase(BaseInstanceTypeTestCase):
         self.assertRaises(exception.NotAuthorized, db.fixed_ip_get,
                           self.ctxt, fixed_ip_id)
 
-    def test_fixed_ip_get_sucsess(self):
+    def test_fixed_ip_get_success(self):
         adress = 'fixed_ip_adress'
         instance_uuid = self._create_instance()
         network_id = db.network_create_safe(self.ctxt, {})['id']
@@ -3165,6 +3174,11 @@ class FixedIPTestCase(BaseInstanceTypeTestCase):
         self.assertRaises(exception.FixedIpNotFoundForAddress,
                           db.fixed_ip_get_by_address_detailed, self.ctxt, 'x')
 
+    def test_fixed_ip_get_by_address_with_data_error_exception(self):
+        self.mock_db_query_first_to_raise_data_error_exception()
+        self.assertRaises(exception.FixedIpInvalid,
+                          db.fixed_ip_get_by_address_detailed, self.ctxt, 'x')
+
     def test_fixed_ip_get_by_address_detailed_sucsess(self):
         adress = 'fixed_ip_adress_123'
         instance_uuid = self._create_instance()
@@ -3246,6 +3260,13 @@ class FloatingIpTestCase(test.TestCase, ModelsObjectComparatorMixin):
             'interface': 'fake_interface',
         }
 
+    def mock_db_query_first_to_raise_data_error_exception(self):
+        self.mox.StubOutWithMock(query.Query, 'first')
+        query.Query.first().AndRaise(exc.DataError(mox.IgnoreArg(),
+                                                   mox.IgnoreArg(),
+                                                   mox.IgnoreArg()))
+        self.mox.ReplayAll()
+
     def _create_floating_ip(self, values):
         if not values:
             values = {}
@@ -3266,6 +3287,11 @@ class FloatingIpTestCase(test.TestCase, ModelsObjectComparatorMixin):
         self.assertRaises(exception.FloatingIpNotFound,
                           db.floating_ip_get, self.ctxt, 100500)
 
+    def test_floating_ip_get_with_long_id_not_found(self):
+        self.mock_db_query_first_to_raise_data_error_exception()
+        self.assertRaises(exception.InvalidID,
+                          db.floating_ip_get, self.ctxt, 123456789101112)
+
     def test_floating_ip_get_pools(self):
         values = [
             {'address': '0.0.0.0', 'pool': 'abc'},
@@ -3571,6 +3597,12 @@ class FloatingIpTestCase(test.TestCase, ModelsObjectComparatorMixin):
                           db.floating_ip_get_by_address,
                           self.ctxt, 'non_exists_host')
 
+    def test_floating_ip_get_by_invalid_address(self):
+        self.mock_db_query_first_to_raise_data_error_exception()
+        self.assertRaises(exception.InvalidIpAddressError,
+                          db.floating_ip_get_by_address,
+                          self.ctxt, 'non_exists_host')
+
     def test_floating_ip_get_by_fixed_address(self):
         fixed_float = [
             ('1.1.1.1', '2.2.2.1'),
@@ -4186,6 +4218,13 @@ class VirtualInterfaceTestCase(test.TestCase, ModelsObjectComparatorMixin):
             'uuid': str(stdlib_uuid.uuid4())
         }
 
+    def mock_db_query_first_to_raise_data_error_exception(self):
+        self.mox.StubOutWithMock(query.Query, 'first')
+        query.Query.first().AndRaise(exc.DataError(mox.IgnoreArg(),
+                                                   mox.IgnoreArg(),
+                                                   mox.IgnoreArg()))
+        self.mox.ReplayAll()
+
     def _create_virt_interface(self, values):
         v = self._get_base_values()
         v.update(values)
@@ -4222,6 +4261,17 @@ class VirtualInterfaceTestCase(test.TestCase, ModelsObjectComparatorMixin):
                                                            vif['address'])
             self._assertEqualObjects(vif, real_vif)
 
+    def test_virtual_interface_get_by_address_not_found(self):
+        self.assertIsNone(db.virtual_interface_get_by_address(self.ctxt,
+                          "i.nv.ali.ip"))
+
+    def test_virtual_interface_get_by_address_data_error_exception(self):
+        self.mock_db_query_first_to_raise_data_error_exception()
+        self.assertRaises(exception.InvalidIpAddressError,
+                          db.virtual_interface_get_by_address,
+                          self.ctxt,
+                          "i.nv.ali.ip")
+
     def test_virtual_interface_get_by_uuid(self):
         vifs = [self._create_virt_interface({}),
                 self._create_virt_interface({})]