From d77ff8f51629bfd4703588bbabe00999f61a5f92 Mon Sep 17 00:00:00 2001 From: Anna Khmelnitsky Date: Fri, 28 Jul 2017 08:26:18 -0700 Subject: [PATCH] Nsx policy: Support icmp protocol in classifier Icmp is provided by separate service in nsx policy. In addition, limit nsx policy to ipv4 only since ipv6 is not supported yet by the backend. Change-Id: I84bc1fadc3e2e259ddd917abd8e8c455f1ee41b5 --- .../vmware/nsx_policy/nsx_policy_mapping.py | 40 +++++++- .../test_nsx_policy_mapping_driver.py | 96 +++++++++++++++---- 2 files changed, 117 insertions(+), 19 deletions(-) diff --git a/gbpservice/neutron/services/grouppolicy/drivers/vmware/nsx_policy/nsx_policy_mapping.py b/gbpservice/neutron/services/grouppolicy/drivers/vmware/nsx_policy/nsx_policy_mapping.py index d1c1d5ee8..70184fe70 100644 --- a/gbpservice/neutron/services/grouppolicy/drivers/vmware/nsx_policy/nsx_policy_mapping.py +++ b/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,7 +494,10 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver): def delete_policy_classifier_postcommit(self, context): classifier_id = context.current['id'] - self.nsx_policy.service.delete(classifier_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): pass @@ -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) diff --git a/gbpservice/neutron/tests/unit/services/grouppolicy/test_nsx_policy_mapping_driver.py b/gbpservice/neutron/tests/unit/services/grouppolicy/test_nsx_policy_mapping_driver.py index 6a7c7ec2a..fde39b9f1 100644 --- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_nsx_policy_mapping_driver.py +++ b/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', - protocol='TCP', - port_range='80', - direction='bi') + # classifier create + cl = self.create_policy_classifier( + name='test', + protocol='TCP', + port_range='80', + 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')