openstack/oslo.config
Code Issues Proposed changes

Check config default value is correct type

Browse Source 
Given a type of Opt, check that the default value supplied to the Opt is
of the same type as the Opt.

E.g. cfg.StrOpt('user_attribute_ignore', default=99) will log a warning
as the default is not of type string.

Closes-Bug: 1261792
Change-Id: I12cd5f4be82ac5ed0ebb348b358dd8eaf04e4e0d
This commit is contained in:
Tom Cammann 2014-07-08 10:54:15 +01:00
parent e82f6bb933
commit f7c54d9ae2
4 changed files with 80 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
oslo/config/cfg.py
View File

@ -661,6 +661,27 @@ class Opt(object):
if deprecated_name is not None or deprecated_group is not None:
self.deprecated_opts.append(DeprecatedOpt(deprecated_name,
group=deprecated_group))
self._assert_default_is_of_opt_type()
def _default_is_ref(self):
"""Check if default is a reference to another var."""
if isinstance(self.default, six.string_types):
tmpl = self.default.replace('\$', '').replace('$$', '')
return '$' in tmpl
return False
def _assert_default_is_of_opt_type(self):
if (self.default is not None
and not self._default_is_ref()
and hasattr(self.type, 'is_base_type')
and not self.type.is_base_type(self.default)):
# NOTE(tcammann) Change this to raise error after K relase
expected_types = ", ".join(
[t.__name__ for t in self.type.BASE_TYPES])
LOG.debug(('Expected default value of type(s) {0} but got '
'"{1}" of type {2}').format(expected_types,
self.default,
type(self.default).__name__))
def __ne__(self, another):
return vars(self) != vars(another)
@ -999,11 +1020,11 @@ class MultiOpt(Opt):
class MultiStrOpt(MultiOpt):
"""Multi opt with String item type (for backward compatibility)."""
"""Multi opt with MultiString item type (for backward compatibility)."""
def __init__(self, name, **kwargs):
super(MultiStrOpt, self).__init__(name,
item_type=types.String(),
item_type=types.MultiString(),
**kwargs)

45
oslo/config/types.py
View File

