[NSX-P] Relax IP validation for DHCP and v6 subnets

Extend validation relaxation introduced by commit 607afed94, by
allowing for multiple IPs for a given port also on DHCP and IPv6
subnets.

In order to ensure correct processing of DHCP bindings, the
process a binding is created only for the first IP address
allocated in the subnet; the process also ensures that the
address associated with the DHCP binding does not change unless
removed from the port's fixed_ips.

This change applies only to the nsx_p plugin, and the other
constraints on port IP allocation are still in place.

Change-Id: I0271b9be8e73e8e6b9d1b3b51bebc1542efd3d29

vmware_nsx/plugins/common_v3/plugin.py

@@ -614,11 +614,15 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
                 LOG.warning(err_msg)
                 raise n_exc.InvalidInput(error_message=err_msg)
 
-    def _validate_max_ips_per_port(self, context, fixed_ip_list, device_owner):
+    def _validate_max_ips_per_port(self, context, fixed_ip_list, device_owner,
+                                   relax_ip_validation=False):
         """Validate the number of fixed ips on a port
 
         Do not allow multiple ip addresses on a port since the nsx backend
-        cannot add multiple static dhcp bindings with the same port
+        cannot add multiple static dhcp bindings with the same port, unless
+        relax_ip_validation is set to True.
+
+        In any case allow at most 1 IPv4 subnet and 1 IPv6 subnet
         """
         if (device_owner and
             nl_net_utils.is_port_trusted({'device_owner': device_owner})):
@@ -706,9 +710,9 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
         # enabled and other IPs. Also ensure no more than one IP for any
         # DHCP subnet is requested. fixed_ip attribute does not have patch
         # behaviour, so all requested IP allocations must be included in it
-
+        if not relax_ip_validation:
-        for subnet_id, fixed_ips in subnet_fixed_ip_dict.items():
+            for subnet_id, fixed_ips in subnet_fixed_ip_dict.items():
-            validate_subnet_dhcp_and_ipv6_state(subnet_id, fixed_ips)
+                validate_subnet_dhcp_and_ipv6_state(subnet_id, fixed_ips)
 
     def _get_subnets_for_fixed_ips_on_port(self, context, port_data):
         # get the subnet id from the fixed ips of the port
@@ -720,10 +724,14 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
                     for subnet_id in subnet_ids)
         return []
 
-    def _validate_create_port(self, context, port_data):
+    def _validate_create_port(self, context, port_data,
+                              relax_ip_validation=False):
+        # Validate IP address settings. Relax IP validation parameter should
+        # be true to allow multiple IPs from the same subnet
         self._validate_max_ips_per_port(context,
                                         port_data.get('fixed_ips', []),
-                                        port_data.get('device_owner'))
+                                        port_data.get('device_owner'),
+                                        relax_ip_validation)
 
         is_external_net = self._network_is_external(
             context, port_data['network_id'])
@@ -812,7 +820,8 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
                 LOG.warning(err_msg)
                 raise n_exc.InvalidInput(error_message=err_msg)
 
-    def _validate_update_port(self, context, id, original_port, port_data):
+    def _validate_update_port(self, context, id, original_port, port_data,
+                              relax_ip_validation=False):
         qos_selected = validators.is_attr_set(port_data.get
                                               (qos_consts.QOS_POLICY_ID))
         is_external_net = self._network_is_external(
@@ -841,9 +850,12 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
         self._assert_on_device_owner_change(port_data, orig_dev_owner)
         self._assert_on_port_admin_state(port_data, device_owner)
         self._assert_on_port_sec_change(port_data, device_owner)
+        # Validate IP address settings. Relax IP validation parameter should
+        # be true to allow multiple IPs from the same subnet
         self._validate_max_ips_per_port(context,
                                         port_data.get('fixed_ips', []),
-                                        device_owner)
+                                        device_owner,
+                                        relax_ip_validation)
         self._validate_number_of_address_pairs(port_data)
         self._assert_on_vpn_port_change(original_port)
         self._assert_on_lb_port_fixed_ip_change(port_data, orig_dev_owner)

vmware_nsx/plugins/nsx_p/plugin.py

@@ -1647,6 +1647,7 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
             return []
 
         address_bindings = []
+        lladdr = None
         for fixed_ip in port_data['fixed_ips']:
             ip_addr = fixed_ip['ip_address']
             mac_addr = self._format_mac_address(port_data['mac_address'])
@@ -1657,7 +1658,7 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
             # add address binding for link local ipv6 address, otherwise
             # neighbor discovery will be blocked by spoofguard.
             # for now only one ipv6 address is allowed
-            if netaddr.IPAddress(ip_addr).version == 6:
+            if netaddr.IPAddress(ip_addr).version == 6 and not lladdr:
                 lladdr = netaddr.EUI(mac_addr).ipv6_link_local()
                 binding = self.nsxpolicy.segment_port.build_address_binding(
                     lladdr, mac_addr)
@@ -1904,8 +1905,36 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
             return
         net_id = port['network_id']
 
