From 9caf4d095445f8220ce2d78aa91c12dc40b2aaea Mon Sep 17 00:00:00 2001
From: Sridar Kandaswamy <skandasw@cisco.com>
Date: Tue, 20 Sep 2016 16:39:07 -0700
Subject: [PATCH] Fix KeyError when fw rule associated with a policy is updated

Fix issue in plugin code that was trying to reference the policy-id
from the rule object which is no longer correct.

Change-Id: I59e061a1ce383f5a783e72aab1ab4a07c88a0d30
Closes-Bug: #1623953
---
 .../services/firewall/fwaas_plugin_v2.py      |  8 +-
 .../services/firewall/test_fwaas_plugin_v2.py | 77 +++++++++++++++++++
 2 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/neutron_fwaas/services/firewall/fwaas_plugin_v2.py b/neutron_fwaas/services/firewall/fwaas_plugin_v2.py
index d9a4ab5c5..388a8e877 100644
--- a/neutron_fwaas/services/firewall/fwaas_plugin_v2.py
+++ b/neutron_fwaas/services/firewall/fwaas_plugin_v2.py
@@ -189,7 +189,7 @@ class FirewallPluginV2(
 
     def _ensure_update_firewall_policy(self, context, firewall_policy_id):
         firewall_policy = self.get_firewall_policy(context, firewall_policy_id)
-        if firewall_policy and 'firewall_list' in firewall_policy:
+        if firewall_policy:
             ing_fwg_ids, eg_fwg_ids = self._get_fwgs_with_policy(context,
                 firewall_policy_id)
             for fwg_id in list(set(ing_fwg_ids + eg_fwg_ids)):
@@ -341,7 +341,7 @@ class FirewallPluginV2(
         self._ensure_update_firewall_rule(context, id)
         fwr = super(FirewallPluginV2,
                     self).update_firewall_rule(context, id, firewall_rule)
-        firewall_policy_id = fwr['firewall_policy_id']
-        if firewall_policy_id:
-            self._rpc_update_firewall_policy(context, firewall_policy_id)
+        fwp_ids = self._get_policies_with_rule(context, id)
+        for fwp_id in fwp_ids:
+            self._rpc_update_firewall_policy(context, fwp_id)
         return fwr
diff --git a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py
index 12f52f86e..a10994eae 100644
--- a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py
+++ b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py
@@ -487,3 +487,80 @@ class TestFirewallPluginBasev2(TestFirewallRouterPortBase,
                 r['router']['id'],
                 s2['subnet']['id'],
                 None)
+
+    def test_update_firewall_rule_on_active_fwg(self):
+        name = "new_firewall_rule1"
+        attrs = self._get_test_firewall_rule_attrs(name)
+        with self.router(name='router1', admin_state_up=True,
+            tenant_id=self._tenant_id) as r, \
+                self.subnet() as s1:
+
+            body = self._router_interface_action(
+                'add',
+                r['router']['id'],
+                s1['subnet']['id'],
+                None)
+            port_id1 = body['port_id']
+            with self.firewall_rule() as fwr:
+                with self.firewall_policy(
+                    firewall_rules=[fwr['firewall_rule']['id']]) as fwp:
+                    fwp_id = fwp['firewall_policy']['id']
+                    with self.firewall_group(
+                        name='test',
+                        ingress_firewall_policy_id=fwp_id,
+                        egress_firewall_policy_id=fwp_id, ports=[port_id1],
+                        admin_state_up=True) as fwg1:
+                        self.assertEqual(const.PENDING_CREATE,
+                             fwg1['firewall_group']['status'])
+
+                        ctx = context.get_admin_context()
+                        self.callbacks.set_firewall_group_status(ctx,
+                            fwg1['firewall_group']['id'], const.ACTIVE)
+                        data = {'firewall_rule': {'name': name}}
+                        req = self.new_update_request('firewall_rules', data,
+                            fwr['firewall_rule']['id'])
+                        res = self.deserialize(self.fmt,
+                                               req.get_response(self.ext_api))
+                        for k, v in six.iteritems(attrs):
+                            self.assertEqual(v, res['firewall_rule'][k])
+
+            self._router_interface_action('remove',
+                r['router']['id'],
+                s1['subnet']['id'],
+                None)
+
+    def test_update_firewall_rule_on_pending_create_fwg(self):
+        """update should fail"""
+        name = "new_firewall_rule1"
+        with self.router(name='router1', admin_state_up=True,
+            tenant_id=self._tenant_id) as r, \
+                self.subnet() as s1:
+
+            body = self._router_interface_action(
+                'add',
+                r['router']['id'],
+                s1['subnet']['id'],
+                None)
+            port_id1 = body['port_id']
+            with self.firewall_rule() as fwr:
+                with self.firewall_policy(
+                    firewall_rules=[fwr['firewall_rule']['id']]) as fwp:
+                    fwp_id = fwp['firewall_policy']['id']
+                    with self.firewall_group(
+                        name='test',
+                        ingress_firewall_policy_id=fwp_id,
+                        egress_firewall_policy_id=fwp_id, ports=[port_id1],
+                        admin_state_up=True) as fwg1:
+                        self.assertEqual(const.PENDING_CREATE,
+                             fwg1['firewall_group']['status'])
+
+                        data = {'firewall_rule': {'name': name}}
+                        req = self.new_update_request('firewall_rules', data,
+                            fwr['firewall_rule']['id'])
+                        res = req.get_response(self.ext_api)
+                        self.assertEqual(409, res.status_int)
+
+            self._router_interface_action('remove',
+                r['router']['id'],
+                s1['subnet']['id'],
+                None)