Skip to content

Commit 93cf601

Browse files
Zuulopenstack-gerrit
Zuul
authored and
openstack-gerrit
committed
Merge "Disallow subnet cidr of :: without PD" into stable/2023.1
2 parents 69f64a1 + 45acc0c commit 93cf601

File tree

4 files changed

+59
-28
lines changed

4 files changed

+59
-28
lines changed

‎doc/source/admin/config-ipv6.rst‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -686,18 +686,19 @@ First, create a network and IPv6 subnet:
686686
+---------------------------+--------------------------------------+
687687
688688
$ openstack subnet create --ip-version 6 --ipv6-ra-mode slaac \
689-
--ipv6-address-mode slaac --use-default-subnet-pool \
689+
--ipv6-address-mode slaac --use-prefix-delegation \
690690
--network ipv6-pd ipv6-pd-1
691691
+------------------------+--------------------------------------+
692692
| Field | Value |
693693
+------------------------+--------------------------------------+
694-
| allocation_pools | ::2-::ffff:ffff:ffff:ffff |
694+
| allocation_pools | ::1-::ffff:ffff:ffff:ffff |
695695
| cidr | ::/64 |
696696
| created_at | 2017-01-25T19:31:53Z |
697697
| description | |
698698
| dns_nameservers | |
699+
| dns_publish_fixed_ip | None |
699700
| enable_dhcp | True |
700-
| gateway_ip | ::1 |
701+
| gateway_ip | :: |
701702
| headers | |
702703
| host_routes | |
703704
| id | 1319510d-c92c-4532-bf5d-8bcf3da761a1 |
@@ -710,9 +711,8 @@ First, create a network and IPv6 subnet:
710711
| revision_number | 2 |
711712
| service_types | |
712713
| subnetpool_id | prefix_delegation |
713-
| tags | [] |
714+
| tags | |
714715
| updated_at | 2017-01-25T19:31:53Z |
715-
| use_default_subnetpool | True |
716716
+------------------------+--------------------------------------+
717717
718718
The subnet is initially created with a temporary CIDR before one can be

‎neutron/db/db_base_plugin_v2.py‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -595,7 +595,7 @@ def _validate_ip_version(self, ip_version, addr, name):
595595
"the ip_version '%(ip_version)s'") % data
596596
raise exc.InvalidInput(error_message=msg)
597597

598-
def _validate_subnet(self, context, s, cur_subnet=None):
598+
def _validate_subnet(self, context, s, cur_subnet=None, is_pd=False):
599599
"""Validate a subnet spec."""
600600

601601
# This method will validate attributes which may change during
@@ -613,6 +613,7 @@ def _validate_subnet(self, context, s, cur_subnet=None):
613613
has_cidr = False
614614
if validators.is_attr_set(s.get('cidr')):
615615
self._validate_ip_version(ip_ver, s['cidr'], 'cidr')
616+
net = netaddr.IPNetwork(s['cidr'])
616617
has_cidr = True
617618

618619
# TODO(watanabe.isao): After we found a way to avoid the re-sync
@@ -621,15 +622,21 @@ def _validate_subnet(self, context, s, cur_subnet=None):
621622
dhcp_was_enabled = cur_subnet.enable_dhcp
622623
else:
623624
dhcp_was_enabled = False
625+
# A subnet cidr of '::' is invalid, unless the caller has
626+
# indicated they are doing Prefix Delegation,
627+
# see https://bugs.launchpad.net/neutron/+bug/2028159
628+
if (has_cidr and ip_ver == constants.IP_VERSION_6 and
629+
net.network == netaddr.IPAddress('::') and not
630+
is_pd):
631+
error_message = _("IPv6 subnet '::' is not supported")
632+
raise exc.InvalidInput(error_message=error_message)
624633
if has_cidr and s.get('enable_dhcp') and not dhcp_was_enabled:
625-
subnet_prefixlen = netaddr.IPNetwork(s['cidr']).prefixlen
626634
error_message = _("Subnet has a prefix length that is "
627635
"incompatible with DHCP service enabled")
628-
if ((ip_ver == 4 and subnet_prefixlen > 30) or
629-
(ip_ver == 6 and subnet_prefixlen > 126)):
636+
if ((ip_ver == 4 and net.prefixlen > 30) or
637+
(ip_ver == 6 and net.prefixlen > 126)):
630638
raise exc.InvalidInput(error_message=error_message)
631639

632-
net = netaddr.IPNetwork(s['cidr'])
633640
if net.is_multicast():
634641
error_message = _("Multicast IP subnet is not supported "
635642
"if enable_dhcp is True")
@@ -874,9 +881,11 @@ def _create_subnet_precommit(self, context, subnet):
874881
raise exc.BadRequest(resource='subnets', msg=msg)
875882

