In some cases there is a need for a blank value of the "router" extra
dhcp option. This fix addresses this issue by introducing a simple
mechanism in the code that allows specific extra dhcp options to be left
blank by applying a different opt_value validation of the data received
from the REST call and different validation on the value that will be
written to the DB.
This fix also takes into consideration bug #
1257467 which claims that
in case of a blank "server-ip-address" option a segmentation fault
occurs in dnsmasq. I did not check this claim with a newer dnsmasq
version since it seemed logical that the list of potentially blank
options should be limited to options that are known to work well when
blank and have functional justification for being blank (e.g. "router")
APIImpact
"router" and "classless-static-route" extra dhcp options can be blank
(e.g. opt-name="router", opt-value="")
DocImpact
During port creation/update, Specific extra-dhcp-options can be left
blank ("router" and/or "classless-static-route"). This causes dnsmasq
to have an empty option in the "opts" file related to the network to
which the port is related.
For example:
tag:tag0,option:classless-static-route,
tag:tag0,option:router,
Closes-Bug: #
1437695
Related-Bug: #
1257467
Change-Id: I0ac76e132c4bd86da39863674d4cf8a22dad7034
"""Mixin class to add extra options to the DHCP opts file
and associate them to a port.
"""
+
+ def _is_valid_opt_value(self, opt_name, opt_value):
+
+ # If the dhcp opt is blank-able, it shouldn't be saved to the DB in
+ # case that the value is None
+ if opt_name in edo_ext.VALID_BLANK_EXTRA_DHCP_OPTS:
+ return opt_value is not None
+
+ # Otherwise, it shouldn't be saved to the DB in case that the value
+ # is None or empty
+ return bool(opt_value)
+
def _process_port_create_extra_dhcp_opts(self, context, port,
extra_dhcp_opts):
if not extra_dhcp_opts:
return port
with context.session.begin(subtransactions=True):
for dopt in extra_dhcp_opts:
- if dopt['opt_value']:
+ if self._is_valid_opt_value(dopt['opt_name'],
+ dopt['opt_value']):
ip_version = dopt.get('ip_version', 4)
db = ExtraDhcpOpt(
port_id=port['id'],
if upd_rec['opt_value'] is None:
context.session.delete(opt)
else:
- if opt['opt_value'] != upd_rec['opt_value']:
+ if (self._is_valid_opt_value(
+ opt['opt_name'],
+ upd_rec['opt_value']) and
+ opt['opt_value'] !=
+ upd_rec['opt_value']):
opt.update(
{'opt_value': upd_rec['opt_value']})
break
else:
- if upd_rec['opt_value'] is not None:
+ if self._is_valid_opt_value(
+ upd_rec['opt_name'],
+ upd_rec['opt_value']):
ip_version = upd_rec.get('ip_version', 4)
db = ExtraDhcpOpt(
port_id=id,
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
-# http://www.apache.org/licenses/LICENSE-2.0
+# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
message = _("Invalid data format for extra-dhcp-opt: %(data)s")
-def _validate_list_of_dict_or_none(data, key_specs=None):
+# Valid blank extra dhcp opts
+VALID_BLANK_EXTRA_DHCP_OPTS = ('router', 'classless-static-route')
+
+# Common definitions for maximum string field length
+DHCP_OPT_NAME_MAX_LEN = 64
+DHCP_OPT_VALUE_MAX_LEN = 255
+
+EXTRA_DHCP_OPT_KEY_SPECS = {
+ 'id': {'type:uuid': None, 'required': False},
+ 'opt_name': {'type:not_empty_string': DHCP_OPT_NAME_MAX_LEN,
+ 'required': True},
+ 'opt_value': {'type:not_empty_string_or_none': DHCP_OPT_VALUE_MAX_LEN,
+ 'required': True},
+ 'ip_version': {'convert_to': attr.convert_to_int,
+ 'type:values': [4, 6],
+ 'required': False}
+}
+
+
+def _validate_extra_dhcp_opt(data, key_specs=None):
if data is not None:
if not isinstance(data, list):
raise ExtraDhcpOptBadData(data=data)
for d in data:
- msg = attr._validate_dict(d, key_specs)
+ if d['opt_name'] in VALID_BLANK_EXTRA_DHCP_OPTS:
+ msg = attr._validate_string_or_none(d['opt_value'],
+ DHCP_OPT_VALUE_MAX_LEN)
+ else:
+ msg = attr._validate_dict(d, key_specs)
if msg:
raise ExtraDhcpOptBadData(data=msg)
-attr.validators['type:list_of_dict_or_none'] = _validate_list_of_dict_or_none
+attr.validators['type:list_of_extra_dhcp_opts'] = _validate_extra_dhcp_opt
# Attribute Map
EXTRADHCPOPTS = 'extra_dhcp_opts'
-# Common definitions for maximum string field length
-DHCP_OPT_NAME_MAX_LEN = 64
-DHCP_OPT_VALUE_MAX_LEN = 255
-
CLIENT_ID = "client-id"
EXTENDED_ATTRIBUTES_2_0 = {
'ports': {
- EXTRADHCPOPTS:
- {'allow_post': True,
- 'allow_put': True,
- 'is_visible': True,
- 'default': None,
- 'validate': {
- 'type:list_of_dict_or_none': {
- 'id': {'type:uuid': None, 'required': False},
- 'opt_name': {'type:not_empty_string': DHCP_OPT_NAME_MAX_LEN,
- 'required': True},
- 'opt_value': {'type:not_empty_string_or_none':
- DHCP_OPT_VALUE_MAX_LEN,
- 'required': True},
- 'ip_version': {'convert_to': attr.convert_to_int,
- 'type:values': [4, 6],
- 'required': False}}}}}}
+ EXTRADHCPOPTS: {
+ 'allow_post': True,
+ 'allow_put': True,
+ 'is_visible': True,
+ 'default': None,
+ 'validate': {
+ 'type:list_of_extra_dhcp_opts': EXTRA_DHCP_OPT_KEY_SPECS
+ }
+ }
+ }
+}
class Extra_dhcp_opt(extensions.ExtensionDescriptor):
self._check_opts(expected,
port['port'][edo_ext.EXTRADHCPOPTS])
+ def test_create_port_with_empty_router_extradhcpopts(self):
+ opt_list = [{'opt_name': 'router',
+ 'opt_value': ''},
+ {'opt_name': 'server-ip-address',
+ 'opt_value': '123.123.123.456'},
+ {'opt_name': 'tftp-server',
+ 'opt_value': '123.123.123.123'}]
+
+ params = {edo_ext.EXTRADHCPOPTS: opt_list,
+ 'arg_list': (edo_ext.EXTRADHCPOPTS,)}
+
+ with self.port(**params) as port:
+ self._check_opts(opt_list,
+ port['port'][edo_ext.EXTRADHCPOPTS])
+
def test_create_port_with_extradhcpopts_ipv4_opt_version(self):
opt_list = [{'opt_name': 'bootfile-name',
'opt_value': 'pxelinux.0',
res = req.get_response(self.api)
self.assertEqual(res.status_int, webob.exc.HTTPBadRequest.code)
+ def test_update_port_with_blank_router_extradhcpopt(self):
+ opt_list = [{'opt_name': 'bootfile-name',
+ 'opt_value': 'pxelinux.0',
+ 'ip_version': 4},
+ {'opt_name': 'tftp-server',
+ 'opt_value': '123.123.123.123',
+ 'ip_version': 4},
+ {'opt_name': 'router',
+ 'opt_value': '123.123.123.1',
+ 'ip_version': 4}]
+ upd_opts = [{'opt_name': 'router',
+ 'opt_value': '',
+ 'ip_version': 4}]
+ expected_opts = copy.deepcopy(opt_list)
+ for i in expected_opts:
+ if i['opt_name'] == upd_opts[0]['opt_name']:
+ i['opt_value'] = upd_opts[0]['opt_value']
+ break
+
+ self._test_update_port_with_extradhcpopts(opt_list, upd_opts,
+ expected_opts)
+
def test_update_port_with_extradhcpopts_ipv6_change_value(self):
opt_list = [{'opt_name': 'bootfile-name',
'opt_value': 'pxelinux.0',
Code Review / openstack-build / neutron-build.git / commitdiff