From 58e0b8bafbadb044b560efaac81e9ee20fcf6b53 Mon Sep 17 00:00:00 2001
From: Numan Siddique <numan.siddique@enovance.com>
Date: Sat, 11 Oct 2014 17:38:05 +0530
Subject: [PATCH] Fix KeyError in dhcp_rpc when plugin.port_update raise
 exception

KeyError exception is seen because of following reasons

* DhcpRpcCallback._port_action() is called by two functions
   -  DhcpRpcCallback.create_dchp_port()
   -  DhcpRpcCallback.update_dhcp_port()

* When create_dhcp_port() function calls _port_action(), the
  function argument 'port' will have the body as
  {'port': {'network_id': foo_network_id, 'fixed_ips': [..] ...}

* When update_dhcp_port() function calls _port_action(), the
  function argument 'port' will have the body as
  {'id': port_id, 'port': {{'port': {'network_id': foo_network_id,
			            'fixed_ips': [..] ...}}

* If an exception occurs when _port_action() calls plugin.create_port(),
  network id is accessed as
  net_id = port['port']['network_id']

* If an exception occurs when _port_action() calls plugin.update_port(),
  network id is accessed as
  net_id = port['port']['network_id']
  which is causing the KeyError. network_id should have been accessed as
  net_id = port['port']['port']['network_id']

This patch fixes the issue by making the _port_action() take the
same port body. update_dhcp_port() insteading of passing the port_id
and port information in a single argument, it now adds port_id
in the port body itself.

Change-Id: I70b92fa20b421b05ca2053a9a57f62db726f7625
Closes-bug: #1378508
---
 neutron/api/rpc/handlers/dhcp_rpc.py |  8 +++---
 neutron/tests/unit/test_dhcp_rpc.py  | 37 +++++++++++++++++++++++++---
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/neutron/api/rpc/handlers/dhcp_rpc.py b/neutron/api/rpc/handlers/dhcp_rpc.py
index 56016be708..58317eac4b 100644
--- a/neutron/api/rpc/handlers/dhcp_rpc.py
+++ b/neutron/api/rpc/handlers/dhcp_rpc.py
@@ -60,7 +60,7 @@ class DhcpRpcCallback(n_rpc.RpcCallback):
             if action == 'create_port':
                 return plugin.create_port(context, port)
             elif action == 'update_port':
-                return plugin.update_port(context, port['id'], port['port'])
+                return plugin.update_port(context, port['id'], port)
             else:
                 msg = _('Unrecognized action')
                 raise n_exc.Invalid(message=msg)
@@ -282,13 +282,11 @@ class DhcpRpcCallback(n_rpc.RpcCallback):
     def update_dhcp_port(self, context, **kwargs):
         """Update the dhcp port."""
         host = kwargs.get('host')
-        port_id = kwargs.get('port_id')
         port = kwargs.get('port')
+        port['id'] = kwargs.get('port_id')
         LOG.debug(_('Update dhcp port %(port)s '
                     'from %(host)s.'),
                   {'port': port,
                    'host': host})
         plugin = manager.NeutronManager.get_plugin()
-        return self._port_action(plugin, context,
-                                 {'id': port_id, 'port': port},
-                                 'update_port')
+        return self._port_action(plugin, context, port, 'update_port')
diff --git a/neutron/tests/unit/test_dhcp_rpc.py b/neutron/tests/unit/test_dhcp_rpc.py
index 6a2ed16d7e..2c4c5c9e6e 100644
--- a/neutron/tests/unit/test_dhcp_rpc.py
+++ b/neutron/tests/unit/test_dhcp_rpc.py
@@ -161,13 +161,44 @@ class TestDhcpRpcCallback(base.BaseTestCase):
         self.plugin.assert_has_calls(expected)
         return retval
 
-    def test_update_dhcp_port(self):
+    def test_update_dhcp_port_verify_port_action_port_dict(self):
+        port = {'port': {'network_id': 'foo_network_id',
+                         'device_owner': constants.DEVICE_OWNER_DHCP,
+                         'fixed_ips': [{'subnet_id': 'foo_subnet_id'}]}
+                }
+        expected_port = {'port': {'network_id': 'foo_network_id',
+                                  'device_owner': constants.DEVICE_OWNER_DHCP,
+                                  'fixed_ips': [{'subnet_id': 'foo_subnet_id'}]
+                                  },
+                         'id': 'foo_port_id'
+                         }
+
+        def _fake_port_action(plugin, context, port, action):
+            self.assertEqual(expected_port, port)
+
+        self.callbacks._port_action = _fake_port_action
         self.callbacks.update_dhcp_port(mock.Mock(),
                                         host='foo_host',
                                         port_id='foo_port_id',
-                                        port=mock.Mock())
+                                        port=port)
+
+    def test_update_dhcp_port(self):
+        port = {'port': {'network_id': 'foo_network_id',
+                         'device_owner': constants.DEVICE_OWNER_DHCP,
+                         'fixed_ips': [{'subnet_id': 'foo_subnet_id'}]}
+                }
+        expected_port = {'port': {'network_id': 'foo_network_id',
+                                  'device_owner': constants.DEVICE_OWNER_DHCP,
+                                  'fixed_ips': [{'subnet_id': 'foo_subnet_id'}]
+                                  },
+                         'id': 'foo_port_id'
+                         }
+        self.callbacks.update_dhcp_port(mock.Mock(),
+                                        host='foo_host',
+                                        port_id='foo_port_id',
+                                        port=port)
         self.plugin.assert_has_calls(
-            mock.call.update_port(mock.ANY, 'foo_port_id', mock.ANY))
+            mock.call.update_port(mock.ANY, 'foo_port_id', expected_port))
 
     def test_get_dhcp_port_existing(self):
         port_retval = dict(id='port_id', fixed_ips=[dict(subnet_id='a')])