-        for fixed_ip in self._filter_ipv4_dhcp_fixed_ips(
+        # If we have more than a single IP we need to fetch existing bindings
-            context, port['fixed_ips']):
+        # before deciding which one we need to write for the current port
+        v4_fixed_ips = self._filter_ipv4_dhcp_fixed_ips(
+            context, port['fixed_ips'])
+        v6_fixed_ips = self._filter_ipv6_dhcp_fixed_ips(
+            context, port['fixed_ips'])
+        if len(v4_fixed_ips) > 1 or len(v6_fixed_ips) > 1:
+            cur_bindings = self.nsxpolicy.segment_dhcp_static_bindings.list(
+                segment_id)
+            for binding in cur_bindings:
+                # Careful about V4/V6 bindings!
+                binding_ip = None
+                try:
+                    binding_ip = binding['ip_address']
+                except KeyError:
+                    ips = binding.get('ip_addresses', [])
+                    if ips:
+                        binding_ip = ips[0]
+                if not binding_ip:
+                    continue
+                if any([binding_ip == ip_v4['ip_address']
+                        for ip_v4 in v4_fixed_ips]):
+                    # Prevent updating v4 binding. Keep current binding.
+                    v4_fixed_ips = []
+                if any([binding_ip == ip_v6['ip_address']
+                        for ip_v6 in v6_fixed_ips]):
+                    # Prevent updating v6 binding. Keep current binding.
+                    v6_fixed_ips = []
+
+        for fixed_ip in v4_fixed_ips:
             # There will be only one ipv4 ip here
             binding_id = port['id'] + '-ipv4'
             name = 'IPv4 binding for port %s' % port['id']
@@ -1929,9 +1958,9 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
                 lease_time=cfg.CONF.nsx_p.dhcp_lease_time,
                 mac_address=port['mac_address'],
                 options=options)
+            break
 
-        for fixed_ip in self._filter_ipv6_dhcp_fixed_ips(
+        for fixed_ip in v6_fixed_ips:
-            context, port['fixed_ips']):
             # There will be only one ipv6 ip here
             binding_id = port['id'] + '-ipv6'
             name = 'IPv6 binding for port %s' % port['id']
@@ -1947,12 +1976,12 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
                 ip_addresses=[ip],
                 lease_time=cfg.CONF.nsx_p.dhcp_lease_time,
                 mac_address=port['mac_address'])
+            break
 
     def _add_port_policy_dhcp_binding(self, context, port):
         net_id = port['network_id']
         if not self._is_dhcp_network(context, net_id):
             return
-
         segment_id = self._get_network_nsx_segment_id(context, net_id)
         self._add_or_overwrite_port_policy_dhcp_binding(
             context, port, segment_id)
@@ -2106,7 +2135,8 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
     def create_port(self, context, port, l2gw_port_check=False):
         port_data = port['port']
         # validate the new port parameters
-        self._validate_create_port(context, port_data)
+        self._validate_create_port(context, port_data,
+                                   relax_ip_validation=True)
         self._assert_on_resource_admin_state_down(port_data)
 
         # Validate the vnic type (the same types as for the NSX-T plugin)
@@ -2314,7 +2344,7 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
             self._remove_provider_security_groups_from_list(original_port)
             port_data = port['port']
             self._validate_update_port(context, port_id, original_port,
-                                       port_data)
+                                       port_data, relax_ip_validation=True)
             self._assert_on_resource_admin_state_down(port_data)
             validate_port_sec = self._should_validate_port_sec_on_update_port(
                 port_data)
@@ -2322,12 +2352,6 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
                 context, original_port['network_id'])
             if is_external_net:
                 self._assert_on_external_net_with_compute(port_data)
-            device_owner = (port_data['device_owner']
-                            if 'device_owner' in port_data
-                            else original_port.get('device_owner'))
-            self._validate_max_ips_per_port(context,
-                                            port_data.get('fixed_ips', []),
-                                            device_owner)
 
             direct_vnic_type = self._validate_port_vnic_type(
                 context, port_data, original_port['network_id'])

vmware_nsx/tests/unit/nsx_p/test_plugin.py

