Ensure members without subnet belong to VIP subnet or fail

This patch ensures the members without subnet are only accepted
if the IP of the member belongs to the CIDR of the VIP subnet,
as that is the subnet associated to the loadbalancer used to
obtain the subnet for the members that do not have it.

Closes-Bug: #1982111

Change-Id: I0fd90c9329a2ec43823813542f263845562c45f2
(cherry picked from commit 136b829579)
This commit is contained in:
Luis Tomas Bolivar 2022-07-13 16:39:22 +02:00
parent d052e39af7
commit f8de811c17
5 changed files with 96 additions and 25 deletions

View File

@ -238,11 +238,15 @@ class OvnProviderDriver(driver_base.ProviderDriver):
admin_state_up = member.admin_state_up
subnet_id = member.subnet_id
if (isinstance(subnet_id, o_datamodels.UnsetType) or not subnet_id):
subnet_id = self._ovn_helper._get_subnet_from_pool(member.pool_id)
if not subnet_id:
subnet_id, subnet_cidr = self._ovn_helper._get_subnet_from_pool(
member.pool_id)
if not (subnet_id and
self._ovn_helper._check_ip_in_subnet(member.address,
subnet_cidr)):
msg = _('Subnet is required, or Loadbalancer associated with '
'Pool must have a subnet, for Member creation '
'with OVN Provider Driver')
'with OVN Provider Driver if it is not the same as '
'LB VIP subnet')
raise driver_exceptions.UnsupportedOptionError(
user_fault_string=msg,
operator_fault_string=msg)
@ -278,8 +282,18 @@ class OvnProviderDriver(driver_base.ProviderDriver):
# the member is deleted, Octavia send the object without subnet_id.
subnet_id = member.subnet_id
if (isinstance(subnet_id, o_datamodels.UnsetType) or not subnet_id):
subnet_id = self._ovn_helper._get_subnet_from_pool(
subnet_id, subnet_cidr = self._ovn_helper._get_subnet_from_pool(
member.pool_id)
if not (subnet_id and
self._ovn_helper._check_ip_in_subnet(member.address,
subnet_cidr)):
msg = _('Subnet is required, or Loadbalancer associated with '
'Pool must have a subnet, for Member deletion if it is'
'with OVN Provider Driver if it is not the same as '
'LB VIP subnet')
raise driver_exceptions.UnsupportedOptionError(
user_fault_string=msg,
operator_fault_string=msg)
request_info = {'id': member.member_id,
'address': member.address,
@ -308,8 +322,18 @@ class OvnProviderDriver(driver_base.ProviderDriver):
# the member is updated, Octavia send the object without subnet_id.
subnet_id = old_member.subnet_id
if (isinstance(subnet_id, o_datamodels.UnsetType) or not subnet_id):
subnet_id = self._ovn_helper._get_subnet_from_pool(
subnet_id, subnet_cidr = self._ovn_helper._get_subnet_from_pool(
old_member.pool_id)
if not (subnet_id and
self._ovn_helper._check_ip_in_subnet(new_member.address,
subnet_cidr)):
msg = _('Subnet is required, or Loadbalancer associated with '
'Pool must have a subnet, for Member update '
'with OVN Provider Driver if it is not the same as '
'LB VIP subnet')
raise driver_exceptions.UnsupportedOptionError(
user_fault_string=msg,
operator_fault_string=msg)
# Validate monitoring options if present
self._check_member_monitor_options(new_member)
@ -346,19 +370,23 @@ class OvnProviderDriver(driver_base.ProviderDriver):
subnet_id = member.subnet_id
if (isinstance(subnet_id, o_datamodels.UnsetType) or
not subnet_id):
pool_subnet_id = (
pool_subnet_id
if pool_subnet_id
else self._ovn_helper._get_subnet_from_pool(pool_id))
# NOTE(mjozefcz): We need to have subnet_id information.
if not pool_subnet_id:
pool_subnet_id, pool_subnet_cidr = (
self._ovn_helper._get_subnet_from_pool(pool_id))
if pool_subnet_id:
if (self._ovn_helper._check_ip_in_subnet(
member.address, pool_subnet_cidr)):
member.subnet_id = pool_subnet_id
# NOTE(mjozefcz): We need to have subnet_id information.
if not member.subnet_id:
msg = _('Subnet is required, or Loadbalancer associated '
'with Pool must have a subnet, for Member '
'creation with OVN Provider Driver')
'batch update with OVN Provider Driver if it is '
'not the same as LB VIP subnet')
raise driver_exceptions.UnsupportedOptionError(
user_fault_string=msg,
operator_fault_string=msg)
member.subnet_id = pool_subnet_id
admin_state_up = member.admin_state_up
if isinstance(admin_state_up, o_datamodels.UnsetType):
admin_state_up = True

View File

@ -491,13 +491,25 @@ class OvnProviderHelper():
ovn_lb = self._find_ovn_lb_with_pool_key(pool_key)
return pool_key, ovn_lb
def _check_ip_in_subnet(self, ip, subnet):
return (netaddr.IPAddress(ip) in netaddr.IPNetwork(subnet))
def _get_subnet_from_pool(self, pool_id):
pool = self._octavia_driver_lib.get_pool(pool_id)
if not pool:
return
return None, None
lb = self._octavia_driver_lib.get_loadbalancer(pool.loadbalancer_id)
if lb and lb.vip_subnet_id:
return lb.vip_subnet_id
neutron_client = clients.get_neutron_client()
try:
subnet = neutron_client.show_subnet(lb.vip_subnet_id)
vip_subnet_cidr = subnet['subnet']['cidr']
except n_exc.NotFound:
LOG.warning('Subnet %s not found while trying to '
'fetch its data.', lb.vip_subnet_id)
return None, None
return lb.vip_subnet_id, vip_subnet_cidr
return None, None
def _execute_commands(self, commands):
with self.ovn_nbdb_api.transaction(check_error=True) as txn:

View File

@ -62,12 +62,14 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase,
self.fake_neutron_client.show_port = self._mock_show_port
self.fake_neutron_client.delete_port.return_value = True
self._local_net_cache = {}
self._local_cidr_cache = {}
self._local_port_cache = {'ports': []}
self.core_plugin = directory.get_plugin()
def _mock_show_subnet(self, subnet_id):
subnet = {}
subnet['network_id'] = self._local_net_cache[subnet_id]
subnet['cidr'] = self._local_cidr_cache[subnet_id]
return {'subnet': subnet}
def _mock_list_ports(self, **kwargs):
@ -253,6 +255,7 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase,
cidr)
subnet = self.deserialize(self.fmt, res)['subnet']
self._local_net_cache[subnet['id']] = n1['network']['id']
self._local_cidr_cache[subnet['id']] = subnet['cidr']
port = self._make_port(self.fmt, n1['network']['id'])
if router_id:

View File

@ -227,7 +227,11 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
self.mock_get_subnet_from_pool = mock.patch.object(
ovn_helper.OvnProviderHelper,
'_get_subnet_from_pool',
return_value=None).start()
return_value=(None, None)).start()
self.mock_check_ip_in_subnet = mock.patch.object(
ovn_helper.OvnProviderHelper,
'_check_ip_in_subnet',
return_value=True).start()
def test_check_for_allowed_cidrs_exception(self):
self.assertRaises(exceptions.UnsupportedOptionError,
@ -301,7 +305,18 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
def test_member_create_no_subnet_provided_get_from_pool(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id)
self.ref_member.subnet_id, '198.52.100.0/24')
self.driver._ovn_helper._check_ip_in_subnet.return_value = False
self.ref_member.subnet_id = data_models.UnsetType()
self.assertRaises(exceptions.UnsupportedOptionError,
self.driver.member_create, self.ref_member)
self.ref_member.subnet_id = None
self.assertRaises(exceptions.UnsupportedOptionError,
self.driver.member_create, self.ref_member)
def test_member_create_no_subnet_provided_get_from_pool_failed(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id, '198.52.100.0/24')
member = copy.copy(self.ref_member)
member.subnet_id = data_models.UnsetType()
self._test_member_create(member)
@ -349,7 +364,7 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
def test_member_update_missing_subnet_id(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id)
self.ref_member.subnet_id, '198.52.100.0/24')
info = {'id': self.update_member.member_id,
'address': self.ref_member.address,
'protocol_port': self.ref_member.protocol_port,
@ -366,7 +381,7 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
def test_member_update_unset_admin_state_up(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id)
self.ref_member.subnet_id, '198.52.100.0/24')
self.update_member.admin_state_up = data_models.UnsetType()
info = {'id': self.update_member.member_id,
'address': self.ref_member.address,
@ -428,10 +443,19 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
def test_member_batch_update_missing_subnet_id_get_from_pool(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id)
self.ref_member.subnet_id, '198.52.100.0/24')
self.ref_member.subnet_id = None
self.driver.member_batch_update(self.pool_id, [self.ref_member])
def test_member_batch_update_missing_subnet_id_get_from_pool_fail(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id, '198.52.100.0/24')
self.driver._ovn_helper._check_ip_in_subnet.return_value = False
self.ref_member.subnet_id = None
self.assertRaises(exceptions.UnsupportedOptionError,
self.driver.member_batch_update,
self.pool_id, [self.ref_member])
def test_member_update_failure(self):
self.assertRaises(exceptions.UnsupportedOptionError,
self.driver.member_update, self.ref_member,
@ -468,7 +492,7 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
def test_member_delete_missing_subnet_id(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id)
self.ref_member.subnet_id, '198.52.100.0/24')
info = {'id': self.ref_member.member_id,
'address': self.ref_member.address,
'protocol_port': self.ref_member.protocol_port,

View File

@ -374,7 +374,11 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
found = f(self.ovn_lb.id, protocol='tcp')
self.assertEqual(found, self.ovn_lb)
def test__get_subnet_from_pool(self):
@mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
def test__get_subnet_from_pool(self, net_cli):
net_cli.return_value.show_subnet.return_value = {
'subnet': {'cidr': '10.22.33.0/24'}}
f = self.helper._get_subnet_from_pool
lb = data_models.LoadBalancer(
@ -393,12 +397,12 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
with mock.patch.object(self.helper, '_octavia_driver_lib') as dlib:
dlib.get_pool.return_value = None
found = f('not_found')
self.assertIsNone(found)
self.assertEqual((None, None), found)
dlib.get_pool.return_value = lb_pool
dlib.get_loadbalancer.return_value = lb
found = f(self.pool_id)
self.assertEqual(found, lb.vip_subnet_id)
self.assertEqual(found, (lb.vip_subnet_id, '10.22.33.0/24'))
def test__get_subnet_from_pool_lb_no_vip_subnet_id(self):
f = self.helper._get_subnet_from_pool
@ -418,12 +422,12 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
with mock.patch.object(self.helper, '_octavia_driver_lib') as dlib:
dlib.get_pool.return_value = None
found = f('not_found')
self.assertIsNone(found)
self.assertEqual((None, None), found)
dlib.get_pool.return_value = lb_pool
dlib.get_loadbalancer.return_value = lb
found = f(self.pool_id)
self.assertIsNone(found)
self.assertEqual((None, None), found)
def test__get_or_create_ovn_lb_no_lb_found(self):
self.mock_find_ovn_lbs.stop()