x/vmware-nsx
Code Issues Proposed changes

FWaaS - fix reordering of rules in policy

Browse Source 
Due to a recent change, reodering of rules within the
same policy was failing. This is fixed by checking if
the rules belong to the same policy we allow reordering.

There was also a missing call to reorder due to which
the position number on the rules was not reflected correctly
after the reordering. This is also fixed.

Closes bug: #1226941

Change-Id: I7f52e8b9d578c290ace3bb615bf68bd213398303
This commit is contained in:
Sumit Naiksatam 2013-09-17 23:27:20 -07:00
parent 9361141feb
commit 750ff54967
2 changed files with 53 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
neutron/db/firewall/firewall_db.py
View File

@ -184,8 +184,10 @@ class Firewall_db_mixin(firewall.FirewallPluginBase, base_db.CommonDbMixin):
raise firewall.FirewallRuleNotFound(firewall_rule_id= raise firewall.FirewallRuleNotFound(firewall_rule_id=
fwrule_id) fwrule_id)
elif rules_dict[fwrule_id]['firewall_policy_id']: elif rules_dict[fwrule_id]['firewall_policy_id']:
raise firewall.FirewallRuleInUse( if (rules_dict[fwrule_id]['firewall_policy_id'] !=
firewall_rule_id=fwrule_id) fwp_db['id']):
raise firewall.FirewallRuleInUse(
firewall_rule_id=fwrule_id)
# New list of rules is valid so we will first reset the existing # New list of rules is valid so we will first reset the existing
# list and then add each rule in order. # list and then add each rule in order.
# Note that the list could be empty in which case we interpret # Note that the list could be empty in which case we interpret
@ -193,6 +195,7 @@ class Firewall_db_mixin(firewall.FirewallPluginBase, base_db.CommonDbMixin):
fwp_db.firewall_rules = [] fwp_db.firewall_rules = []
for fwrule_id in rule_id_list: for fwrule_id in rule_id_list:
fwp_db.firewall_rules.append(rules_dict[fwrule_id]) fwp_db.firewall_rules.append(rules_dict[fwrule_id])
fwp_db.firewall_rules.reorder()
fwp_db.audited = False fwp_db.audited = False
def _process_rule_for_policy(self, context, firewall_policy_id, def _process_rule_for_policy(self, context, firewall_policy_id,

48
neutron/tests/unit/db/firewall/test_db_firewall.py
View File

@ -420,6 +420,54 @@ class TestFirewallDBPlugin(FirewallPluginDbTestCase):
for k, v in attrs.iteritems(): for k, v in attrs.iteritems():
self.assertEqual(res['firewall_policy'][k], v) self.assertEqual(res['firewall_policy'][k], v)
def test_update_firewall_policy_reorder_rules(self):
attrs = self._get_test_firewall_policy_attrs()
with self.firewall_policy() as fwp:
with contextlib.nested(self.firewall_rule(name='fwr1',
no_delete=True),
self.firewall_rule(name='fwr2',
no_delete=True),
self.firewall_rule(name='fwr3',
no_delete=True),
self.firewall_rule(name='fwr4',
no_delete=True)) as fr:
fw_rule_ids = [fr[2]['firewall_rule']['id'],
fr[3]['firewall_rule']['id']]
data = {'firewall_policy':
{'firewall_rules': fw_rule_ids}}
req = self.new_update_request('firewall_policies', data,
fwp['firewall_policy']['id'])
req.get_response(self.ext_api)
# shuffle the rules, add more rules
fw_rule_ids = [fr[1]['firewall_rule']['id'],
fr[3]['firewall_rule']['id'],
fr[2]['firewall_rule']['id'],
fr[0]['firewall_rule']['id']]
attrs['firewall_rules'] = fw_rule_ids
data = {'firewall_policy':
{'firewall_rules': fw_rule_ids}}
req = self.new_update_request('firewall_policies', data,
fwp['firewall_policy']['id'])
res = self.deserialize(self.fmt,
req.get_response(self.ext_api))
rules = []
for rule_id in fw_rule_ids:
req = self.new_show_request('firewall_rules',
rule_id,
fmt=self.fmt)
res = self.deserialize(self.fmt,
req.get_response(self.ext_api))
rules.append(res['firewall_rule'])
self.assertEqual(rules[0]['position'], 1)
self.assertEqual(rules[0]['id'], fr[1]['firewall_rule']['id'])
self.assertEqual(rules[1]['position'], 2)
self.assertEqual(rules[1]['id'], fr[3]['firewall_rule']['id'])
self.assertEqual(rules[2]['position'], 3)
self.assertEqual(rules[2]['id'], fr[2]['firewall_rule']['id'])
self.assertEqual(rules[3]['position'], 4)
self.assertEqual(rules[3]['id'], fr[0]['firewall_rule']['id'])
def test_update_firewall_policy_with_non_existing_rule(self): def test_update_firewall_policy_with_non_existing_rule(self):
attrs = self._get_test_firewall_policy_attrs() attrs = self._get_test_firewall_policy_attrs()