From 6ea681d5b3a90e70c95fa9d9dbf709eafec46b3e Mon Sep 17 00:00:00 2001
From: Justin Hammond <justin.hammond@rackspace.com>
Date: Tue, 13 Oct 2015 14:05:57 -0500
Subject: [PATCH] Prevents PUT on ip_addresses with empty port list

JIRA:NCP-1631
---
 quark/plugin_modules/ip_addresses.py          |  8 +++-
 .../plugin_modules/test_shared_ips.py         | 41 ++++++++-----------
 .../tests/plugin_modules/test_ip_addresses.py |  9 ++--
 3 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/quark/plugin_modules/ip_addresses.py b/quark/plugin_modules/ip_addresses.py
index 514deaa..8350d2b 100644
--- a/quark/plugin_modules/ip_addresses.py
+++ b/quark/plugin_modules/ip_addresses.py
@@ -281,7 +281,13 @@ def update_ip_address(context, id, ip_address):
                 msg = "Modification of reset_allocation_time requires admin"
                 raise webob.exc.HTTPForbidden(detail=msg)
 
-        port_ids = ip_address['ip_address'].get('port_ids')
+        port_ids = ip_address['ip_address'].get('port_ids', None)
+
+        if port_ids is not None and not port_ids:
+            raise exceptions.BadRequest(
+                resource="ip_addresses",
+                msg="Cannot be updated with empty port_id list")
+
         if iptype == ip_types.SHARED:
             has_owner = address.has_any_shared_owner()
 
diff --git a/quark/tests/functional/plugin_modules/test_shared_ips.py b/quark/tests/functional/plugin_modules/test_shared_ips.py
index 1bed4f8..dde41ec 100644
--- a/quark/tests/functional/plugin_modules/test_shared_ips.py
+++ b/quark/tests/functional/plugin_modules/test_shared_ips.py
@@ -118,6 +118,23 @@ class QuarkSharedIPs(BaseFunctionalTest):
             with self.assertRaises(exceptions.BadRequest):
                 ip_api.update_ip_address(self.context, ip['id'], shared_ip)
 
+    def test_update_shared_ip_with_empty_port_id_list_will_error(self):
+
+        with self._stubs(self.network, self.subnet, self.ports_info4) as (
+                net, sub, ports):
+
+            port_ids = [ports[0]['id'], ports[1]['id']]
+            shared_ip = {'ip_address': dict(port_ids=port_ids,
+                                            network_id=net['id'],
+                                            version=4)}
+            ip = ip_api.create_ip_address(self.context, shared_ip)
+            self.assertEqual(ip_types.SHARED, ip['type'])
+            port_ids = []
+            shared_ip = {'ip_addresses': dict(port_ids=port_ids)}
+
+            with self.assertRaises(exceptions.BadRequest):
+                ip_api.update_ip_address(self.context, ip['id'], shared_ip)
+
     def test_update_shared_ip_with_garbage_will_error(self):
 
         with self._stubs(self.network, self.subnet, self.ports_info4) as (
@@ -269,30 +286,6 @@ class QuarkSharedIPs(BaseFunctionalTest):
 
             ip_api.update_ip_address(self.context, ip['id'], shared_ip)
 
-    def test_update_shared_ip_with_owned_port_error_deallocate(self):
-
-        with self._stubs(self.network, self.subnet, self.ports_info4) as (
-                net, sub, ports):
-
-            port_ids = [ports[0]['id'], ports[1]['id']]
-            p_id = ports[0]['id']
-
-            shared_ip = {'ip_address': dict(port_ids=port_ids,
-                                            network_id=net['id'],
-                                            version=4)}
-            ip = ip_api.create_ip_address(self.context, shared_ip)
-            self.assertEqual(ip_types.SHARED, ip['type'])
-
-            port_ip_update = ip_api.update_port_for_ip_address
-            port_ip_update(self.context, ip['id'], p_id,
-                           self._make_port_body('derp'))
-
-            port_ids = []
-            shared_ip = {'ip_address': dict(port_ids=port_ids)}
-
-            with self.assertRaises(self.disassociate_exception):
-                ip_api.update_ip_address(self.context, ip['id'], shared_ip)
-
     def test_update_shared_ip_with_owned_port_error(self):
 
         with self._stubs(self.network, self.subnet, self.ports_info4) as (
diff --git a/quark/tests/plugin_modules/test_ip_addresses.py b/quark/tests/plugin_modules/test_ip_addresses.py
index 4f19ffd..405d668 100644
--- a/quark/tests/plugin_modules/test_ip_addresses.py
+++ b/quark/tests/plugin_modules/test_ip_addresses.py
@@ -553,16 +553,15 @@ class TestQuarkUpdateIPAddress(test_quark_plugin.TestQuarkPlugin):
                                                      ip_address)
             self.assertEqual(response['port_ids'], [])
 
-    def test_update_ip_address_empty_ports_delete(self):
+    def test_update_ip_address_empty_ports_does_not_delete_but_errors(self):
         port = dict(id=1, network_id=2, ip_addresses=[])
         ip = dict(id=1, address=3232235876, address_readable="192.168.1.100",
                   subnet_id=1, network_id=2, version=4)
         with self._stubs(ports=[port], addr=ip, addr_ports=True):
             ip_address = {'ip_address': {'port_ids': []}}
-            response = self.plugin.update_ip_address(self.context,
-                                                     ip['id'],
-                                                     ip_address)
-            self.assertEqual(response['port_ids'], [])
+            with self.assertRaises(exceptions.BadRequest):
+                self.plugin.update_ip_address(self.context, ip['id'],
+                                              ip_address)
 
 
 class TestQuarkGetIpAddress(test_quark_plugin.TestQuarkPlugin):