From aa149a7cf4e9a4e8c2cef9e89e0b77f9f6181f08 Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Sun, 16 Jul 2017 10:33:06 +0300 Subject: [PATCH] NSX|v: refactor shared router FW rules creation reduce code duplication by using the general nsx-v plugin code for FW rules. Change-Id: Ic7e93b3f0d7f27c36ab53e8f69aed996843199eb --- .../nsx_v/drivers/shared_router_driver.py | 49 ++++++--------- vmware_nsx/plugins/nsx_v/plugin.py | 59 +++++++++++-------- 2 files changed, 53 insertions(+), 55 deletions(-) diff --git a/vmware_nsx/plugins/nsx_v/drivers/shared_router_driver.py b/vmware_nsx/plugins/nsx_v/drivers/shared_router_driver.py index e0df7a3215..1be4278fd6 100644 --- a/vmware_nsx/plugins/nsx_v/drivers/shared_router_driver.py +++ b/vmware_nsx/plugins/nsx_v/drivers/shared_router_driver.py @@ -284,56 +284,41 @@ class RouterSharedDriver(router_driver.RouterBaseDriver): target_router_id, router_ids, allow_external=True): - fake_fw_rules = [] + fw_rules = [] for router_id in router_ids: + # Add FW rules per single router router_qry = context.session.query(l3_db_models.Router) router = router_qry.filter_by(id=router_id).one() - subnet_cidrs = self.plugin._find_router_subnets_cidrs( - context, router['id']) - routes = self.plugin._get_extra_routes_by_router_id( - context, router['id']) - subnet_cidrs.extend([route['destination'] for route in routes]) - if subnet_cidrs: - # Add fw rule to open subnets firewall flows and static routes - # relative flows - fake_subnet_fw_rule = { - 'name': 'Subnet Rule', - 'action': 'allow', - 'enabled': True, - 'source_ip_address': subnet_cidrs, - 'destination_ip_address': subnet_cidrs} - fake_fw_rules.append(fake_subnet_fw_rule) - _, dnat_rules = self.plugin._get_nat_rules(context, router) - dnat_cidrs = [rule['dst'] for rule in dnat_rules] - if dnat_cidrs: - # Fake fw rule to open dnat firewall flows - fake_dnat_fw_rule = { - 'name': 'DNAT Rule', - 'action': 'allow', - 'enabled': True, - 'destination_ip_address': dnat_cidrs} - fake_fw_rules.append(fake_dnat_fw_rule) + + # subnet rules to allow east-west traffic + subnet_rules = self.plugin._get_subnet_fw_rules(context, router) + if subnet_rules: + fw_rules.extend(subnet_rules) + + # DNAT rules + dnat_rule = self.plugin._get_dnat_fw_rule(context, router) + if dnat_rule: + fw_rules.append(dnat_rule) # Add rule for not NAT-ed allocation pools alloc_pool_rule = self.plugin._get_allocation_pools_fw_rule( context, router) if alloc_pool_rule: - fake_fw_rules.append(alloc_pool_rule) + fw_rules.append(alloc_pool_rule) # Add no-snat rules nosnat_fw_rules = self.plugin._get_nosnat_subnets_fw_rules( context, router) - fake_fw_rules.extend(nosnat_fw_rules) + fw_rules.extend(nosnat_fw_rules) # If metadata service is enabled, block access to inter-edge network if self.plugin.metadata_proxy_handler: - fake_fw_rules += ( - nsx_v_md_proxy.get_router_fw_rules()) + fw_rules += nsx_v_md_proxy.get_router_fw_rules() # TODO(asarfaty): Add fwaas rules when fwaas supports shared routers - fake_fw = {'firewall_rule_list': fake_fw_rules} + fw = {'firewall_rule_list': fw_rules} edge_utils.update_firewall(self.nsx_v, context, target_router_id, - fake_fw, allow_external=allow_external) + fw, allow_external=allow_external) def update_routes(self, context, router_id, nexthop): edge_id = edge_utils.get_router_edge_id(context, router_id) diff --git a/vmware_nsx/plugins/nsx_v/plugin.py b/vmware_nsx/plugins/nsx_v/plugin.py index 04ba1068f4..05f5ac9344 100644 --- a/vmware_nsx/plugins/nsx_v/plugin.py +++ b/vmware_nsx/plugins/nsx_v/plugin.py @@ -3398,6 +3398,35 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin, 'source_vnic_groups': ["external"], 'destination_ip_address': no_nat_cidrs} + def _get_dnat_fw_rule(self, context, router): + # Get FW rule to open dnat firewall flows + _, dnat_rules = self._get_nat_rules(context, router) + dnat_cidrs = [rule['dst'] for rule in dnat_rules] + if dnat_cidrs: + return { + 'name': DNAT_RULE_NAME, + 'action': 'allow', + 'enabled': True, + 'destination_ip_address': dnat_cidrs} + + def _get_subnet_fw_rules(self, context, router): + # Get FW rule/s to open subnets firewall flows and static routes + # relative flows + fw_rules = [] + subnet_cidrs = self._find_router_subnets_cidrs(context, router['id']) + routes = self._get_extra_routes_by_router_id(context, router['id']) + subnet_cidrs.extend([route['destination'] for route in routes]) + #TODO(asarfaty): need a separate rule per address scope + if subnet_cidrs: + subnet_fw_rule = { + 'name': SUBNET_RULE_NAME, + 'action': 'allow', + 'enabled': True, + 'source_ip_address': subnet_cidrs, + 'destination_ip_address': subnet_cidrs} + fw_rules.append(subnet_fw_rule) + return fw_rules + def _update_nat_rules(self, context, router, router_id=None): snat, dnat = self._get_nat_rules(context, router) if not router_id: @@ -3608,22 +3637,13 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin, """ fw_rules = [] router_with_firewall = True if fwaas_rules is not None else False - neutron_id = router_db['id'] edge_id = self._get_edge_id_by_rtr_id(context, router_id) - # Add FW rule to open subnets firewall flows and static routes + # Add FW rule/s to open subnets firewall flows and static routes # relative flows - subnet_cidrs = self._find_router_subnets_cidrs(context, neutron_id) - routes = self._get_extra_routes_by_router_id(context, neutron_id) - subnet_cidrs.extend([route['destination'] for route in routes]) - if subnet_cidrs: - subnet_fw_rule = { - 'name': SUBNET_RULE_NAME, - 'action': 'allow', - 'enabled': True, - 'source_ip_address': subnet_cidrs, - 'destination_ip_address': subnet_cidrs} - fw_rules.append(subnet_fw_rule) + subnet_rules = self._get_subnet_fw_rules(context, router_db) + if subnet_rules: + fw_rules.extend(subnet_rules) # If metadata service is enabled, block access to inter-edge network if self.metadata_proxy_handler: @@ -3634,16 +3654,9 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin, fw_rules += fwaas_rules if not router_with_firewall: - # Add FW rule to open dnat firewall flows - _, dnat_rules = self._get_nat_rules(context, router_db) - dnat_cidrs = [rule['dst'] for rule in dnat_rules] - if dnat_cidrs: - dnat_fw_rule = { - 'name': DNAT_RULE_NAME, - 'action': 'allow', - 'enabled': True, - 'destination_ip_address': dnat_cidrs} - fw_rules.append(dnat_fw_rule) + dnat_rule = self._get_dnat_fw_rule(context, router_db) + if dnat_rule: + fw_rules.append(dnat_rule) # Add rule for not NAT-ed allocation pools alloc_pool_rule = self._get_allocation_pools_fw_rule(