876883
validate = True
884+
is_pd = False
877885
if subnetpool_id:
878886
self.ipam.validate_pools_with_subnetpool(s)
879887
if subnetpool_id == constants.IPV6_PD_POOL_ID:
888+
is_pd = True
880889
if has_cidr:
881890
# We do not currently support requesting a specific
882891
# cidr with IPv6 prefix delegation. Set the subnetpool_id
@@ -894,7 +903,7 @@ def _create_subnet_precommit(self, context, subnet):
894903
raise exc.BadRequest(resource='subnets', msg=msg)
895904

896905
if validate:
897-
self._validate_subnet(context, s)
906+
self._validate_subnet(context, s, is_pd=is_pd)
898907

899908
with db_api.CONTEXT_WRITER.using(context):
900909
network = self._get_network(context,
@@ -950,7 +959,8 @@ def _update_subnet_precommit(self, context, id, subnet):
950959
# Fill 'network_id' field with the current value since this is expected
951960
# by _validate_segment() in ipam_pluggable_backend.
952961
s['network_id'] = subnet_obj.network_id
953-
self._validate_subnet(context, s, cur_subnet=subnet_obj)
962+
is_pd = s['subnetpool_id'] == constants.IPV6_PD_POOL_ID
963+
self._validate_subnet(context, s, cur_subnet=subnet_obj, is_pd=is_pd)
954964
db_pools = [netaddr.IPRange(p.start, p.end)
955965
for p in subnet_obj.allocation_pools]
956966

‎neutron/tests/unit/db/test_db_base_plugin_v2.py‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ def _create_subnet(self, fmt, net_id, cidr,
397397
data = {'subnet': {'network_id': net_id,
398398
'ip_version': constants.IP_VERSION_4,
399399
'tenant_id': self._tenant_id}}
400-
if cidr:
400+
if cidr and cidr is not constants.ATTR_NOT_SPECIFIED:
401401
data['subnet']['cidr'] = cidr
402402
for arg in ('ip_version', 'tenant_id', 'subnetpool_id', 'prefixlen',
403403
'enable_dhcp', 'allocation_pools', 'segment_id',
@@ -774,7 +774,9 @@ def subnet(self, network=None,
774774
set_context=False):
775775
if project_id:
776776
tenant_id = project_id
777-
cidr = netaddr.IPNetwork(cidr) if cidr else None
777+
if (cidr is not None and
778+
cidr != constants.ATTR_NOT_SPECIFIED):
779+
cidr = netaddr.IPNetwork(cidr)
778780
if (gateway_ip is not None and
779781
gateway_ip != constants.ATTR_NOT_SPECIFIED):
780782
gateway_ip = netaddr.IPAddress(gateway_ip)
@@ -947,7 +949,7 @@ def _validate_resource(self, resource, keys, res_name):
947949
if (k == 'gateway_ip' and ipv6_zero_gateway and
948950
keys[k][-3:] == "::0"):
949951
self.assertEqual(keys[k][:-1], resource[res_name][k])
950-
else:
952+
elif keys[k] != constants.ATTR_NOT_SPECIFIED:
951953
self.assertEqual(keys[k], resource[res_name][k])
952954

953955

@@ -3265,7 +3267,6 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
32653267

32663268
def _test_create_subnet(self, network=None, expected=None, **kwargs):
32673269
keys = kwargs.copy()
3268-
keys.setdefault('cidr', '10.0.0.0/24')
32693270
keys.setdefault('ip_version', constants.IP_VERSION_4)
32703271
keys.setdefault('enable_dhcp', True)
32713272
with self.subnet(network=network, **keys) as subnet:
@@ -4005,17 +4006,25 @@ def test_create_subnet_ipv6_gw_values(self):
40054006
@testtools.skipIf(tools.is_bsd(), 'bug/1484837')
40064007
def test_create_subnet_ipv6_pd_gw_values(self):
40074008
cidr = constants.PROVISIONAL_IPV6_PD_PREFIX
4009+
subnetpool_id = constants.IPV6_PD_POOL_ID
40084010
# Gateway is last IP in IPv6 DHCPv6 Stateless subnet
40094011
gateway = '::ffff:ffff:ffff:ffff'
40104012
allocation_pools = [{'start': '::1',
40114013
'end': '::ffff:ffff:ffff:fffe'}]
40124014
expected = {'gateway_ip': gateway,
40134015
'cidr': cidr,
40144016
'allocation_pools': allocation_pools}
4017+
# We do not specify a CIDR in the API call for a PD subnet, as it
4018+
# is unsupported. Instead we specify the subnetpool_id as
4019+
# "prefix_delegation" which is what happens via OSC's
4020+
# --use-prefix-delegation argument. But the expected result is a
4021+
# subnet object with the "::/64" PD prefix. Same comment applies below.
40154022
self._test_create_subnet(expected=expected, gateway_ip=gateway,
4016-
cidr=cidr, ip_version=constants.IP_VERSION_6,
4023+
cidr=constants.ATTR_NOT_SPECIFIED,
4024+
ip_version=constants.IP_VERSION_6,
40174025
ipv6_ra_mode=constants.DHCPV6_STATELESS,
4018-
ipv6_address_mode=constants.DHCPV6_STATELESS)
4026+
ipv6_address_mode=constants.DHCPV6_STATELESS,
4027+
subnetpool_id=subnetpool_id)
40194028
# Gateway is first IP in IPv6 DHCPv6 Stateless subnet
40204029
gateway = '::1'
40214030
allocation_pools = [{'start': '::2',
@@ -4024,18 +4033,22 @@ def test_create_subnet_ipv6_pd_gw_values(self):
40244033
'cidr': cidr,
40254034
'allocation_pools': allocation_pools}
40264035
self._test_create_subnet(expected=expected, gateway_ip=gateway,
4027-
cidr=cidr, ip_version=constants.IP_VERSION_6,
4036+
cidr=constants.ATTR_NOT_SPECIFIED,
4037+
ip_version=constants.IP_VERSION_6,
40284038
ipv6_ra_mode=constants.DHCPV6_STATELESS,
4029-
ipv6_address_mode=constants.DHCPV6_STATELESS)
4039+
ipv6_address_mode=constants.DHCPV6_STATELESS,
4040+
subnetpool_id=subnetpool_id)
40304041
# If gateway_ip is not specified and the subnet is using prefix
40314042
# delegation, until the CIDR is assigned, this value should be first
40324043
# IP from the subnet
40334044
expected = {'gateway_ip': str(netaddr.IPNetwork(cidr).network),
40344045
'cidr': cidr}
40354046
self._test_create_subnet(expected=expected,
4036-
cidr=cidr, ip_version=constants.IP_VERSION_6,
4047+
cidr=constants.ATTR_NOT_SPECIFIED,
4048+
ip_version=constants.IP_VERSION_6,
40374049
ipv6_ra_mode=constants.IPV6_SLAAC,
4038-
ipv6_address_mode=constants.IPV6_SLAAC)
4050+
ipv6_address_mode=constants.IPV6_SLAAC,
4051+
subnetpool_id=subnetpool_id)
40394052

40404053
def test_create_subnet_gw_outside_cidr_returns_201(self):
40414054
with self.network() as network:
@@ -4132,14 +4145,21 @@ def test_create_subnet_with_v6_allocation_pool(self):
41324145
allocation_pools=allocation_pools)
41334146