@ -19,9 +19,18 @@ Use these classes as values for the `type` argument to
"""
import netaddr
import six
class String(object):
class ConfigType(object):
BASE_TYPES = (None,)
def is_base_type(self, other):
return isinstance(other, self.BASE_TYPES)
class String(ConfigType):
"""String type.
@ -35,6 +44,8 @@ class String(object):
container types like List.
"""
BASE_TYPES = six.string_types
def __init__(self, choices=None, quotes=False):
super(String, self).__init__()
self.choices = choices
@ -69,17 +80,23 @@ class String(object):
)
class Boolean(object):
class MultiString(String):
BASE_TYPES = six.string_types + (list,)
class Boolean(ConfigType):
"""Boolean type.
Values are case insensitive and can be set using
1/0, yes/no, true/false or on/off.
"""
TRUE_VALUES = ['true', '1', 'on', 'yes']
FALSE_VALUES = ['false', '0', 'off', 'no']
BASE_TYPES = (bool,)
def __call__(self, value):
if isinstance(value, bool):
return value
@ -99,7 +116,7 @@ class Boolean(object):
return self.__class__ == other.__class__
class Integer(object):
class Integer(ConfigType):
"""Integer type.
@ -110,6 +127,8 @@ class Integer(object):
:param max: Optional check that value is less than or equal to max
"""
BASE_TYPES = six.integer_types
def __init__(self, min=None, max=None):
super(Integer, self).__init__()
self.min = min
@ -156,10 +175,13 @@ class Integer(object):
)
class Float(object):
class Float(ConfigType):
"""Float type."""
# allow float to be set from int
BASE_TYPES = six.integer_types + (float,)
def __call__(self, value):
if isinstance(value, float):
return value
@ -173,7 +195,7 @@ class Float(object):
return self.__class__ == other.__class__
class List(object):
class List(ConfigType):
"""List type.
@ -189,6 +211,8 @@ class List(object):
:param bounds: if True, value should be inside "[" and "]" pair
"""
BASE_TYPES = (list,)
def __init__(self, item_type=None, bounds=False):
super(List, self).__init__()
@ -247,7 +271,7 @@ class List(object):
)
class Dict(object):
class Dict(ConfigType):
"""Dictionary type.
@ -260,6 +284,8 @@ class Dict(object):
:param bounds: if True, value should be inside "{" and "}" pair
"""
BASE_TYPES = (dict,)
def __init__(self, value_type=None, bounds=False):
super(Dict, self).__init__()
@ -335,7 +361,7 @@ class Dict(object):
)
class IPAddress(object):
class IPAddress(ConfigType):
"""IP address type
@ -346,7 +372,10 @@ class IPAddress(object):
"""
BASE_TYPES = six.string_types
def __init__(self, version=None):
super(IPAddress, self).__init__()
version_checkers = {
None: self._check_both_versions,
4: self._check_ipv4,

40
tests/test_cfg.py
View File

@ -368,6 +368,9 @@ class CliOptsTestCase(BaseTestCase):
('float_arg_deprecated_group_and_name',
dict(opt_class=cfg.FloatOpt, default=None,
cli_args=['--old-oof', '2.0'], value=2.0, deps=('oof', 'old'))),
('float_default_as_integer',
dict(opt_class=cfg.FloatOpt, default=2,
cli_args=['--old-oof', '2.0'], value=2.0, deps=('oof', 'old'))),
('ipv4addr_arg',
dict(opt_class=IPv4Opt, default=None,
cli_args=['--foo', '192.168.0.1'], value='192.168.0.1',
@ -849,17 +852,10 @@ class ConfigFileOptsTestCase(BaseTestCase):
self.assertTrue(hasattr(self.conf, 'foo'))
self.assertEqual(self.conf.foo, 666)
def test_conf_file_int_wrong_default(self):
self.conf.register_opt(cfg.IntOpt('foo', default='t666'))
paths = self.create_tempfiles([('test',
'[DEFAULT]\n')])
self.conf(['--config-file', paths[0]])
self.assertRaises(AttributeError,
getattr,
self.conf,
'foo')
@mock.patch.object(cfg, 'LOG')
def test_conf_file_int_wrong_default(self, mock_log):
cfg.IntOpt('foo', default='666')
mock_log.debug.assert_call_count(1)
def test_conf_file_int_value(self):
self.conf.register_opt(cfg.IntOpt('foo'))
@ -922,17 +918,10 @@ class ConfigFileOptsTestCase(BaseTestCase):
self.assertTrue(hasattr(self.conf, 'foo'))
self.assertEqual(self.conf.foo, 6.66)
def test_conf_file_float_default_wrong_type(self):
self.conf.register_opt(cfg.FloatOpt('foo', default='foobar6.66'))
paths = self.create_tempfiles([('test',
'[DEFAULT]\n')])
self.conf(['--config-file', paths[0]])
self.assertRaises(AttributeError,
getattr,
self.conf,
'foo')
@mock.patch.object(cfg, 'LOG')
def test_conf_file_float_default_wrong_type(self, mock_log):
cfg.FloatOpt('foo', default='foobar6.66')
mock_log.debug.assert_call_count(1)
def test_conf_file_float_value(self):
self.conf.register_opt(cfg.FloatOpt('foo'))
@ -995,6 +984,13 @@ class ConfigFileOptsTestCase(BaseTestCase):
self.assertTrue(hasattr(self.conf, 'foo'))
self.assertEqual(self.conf.foo, ['bar'])
@mock.patch.object(cfg, 'LOG')
def test_conf_file_list_default_wrong_type(self, mock_log):
cfg.ListOpt('foo', default=25)
mock_log.debug.assert_called_once_with(
'Expected default value of type(s) list but '
'got "25" of type int')
def test_conf_file_list_value(self):
self.conf.register_opt(cfg.ListOpt('foo'))

3
tests/test_generator.py
View File

@ -55,8 +55,9 @@ class GeneratorTestCase(base.BaseTestCase):
deprecated_group='group1',
deprecated_name='foobar',
help='deprecated'),
# Unknown Opt default must be a string
'unknown_type': cfg.Opt('unknown_opt',
default=123,
default='123',
help='unknown'),
'str_opt': cfg.StrOpt('str_opt',
default='foo bar',