x/vmware-nsx
Code Issues Proposed changes

NSX-P+v3: Better handling for NSX address bindings

Browse Source 
When allowed address pairs are specified they can overlap with
fixed IPs, especially when they specify CIDRs.
The plugin should not fail to create/update neutron ports in this
case, but should instead properly handle NSX address bindings.

Change-Id: I145950ebe5769490f1c05729d94869dfa2e7d856
This commit is contained in:
Salvatore Orlando 2021-07-07 13:32:59 -07:00
parent 6638cbb70f
commit 73bf4bbe31
4 changed files with 146 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
vmware_nsx/plugins/nsx_p/plugin.py
View File

@ -1670,6 +1670,12 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base):
self._format_mac_address(pair['mac_address']))
address_bindings.append(binding)
for binding1 in address_bindings[:]:
for binding2 in address_bindings[:]:
cidr1 = netaddr.IPNetwork(binding1.ip_address)
cidr2 = netaddr.IPNetwork(binding2.ip_address)
if cidr1 != cidr2 and cidr1 in cidr2:
address_bindings.remove(binding1)
return address_bindings
def _get_network_nsx_id(self, context, network_id):

7
vmware_nsx/plugins/nsx_v3/plugin.py
View File

@ -1200,6 +1200,13 @@ class NsxV3Plugin(nsx_plugin_common.NsxPluginV3Base,
address_bindings.append(nsx_resources.PacketAddressClassifier(
pair['ip_address'], pair['mac_address'], None))
for binding1 in address_bindings[:]:
for binding2 in address_bindings[:]:
cidr1 = netaddr.IPNetwork(binding1.ip_address)
cidr2 = netaddr.IPNetwork(binding2.ip_address)
if cidr1 != cidr2 and cidr1 in cidr2:
address_bindings.remove(binding1)
return address_bindings
def _get_qos_profile_id(self, context, policy_id):

71
vmware_nsx/tests/unit/nsx_p/test_plugin.py
View File

@ -1263,6 +1263,77 @@ class NsxPTestPorts(common_v3.NsxV3TestPorts,
mock_list_bindings.assert_not_called()
mock_create_bindings.assert_not_called()
def test_update_port_ip_in_allowed_pair_cidr(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)
with mock.patch.object(
self.plugin.nsxpolicy.segment_port,
'create_or_overwrite') as mock_port:
put_data = {
'port': {
'allowed_address_pairs': [
{'ip_address': subnet['subnet']['cidr']}
]
}
}
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)
self.assertEqual(1, len(mock_port.mock_calls))
_n, _a, kwargs = mock_port.mock_calls[0]
actual_binding = kwargs['address_bindings'][0]
self.assertEqual(
subnet['subnet']['cidr'],
actual_binding.ip_address)
def test_update_port_ip_not_in_allowed_pair_cidr(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)
fixed_ip = (
port['port']['fixed_ips'][0]['ip_address'])
with mock.patch.object(
self.plugin.nsxpolicy.segment_port,
'create_or_overwrite') as mock_port:
put_data = {
'port': {
'allowed_address_pairs': [
{'ip_address': '1.2.3.0/24'}
]
}
}
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)
self.assertEqual(1, len(mock_port.mock_calls))
_n, _a, kwargs = mock_port.mock_calls[0]
actual_bindings = kwargs['address_bindings']
addresses = set([b.ip_address for b in actual_bindings])
self.assertEqual(
set([fixed_ip, '1.2.3.0/24']),
addresses)
class NsxPTestSubnets(common_v3.NsxV3TestSubnets,
NsxPPluginTestCaseMixin):

62
vmware_nsx/tests/unit/nsx_v3/test_plugin.py
View File

@ -1503,6 +1503,68 @@ class TestPortsV2(common_v3.NsxV3SubnetMixin,
self.plugin.update_port(self.ctx, port['id'], data)
get_profile.assert_called_once_with(self.ctx, policy_id)
def test_update_port_ip_in_allowed_pair_cidr(self):
with self.network() as network:
with self.subnet(network) as subnet:
data = {'port': {
'network_id': network['network']['id'],
'tenant_id': self._tenant_id,
'admin_state_up': True,
'name': 'meh',
'device_id': 'fake_device',
'device_owner': 'fake_owner',
'fixed_ips': [
{'subnet_id': subnet['subnet']['id']}],
'mac_address': '00:00:00:00:00:01'}
}
port = self.plugin.create_port(self.ctx, data)
with mock.patch.object(self.plugin.nsxlib.logical_port,
'update') as mock_port:
self.plugin.update_port(
self.ctx, port['id'],
{'port': {
'allowed_address_pairs': [
{'ip_address': subnet['subnet']['cidr']}
]}})
self.assertEqual(1, len(mock_port.mock_calls))
_n, _a, kwargs = mock_port.mock_calls[0]
actual_binding = kwargs['address_bindings'][0]
self.assertEqual(
subnet['subnet']['cidr'],
actual_binding.ip_address)
def test_update_port_ip_not_in_allowed_pair_cidr(self):
with self.network() as network:
with self.subnet(network) as subnet:
data = {'port': {
'network_id': network['network']['id'],
'tenant_id': self._tenant_id,
'admin_state_up': True,
'name': 'meh',
'device_id': 'fake_device',
'device_owner': 'fake_owner',
'fixed_ips': [
{'subnet_id': subnet['subnet']['id']}],
'mac_address': '00:00:00:00:00:01'}
}
port = self.plugin.create_port(self.ctx, data)
fixed_ip = port['fixed_ips'][0]['ip_address']
with mock.patch.object(self.plugin.nsxlib.logical_port,
'update') as mock_port:
self.plugin.update_port(
self.ctx, port['id'],
{'port': {
'allowed_address_pairs': [
{'ip_address': '1.2.3.0/24'}
]}})
self.assertEqual(1, len(mock_port.mock_calls))
_n, _a, kwargs = mock_port.mock_calls[0]
actual_bindings = kwargs['address_bindings']
addresses = set([b.ip_address for b in actual_bindings])
self.assertEqual(
set([fixed_ip, '1.2.3.0/24']),
addresses)
def _get_ports_with_fields(self, tenid, fields, expected_count):
pl = directory.get_plugin()
ctx = context.Context(user_id=None, tenant_id=tenid,