x/vmware-nsx
Code Issues Proposed changes

NSX|V fix FWaaS rules order when router is added to FW

Browse Source 
previous patch took care of the rule order when recreating the edge fw rules.
This patch takes care of the same when the rotuer is added to the firewall.
The FWaaS rules should be added at the correct location between the
rest of the router rules.

Change-Id: I8ac52e6e476b214ded7342d1473670c1d31befef
This commit is contained in:
Adit Sarfaty 2017-04-30 10:29:20 +03:00
parent 630a78546d
commit 8de331c4d6
4 changed files with 77 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
vmware_nsx/plugins/nsx_v/plugin.py
View File

@ -3336,7 +3336,7 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
router['id'])
if subnet_cidrs:
no_snat_fw_rules.append({
'name': 'No SNAT Rule',
'name': edge_firewall_driver.NO_SNAT_RULE_NAME,
'action': 'allow',
'enabled': True,
'source_vnic_groups': ["external"],
@ -3516,6 +3516,8 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
def _update_subnets_and_dnat_firewall(self, context, router,
router_id=None):
# Note(asarfaty): If changing the order or names of rules here,
# please take care of fwaas _get_other_backend_rules too.
fw_rules = []
if not router_id:
router_id = router['id']
@ -3527,7 +3529,7 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
subnet_cidrs.extend([route['destination'] for route in routes])
if subnet_cidrs:
subnet_fw_rule = {
'name': 'Subnet Rule',
'name': edge_firewall_driver.SUBNET_RULE_NAME,
'action': 'allow',
'enabled': True,
'source_ip_address': subnet_cidrs,
@ -3553,7 +3555,7 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
dnat_cidrs = [rule['dst'] for rule in dnat_rules]
if dnat_cidrs:
dnat_fw_rule = {
'name': 'DNAT Rule',
'name': edge_firewall_driver.DNAT_RULE_NAME,
'action': 'allow',
'enabled': True,
'destination_ip_address': dnat_cidrs}

4
vmware_nsx/plugins/nsx_v/vshield/edge_firewall_driver.py
View File

@ -32,6 +32,10 @@ FWAAS_DENY = "deny"
FWAAS_REJECT = "reject"
FWAAS_ALLOW_EXT_RULE_NAME = 'Allow To External'
SUBNET_RULE_NAME = 'Subnet Rule'
DNAT_RULE_NAME = 'DNAT Rule'
NO_SNAT_RULE_NAME = 'No SNAT Rule'
class EdgeFirewallDriver(object):
"""Implementation of driver APIs for

34
vmware_nsx/services/fwaas/nsx_v/edge_fwaas_driver.py
View File

@ -131,6 +131,9 @@ class EdgeFwaasDriver(fwaas_base.FwaasDriverBase):
Those rules should stay on the backend firewall, when updating the
FWaaS rules.
Return it as 2 separate lists of rules, which should go before/after
the fwaas rules.
"""
try:
backend_fw = self._nsxv.get_firewall(context, edge_id)
@ -138,19 +141,33 @@ class EdgeFwaasDriver(fwaas_base.FwaasDriverBase):
except vcns_exc.VcnsApiException:
# Need to create a new one
backend_rules = []
# remove old FWaaS rules from the rules list.
# also delete the allow-external rule, if it is there.
# If necessary - we will add it again later
other_rules = []
before_rules = []
after_rules = []
go_after = False
for rule_item in backend_rules:
rule = rule_item['firewall_rule']
rule_name = rule.get('name', '')
if (not rule_name.startswith(RULE_NAME_PREFIX) and
fwaas_rule = rule_name.startswith(RULE_NAME_PREFIX)
if fwaas_rule:
# reached the fwaas part, the rest of the rules should be
# in the 'after' list
go_after = True
if (rule_name == edge_firewall_driver.DNAT_RULE_NAME or
rule_name == edge_firewall_driver.NO_SNAT_RULE_NAME):
# passed the fwaas part, the rest of the rules should be
# in the 'after' list
go_after = True
if (not fwaas_rule and
not self._is_allow_external_rule(rule)):
other_rules.append(rule)
if go_after:
after_rules.append(rule)
else:
before_rules.append(rule)
return other_rules
return before_rules, after_rules
def _set_rules_on_edge(self, context, edge_id, fw_id, translated_rules,
allow_external=False):
@ -163,10 +180,13 @@ class EdgeFwaasDriver(fwaas_base.FwaasDriverBase):
firewall. It should only be True when the firewall is being deleted.
"""
# Get the existing backend rules which do not belong to FWaaS
backend_rules = self._get_other_backend_rules(context, edge_id)
backend_rules, after_rules = self._get_other_backend_rules(
context, edge_id)
# add new FWaaS rules at the end by their original order
# add new FWaaS rules at the correct location by their original order
backend_rules.extend(translated_rules)
backend_rules.extend(after_rules)
# update the backend
try:
with locking.LockManager.get_lock(str(edge_id)):

49
vmware_nsx/tests/unit/nsx_v/test_fwaas_driver.py
View File

@ -97,8 +97,8 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase):
"_get_routers_edges", return_value=edges):
func('nsx', apply_list, firewall)
self.assertEqual(router_count, update_fw.call_count)
bakend_rules = update_fw.call_args[0][1]['firewall_rule_list']
self.assertEqual(len(rule_list), len(bakend_rules))
backend_rules = update_fw.call_args[0][1]['firewall_rule_list']
self.assertEqual(len(rule_list), len(backend_rules))
allow_ext = update_fw.call_args[1]['allow_external']
self.assertEqual(False, allow_ext)
@ -109,8 +109,8 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase):
"update_firewall") as update_fw:
self.firewall.create_firewall('nsx', apply_list, firewall)
self.assertEqual(1, update_fw.call_count)
bakend_rules = update_fw.call_args[0][1]['firewall_rule_list']
self.assertEqual(0, len(bakend_rules))
backend_rules = update_fw.call_args[0][1]['firewall_rule_list']
self.assertEqual(0, len(backend_rules))
allow_ext = update_fw.call_args[1]['allow_external']
self.assertEqual(False, allow_ext)
@ -124,6 +124,39 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase):
def test_update_firewall_with_rules(self):
self._setup_firewall_with_rules(self.firewall.update_firewall)
def test_create_firewall_with_existing_backend_rules(self):
apply_list = self._fake_apply_list()
rule_list = self._fake_rules_v4()
firewall = self._fake_firewall(rule_list)
edge_id = 'edge-1'
# backend fw already has 3 rules.
# The fwaas rule should come in the middle, and replace the fwaas rule
existing_rules = [{'name': 'Subnet Rule',
'action': 'accept',
'enabled': True},
{'name': 'Fwaas-1',
'action': 'deny',
'enabled': True},
{'name': 'DNAT Rule',
'action': 'accept',
'enabled': True}]
for rule in existing_rules:
self.firewall._nsxv.vcns.add_firewall_rule(edge_id, rule)
with mock.patch.object(self.firewall._nsxv,
"update_firewall") as update_fw,\
mock.patch.object(self.firewall,
"_get_routers_edges", return_value=[edge_id]):
self.firewall.create_firewall('nsx', apply_list, firewall)
self.assertEqual(1, update_fw.call_count)
backend_rules = update_fw.call_args[0][1]['firewall_rule_list']
self.assertEqual(len(rule_list) + len(existing_rules) - 1,
len(backend_rules))
self.assertEqual('Subnet Rule', backend_rules[0]['name'])
self.assertEqual('DNAT Rule', backend_rules[-1]['name'])
allow_ext = update_fw.call_args[1]['allow_external']
self.assertEqual(False, allow_ext)
def test_delete_firewall(self):
apply_list = self._fake_apply_list()
firewall = self._fake_firewall_no_rule()
@ -131,8 +164,8 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase):
"update_firewall") as update_fw:
self.firewall.delete_firewall('nsx', apply_list, firewall)
self.assertEqual(1, update_fw.call_count)
bakend_rules = update_fw.call_args[0][1]['firewall_rule_list']
self.assertEqual(0, len(bakend_rules))
backend_rules = update_fw.call_args[0][1]['firewall_rule_list']
self.assertEqual(0, len(backend_rules))
allow_ext = update_fw.call_args[1]['allow_external']
self.assertEqual(True, allow_ext)
@ -144,8 +177,8 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase):
"update_firewall") as update_fw:
self.firewall.create_firewall('nsx', apply_list, firewall)
self.assertEqual(1, update_fw.call_count)
bakend_rules = update_fw.call_args[0][1]['firewall_rule_list']
self.assertEqual(0, len(bakend_rules))
backend_rules = update_fw.call_args[0][1]['firewall_rule_list']
self.assertEqual(0, len(backend_rules))
allow_ext = update_fw.call_args[1]['allow_external']
self.assertEqual(False, allow_ext)