diff --git a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/apic_mapping.py b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/apic_mapping.py index 04584580c..da79f9e14 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/apic_mapping.py +++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/apic_mapping.py @@ -51,7 +51,6 @@ from oslo_serialization import jsonutils import sqlalchemy as sa from gbpservice.neutron.db.grouppolicy import group_policy_mapping_db as gpdb -from gbpservice.neutron.extensions import driver_proxy_group as proxy_group from gbpservice.neutron.extensions import group_policy as gpolicy from gbpservice.neutron.services.grouppolicy.common import constants as g_const from gbpservice.neutron.services.grouppolicy.common import exceptions as gpexc @@ -199,6 +198,10 @@ class ExplicitPortOverlap(gpexc.GroupPolicyBadRequest): 'in network %(net)s') +class AdminOnlyOperation(gpexc.GroupPolicyBadRequest): + message = _("This operation is reserved to admins") + + class TenantSpecificNatEpg(model_base.BASEV2): """Tenants that use a specific NAT EPG for an external segment.""" __tablename__ = 'gp_apic_tenant_specific_nat_epg' @@ -208,6 +211,7 @@ class TenantSpecificNatEpg(model_base.BASEV2): primary_key=True) tenant_id = sa.Column(sa.String(36), primary_key=True) + REVERSE_PREFIX = 'reverse-' SHADOW_PREFIX = 'Shd-' AUTO_PREFIX = 'Auto-' @@ -224,6 +228,7 @@ REVERTIBLE_PROTOCOLS = [n_constants.PROTO_NAME_TCP.lower(), n_constants.PROTO_NAME_UDP.lower(), n_constants.PROTO_NAME_ICMP.lower()] PROXY_PORT_PREFIX = "opflex_proxy:" +EOC_PREFIX = "opflex_eoc:" ICMP_REPLY_TYPES = ['echo-rep', 'dst-unreach', 'src-quench', 'time-exceeded'] @@ -430,22 +435,36 @@ class ApicMappingDriver(api.ResourceMappingDriver, owned=own_addr, inject_default_route= l2p['inject_default_route']) self._add_vrf_details(context, details) - if self._is_pt_chain_head(context, pt, ptg, owned_ips=own_addr): + if self._is_pt_chain_head(context, pt, ptg, owned_ips=own_addr, + port_id=port_id): # is a relevant proxy_gateway, push all the addresses from this # chain to this PT extra_map = details - master_mac = self._is_master_owner(context, pt, - owned_ips=own_addr) - if master_mac: + master_port = self._is_master_owner(context, pt, + owned_ips=own_addr) + if master_port: extra_map = details['extra_details'].setdefault( - master_mac, {'extra_ips': [], 'floating_ip': [], - 'ip_mapping': [], 'host_snat_ips': []}) - if bool(master_mac) == bool(pt['cluster_id']): + master_port['mac_address'], + {'extra_ips': [], 'floating_ip': [], + 'ip_mapping': [], 'host_snat_ips': []}) + if bool(master_port) == bool(pt['cluster_id']): l3_policy = context._plugin.get_l3_policy( context, l2p['l3_policy_id']) - while ptg['proxied_group_id']: + proxied_ptgs = [] + while ptg.get('proxied_group_id'): proxied = self.gbp_plugin.get_policy_target_group( context, ptg['proxied_group_id']) + proxied_ptgs.append(proxied) + ptg = proxied + # Retrieve PTGs explicitly proxied + descriptions = [EOC_PREFIX + port_id] + if master_port: + # Also retrieve groups proxied by master of the cluster + descriptions.append(EOC_PREFIX + master_port['id']) + proxied_ptgs.extend( + self._get_policy_target_groups( + context, filters={'description': descriptions})) + for proxied in proxied_ptgs: for port in self._get_ptg_ports(proxied): extra_map['extra_ips'].extend( [x['ip_address'] for x in port['fixed_ips'] + @@ -460,7 +479,6 @@ class ApicMappingDriver(api.ResourceMappingDriver, if not extra_map['host_snat_ips']: extra_map['host_snat_ips'].extend( host_snat_ips) - ptg = proxied else: LOG.info(_LI("Active master has changed for PT %s"), pt['id']) @@ -858,6 +876,10 @@ class ApicMappingDriver(api.ResourceMappingDriver, context._plugin_context, context.current['id']) db_group.l2_policy_id = proxied['l2_policy_id'] context.current['l2_policy_id'] = proxied['l2_policy_id'] + if context.current['description']: + if (EOC_PREFIX in context.current['description'] + and not context._plugin_context.is_admin): + raise AdminOnlyOperation() else: self.name_mapper.has_valid_name(context.current) @@ -1192,6 +1214,11 @@ class ApicMappingDriver(api.ResourceMappingDriver, context.current['subnets']): raise ExplicitSubnetAssociationNotSupported() self._reject_shared_update(context, 'policy_target_group') + if (context.current['description'] != + context.original['description']): + if (EOC_PREFIX in context.current['description'] + and not context._plugin_context.is_admin): + raise AdminOnlyOperation() def update_policy_target_group_postcommit(self, context): if not self.name_mapper._is_apic_reference(context.current): @@ -2420,13 +2447,15 @@ class ApicMappingDriver(api.ResourceMappingDriver, object): return apic_manager.TENANT_COMMON else: - if object.get('proxied_group_id'): # Then it's a proxy PTG - # Even though they may belong to a different tenant, + if object.get('proxied_group_id') or object.get( + 'enforce_service_chains') is False: + # Then it's a proxy + # PTG Even though they may belong to a different tenant, # the proxy PTGs will be created on the L2P's tenant to # make APIC happy l2p = self.gbp_plugin.get_l2_policy( nctx.get_admin_context(), object['l2_policy_id']) - return self.name_mapper.tenant(l2p) + return self._tenant_by_sharing_policy(l2p) else: return self.name_mapper.tenant(object) @@ -2545,12 +2574,11 @@ class ApicMappingDriver(api.ResourceMappingDriver, bd_name = self.name_mapper.policy_target_group( context, proxied, prefix=SHADOW_PREFIX) ptg_name = self.name_mapper.policy_target_group(context, proxied) - is_l2 = context.current['proxy_type'] == proxy_group.PROXY_TYPE_L2 with self.apic_manager.apic.transaction(None) as trs: # Create shadow BD to host the proxied EPG self.apic_manager.ensure_bd_created_on_apic( tenant, bd_name, ctx_owner=ctx_owner, ctx_name=l3_policy_name, - allow_broadcast=is_l2, unicast_route=False, transaction=trs, + allow_broadcast=True, unicast_route=False, transaction=trs, enforce_subnet_check=False) # Move current PTG to different BD self.apic_manager.ensure_epg_created( @@ -3175,6 +3203,17 @@ class ApicMappingDriver(api.ResourceMappingDriver, def _notify_head_chain_ports(self, ptg_id): context = nctx.get_admin_context() ptg = self.gbp_plugin.get_policy_target_group(context, ptg_id) + explicit_eoc_id = self._extract_ptg_explicit_eoc(context, ptg) + if explicit_eoc_id: + # Could be a cluster master + updates = [explicit_eoc_id] + eoc_pt = self._port_id_to_pt(context, explicit_eoc_id) + if eoc_pt: + updates.extend( + x['port_id'] for x in self.gbp_plugin.get_policy_targets( + context, {'cluster_id': [eoc_pt['id']]})) + for update in updates: + self._notify_port_update(context, update) # to avoid useless double notification exit now if no proxy if not ptg.get('proxy_group_id'): return @@ -3281,33 +3320,48 @@ class ApicMappingDriver(api.ResourceMappingDriver, return opt.lower() in ['true', 'yes', '1'] return False - def _is_pt_chain_head(self, plugin_context, pt, ptg=None, owned_ips=None): - if pt: - ptg = ptg or self._get_policy_target_group( - plugin_context, pt['policy_target_group_id']) - # Check whenther PTG is the end of a chain - chain_end = bool(ptg.get('proxied_group_id') and - not ptg.get('proxy_group')) - if chain_end: - cluster_id = pt['cluster_id'] - if cluster_id: - # The master PT must be a proxy gateway for this to be - # eligible as the chain head - master_pt = self._get_pt_cluster_master(plugin_context, pt) + def _is_pt_chain_head(self, plugin_context, pt, ptg=None, owned_ips=None, + port_id=None): + if not pt: + return False - # Verify whether this is the active PT - return bool(master_pt['proxy_gateway'] and - self._is_master_owner(plugin_context, pt, - master_pt=master_pt, - owned_ips=owned_ips)) - # regular PT not part of a cluster, return if proxy gateway - return bool(pt['proxy_gateway']) + master_pt = None + if port_id or pt.get('cluster_id'): + descr = [] + if port_id: + descr.append(EOC_PREFIX + port_id) + if pt.get('cluster_id'): + master_pt = self._get_pt_cluster_master(plugin_context, pt) + descr.append(EOC_PREFIX + master_pt['port_id']) + if bool(self._get_policy_target_groups( + plugin_context, filters={'description': descr})): + return True + + ptg = ptg or self._get_policy_target_group( + plugin_context, pt['policy_target_group_id']) + # Check whether PTG is the end of a chain + chain_end = bool(ptg.get('proxied_group_id') and + not ptg.get('proxy_group')) + if chain_end: + cluster_id = pt['cluster_id'] + if cluster_id: + # The master PT must be a proxy gateway for this to be + # eligible as the chain head + master_pt = master_pt or self._get_pt_cluster_master( + plugin_context, pt) + # Verify whether this is the active PT + return bool(master_pt['proxy_gateway'] and + self._is_master_owner(plugin_context, pt, + master_pt=master_pt, + owned_ips=owned_ips)) + # regular PT not part of a cluster, return if proxy gateway + return bool(pt['proxy_gateway']) def _is_master_owner(self, plugin_context, pt, master_pt=None, owned_ips=None): """Verifies if the port owns the master address. - Returns the master MAC address or False + Returns the master port address or False """ if pt['cluster_id']: master_pt = master_pt or self._get_pt_cluster_master( @@ -3319,10 +3373,9 @@ class ApicMappingDriver(api.ResourceMappingDriver, master_port = self._get_port(plugin_context, master_pt['port_id']) master_addresses = set([x['ip_address'] for x in master_port['fixed_ips']]) - master_mac = master_port['mac_address'] if bool(owned_addresses & master_addresses): - return master_mac - return False + return master_port + return None def _get_owned_addresses(self, plugin_context, port_id): return set(self.ha_ip_handler.get_ha_ipaddresses_for_port(port_id)) @@ -3807,3 +3860,13 @@ class ApicMappingDriver(api.ResourceMappingDriver, 'for external segment %(es)s', {'tenant': nat_epg_tenant, 'epg': nat_epg_name, 'es': es['id']}) + + def _extract_ptg_explicit_eoc(self, plugin_context, ptg): + """Extract PTG End of Chain + + Given a PTG, retrieves the explicit End of the Chain from its + description and returns a Neutron port + :return: Neutron port or None + """ + if EOC_PREFIX in ptg.get('description', ''): + return ptg['description'][len(EOC_PREFIX):] diff --git a/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py b/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py index 6f9cf2d8c..743d0dbe3 100644 --- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py +++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py @@ -244,6 +244,13 @@ class ApicMappingTestCase( sys.modules["apicapi"] = self.saved_apicapi super(ApicMappingTestCase, self).tearDown() + def _get_pts_addresses(self, pts): + addresses = [] + for pt in pts: + port = self._get_object('ports', pt['port_id'], self.api)['port'] + addresses.extend([x['ip_address'] for x in port['fixed_ips']]) + return addresses + def _build_external_dict(self, name, cidr_exposed, is_edge_nat=False): ext_info = { 'enable_nat': 'True' if self.nat_enabled else 'False' @@ -553,6 +560,24 @@ class TestPolicyTarget(ApicMappingTestCase): details['host_snat_ip']) self.assertEqual(24, details['prefixlen']) + def test_get_gbp_details_extra_ips_explicit_eoc(self): + ptg = self.create_policy_target_group( + name="ptg1")['policy_target_group'] + pt1 = self.create_policy_target( + policy_target_group_id=ptg['id'])['policy_target'] + ptg2 = self.create_policy_target_group( + name="ptg2", + description='opflex_eoc:' + pt1['port_id'], + is_admin_context=True)['policy_target_group'] + pt2 = self.create_policy_target( + policy_target_group_id=ptg2['id'])['policy_target'] + mapping = self.driver.get_gbp_details(context.get_admin_context(), + device='tap%s' % pt1['port_id'], host='h2') + + # Extra ip set for pt2 + ips = self._get_pts_addresses([pt2]) + self.assertEqual(set(ips), set(mapping['extra_ips'])) + def test_snat_pool_subnet_deletion(self): self._mock_external_dict([('supported', '192.168.0.2/24')]) self.driver.apic_manager.ext_net_dict[ @@ -1100,6 +1125,55 @@ class TestPolicyTarget(ApicMappingTestCase): self.assertEqual('1.1.1.1', entries[0].ha_ip_address) self.assertEqual('1.1.1.1', entries[1].ha_ip_address) + def test_explicit_end_of_chain(self): + self.driver._notify_port_update = mock.Mock() + ptg = self.create_policy_target_group( + name="ptg1")['policy_target_group'] + pt1 = self.create_policy_target( + policy_target_group_id=ptg['id'])['policy_target'] + ptg2 = self.create_policy_target_group( + name="ptg2", + description='opflex_eoc:' + pt1['port_id'], + is_admin_context=True)['policy_target_group'] + self.create_policy_target(policy_target_group_id=ptg2['id']) + # pt1 notified + self.driver._notify_port_update.assert_called_once_with( + mock.ANY, pt1['port_id']) + + def test_explicit_end_of_chain_cluster(self): + self.driver._notify_port_update = mock.Mock() + ptg = self.create_policy_target_group( + name="ptg1")['policy_target_group'] + pt1 = self.create_policy_target( + policy_target_group_id=ptg['id'])['policy_target'] + # Same cluster + pt2 = self.create_policy_target( + policy_target_group_id=ptg['id'], + cluster_id=pt1['id'])['policy_target'] + pt3 = self.create_policy_target( + policy_target_group_id=ptg['id'], + cluster_id=pt1['id'])['policy_target'] + ptg2 = self.create_policy_target_group( + name="ptg2", + description='opflex_eoc:' + pt1['port_id'], + is_admin_context=True)['policy_target_group'] + self.create_policy_target(policy_target_group_id=ptg2['id']) + # pt1, 2 and 3 notified + expected_calls = [ + mock.call(mock.ANY, pt1['port_id']), + mock.call(mock.ANY, pt2['port_id']), + mock.call(mock.ANY, pt3['port_id'])] + self._check_call_list( + expected_calls, self.driver._notify_port_update.call_args_list) + + def test_explicit_eoc_raises(self): + ptg = self.create_policy_target_group( + name="ptg1")['policy_target_group'] + self.create_policy_target_group( + name="ptg2", description='opflex_eoc:', expected_res_status=400) + self.update_policy_target_group( + ptg['id'], description='opflex_eoc:', expected_res_status=400) + class TestPolicyTargetVlanNetwork(ApicMappingVlanTestCase, TestPolicyTarget): diff --git a/gbpservice/neutron/tests/unit/services/servicechain/ncp/test_tscp_apic_mapping.py b/gbpservice/neutron/tests/unit/services/servicechain/ncp/test_tscp_apic_mapping.py index b828d9b18..e25f2a6cf 100644 --- a/gbpservice/neutron/tests/unit/services/servicechain/ncp/test_tscp_apic_mapping.py +++ b/gbpservice/neutron/tests/unit/services/servicechain/ncp/test_tscp_apic_mapping.py @@ -809,13 +809,6 @@ class TestApicChains(ApicMappingStitchingPlumberGBPTestCase, class TestProxyGroup(ApicMappingStitchingPlumberGBPTestCase): - def _get_pts_addresses(self, pts): - addresses = [] - for pt in pts: - port = self._get_object('ports', pt['port_id'], self.api)['port'] - addresses.extend([x['ip_address'] for x in port['fixed_ips']]) - return addresses - def _proxy_tenant(self, ptg, admin_proxy): return 'admin' if admin_proxy else ptg['tenant_id'] @@ -926,7 +919,7 @@ class TestProxyGroup(ApicMappingStitchingPlumberGBPTestCase): # Shadow BD created self.mgr.ensure_bd_created_on_apic.assert_called_once_with( ptg['tenant_id'], 'Shd-' + ptg['id'], ctx_owner=l3p['tenant_id'], - ctx_name=l3p['id'], allow_broadcast=False, unicast_route=False, + ctx_name=l3p['id'], allow_broadcast=True, unicast_route=False, transaction=mock.ANY, enforce_subnet_check=False) # Proxied PTG moved expected_calls = [ @@ -1217,7 +1210,7 @@ class TestProxyGroup(ApicMappingStitchingPlumberGBPTestCase): # Shadow BD created in non-admin tenant self.mgr.ensure_bd_created_on_apic.assert_called_once_with( 'non-admin', 'Shd-' + ptg['id'], ctx_owner=l3p['tenant_id'], - ctx_name=l3p['id'], allow_broadcast=False, unicast_route=False, + ctx_name=l3p['id'], allow_broadcast=True, unicast_route=False, transaction=mock.ANY, enforce_subnet_check=False) # Proxied PTG moved expected_calls = [