Merge "Nsx policy: Support icmp protocol in classifier"

gbpservice/neutron/services/grouppolicy/drivers/vmware/nsx_policy/nsx_policy_mapping.py

@@ -61,6 +61,21 @@ class ProxyGroupsNotSupported(gpexc.GroupPolicyBadRequest):
     message = ("Proxy groups are not supported with %s." % DRIVER_NAME)
 
 
+#TODO(annak): remove when ipv6 is supported + add support for ICMPv6 service
+class Ipv6NotSupported(gpexc.GroupPolicyBadRequest):
+    message = ("Ipv6 not supported with %s" % DRIVER_NAME)
+
+
+class UpdateClassifierProtocolNotSupported(gpexc.GroupPolicyBadRequest):
+    message = ("Update operation on classifier protocol is not supported "
+               "with %s" % DRIVER_NAME)
+
+
+class ProtocolNotSupported(gpexc.GroupPolicyBadRequest):
+    message = ("Unsupported classifier protocol. Only icmp, tcp and udp are "
+               "supported with %s" % DRIVER_NAME)
+
+
 def in_name(name):
     return name + '_I'
 
@@ -356,11 +371,19 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
               self).create_policy_action_postcommit(context)
 
     def create_policy_classifier_precommit(self, context):
-        pass
+        if context.current['protocol'] not in ('icmp', 'tcp', 'udp'):
+            raise ProtocolNotSupported()
 
     def create_policy_classifier_postcommit(self, context):
         classifier = context.current
 
+        if classifier['protocol'] == 'icmp':
+            self.nsx_policy.icmp_service.create_or_overwrite(
+                name=classifier['name'],
+                service_id=classifier['id'],
+                description=classifier['description'])
+            return
+
         port_range = classifier['port_range'].split(':', 1)
         lower = int(port_range[0])
         upper = int(port_range[-1]) + 1
@@ -471,6 +494,9 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
 
     def delete_policy_classifier_postcommit(self, context):
         classifier_id = context.current['id']
+        if context.current['protocol'] == 'icmp':
+            self.nsx_policy.icmp_service.delete(classifier_id)
+        else:
             self.nsx_policy.service.delete(classifier_id)
 
     def delete_policy_rule_set_precommit(self, context):
@@ -525,7 +551,15 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
         raise UpdateOperationNotSupported()
 
     def update_policy_classifier_precommit(self, context):
-        pass
+        if context.current['protocol'] != context.original['protocol']:
+            raise UpdateClassifierProtocolNotSupported()
 
     def update_policy_classifier_postcommit(self, context):
         self.create_policy_classifier_postcommit(context)
+
+    def create_l3_policy_precommit(self, context):
+        if context.current['ip_version'] != 4:
+            raise Ipv6NotSupported()
+
+        super(NsxPolicyMappingDriver,
+              self).create_l3_policy_precommit(context)

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

@@ -76,9 +76,16 @@ class NsxPolicyMappingTestCase(test_rmd.ResourceMappingTestCase):
         return mock.patch.object(self.nsx_policy.service,
                                  'create_or_overwrite')
 
+    def _mock_icmp_service_create(self):
+        return mock.patch.object(self.nsx_policy.icmp_service,
+                                 'create_or_overwrite')
+
     def _mock_service_delete(self):
         return mock.patch.object(self.nsx_policy.service, 'delete')
 
+    def _mock_icmp_service_delete(self):
+        return mock.patch.object(self.nsx_policy.icmp_service, 'delete')
+
     def _mock_profile_create(self):
         return mock.patch.object(self.nsx_policy.comm_profile,
                                  'create_or_overwrite')
@@ -156,15 +163,16 @@ class NsxPolicyMappingTestCase(test_rmd.ResourceMappingTestCase):
 
 class TestPolicyClassifier(NsxPolicyMappingTestCase):
 
-    def test_create(self):
-        # Create non-first classifier within tenant
-        # Should not trigger domain generation on backend
-        with self._mock_service_create() as service_create_call:
+    def test_l4_lifecycle(self):
+        with self._mock_service_create() as service_create_call, \
+            self._mock_service_delete() as service_delete_call:
 
-            self.create_policy_classifier(name='test',
+            # classifier create
+            cl = self.create_policy_classifier(
+                name='test',
                 protocol='TCP',
                 port_range='80',
-                                          direction='bi')
+                direction='bi')['policy_classifier']
 
             # verify API call to create the service
             service_create_call.assert_called_with(
@@ -174,6 +182,26 @@ class TestPolicyClassifier(NsxPolicyMappingTestCase):
                 dest_ports=['80'],
                 service_id=mock.ANY)
 
+            service_create_call.reset_mock()
+
+            # classifier update
+            cl = self.update_policy_classifier(
+                    cl['id'],
+                    port_range='443',
+                    direction='in')['policy_classifier']
+
+            service_create_call.assert_called_with(
+                name='test',
+                description=mock.ANY,
+                protocol='tcp',
+                dest_ports=['443'],
+                service_id=mock.ANY)
+
+            # classifier delete
+            self.delete_policy_classifier(cl['id'])
+
+            service_delete_call.assert_called_with(cl['id'])
+
     def test_create_port_range(self):
         with self._mock_service_create() as service_create_call:
 
@@ -190,18 +218,45 @@ class TestPolicyClassifier(NsxPolicyMappingTestCase):
                 dest_ports=port_list,
                 service_id=mock.ANY)
 
-    def test_delete(self):
-        with self._mock_service_create(),\
-            self._mock_service_delete() as service_delete_call:
+    def test_icmp_lifecycle(self):
+        with self._mock_icmp_service_create() as service_create_call, \
+            self._mock_icmp_service_delete() as service_delete_call:
 
-            classifier = self.create_policy_classifier(
+            cl = self.create_policy_classifier(
                 name='test',
-                protocol='TCP',
-                port_range='80',
+                protocol='icmp',
                 direction='bi')['policy_classifier']
-            self.delete_policy_classifier(classifier['id'])
 
-            service_delete_call.assert_called_with(classifier['id'])
+            # verify API call to create the service
+            service_create_call.assert_called_with(
+                name='test',
+                description=mock.ANY,
+                service_id=mock.ANY)
+
+            self.delete_policy_classifier(cl['id'])
+
+            service_delete_call.assert_called_with(cl['id'])
+
+    def test_update_protocol_fails(self):
+        with self._mock_icmp_service_create():
+
+            cl = self.create_policy_classifier(
+                name='test',
+                protocol='icmp',
+                direction='bi')['policy_classifier']
+
+            self.assertRaises(webob.exc.HTTPClientError,
+                              self.update_policy_classifier,
+                              cl['id'],
+                              protocol='tcp',
+                              dest_ports=['80'])
+
+    def test_icmpv6_protocol_failsi(self):
+        self.assertRaises(webob.exc.HTTPClientError,
+                          self.create_policy_classifier,
+                          name='test',
+                          protocol='58',
+                          direction='bi')
 
 
 class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
@@ -858,3 +913,12 @@ class TestPolicyTargetTag(NsxPolicyMappingTestCase):
 
         # policy target deletion should not affect backend policy-wise
         self.delete_policy_target(target['id'])
+
+
+class TestL3Policy(NsxPolicyMappingTestCase):
+
+    def test_ipv6_supported(self):
+        self.assertRaises(webob.exc.HTTPClientError,
+                          self.create_l3_policy,
+                          ip_version=6,
+                          ip_pool='1001::0/64')