port extra attribute extension for gbp mapping

Just a few lines of code to propose a way for internal users to
specify extra port attributes when creating a PT.
There's no persistence or API changes involved, and the workflow
will look like the "implicit port" workflow. The advantage of
having this is that some tweaks can be made to the Neutron port
by still having GBP as the port owner.

Closes-Bug: 1527360
Change-Id: Iebcda544d0b5aee3e02e4828228d662997008251
(cherry picked from commit 0d5cedc413)

gbpservice/neutron/db/grouppolicy/group_policy_mapping_db.py

@@ -20,6 +20,7 @@ from sqlalchemy import orm
 from gbpservice.neutron.db import gbp_quota_db as gquota
 from gbpservice.neutron.db.grouppolicy import group_policy_db as gpdb
 from gbpservice.neutron.extensions import group_policy as gpolicy
+from gbpservice.neutron.services.grouppolicy.common import exceptions
 
 
 LOG = logging.getLogger(__name__)
@@ -110,10 +111,11 @@ class GroupPolicyMappingDbPlugin(gpdb.GroupPolicyDbPlugin):
     """Group Policy Mapping interface implementation using SQLAlchemy models.
     """
 
-    def _make_policy_target_dict(self, pt, fields=None):
+    def _make_policy_target_dict(self, pt, fields=None, **kwargs):
         res = super(GroupPolicyMappingDbPlugin,
                     self)._make_policy_target_dict(pt)
         res['port_id'] = pt.port_id
+        res.update(kwargs)
         return self._fields(res, fields)
 
     def _make_policy_target_group_dict(self, ptg, fields=None):
@@ -226,11 +228,35 @@ class GroupPolicyMappingDbPlugin(gpdb.GroupPolicyDbPlugin):
             context.session.query(PTGToSubnetAssociation).filter_by(
                 subnet_id=subnet_id)]
 
+    def _validate_pt_port_exta_attributes(self, context, pt):
+        attributes = pt.get('port_attributes')
+        if attributes:
+            # Check network ID not overridden
+            if 'network_id' in attributes:
+                raise exceptions.InvalidPortExtraAttributes(
+                    attribute='network_id', reason='read only attribute')
+            if 'fixed_ips' in attributes:
+                ptg = self.get_policy_target_group(
+                    context, pt['policy_target_group_id'])
+                subnets = ptg['subnets']
+                for fixed_ip in attributes.get('fixed_ips'):
+                    if fixed_ip['subnet_id'] not in subnets:
+                        raise exceptions.InvalidPortExtraAttributes(
+                            attribute='fixed_ips:subnet_id',
+                            reason='subnet not in PTG')
+            if 'allowed_address_pairs' in attributes:
+                # REVISIT(ivar); Could be allowed with certain restrictions,
+                # but we don't have a use case for it right now
+                raise exceptions.InvalidPortExtraAttributes(
+                    attribute='allowed_address_pairs',
+                    reason='read only attribute')
+
     @log.log
     def create_policy_target(self, context, policy_target):
         pt = policy_target['policy_target']
         tenant_id = self._get_tenant_id_for_create(context, pt)
         with context.session.begin(subtransactions=True):
+            self._validate_pt_port_exta_attributes(context, pt)
             pt_db = PolicyTargetMapping(id=uuidutils.generate_uuid(),
                                         tenant_id=tenant_id,
                                         name=pt['name'],
@@ -240,7 +266,8 @@ class GroupPolicyMappingDbPlugin(gpdb.GroupPolicyDbPlugin):
                                         port_id=pt['port_id'],
                                         cluster_id=pt['cluster_id'])
             context.session.add(pt_db)
-        return self._make_policy_target_dict(pt_db)
+        return self._make_policy_target_dict(
+            pt_db, port_attributes=pt.get('port_attributes', {}))
 
     @log.log
     def get_policy_targets_count(self, context, filters=None):

gbpservice/neutron/services/grouppolicy/common/exceptions.py

@@ -94,6 +94,11 @@ class InvalidPortForPTG(GroupPolicyBadRequest):
                 "%(policy_target_group_id)s.")
 
 
