From 45ab0ca06436adade666a287aeee875974852050 Mon Sep 17 00:00:00 2001 From: asarfaty Date: Thu, 12 Nov 2020 14:16:19 +0200 Subject: [PATCH] NSX|V: Fix FW rule id for distributed routers Commit I817d9434715d7bd3cba266575321d4c89bf173e4 added the vnic-id to the rule so it will be unique. But for distributed router this is not good enough as the vnix is not part of the rule. Instead the driver will add each rule only once per firewall-group, and add the firewall group id instead of the vnic for distributed routers Change-Id: If775cc7aeb9e3edb64462484bb1b2714a7d30073 --- .../fwaas/nsx_v/fwaas_callbacks_v2.py | 26 +++++++++++++------ .../tests/unit/nsx_v/test_fwaas_v2_driver.py | 8 +++--- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/vmware_nsx/services/fwaas/nsx_v/fwaas_callbacks_v2.py b/vmware_nsx/services/fwaas/nsx_v/fwaas_callbacks_v2.py index 69e2aa159a..ff5e49d055 100644 --- a/vmware_nsx/services/fwaas/nsx_v/fwaas_callbacks_v2.py +++ b/vmware_nsx/services/fwaas/nsx_v/fwaas_callbacks_v2.py @@ -83,17 +83,22 @@ class NsxvFwaasCallbacksV2(com_callbacks.NsxFwaasCallbacksV2): ctx_elevated, router_id) fw_rules = [] + fwg_ids = [] + router_dict = {} # Add firewall rules per port attached to a firewall group for port in router_interfaces: fwg = self.get_port_fwg(ctx_elevated, port['id']) if fwg: - router_dict = {} - self.core_plugin._extend_nsx_router_dict( - router_dict, router_db) + if not router_dict: + self.core_plugin._extend_nsx_router_dict( + router_dict, router_db) if router_dict['distributed']: # The vnic_id is ignored for distributed routers, so # each rule will be applied to all the interfaces. vnic_id = None + # if rules for this fwg where already added skip it + if fwg['id'] in fwg_ids: + continue else: # get the interface vnic edge_vnic_bind = nsxv_db.get_edge_vnic_binding( @@ -102,6 +107,7 @@ class NsxvFwaasCallbacksV2(com_callbacks.NsxFwaasCallbacksV2): # Add the FWaaS rules for this port fw_rules.extend( self.get_port_translated_rules(vnic_id, fwg)) + fwg_ids.append(fwg['id']) return fw_rules @@ -118,12 +124,14 @@ class NsxvFwaasCallbacksV2(com_callbacks.NsxFwaasCallbacksV2): firewall_group['ingress_rule_list'], replace_dest=vnic_id, logged=logged, - is_ingress=True)) + is_ingress=True, + fwg_id=firewall_group['id'])) port_rules.extend(self.translate_rules( firewall_group['egress_rule_list'], replace_src=vnic_id, logged=logged, - is_ingress=False)) + is_ingress=False, + fwg_id=firewall_group['id'])) # Add ingress/egress block rules for this port default_ingress = {'name': "Block port ingress", @@ -140,7 +148,7 @@ class NsxvFwaasCallbacksV2(com_callbacks.NsxFwaasCallbacksV2): return port_rules def translate_rules(self, fwaas_rules, replace_dest=None, replace_src=None, - logged=False, is_ingress=True): + logged=False, is_ingress=True, fwg_id=None): translated_rules = [] for rule in fwaas_rules: if not rule['enabled']: @@ -157,10 +165,12 @@ class NsxvFwaasCallbacksV2(com_callbacks.NsxFwaasCallbacksV2): # update rules ID to prevent DB duplications in # NsxvEdgeFirewallRuleBinding if is_ingress: - rule['id'] = ('ingress-%s-%s' % (replace_dest, + rule['id'] = ('ingress-%s-%s' % (replace_dest or + fwg_id[:15], rule['id']))[:36] else: - rule['id'] = ('egress-%s-%s' % (replace_src, + rule['id'] = ('egress-%s-%s' % (replace_src or + fwg_id[:15], rule['id']))[:36] # source & destination should be lists if (rule.get('destination_ip_address') and diff --git a/vmware_nsx/tests/unit/nsx_v/test_fwaas_v2_driver.py b/vmware_nsx/tests/unit/nsx_v/test_fwaas_v2_driver.py index 70597ffc7a..8407dba1dc 100644 --- a/vmware_nsx/tests/unit/nsx_v/test_fwaas_v2_driver.py +++ b/vmware_nsx/tests/unit/nsx_v/test_fwaas_v2_driver.py @@ -121,7 +121,7 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase): def _fake_translated_rules(self, rules_list, nsx_port_id, is_ingress=True, - logged=False): + logged=False, fwg_id=None): translated_rules = copy.copy(rules_list) for rule in translated_rules: if logged: @@ -152,10 +152,10 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase): (rule.get('name') or rule['id']))[:30] if rule.get('id'): if is_ingress: - rule['id'] = ('ingress-%s-%s' % (nsx_port_id, + rule['id'] = ('ingress-%s-%s' % (nsx_port_id or fwg_id, rule['id']))[:36] else: - rule['id'] = ('egress-%s-%s' % (nsx_port_id, + rule['id'] = ('egress-%s-%s' % (nsx_port_id or fwg_id, rule['id']))[:36] return translated_rules @@ -356,7 +356,7 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase): return_value=self.distributed_router): func('nsx', apply_list, firewall) expected_rules = self._fake_translated_rules( - rule_list, None, is_ingress=is_ingress) + [ + rule_list, None, is_ingress=is_ingress, fwg_id=FAKE_FW_ID) + [ {'name': "Block port ingress", 'action': edge_firewall_driver.FWAAS_DENY, 'logged': False},