diff --git a/vmware_nsx/plugins/nsx_v/plugin.py b/vmware_nsx/plugins/nsx_v/plugin.py index 8392d71029..b2ebb5a25e 100644 --- a/vmware_nsx/plugins/nsx_v/plugin.py +++ b/vmware_nsx/plugins/nsx_v/plugin.py @@ -4277,7 +4277,7 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin, self.fwaas_callbacks.should_apply_firewall_to_router( context, router_db, router_id)): fwaas_rules = self.fwaas_callbacks.get_fwaas_rules_for_router( - context, router_db['id'], edge_id) + context, router_db['id'], router_db, edge_id) if fwaas_rules: fw_rules += fwaas_rules 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 b071524801..ef007f3106 100644 --- a/vmware_nsx/services/fwaas/nsx_v/fwaas_callbacks_v2.py +++ b/vmware_nsx/services/fwaas/nsx_v/fwaas_callbacks_v2.py @@ -75,7 +75,8 @@ class NsxvFwaasCallbacksV2(com_callbacks.NsxFwaasCallbacksV2): return True - def get_fwaas_rules_for_router(self, context, router_id, edge_id): + def get_fwaas_rules_for_router(self, context, router_id, router_db, + edge_id): """Return the list of (translated) FWaaS rules for this router.""" ctx_elevated = context.elevated() router_interfaces = self.core_plugin._get_router_interfaces( @@ -86,10 +87,18 @@ class NsxvFwaasCallbacksV2(com_callbacks.NsxFwaasCallbacksV2): for port in router_interfaces: fwg = self.get_port_fwg(ctx_elevated, port['id']) if fwg: - # get the interface vnic - edge_vnic_bind = nsxv_db.get_edge_vnic_binding( - context.session, edge_id, port['network_id']) - vnic_id = 'vnic-index-%s' % edge_vnic_bind.vnic_index + 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 + else: + # get the interface vnic + edge_vnic_bind = nsxv_db.get_edge_vnic_binding( + context.session, edge_id, port['network_id']) + vnic_id = 'vnic-index-%s' % edge_vnic_bind.vnic_index # Add the FWaaS rules for this port fw_rules.extend( self.get_port_translated_rules(vnic_id, fwg)) @@ -117,15 +126,16 @@ class NsxvFwaasCallbacksV2(com_callbacks.NsxFwaasCallbacksV2): is_ingress=False)) # Add ingress/egress block rules for this port - port_rules.extend([ - {'name': "Block port ingress", + default_ingress = {'name': "Block port ingress", 'action': edge_firewall_driver.FWAAS_DENY, - 'destination_vnic_groups': [vnic_id], - 'logged': logged}, - {'name': "Block port egress", + 'logged': logged} + default_egress = {'name': "Block port egress", 'action': edge_firewall_driver.FWAAS_DENY, - 'source_vnic_groups': [vnic_id], - 'logged': logged}]) + 'logged': logged} + if vnic_id: + default_ingress['destination_vnic_groups'] = [vnic_id] + default_egress['source_vnic_groups'] = [vnic_id] + port_rules.extend([default_ingress, default_egress]) return port_rules 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 f469f182f0..75bda89651 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 @@ -49,7 +49,13 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase): # Start some mocks self.router = {'id': FAKE_ROUTER_ID, - 'external_gateway_info': {'network_id': 'external'}} + 'external_gateway_info': {'network_id': 'external'}, + 'nsx_attributes': {'distributed': False, + 'router_type': 'exclusive'}} + self.distributed_router = {'id': FAKE_ROUTER_ID, + 'external_gateway_info': {'network_id': 'external'}, + 'nsx_attributes': {'distributed': True, + 'router_type': 'exclusive'}} mock.patch.object(self.plugin, '_get_router', return_value=self.router).start() mock.patch.object(self.plugin, 'get_router', @@ -121,11 +127,13 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase): if is_ingress: if (not rule.get('destination_ip_address') or rule['destination_ip_address'].startswith('0.0.0.0')): - rule['destination_vnic_groups'] = ['vnic-index-1'] + if nsx_port_id: + rule['destination_vnic_groups'] = [nsx_port_id] else: if (not rule.get('source_ip_address') or rule['source_ip_address'].startswith('0.0.0.0')): - rule['source_vnic_groups'] = ['vnic-index-1'] + if nsx_port_id: + rule['source_vnic_groups'] = [nsx_port_id] if rule.get('destination_ip_address'): if rule['destination_ip_address'].startswith('0.0.0.0'): del rule['destination_ip_address'] @@ -177,14 +185,20 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase): return self._fake_firewall_group( rule_list, is_ingress=is_ingress, admin_state_up=False) - def _fake_apply_list(self): - router_inst = self.router + def _fake_apply_list_template(self, router): + router_inst = router router_info_inst = mock.Mock() router_info_inst.router = router_inst router_info_inst.router_id = FAKE_ROUTER_ID apply_list = [(router_info_inst, FAKE_PORT_ID)] return apply_list + def _fake_apply_list(self): + return self._fake_apply_list_template(self.router) + + def _fake_distributed_apply_list(self): + return self._fake_apply_list_template(self.distributed_router) + def test_create_firewall_no_rules(self): apply_list = self._fake_apply_list() firewall = self._fake_empty_firewall_group() @@ -311,3 +325,74 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase): update_fw.assert_called_once_with( self.plugin.nsx_v, mock.ANY, FAKE_ROUTER_ID, {'firewall_rule_list': []}) + + def _setup_dist_router_firewall_with_rules(self, func, is_ingress=True, + is_conflict=False, + cidr='10.24.4.0/24'): + apply_list = self._fake_distributed_apply_list() + rule_list = self._fake_rules_v4(is_ingress=is_ingress, + is_conflict=is_conflict, cidr=cidr) + firewall = self._fake_firewall_group(rule_list, is_ingress=is_ingress) + with mock.patch.object(self.plugin.fwaas_callbacks, 'get_port_fwg', + return_value=firewall),\ + mock.patch.object(self.plugin.fwaas_callbacks, + '_get_port_firewall_group_id', + return_value=FAKE_FW_ID),\ + mock.patch.object(self.plugin.fwaas_callbacks, + '_get_fw_group_from_plugin', + return_value=firewall),\ + mock.patch.object(edge_utils, "update_firewall") as update_fw,\ + mock.patch.object(edge_utils, 'get_router_edge_id', + return_value='edge-1'),\ + mock.patch.object(self.plugin.edge_manager, 'get_plr_by_tlr_id', + return_value=FAKE_ROUTER_ID),\ + mock.patch.object(self.plugin, '_get_router', + return_value=self.distributed_router),\ + mock.patch.object(self.plugin, 'get_router', + return_value=self.distributed_router): + func('nsx', apply_list, firewall) + expected_rules = self._fake_translated_rules( + rule_list, None, is_ingress=is_ingress) + [ + {'name': "Block port ingress", + 'action': edge_firewall_driver.FWAAS_DENY, + 'logged': False}, + {'name': "Block port egress", + 'action': edge_firewall_driver.FWAAS_DENY, + 'logged': False}] + + update_fw.assert_called_once_with( + self.plugin.nsx_v, mock.ANY, FAKE_ROUTER_ID, + {'firewall_rule_list': expected_rules}) + + def test_create_dist_router_firewall_with_ingress_rules(self): + self._setup_dist_router_firewall_with_rules( + self.firewall.create_firewall_group) + + def test_update_dist_router_firewall_with_ingress_rules(self): + self._setup_dist_router_firewall_with_rules( + self.firewall.update_firewall_group) + + def test_create_dist_router_firewall_with_egress_rules(self): + self._setup_dist_router_firewall_with_rules( + self.firewall.create_firewall_group, + is_ingress=False) + + def test_create_dist_router_firewall_with_illegal_cidr(self): + self._setup_dist_router_firewall_with_rules( + self.firewall.create_firewall_group, + cidr='0.0.0.0/24') + + def test_update_dist_router_firewall_with_egress_rules(self): + self._setup_dist_router_firewall_with_rules( + self.firewall.update_firewall_group, + is_ingress=False) + + def test_update_dist_router_firewall_with_egress_conflicting_rules(self): + self._setup_dist_router_firewall_with_rules( + self.firewall.update_firewall_group, + is_ingress=False, is_conflict=True) + + def test_update_dist_router_firewall_with_ingress_conflicting_rules(self): + self._setup_dist_router_firewall_with_rules( + self.firewall.update_firewall_group, + is_ingress=True, is_conflict=True)