41344147
@testtools.skipIf(tools.is_bsd(), 'bug/1484837')
4135-
def test_create_subnet_with_v6_pd_allocation_pool(self):
4148+
def test_create_subnet_with_v6_pd_allocation_pool_returns_400(self):
41364149
gateway_ip = '::1'
41374150
cidr = constants.PROVISIONAL_IPV6_PD_PREFIX
41384151
allocation_pools = [{'start': '::2',
41394152
'end': '::ffff:ffff:ffff:fffe'}]
4140-
self._test_create_subnet(gateway_ip=gateway_ip,
4141-
cidr=cidr, ip_version=constants.IP_VERSION_6,
4142-
allocation_pools=allocation_pools)
4153+
# Creating a subnet object with the "::/64" PD prefix is invalid
4154+
# unless the subnetpool_id is also passed as "prefix_delegation"
4155+
with testlib_api.ExpectedException(
4156+
webob.exc.HTTPClientError) as ctx_manager:
4157+
self._test_create_subnet(gateway_ip=gateway_ip,
4158+
cidr=cidr,
4159+
ip_version=constants.IP_VERSION_6,
4160+
allocation_pools=allocation_pools)
4161+
self.assertEqual(webob.exc.HTTPClientError.code,
4162+
ctx_manager.exception.code)
41434163

41444164
def test_create_subnet_with_large_allocation_pool(self):
41454165
gateway_ip = '10.0.0.1'
@@ -4350,10 +4370,10 @@ def _test_validate_subnet_ipv6_pd_modes(self, cur_subnet=None,
43504370
for mode, value in modes.items():
43514371
new_subnet[mode] = value
43524372
if expect_success:
4353-
plugin._validate_subnet(ctx, new_subnet, cur_subnet)
4373+
plugin._validate_subnet(ctx, new_subnet, cur_subnet, True)
43544374
else:
43554375
self.assertRaises(lib_exc.InvalidInput, plugin._validate_subnet,
4356-
ctx, new_subnet, cur_subnet)
4376+
ctx, new_subnet, cur_subnet, True)
43574377

43584378
def test_create_subnet_ipv6_ra_modes(self):
43594379
# Test all RA modes with no address mode specified

‎neutron/tests/unit/db/test_ipam_pluggable_backend.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ def test_test_fixed_ips_for_port_pd_gateway(self):
374374
context = mock.Mock()
375375
pluggable_backend = ipam_pluggable_backend.IpamPluggableBackend()
376376
with self.subnet(cidr=constants.PROVISIONAL_IPV6_PD_PREFIX,
377+
subnetpool_id=constants.IPV6_PD_POOL_ID,
377378
ip_version=constants.IP_VERSION_6) as subnet:
378379
subnet = subnet['subnet']
379380
fixed_ips = [{'subnet_id': subnet['id'],

0 commit comments

Comments
 (0)