+class InvalidPortExtraAttributes(GroupPolicyBadRequest):
+    message = _("Port extra attribute %(attribute)s is invalid for the "
+                "following reason: %(reason)s")
+
+
 class InvalidSubnetForPTG(GroupPolicyBadRequest):
     message = _("Subnet %(subnet_id)s does not belong to network "
                 "%(network_id)s associated with L2P %(l2p_id)s for PTG "

gbpservice/neutron/services/grouppolicy/drivers/resource_mapping.py

@@ -1326,6 +1326,7 @@ class ResourceMappingDriver(api.PolicyDriver, local_api.LocalAPI,
                          'admin_state_up': True}
                 if context.current.get('group_default_gateway'):
                     attrs['fixed_ips'][0]['ip_address'] = subnet['gateway_ip']
+                attrs.update(context.current.get('port_attributes', {}))
                 port = self._create_port(context._plugin_context, attrs)
                 port_id = port['id']
                 self._mark_port_owned(context._plugin_context.session, port_id)
@@ -2449,10 +2450,10 @@ class ResourceMappingDriver(api.PolicyDriver, local_api.LocalAPI,
             if subnet == port_subnet_id:
                 break
         else:
-            raise exc.InvalidPortForPTG(port_id=port_id,
-                                ptg_subnet_id=",".join(ptg.get('subnets')),
-                                port_subnet_id=port_subnet_id,
-                                policy_target_group_id=ptg_id)
+            raise exc.InvalidPortForPTG(
+                port_id=port_id, ptg_subnet_id=",".join(ptg.get('subnets')),
+                port_subnet_id=port_subnet_id,
+                policy_target_group_id=ptg_id)
 
     def _get_ptg_l3p(self, context, ptg):
         l3p_id = context._plugin.get_l2_policy(

gbpservice/neutron/services/grouppolicy/plugin.py

@@ -352,6 +352,8 @@ class GroupPolicyPlugin(group_policy_mapping_db.GroupPolicyMappingDbPlugin):
                               result['id'])
                 self.delete_policy_target(context, result['id'])
 
+        # Strip the extra port attributes
+        result.pop('port_attributes', None)
         return result
 
     @log.log

gbpservice/neutron/tests/unit/db/grouppolicy/test_group_policy_mapping_db.py

@@ -13,12 +13,14 @@
 
 import webob.exc
 
+from neutron import context as nctx
 from neutron.db import api as db_api
 from neutron.db import model_base
 from neutron.tests.unit.extensions import test_l3
 from neutron.tests.unit import testlib_api
 
 from gbpservice.neutron.db.grouppolicy import group_policy_mapping_db as gpmdb
+from gbpservice.neutron.services.grouppolicy.common import exceptions as gpexc
 from gbpservice.neutron.tests.unit.db.grouppolicy import (
     test_group_policy_db as tgpdb)
 
@@ -244,3 +246,25 @@ class TestMappedGroupResourceAttrs(GroupPolicyMappingDbTestCase):
                 self._test_list_resources(
                     'external_segment', [external_segments[0]],
                     query_params='subnet_id=' + subnets[0])
+
+    def test_pt_port_extra_attributes_fail(self):
+        ptg = self.create_policy_target_group()['policy_target_group']
+        ctx = nctx.get_admin_context()
+        self.assertRaises(gpexc.InvalidPortExtraAttributes,
+                          self._gbp_plugin.create_policy_target,
+                          ctx, {'policy_target': {
+                                    'description': '', 'name': '',
+                                    'port_id': None, 'cluster_id': '',
+                                    'policy_target_group_id': ptg['id'],
+                                    'port_attributes': {'network_id': ''},
+                                    'tenant_id': self._tenant_id}})
+
+        self.assertRaises(gpexc.InvalidPortExtraAttributes,
+                          self._gbp_plugin.create_policy_target,
+                          ctx, {'policy_target': {
+                                    'description': '', 'name': '',
+                                    'port_id': None, 'cluster_id': '',
+                                    'policy_target_group_id': ptg['id'],
+                                    'port_attributes': {
+                                        'allowed_address_pairs': ''},
+                                    'tenant_id': self._tenant_id}})

gbpservice/neutron/tests/unit/services/grouppolicy/test_resource_mapping.py

@@ -37,6 +37,7 @@ from gbpservice.common import utils
 from gbpservice.neutron.db.grouppolicy import group_policy_db as gpdb
 from gbpservice.neutron.db import servicechain_db
 from gbpservice.neutron.services.grouppolicy.common import constants as gconst
+from gbpservice.neutron.services.grouppolicy.common import exceptions as gpexc
 from gbpservice.neutron.services.grouppolicy import config
 from gbpservice.neutron.services.grouppolicy.drivers import chain_mapping
 from gbpservice.neutron.services.grouppolicy.drivers import nsp_manager
@@ -779,6 +780,97 @@ class TestPolicyTarget(ResourceMappingTestCase, TestClusterIdMixin):
                 self.create_policy_target(
                     policy_target_group_id=ptg['id'], expected_res_status=500)
 
+    def test_port_extra_attributes(self, extra=None):
+        extra = extra or {}
+        ptg = self.create_policy_target_group()['policy_target_group']
+        # Issue internal call
+        ctx = nctx.get_admin_context()
+        data = {'description': '', 'name': '',
+                'port_id': None, 'cluster_id': '',
+                'policy_target_group_id': ptg['id'],
+                'port_attributes': {'mac_address':
+                                    'aa:bb:cc:dd:ee:ff'},
+                'tenant_id': self._tenant_id}
+        data.update(extra)
+        pt = self._gbp_plugin.create_policy_target(
+            ctx, {'policy_target': data})
+        port = self._get_object('ports', pt['port_id'], self.api)['port']
+        self.assertEqual('aa:bb:cc:dd:ee:ff', port['mac_address'])
+
+    def test_weird_port_extra_attributes_ignored(self, extra=None):
+        extra = extra or {}
+        ptg = self.create_policy_target_group()['policy_target_group']
+        # Calling an internal API, all additional weird attributes are ignored.
+        # This will change once we go to a separated server
+        ctx = nctx.get_admin_context()
+        data = {'description': '', 'name': '',
+                'port_id': None, 'cluster_id': '',
+                'policy_target_group_id': ptg['id'],
+                'port_attributes': {'non_port_attribute': ''},
+                'tenant_id': self._tenant_id}
+        data.update(extra)
+        pt = self._gbp_plugin.create_policy_target(
+            ctx, {'policy_target': data})
+        self.assertIsNotNone(pt['id'])
+
+    def test_port_extra_attributes_fixed_ips(self, extra=None):
+        extra = extra or {}
+        l2p = self.create_l2_policy()['l2_policy']
+        network = self._get_object('networks', l2p['network_id'], self.api)
+        with self.subnet(network=network, cidr='10.10.1.0/24') as subnet:
+            subnet = subnet['subnet']
+            ptg = self.create_policy_target_group(
+                subnets=[subnet['id']],
+                l2_policy_id=l2p['id'])['policy_target_group']
+            fixed_ips = [{'subnet_id': subnet['id'],
+                          'ip_address': '10.10.1.10'}]
+            ctx = nctx.get_admin_context()
+            data = {'description': '', 'name': '',
+                    'port_id': None, 'cluster_id': '',
+                    'policy_target_group_id': ptg['id'],
+                    'port_attributes': {'fixed_ips': fixed_ips},
+                    'tenant_id': self._tenant_id}
+            data.update(extra)
+            pt = self._gbp_plugin.create_policy_target(
+                ctx, {'policy_target': data})
+            port = self._get_object('ports', pt['port_id'], self.api)['port']
+            self.assertEqual(fixed_ips, port['fixed_ips'])
+
+            # Now use a different subnet
+            ctx = nctx.get_admin_context()
+            data = {'description': '', 'name': '', 'port_id': None,
+                    'cluster_id': '', 'policy_target_group_id': ptg['id'],
+                    'port_attributes': {'fixed_ips': [{
+                        'subnet_id': 'not_in_ptg',
+                        'ip_address': '10.10.1.10'}]},
+                    'tenant_id': self._tenant_id}
+            data.update(extra)
+            self.assertRaises(gpexc.InvalidPortExtraAttributes,
+                              self._gbp_plugin.create_policy_target,
+                              ctx, {'policy_target': data})
+
+    def test_port_extra_attributes_implicit(self, extra=None):
+        extra = extra or {}
+        ptg = self.create_policy_target_group()['policy_target_group']
+        ctx = nctx.get_admin_context()
+        data = {'description': '', 'name': '',
+                'port_id': None, 'cluster_id': '',
+                'policy_target_group_id': ptg['id'],
+                'port_attributes': {'mac_address':
+                                    'aa:bb:cc:dd:ee:ff'},
+                'tenant_id': self._tenant_id}
+        data.update(extra)
+        pt = self._gbp_plugin.create_policy_target(
+            ctx, {'policy_target': data})
+        # Port exists
+        self._get_object('ports', pt['port_id'], self.api,
+                         expected_res_status=200)
+
+        self.delete_policy_target(pt['id'], expected_res_status=204)
+        # Port is gone since owned by GBP
+        self._get_object('ports', pt['port_id'], self.api,
+                         expected_res_status=404)
+
 
 class TestPolicyTargetGroupWithDNSConfiguration(ResourceMappingTestCase):
 

gbpservice/neutron/tests/unit/services/grouppolicy/test_resource_mapping_proxy_group.py

@@ -314,6 +314,22 @@ class TestPolicyTarget(ResourceMappingProxyGroupGBPTestCase,
         super(TestPolicyTarget, self).test_implicit_port_lifecycle(
             proxy_ip_pool='182.169.0.0/16')
 
+    def test_weird_port_extra_attributes_ignored(self):
+        super(TestPolicyTarget, self).test_weird_port_extra_attributes_ignored(
+            extra={'proxy_gateway': False, 'group_default_gateway': False})
+
+    def test_port_extra_attributes(self):
+        super(TestPolicyTarget, self).test_port_extra_attributes(
+            extra={'proxy_gateway': False, 'group_default_gateway': False})
+
+    def test_port_extra_attributes_fixed_ips(self):
+        super(TestPolicyTarget, self).test_port_extra_attributes_fixed_ips(
+            extra={'proxy_gateway': False, 'group_default_gateway': False})
+
+    def test_port_extra_attributes_implicit(self):
+        super(TestPolicyTarget, self).test_port_extra_attributes_implicit(
+            extra={'proxy_gateway': False, 'group_default_gateway': False})
+
 
 class TestPolicyTargetGroup(ResourceMappingProxyGroupGBPTestCase,
                             test_resource_mapping.TestPolicyTargetGroup):