x/vmware-nsx
Code Issues Proposed changes

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

Browse Source
This commit is contained in:
Jenkins 2017-05-02 21:14:43 +00:00 committed by Gerrit Code Review
commit 40724d7034
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)