@@ -1148,6 +1148,121 @@ class NsxPTestPorts(common_v3.NsxV3TestPorts,
                 mock_delete_binding.assert_called_once_with(
                     network['network']['id'], mock.ANY)
 
+    # Tests overriden from base class as they're expected to pass instead
+    # of failing with the policy plugin
+    def test_create_port_additional_ip(self):
+        """Test that creation of port with additional IP fails."""
+        self.plugin.use_policy_dhcp = True
+        with mock.patch.object(
+            self.plugin.nsxpolicy.segment_dhcp_static_bindings,
+            'list') as mock_list_bindings:
+            with mock.patch.object(
+                self.plugin.nsxpolicy.segment_dhcp_static_bindings,
+                'create_or_overwrite_v4') as mock_create_bindings:
+                with self.subnet() as subnet:
+                    data = {'port':
+                            {'network_id': subnet['subnet']['network_id'],
+                             'tenant_id': subnet['subnet']['tenant_id'],
+                             'device_owner': 'compute:meh',
+                             'fixed_ips': [{'subnet_id':
+                                            subnet['subnet']['id']},
+                                           {'subnet_id':
+                                            subnet['subnet']['id']}]}}
+                    port_req = self.new_create_request('ports', data)
+                    res = port_req.get_response(self.api)
+                    self.assertEqual(201, res.status_int)
+                    res = self.deserialize('json', res)
+                    fixed_ips = res['port']['fixed_ips']
+                    subnet_ids = set(item['subnet_id'] for item in fixed_ips)
+                    self.assertEqual(1, len(subnet_ids))
+                    self.assertIn(subnet['subnet']['id'], subnet_ids)
+                    mock_list_bindings.assert_called_once()
+                    mock_create_bindings.assert_called_once_with(
+                        mock.ANY, mock.ANY,
+                        binding_id=mock.ANY,
+                        gateway_address=subnet['subnet']['gateway_ip'],
+                        host_name=mock.ANY,
+                        ip_address=fixed_ips[0]['ip_address'],
+                        lease_time=mock.ANY,
+                        mac_address=res['port']['mac_address'],
+                        options=mock.ANY)
+
+    def test_update_port_add_additional_ip(self):
+        with self.subnet() as subnet:
+            post_data = {
+                'port': {
+                    'network_id': subnet['subnet']['network_id'],
+                    'tenant_id': subnet['subnet']['tenant_id'],
+                    'device_owner': 'compute:meh',
+                    'fixed_ips': [{'subnet_id':
+                                    subnet['subnet']['id']}]}}
+            post_req = self.new_create_request('ports', post_data)
+            res = post_req.get_response(self.api)
+            self.assertEqual(201, res.status_int)
+            port = self.deserialize('json', res)
+            orig_fixed_ip = (
+                port['port']['fixed_ips'][0]['ip_address'])
+            with mock.patch.object(
+                self.plugin.nsxpolicy.segment_dhcp_static_bindings,
+                'list') as mock_list_bindings:
+                with mock.patch.object(
+                    self.plugin.nsxpolicy.segment_dhcp_static_bindings,
+                    'create_or_overwrite_v4') as mock_create_bindings:
+                    mock_list_bindings.return_value = [
+                        {'ip_address': orig_fixed_ip}
+                    ]
+                    put_data = {
+                        'port': {
+                            'device_owner': 'compute:meh',
+                            'fixed_ips': [
+                                {'subnet_id': subnet['subnet']['id'],
+                                 'ip_address': orig_fixed_ip},
+                                {'subnet_id': subnet['subnet']['id']}]}}
+                    put_req = self.new_update_request(
+                        'ports', put_data, port['port']['id'])
+                    put_res = put_req.get_response(self.api)
+                    self.assertEqual(200, put_res.status_int)
+                    upd_port = self.deserialize('json', res)['port']
+                    fixed_ips = upd_port['fixed_ips']
+                    subnet_ids = set(item['subnet_id'] for item in fixed_ips)
+                    self.assertEqual(1, len(subnet_ids))
+                    self.assertIn(subnet['subnet']['id'], subnet_ids)
+                    mock_list_bindings.assert_called_once()
+                    mock_create_bindings.assert_not_called()
+
+    def test_update_port_clear_ip(self):
+        with self.subnet() as subnet:
+            post_data = {
+                'port': {
+                    'network_id': subnet['subnet']['network_id'],
+                    'tenant_id': subnet['subnet']['tenant_id'],
+                    'device_owner': 'compute:meh',
+                    'fixed_ips': [{'subnet_id': subnet['subnet']['id']},
+                                  {'subnet_id': subnet['subnet']['id']}]}}
+            post_req = self.new_create_request('ports', post_data)
+            res = post_req.get_response(self.api)
+            self.assertEqual(201, res.status_int)
+            port = self.deserialize('json', res)
+            with mock.patch.object(
+                self.plugin.nsxpolicy.segment_dhcp_static_bindings,
+                'list') as mock_list_bindings:
+                with mock.patch.object(
+                    self.plugin.nsxpolicy.segment_dhcp_static_bindings,
+                    'create_or_overwrite_v4') as mock_create_bindings:
+                    put_data = {'port':
+                                {'fixed_ips': [],
+                                 secgrp.SECURITYGROUPS: []}}
+                    put_req = self.new_update_request(
+                        'ports', put_data, port['port']['id'])
+                    res = put_req.get_response(self.api)
+                    self.assertEqual(200, res.status_int)
+                    res = self.deserialize('json', res)
+                    fixed_ips = res['port']['fixed_ips']
+                    subnet_ids = set(item['subnet_id'] for item in fixed_ips)
+                    self.assertEqual(0, len(subnet_ids))
+                    mock_list_bindings.assert_not_called()
+                    mock_create_bindings.assert_not_called()
+
 
 class NsxPTestSubnets(common_v3.NsxV3TestSubnets,
                       NsxPPluginTestCaseMixin):