ptg attribute for sc enforcement

Add enforce_service_chains attribute to PTGs as part of the
proxy-group driver extension. When set to False, PTGs won't trigger
service chain creation even when providing a PRS with a redirect
rule.

Change-Id: I78fb098ec4092f2c2b43f0eb41f35ab2fd5e01d9

etc/policy.json

@@ -215,6 +215,7 @@
     "create_policy_target_group": "",
     "create_policy_target_group:shared": "rule:admin_only",
     "create_policy_target_group:service_management": "rule:admin_only",
+    "create_policy_target_group:enforce_service_chains": "rule:admin_only",
     "get_policy_target_group": "rule:admin_or_owner or rule:shared_ptg",
     "update_policy_target_group:shared": "rule:admin_only",
 

gbpservice/neutron/db/grouppolicy/extensions/group_proxy_db.py

@@ -28,6 +28,7 @@ class GroupProxyMapping(model_base.BASEV2):
                                sa.ForeignKey('gp_policy_target_groups.id',
                                              ondelete="SET NULL"))
     proxy_type = sa.Column(sa.String(24))
+    enforce_service_chains = sa.Column(sa.Boolean)
 
 
 class ProxyGatewayMapping(model_base.BASEV2):

gbpservice/neutron/db/migration/alembic_migrations/versions/7afacef00d31_enforce_chains.py

