x/vmware-nsx
Code Issues Proposed changes

NSX|V3 Add validations to DHCP relay

Browse Source 
1. add relay service only if the subnet is with dhcp
2. do not add native dhcp if dhcp relay is configured for this subnet
3. do not allow router creation with dhcp relay and without IPAM
4. do not allow creation of VM port from a network with dhcp relay and
without a router.

Change-Id: I05fa71f69ded69ea58a4e4df0a1f20c963cb3fc5
This commit is contained in:
Adit Sarfaty 2017-09-18 14:10:19 +03:00
parent fc60d37c7d
commit 8a9aac3917
3 changed files with 177 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
vmware_nsx/plugins/nsx_v3/availability_zones.py
View File

@ -201,3 +201,9 @@ class NsxV3AvailabilityZones(common_az.ConfiguredAvailabilityZones):
super(NsxV3AvailabilityZones, self).__init__(
cfg.CONF.nsx_v3.availability_zones,
NsxV3AvailabilityZone)
def dhcp_relay_configured(self):
for az in self.availability_zones.values():
if az.dhcp_relay_service:
return True
return False

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

@ -1382,7 +1382,6 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
def create_subnet(self, context, subnet):
self._validate_address_space(subnet['subnet'])
# TODO(berlin): public external subnet announcement
if (cfg.CONF.nsx_v3.native_dhcp_metadata and
subnet['subnet'].get('enable_dhcp', False)):
@ -1399,8 +1398,12 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
NsxV3Plugin, self).create_subnet(context, subnet)
self._extension_manager.process_create_subnet(context,
subnet['subnet'], created_subnet)
self._enable_native_dhcp(context, network,
created_subnet)
dhcp_relay = self.get_network_az_by_net_id(
context,
subnet['subnet']['network_id']).dhcp_relay_service
if not dhcp_relay:
self._enable_native_dhcp(context, network,
created_subnet)
msg = None
else:
msg = (_("Can not create more than one DHCP-enabled "
@ -1849,6 +1852,52 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
LOG.warning(err_msg)
raise n_exc.InvalidInput(error_message=err_msg)
def _assert_on_dhcp_relay_without_router(self, context, port_data,
original_port=None):
# Prevent creating/updating port with device owner prefix 'compute'
# on a subnet with dhcp relay but no router.
if not original_port:
original_port = port_data
device_owner = port_data.get('device_owner')
if (device_owner is None or
not device_owner.startswith(const.DEVICE_OWNER_COMPUTE_PREFIX)):
# not a compute port
return
if not self.get_network_az_by_net_id(
context,
original_port['network_id']).dhcp_relay_service:
# No dhcp relay for the net of this port
return
# get the subnet id from the fixed ips of the port
if 'fixed_ips' in port_data and port_data['fixed_ips']:
subnet_id = port_data['fixed_ips'][0]['subnet_id']
elif 'fixed_ips' in original_port and original_port['fixed_ips']:
subnet_id = original_port['fixed_ips'][0]['subnet_id']
else:
return
# check only dhcp enabled subnets
subnet = self.get_subnet(context.elevated(), subnet_id)
if not subnet['enable_dhcp']:
return
# check if the subnet is attached to a router
port_filters = {'device_owner': [l3_db.DEVICE_OWNER_ROUTER_INTF],
'network_id': [original_port['network_id']]}
interfaces = self.get_ports(context.elevated(), filters=port_filters)
router_found = False
for interface in interfaces:
if interface['fixed_ips'][0]['subnet_id'] == subnet_id:
router_found = True
break
if not router_found:
err_msg = _("Neutron is configured with DHCP_Relay but no router "
"connected to the subnet")
LOG.warning(err_msg)
raise n_exc.InvalidInput(error_message=err_msg)
def _cleanup_port(self, context, port_id, lport_id):
super(NsxV3Plugin, self).delete_port(context, port_id)
if lport_id:
@ -2220,6 +2269,7 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
self._validate_extra_dhcp_options(dhcp_opts)
self._validate_max_ips_per_port(port_data.get('fixed_ips', []),
port_data.get('device_owner'))
self._assert_on_dhcp_relay_without_router(context, port_data)
# TODO(salv-orlando): Undo logical switch creation on failure
with db_api.context_manager.writer.using(context):
@ -2662,6 +2712,8 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
if is_external_net:
self._assert_on_external_net_with_compute(port_data)
self._assert_on_external_net_port_with_qos(port_data)
self._assert_on_dhcp_relay_without_router(context, port_data,
original_port)
dhcp_opts = port_data.get(ext_edo.EXTRADHCPOPTS)
self._validate_extra_dhcp_options(dhcp_opts)
@ -3001,9 +3053,18 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
LOG.warning(err_msg)
raise n_exc.InvalidInput(error_message=err_msg)
def validate_router_dhcp_relay(self, context):
"""Fail router creation dhcp relay is configured without IPAM"""
if (self._availability_zones_data.dhcp_relay_configured() and
cfg.CONF.ipam_driver == 'internal'):
err_msg = _("Neutron is configured with DHCP_Relay but no IPAM "
"plugin configured.")
LOG.warning(err_msg)
raise n_exc.InvalidInput(error_message=err_msg)
def create_router(self, context, router):
r = router['router']
self.validate_router_dhcp_relay(context)
gw_info = self._extract_external_gw(context, router, is_extract=True)
r['id'] = (r.get('id') or uuidutils.generate_uuid())
tags = self.nsxlib.build_v3_tags_payload(
@ -3322,7 +3383,13 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
port, resource_type='os-neutron-rport-id',
project_name=context.tenant_name)
tags.append({'scope': 'os-subnet-id', 'tag': subnet['id']})
net_az = self.get_network_az_by_net_id(context, network_id)
# Add the dhcp relay service to the NSX interface
relay_service = None
if subnet['enable_dhcp']:
net_az = self.get_network_az_by_net_id(context, network_id)
relay_service = net_az.dhcp_relay_service
self._routerlib.create_logical_router_intf_port_by_ls_id(
logical_router_id=nsx_router_id,
display_name=display_name,
@ -3330,7 +3397,7 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
ls_id=nsx_net_id,
logical_switch_port_id=nsx_port_id,
address_groups=address_groups,
relay_service_uuid=net_az.dhcp_relay_service)
relay_service_uuid=relay_service)
if router_db.gw_port and not router_db.enable_snat:
# TODO(berlin): Announce the subnet on tier0 if enable_snat

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

@ -257,6 +257,18 @@ class NsxV3PluginTestCaseMixin(test_plugin.NeutronDbPluginV2TestCase,
with ctx.session.begin(subtransactions=True):
ctx.session.add(models_v2.Network(id=network_id))
def _enable_dhcp_relay(self):
# Add the relay service to the config and availability zones
cfg.CONF.set_override('native_dhcp_metadata', True, 'nsx_v3')
cfg.CONF.set_override('dhcp_relay_service', NSX_DHCP_RELAY_SRV,
'nsx_v3')
mock_nsx_version = mock.patch.object(
self.plugin.nsxlib, 'feature_supported', return_value=True)
mock_nsx_version.start()
self.plugin.init_availability_zones()
for az in self.plugin.get_azs_list():
az.translate_configured_names_to_uuids(self.plugin.nsxlib)
class TestNetworksV2(test_plugin.TestNetworksV2, NsxV3PluginTestCaseMixin):
@ -458,6 +470,34 @@ class TestSubnetsV2(test_plugin.TestSubnetsV2, NsxV3PluginTestCaseMixin):
def test_subnet_update_ipv4_and_ipv6_pd_slaac_subnets(self):
self.skipTest('Multiple fixed ips on a port are not supported')
def test_subnet_native_dhcp_subnet_enabled(self):
cfg.CONF.set_override('native_dhcp_metadata', True, 'nsx_v3')
with self.network() as network:
with mock.patch.object(self.plugin,
'_enable_native_dhcp') as enable_dhcp,\
self.subnet(network=network, enable_dhcp=True):
# Native dhcp should be set for this subnet
self.assertTrue(enable_dhcp.called)
def test_subnet_native_dhcp_subnet_disabled(self):
cfg.CONF.set_override('native_dhcp_metadata', True, 'nsx_v3')
with self.network() as network:
with mock.patch.object(self.plugin,
'_enable_native_dhcp') as enable_dhcp,\
self.subnet(network=network, enable_dhcp=False):
# Native dhcp should be set for this subnet
self.assertFalse(enable_dhcp.called)
def test_subnet_native_dhcp_with_relay(self):
"""Verify that the relay service is added to the router interface"""
self._enable_dhcp_relay()
with self.network() as network:
with mock.patch.object(self.plugin,
'_enable_native_dhcp') as enable_dhcp,\
self.subnet(network=network, enable_dhcp=True):
# Native dhcp should not be set for this subnet
self.assertFalse(enable_dhcp.called)
class TestPortsV2(test_plugin.TestPortsV2, NsxV3PluginTestCaseMixin,
test_bindings.PortBindingsTestCase,
@ -757,6 +797,29 @@ class TestPortsV2(test_plugin.TestPortsV2, NsxV3PluginTestCaseMixin,
def test_create_port_anticipating_allocation(self):
self.skipTest('Multiple fixed ips on a port are not supported')
def test_create_compute_port_with_relay_no_router(self):
"""Compute port creation should fail
if a network with dhcp relay is not connected to a router
"""
self._enable_dhcp_relay()
with self.network() as network, \
self.subnet(network=network, enable_dhcp=True) as s1:
device_owner = constants.DEVICE_OWNER_COMPUTE_PREFIX + 'X'
data = {'port': {
'network_id': network['network']['id'],
'tenant_id': self._tenant_id,
'name': 'port',
'admin_state_up': True,
'device_id': 'fake_device',
'device_owner': device_owner,
'fixed_ips': [{'subnet_id': s1['subnet']['id']}],
'mac_address': '00:00:00:00:00:01'}
}
self.assertRaises(n_exc.InvalidInput,
self.plugin.create_port,
self.ctx, data)
class DHCPOptsTestCase(test_dhcpopts.TestExtraDhcpOpt,
NsxV3PluginTestCaseMixin):
@ -1478,19 +1541,13 @@ class TestL3NatTestCase(L3NatTest,
{'router': {'admin_state_up': False}},
expected_code=exc.HTTPBadRequest.code)
def test_router_dhcp_relay(self):
# Add the relay service to the config and availability zones
cfg.CONF.set_override('dhcp_relay_service', NSX_DHCP_RELAY_SRV,
'nsx_v3')
mock_nsx_version = mock.patch.object(
self.plugin.nsxlib, 'feature_supported', return_value=True)
mock_nsx_version.start()
self.plugin.init_availability_zones()
for az in self.plugin.get_azs_list():
az.translate_configured_names_to_uuids(self.plugin.nsxlib)
def test_router_dhcp_relay_dhcp_enabled(self):
"""Verify that the relay service is added to the router interface"""
self._enable_dhcp_relay()
with self.network() as network:
with self.subnet(network=network) as s1,\
with mock.patch.object(self.plugin,
'validate_router_dhcp_relay'),\
self.subnet(network=network, enable_dhcp=True) as s1,\
self.router() as r1,\
mock.patch.object(self.plugin.nsxlib.logical_router_port,
'update') as mock_update_port:
@ -1501,6 +1558,35 @@ class TestL3NatTestCase(L3NatTest,
relay_service_uuid=NSX_DHCP_RELAY_SRV,
subnets=mock.ANY)
def test_router_dhcp_relay_dhcp_disabled(self):
"""Verify that the relay service is not added to the router interface
If the subnet do not have enabled dhcp
"""
self._enable_dhcp_relay()
with self.network() as network:
with mock.patch.object(self.plugin,
'validate_router_dhcp_relay'),\
self.subnet(network=network, enable_dhcp=False) as s1,\
self.router() as r1,\
mock.patch.object(self.plugin.nsxlib.logical_router_port,
'update') as mock_update_port:
self._router_interface_action('add', r1['router']['id'],
s1['subnet']['id'], None)
mock_update_port.assert_called_once_with(
mock.ANY,
relay_service_uuid=None,
subnets=mock.ANY)
def test_router_dhcp_relay_no_ipam(self):
"""Verify that a router cannot be created with relay and no ipam"""
# Add the relay service to the config and availability zones
self._enable_dhcp_relay()
self.assertRaises(n_exc.InvalidInput,
self.plugin_instance.create_router,
context.get_admin_context(),
{'router': {'name': 'rtr'}})
class ExtGwModeTestCase(test_ext_gw_mode.ExtGwModeIntTestCase,
L3NatTest):