From 09bbf4d583fc088939ce6e5de0ea75afc6af1423 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 cnix is not part of hte rule. Instead the driver will add each rule only once per firewall-group, and add the firewall group id instead of hte 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 7e0bed377b..4fce2919e7 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},