@@ -0,0 +1,38 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+
+"""enforce_service_chains attribute for PTGs
+
+Revision ID: 64fa77aca090
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '7afacef00d31'
+down_revision = 'c1aab79622fe'
+
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+
+    op.add_column(
+        'gp_group_proxy_mappings',
+        sa.Column('enforce_service_chains', sa.Boolean, default=True)
+    )
+
+
+def downgrade():
+    pass

gbpservice/neutron/db/migration/alembic_migrations/versions/HEAD

@@ -1 +1 @@
-c1aab79622fe
+7afacef00d31

gbpservice/neutron/extensions/driver_proxy_group.py

@@ -74,6 +74,11 @@ EXTENDED_ATTRIBUTES_2_0 = {
             'allow_post': False, 'allow_put': False,
             'validate': {'type:uuid_or_none': None}, 'is_visible': True,
             'enforce_policy': True},
+        'enforce_service_chains': {
+            'allow_post': True, 'allow_put': False, 'default': True,
+            'convert_to': attr.convert_to_boolean,
+            'is_visible': True, 'required_by_policy': True,
+            'enforce_policy': True},
         # TODO(ivar): The APIs should allow the creation of a group with a
         # custom subnet prefix length. It may be useful for both the proxy
         # groups and traditional ones.

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

@@ -160,8 +160,8 @@ class ChainMappingDriver(api.PolicyDriver, local_api.LocalAPI,
 
     @log.log_method_call
     def create_policy_target_group_postcommit(self, context):
-        if (context.current['provided_policy_rule_sets'] and not
-            context.current.get('proxied_group_id')):
+        if (context.current['provided_policy_rule_sets'] and
+                self._is_group_chainable(context, context.current)):
             self._handle_redirect_action(
                 context, context.current['provided_policy_rule_sets'],
                 providing_ptg=context.current)
@@ -186,9 +186,9 @@ class ChainMappingDriver(api.PolicyDriver, local_api.LocalAPI,
         self._cleanup_redirect_action(context)
         # If the spec is changed, then update the chain with new spec
         # If redirect is newly added, create the chain
-        if self._is_redirect_in_policy_rule_sets(
-                context, new_provided_policy_rule_sets) and not (
-                    context.current.get('proxied_group_id')):
+        if (self._is_redirect_in_policy_rule_sets(
+                context, new_provided_policy_rule_sets) and
+                self._is_group_chainable(context, context.current)):
             self._handle_redirect_action(
                 context, curr['provided_policy_rule_sets'],
                 providing_ptg=context.current)
@@ -497,7 +497,7 @@ class ChainMappingDriver(api.PolicyDriver, local_api.LocalAPI,
                     # REVISIT(Magesh): There are concurrency issues here with
                     # concurrent updates to the same PRS, Policy Rule or Action
                     # value
-                    if not ptg_providing_prs.get('proxied_group_id'):
+                    if self._is_group_chainable(context, ptg_providing_prs):
                         self._create_or_update_chain(
                             context, ptg_providing_prs['id'],
                             SCI_CONSUMER_NOT_AVAILABLE, spec_id,
@@ -749,9 +749,10 @@ class ChainMappingDriver(api.PolicyDriver, local_api.LocalAPI,
     def _validate_ptg_prss(self, context, ptg):
         # If the PTG is providing a redirect PRS, it can't provide any more
         # redirect rules
-        if self._prss_redirect_rules(context._plugin_context.session,
-                                     ptg['provided_policy_rule_sets']) > 1:
-                raise exc.PTGAlreadyProvidingRedirectPRS(ptg_id=ptg['id'])
+        if self._is_group_chainable(context, ptg):
+            if self._prss_redirect_rules(context._plugin_context.session,
+                                         ptg['provided_policy_rule_sets']) > 1:
+                    raise exc.PTGAlreadyProvidingRedirectPRS(ptg_id=ptg['id'])
 
     def _handle_classifier_update_notification(self, context):
         # Invoke Service chain update notify hook if protocol or port or
@@ -809,3 +810,16 @@ class ChainMappingDriver(api.PolicyDriver, local_api.LocalAPI,
                          provider_ptg_ids=prs[
                              'providing_policy_target_groups'])])
         return result
+
+    def _is_group_chainable(self, context, group):
+        """Determines whether a group should trigger a chain.
+
+        Non chainable groups:
+        - Proxy groups;
+
+        :param context:
+        :param group:
+        :return: boolean
+        """
+        return (not group.get('proxied_group_id') and
+                group.get('enforce_service_chains', True) is True)

gbpservice/neutron/services/grouppolicy/drivers/extensions/proxy_group_driver.py

@@ -59,6 +59,12 @@ class ProxyGroupDriver(api.ExtensionDriver):
                     policy_target_group_id=result['id']).one())
                 record.proxy_type = data['proxy_type']
                 result['proxy_type'] = data['proxy_type']
+            # Proxy PTGs can't have chains enforced
+            data['enforce_service_chains'] = False
+            record = (session.query(db.GroupProxyMapping).filter_by(
+                policy_target_group_id=result['id']).one())
+            record.enforce_service_chains = data['enforce_service_chains']
+            result['enforce_service_chains'] = data['enforce_service_chains']
         elif attributes.is_attr_set(data.get('proxy_type')):
             raise driver_proxy_group.ProxyTypeSetWithoutProxiedPTG()
 

gbpservice/neutron/tests/etc/test-policy.json

@@ -22,6 +22,7 @@
     "shared_sp": "field:service_profiles:shared=True",
 
     "create_policy_target_group": "",
+    "create_policy_target_group:enforce_service_chains": "rule:admin_only",
     "get_policy_target_group": "rule:admin_or_owner or rule:shared_ptg",
     "create_policy_target_group:service_management": "rule:admin_only",
 

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

@@ -281,20 +281,6 @@ class ApicMappingTestCase(
             self.driver.apic_manager.ext_net_dict.update(
                 self._build_external_dict(x[0], x[1], is_edge_nat=is_edge_nat))
 
-    def _create_simple_policy_rule(self, direction='bi', protocol='tcp',
-                                   port_range=80, shared=False,
-                                   action_type='allow', action_value=None):
-        cls = self.create_policy_classifier(
-            direction=direction, protocol=protocol,
-            port_range=port_range, shared=shared)['policy_classifier']
-
-        action = self.create_policy_action(
-            action_type=action_type, shared=shared,
-            action_value=action_value)['policy_action']
-        return self.create_policy_rule(
-            policy_classifier_id=cls['id'], policy_actions=[action['id']],
-            shared=shared)['policy_rule']
-
     def _bind_port_to_host(self, port_id, host):
         data = {'port': {'binding:host_id': host,
                          'device_owner': 'compute:',

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

@@ -37,11 +37,26 @@ class ExtensionDriverTestCaseMixin(object):
         self.assertEqual('192.168.0.0/16', l3p['proxy_ip_pool'])
         self.assertEqual(28, l3p['proxy_subnet_prefix_length'])
 
+        # No proxy
+        ptg_proxy = self.create_policy_target_group()['policy_target_group']
+        self.assertTrue(ptg_proxy['enforce_service_chains'])
+        ptg_proxy = self.show_policy_target_group(
+            ptg_proxy['id'])['policy_target_group']
+        self.assertTrue(ptg_proxy['enforce_service_chains'])
+        ptg_proxy = self.create_policy_target_group(
+            enforce_service_chains=False,
+            is_admin_context=True)['policy_target_group']
+        self.assertFalse(ptg_proxy['enforce_service_chains'])
+        ptg_proxy = self.show_policy_target_group(
+            ptg_proxy['id'])['policy_target_group']
+        self.assertFalse(ptg_proxy['enforce_service_chains'])
+
         ptg_proxy = self.create_policy_target_group(
             proxied_group_id=ptg['id'])['policy_target_group']
         self.assertIsNone(ptg_proxy['proxy_group_id'])
         self.assertEqual(ptg['id'], ptg_proxy['proxied_group_id'])
         self.assertEqual('l3', ptg_proxy['proxy_type'])
+        self.assertFalse(ptg_proxy['enforce_service_chains'])
 
         # Verify relationship added
         ptg = self.show_policy_target_group(ptg['id'])['policy_target_group']

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

@@ -448,6 +448,20 @@ class ResourceMappingTestCase(test_plugin.GroupPolicyPluginTestCase):
         scs_id = spec['servicechain_spec']['id']
         return scs_id
 
+    def _create_simple_policy_rule(self, direction='bi', protocol='tcp',
+                                   port_range=80, shared=False,
+                                   action_type='allow', action_value=None):
+        cls = self.create_policy_classifier(
+            direction=direction, protocol=protocol,
+            port_range=port_range, shared=shared)['policy_classifier']
+
+        action = self.create_policy_action(
+            action_type=action_type, shared=shared,
+            action_value=action_value)['policy_action']
+        return self.create_policy_rule(
+            policy_classifier_id=cls['id'], policy_actions=[action['id']],
+            shared=shared)['policy_rule']
+
 
 class TestClusterIdMixin(object):
 
@@ -1381,6 +1395,35 @@ class TestPolicyTargetGroup(ResourceMappingTestCase):
         self._unbind_port(port['port']['id'])
         self.delete_policy_target_group(ptg['id'], expected_res_status=204)
 
+    def test_ptg_only_participate_one_prs_when_redirect(self):
+        redirect_rule = self._create_simple_policy_rule(action_type='redirect')
+        simple_rule = self._create_simple_policy_rule()
+        prs_r = self.create_policy_rule_set(
+            policy_rules=[redirect_rule['id']])['policy_rule_set']
+        prs = self.create_policy_rule_set(
+            policy_rules=[simple_rule['id']])['policy_rule_set']
+
+        # Creating PTG with provided redirect and multiple PRS fails
+        self.create_policy_target_group(
+            provided_policy_rule_sets={prs_r['id']: '', prs['id']: ''},
+            consumed_policy_rule_sets={prs['id']: ''},
+            expected_res_status=201)
+
+        action = self.create_policy_action(
+            action_type='redirect')['policy_action']
+        res = self.update_policy_rule(
+            simple_rule['id'], policy_actions=[action['id']],
+            expected_res_status=400)
+        self.assertEqual('PTGAlreadyProvidingRedirectPRS',
+                         res['NeutronError']['type'])
+        # Verify everythin is fine for non chainable PTGs
+        with mock.patch.object(
+                chain_mapping.ChainMappingDriver, '_is_group_chainable',
+                return_value=False):
+            self.update_policy_rule(
+                simple_rule['id'], policy_actions=[action['id']],
+                expected_res_status=200)
+
 
 class TestL2Policy(ResourceMappingTestCase):