Merge "NSX|v: refactor shared router FW rules creation"

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'])
+            # subnet rules to allow east-west traffic
-            routes = self.plugin._get_extra_routes_by_router_id(
+            subnet_rules = self.plugin._get_subnet_fw_rules(context, router)
-                context, router['id'])
+            if subnet_rules:
-            subnet_cidrs.extend([route['destination'] for route in routes])
+                fw_rules.extend(subnet_rules)
-            if subnet_cidrs:
+
-                # Add fw rule to open subnets firewall flows and static routes
+            # DNAT rules
-                # relative flows
+            dnat_rule = self.plugin._get_dnat_fw_rule(context, router)
-                fake_subnet_fw_rule = {
+            if dnat_rule:
-                    'name': 'Subnet Rule',
+                fw_rules.append(dnat_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)
 
             # 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 += (
+            fw_rules += nsx_v_md_proxy.get_router_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)

vmware_nsx/plugins/nsx_v/plugin.py

@@ -3408,6 +3408,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:
@@ -3618,22 +3647,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)
+        subnet_rules = self._get_subnet_fw_rules(context, router_db)
-        routes = self._get_extra_routes_by_router_id(context, neutron_id)
+        if subnet_rules:
-        subnet_cidrs.extend([route['destination'] for route in routes])
+            fw_rules.extend(subnet_rules)
-        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)
 
         # If metadata service is enabled, block access to inter-edge network
         if self.metadata_proxy_handler:
@@ -3644,16 +3664,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_rule = self._get_dnat_fw_rule(context, router_db)
-            _, dnat_rules = self._get_nat_rules(context, router_db)
+            if dnat_rule:
-            dnat_cidrs = [rule['dst'] for rule in dnat_rules]
+                fw_rules.append(dnat_rule)
-            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)
 
             # Add rule for not NAT-ed allocation pools
             alloc_pool_rule = self._get_allocation_pools_fw_rule(