From 14ec2c67eb2a9720b0cf567d3db6174626b594c9 Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Thu, 7 Apr 2016 15:07:19 +0300 Subject: [PATCH] Add Port type to allow configuration of a list of tcp/ip ports There was already a PortOpt, but this is not composable with ListOpt. The new type inherits from Integer and therefore supports min/max value and choices. For example, openstack/vmware-nsx wants to use this to configure a default list of ports for a firewall rule. Change-Id: I83bbec6add8ce2951e94e69ec14bb6d8137d4f0c --- oslo_config/cfg.py | 28 +-- oslo_config/tests/test_cfg.py | 125 +++++++++++++- oslo_config/tests/test_types.py | 161 +++++++++++++++++- oslo_config/types.py | 101 ++++++----- .../notes/add-port_type-8704295c6a56265d.yaml | 5 + 5 files changed, 351 insertions(+), 69 deletions(-) create mode 100644 releasenotes/notes/add-port_type-8704295c6a56265d.yaml diff --git a/oslo_config/cfg.py b/oslo_config/cfg.py index ebed5628..68cb8c21 100644 --- a/oslo_config/cfg.py +++ b/oslo_config/cfg.py @@ -1283,35 +1283,21 @@ class PortOpt(Opt): :param name: the option's name :param choices: Optional sequence of valid values. :param \*\*kwargs: arbitrary keyword arguments passed to :class:`Opt` + :param min: minimum value the port can take + :param max: maximum value the port can take .. versionadded:: 2.6 .. versionchanged:: 3.2 Added *choices* parameter. .. versionchanged:: 3.4 Allow port number with 0. + .. versionchanged:: 3.16 + Added *min* and *max* parameters. """ - PORT_MIN = 0 - PORT_MAX = 65535 - def __init__(self, name, choices=None, **kwargs): - - # choices and min/max are mutally exclusive for Integer type. So check - # if choice are in the range of min/max and only assign choices to - # Integer type. - if choices is not None: - invalid_choices = [] - for choice in choices: - if not self.PORT_MIN <= choice <= self.PORT_MAX: - invalid_choices.append(six.text_type(choice)) - if invalid_choices: - raise ValueError("'Choices' values %(choices)s should be in " - "the range of %(min)d and %(max)d" % - {'choices': ', '.join(invalid_choices), - 'min': self.PORT_MIN, 'max': self.PORT_MAX}) - type = types.Integer(choices=choices, type_name='port value') - else: - type = types.Integer(min=self.PORT_MIN, max=self.PORT_MAX, - type_name='port value') + def __init__(self, name, min=None, max=None, choices=None, **kwargs): + type = types.Port(min=min, max=max, choices=choices, + type_name='port value') super(PortOpt, self).__init__(name, type=type, **kwargs) diff --git a/oslo_config/tests/test_cfg.py b/oslo_config/tests/test_cfg.py index f249c516..8ae6e31d 100644 --- a/oslo_config/tests/test_cfg.py +++ b/oslo_config/tests/test_cfg.py @@ -1538,6 +1538,129 @@ class ConfigFileOptsTestCase(BaseTestCase): self.conf(['--config-file', paths[0]]) self.assertRaises(cfg.ConfigFileValueError, self.conf._get, 'foo') + def test_conf_file_port_list(self): + self.conf.register_opt(cfg.ListOpt('foo', item_type=types.Port())) + + paths = self.create_tempfiles([('test', + '[DEFAULT]\n' + 'foo = 22, 80\n')]) + + self.conf(['--config-file', paths[0]]) + self.assertTrue(hasattr(self.conf, 'foo')) + self.assertEqual([22, 80], self.conf.foo) + + def test_conf_file_port_list_default(self): + self.conf.register_opt(cfg.ListOpt('foo', item_type=types.Port(), + default=[55, 77])) + + paths = self.create_tempfiles([('test', + '[DEFAULT]\n' + 'foo = 22, 80\n')]) + + self.conf(['--config-file', paths[0]]) + self.assertTrue(hasattr(self.conf, 'foo')) + self.assertEqual([22, 80], self.conf.foo) + + def test_conf_file_port_list_only_default(self): + self.conf.register_opt(cfg.ListOpt('foo', item_type=types.Port(), + default=[55, 77])) + + paths = self.create_tempfiles([('test', + '[DEFAULT]\n')]) + + self.conf(['--config-file', paths[0]]) + self.assertTrue(hasattr(self.conf, 'foo')) + self.assertEqual([55, 77], self.conf.foo) + + def test_conf_file_port_list_outside_range(self): + self.conf.register_opt(cfg.ListOpt('foo', item_type=types.Port())) + + paths = self.create_tempfiles([('test', + '[DEFAULT]\n' + 'foo = 1,65536\n')]) + + self.conf(['--config-file', paths[0]]) + self.assertRaises(cfg.ConfigFileValueError, self.conf._get, 'foo') + + def test_conf_file_port_min_max_above_max(self): + self.conf.register_opt(cfg.PortOpt('foo', min=1, max=5)) + + paths = self.create_tempfiles([('test', + '[DEFAULT]\n' + 'foo = 10\n')]) + + self.conf(['--config-file', paths[0]]) + self.assertRaises(cfg.ConfigFileValueError, self.conf._get, 'foo') + + def test_conf_file_port_only_max_above_max(self): + self.conf.register_opt(cfg.PortOpt('foo', max=500)) + + paths = self.create_tempfiles([('test', + '[DEFAULT]\n' + 'foo = 600\n')]) + + self.conf(['--config-file', paths[0]]) + self.assertRaises(cfg.ConfigFileValueError, self.conf._get, 'foo') + + def test_conf_file_port_min_max_below_min(self): + self.conf.register_opt(cfg.PortOpt('foo', min=100, max=500)) + + paths = self.create_tempfiles([('test', + '[DEFAULT]\n' + 'foo = 99\n')]) + + self.conf(['--config-file', paths[0]]) + self.assertRaises(cfg.ConfigFileValueError, self.conf._get, 'foo') + + def test_conf_file_port_only_min_below_min(self): + self.conf.register_opt(cfg.PortOpt('foo', min=1025)) + + paths = self.create_tempfiles([('test', + '[DEFAULT]\n' + 'foo = 1024\n')]) + + self.conf(['--config-file', paths[0]]) + self.assertRaises(cfg.ConfigFileValueError, self.conf._get, 'foo') + + def test_conf_file_port_min_max_in_range(self): + self.conf.register_opt(cfg.PortOpt('foo', min=1025, max=6000)) + + paths = self.create_tempfiles([('test', + '[DEFAULT]\n' + 'foo = 2500\n')]) + + self.conf(['--config-file', paths[0]]) + + self.assertTrue(hasattr(self.conf, 'foo')) + self.assertEqual(2500, self.conf.foo) + + def test_conf_file_port_only_max_in_range(self): + self.conf.register_opt(cfg.PortOpt('foo', max=5000)) + + paths = self.create_tempfiles([('test', + '[DEFAULT]\n' + 'foo = 45\n')]) + + self.conf(['--config-file', paths[0]]) + + self.assertTrue(hasattr(self.conf, 'foo')) + self.assertEqual(45, self.conf.foo) + + def test_conf_file_port_only_min_in_range(self): + self.conf.register_opt(cfg.PortOpt('foo', min=35)) + + paths = self.create_tempfiles([('test', + '[DEFAULT]\n' + 'foo = 45\n')]) + + self.conf(['--config-file', paths[0]]) + + self.assertTrue(hasattr(self.conf, 'foo')) + self.assertEqual(45, self.conf.foo) + + def test_conf_file_port_min_greater_max(self): + self.assertRaises(ValueError, cfg.PortOpt, 'foo', min=55, max=15) + def test_conf_file_multistr_default(self): self.conf.register_opt(cfg.MultiStrOpt('foo', default=['bar'])) @@ -4209,7 +4332,7 @@ class PortChoicesTestCase(BaseTestCase): self.assertRaises(SystemExit, self.conf, ['--port', '8181']) def test_choice_out_range(self): - self.assertRaisesRegexp(ValueError, 'values 65537 should', + self.assertRaisesRegexp(ValueError, 'out of bounds', cfg.PortOpt, 'port', choices=[80, 65537, 0]) def test_conf_file_choice_value(self): diff --git a/oslo_config/tests/test_types.py b/oslo_config/tests/test_types.py index b53817d8..f2d429ec 100644 --- a/oslo_config/tests/test_types.py +++ b/oslo_config/tests/test_types.py @@ -301,16 +301,13 @@ class IntegerTypeTests(TypeTestHelper, unittest.TestCase): def test_choices_with_min_max(self): self.assertRaises(ValueError, types.Integer, - min=10, + min=100, choices=[50, 60]) self.assertRaises(ValueError, types.Integer, - max=100, - choices=[50, 60]) - self.assertRaises(ValueError, - types.Integer, - min=10, max=100, + max=10, choices=[50, 60]) + types.Integer(min=10, max=100, choices=[50, 60]) def test_min_greater_max(self): self.assertRaises(ValueError, @@ -733,3 +730,155 @@ class URITypeTests(TypeTestHelper, unittest.TestCase): self.assertInvalid('http://www.example.com/versions') self.assertConvertedValue('http://www.example.com', 'http://www.example.com') + + +class PortTypeTests(TypeTestHelper, unittest.TestCase): + type = types.Port() + + def test_port(self): + self.assertInvalid(-1) + self.assertInvalid(65536) + self.assertConvertedValue('80', 80) + self.assertConvertedValue('65535', 65535) + + def test_repr(self): + self.assertEqual('Port(min=0, max=65535)', repr(types.Port())) + + def test_repr_with_min(self): + t = types.Port(min=123) + self.assertEqual('Port(min=123, max=65535)', repr(t)) + + def test_repr_with_max(self): + t = types.Port(max=456) + self.assertEqual('Port(min=0, max=456)', repr(t)) + + def test_repr_with_min_and_max(self): + t = types.Port(min=123, max=456) + self.assertEqual('Port(min=123, max=456)', repr(t)) + t = types.Port(min=0, max=0) + self.assertEqual('Port(min=0, max=0)', repr(t)) + + def test_repr_with_choices(self): + t = types.Port(choices=[80, 457]) + self.assertEqual('Port(choices=[80, 457])', repr(t)) + + def test_choices(self): + t = types.Port(choices=[80, 457]) + self.assertRaises(ValueError, t, 1) + self.assertRaises(ValueError, t, 200) + t(80) + t(457) + + def test_invalid_choices(self): + self.assertRaises(ValueError, types.Port, choices=[-1, 457]) + self.assertRaises(ValueError, types.Port, choices=[1, 2, 3, 65536]) + + def test_equal(self): + self.assertTrue(types.Port() == types.Port()) + + def test_equal_with_same_min_and_no_max(self): + self.assertTrue(types.Port(min=123) == types.Port(min=123)) + + def test_equal_with_same_max_and_no_min(self): + self.assertTrue(types.Port(max=123) == types.Port(max=123)) + + def test_equal_with_same_min_and_max(self): + t1 = types.Port(min=1, max=123) + t2 = types.Port(min=1, max=123) + self.assertTrue(t1 == t2) + + def test_equal_with_same_choices(self): + t1 = types.Port(choices=[80, 457]) + t2 = types.Port(choices=[457, 80]) + self.assertTrue(t1 == t2) + + def test_not_equal(self): + self.assertFalse(types.Port(min=123) == types.Port(min=456)) + self.assertFalse(types.Port(choices=[80, 457]) == + types.Port(choices=[80, 40])) + self.assertFalse(types.Port(choices=[80, 457]) == + types.Port()) + + def test_not_equal_to_other_class(self): + self.assertFalse(types.Port() == types.Integer()) + + def test_choices_with_min_max(self): + self.assertRaises(ValueError, + types.Port, + min=100, + choices=[50, 60]) + self.assertRaises(ValueError, + types.Port, + max=10, + choices=[50, 60]) + types.Port(min=10, max=100, choices=[50, 60]) + + def test_min_greater_max(self): + self.assertRaises(ValueError, + types.Port, + min=100, max=50) + self.assertRaises(ValueError, + types.Port, + min=-50, max=-100) + self.assertRaises(ValueError, + types.Port, + min=0, max=-50) + self.assertRaises(ValueError, + types.Port, + min=50, max=0) + + def test_illegal_min(self): + self.assertRaises(ValueError, + types.Port, + min=-1, max=50) + self.assertRaises(ValueError, + types.Port, + min=-50) + + def test_illegal_max(self): + self.assertRaises(ValueError, + types.Port, + min=100, max=65537) + self.assertRaises(ValueError, + types.Port, + max=100000) + + def test_with_max_and_min(self): + t = types.Port(min=123, max=456) + self.assertRaises(ValueError, t, 122) + t(123) + t(300) + t(456) + self.assertRaises(ValueError, t, 0) + self.assertRaises(ValueError, t, 457) + + def test_with_min_zero(self): + t = types.Port(min=0, max=456) + self.assertRaises(ValueError, t, -1) + t(0) + t(123) + t(300) + t(456) + self.assertRaises(ValueError, t, -201) + self.assertRaises(ValueError, t, 457) + + def test_with_max_zero(self): + t = types.Port(max=0) + self.assertRaises(ValueError, t, 1) + t(0) + + def test_with_choices_list(self): + t = types.Port(choices=[80, 457]) + self.assertRaises(ValueError, t, 1) + self.assertRaises(ValueError, t, 200) + self.assertRaises(ValueError, t, -457) + t(80) + t(457) + + def test_with_choices_tuple(self): + t = types.Port(choices=(80, 457)) + self.assertRaises(ValueError, t, 1) + self.assertRaises(ValueError, t, 200) + self.assertRaises(ValueError, t, -457) + t(80) + t(457) diff --git a/oslo_config/types.py b/oslo_config/types.py index 892b5893..07d5188b 100644 --- a/oslo_config/types.py +++ b/oslo_config/types.py @@ -248,12 +248,9 @@ class Number(ConfigType): """Number class, base for Integer and Float. :param min: Optional check that value is greater than or equal to min. - Mutually exclusive with 'choices'. :param max: Optional check that value is less than or equal to max. - Mutually exclusive with 'choices'. :param type_name: Type name to be used in the sample config file. - :param choices: Optional sequence of valid values. Mutually exclusive - with 'min/max'. + :param choices: Optional sequence of valid values. :param num_type: the type of number used for casting (i.e int, float) .. versionadded:: 3.14 @@ -263,15 +260,14 @@ class Number(ConfigType): min=None, max=None, choices=None): super(Number, self).__init__(type_name=type_name) - # Validate the choices and limits - if choices is not None: - if min is not None or max is not None: - raise ValueError("'choices' and 'min/max' cannot both be " - "specified") - else: - if min is not None and max is not None and max < min: - raise ValueError('Max value is less than min value') - + if min is not None and max is not None and max < min: + raise ValueError('Max value is less than min value') + invalid_choices = [c for c in choices or [] + if (min is not None and min > c) + or (max is not None and max < c)] + if invalid_choices: + raise ValueError("Choices %s are out of bounds [%s..%s]" + % (invalid_choices, min, max)) self.min = min self.max = max self.choices = choices @@ -281,31 +277,21 @@ class Number(ConfigType): if not isinstance(value, self.num_type): s = str(value).strip() if s == '': - value = None - else: - value = self.num_type(value) + return None + value = self.num_type(value) - if value is not None: - if self.choices is not None: - self._check_choices(value) - else: - self._check_range(value) - - return value - - def _check_choices(self, value): - if value in self.choices: - return + if self.choices is None: + if self.min is not None and value < self.min: + raise ValueError('Should be greater than or equal to %g' % + self.min) + if self.max is not None and value > self.max: + raise ValueError('Should be less than or equal to %g' % + self.max) else: - raise ValueError('Valid values are %r, but found %g' % ( - self.choices, value)) - - def _check_range(self, value): - if self.min is not None and value < self.min: - raise ValueError('Should be greater than or equal to %g' % - self.min) - if self.max is not None and value > self.max: - raise ValueError('Should be less than or equal to %g' % self.max) + if value not in self.choices: + raise ValueError('Valid values are %r, but found %g' % ( + self.choices, value)) + return value def __repr__(self): props = [] @@ -343,12 +329,9 @@ class Integer(Number): If value is whitespace or empty string will return None. :param min: Optional check that value is greater than or equal to min. - Mutually exclusive with 'choices'. :param max: Optional check that value is less than or equal to max. - Mutually exclusive with 'choices'. :param type_name: Type name to be used in the sample config file. - :param choices: Optional sequence of valid values. Mutually exclusive - with 'min/max'. + :param choices: Optional sequence of valid values. .. versionchanged:: 2.4 The class now honors zero for *min* and *max* parameters. @@ -358,6 +341,10 @@ class Integer(Number): .. versionchanged:: 3.2 Added *choices* parameter. + + .. versionchanged:: 3.16 + *choices* is no longer mutually exclusive with *min*/*max*. If those are + supplied, all choices are verified to be within the range. """ def __init__(self, min=None, max=None, type_name='integer value', @@ -380,13 +367,45 @@ class Float(Number): .. versionchanged:: 3.14 - Added *min* and *max* parameters. + Added *min* and *max* parameters. If *choices* are also supplied, they + must be within the range. """ def __init__(self, min=None, max=None, type_name='floating point value'): super(Float, self).__init__(float, type_name, min=min, max=max) +class Port(Integer): + + """Port type + + Represents a L4 Port. + + :param type_name: Type name to be used in the sample config file. + :param choices: Optional sequence of valid values. + :param min: Optional check that value is greater than or equal to min. + :param max: Optional check that value is less than or equal to max. + + .. versionadded:: 3.16 + """ + + PORT_MIN = 0 + PORT_MAX = 65535 + + def __init__(self, min=None, max=None, type_name='port', choices=None): + min = self.PORT_MIN if min is None else min + max = self.PORT_MAX if max is None else max + if min < self.PORT_MIN: + raise ValueError('Min value cannot be less than %(min)d', + {'min': self.PORT_MIN}) + if max > self.PORT_MAX: + raise ValueError('Max value cannot be more than %(max)d', + {'max': self.PORT_MAX}) + + super(Port, self).__init__(min=min, max=max, type_name=type_name, + choices=choices) + + class List(ConfigType): """List type. diff --git a/releasenotes/notes/add-port_type-8704295c6a56265d.yaml b/releasenotes/notes/add-port_type-8704295c6a56265d.yaml new file mode 100644 index 00000000..9896164c --- /dev/null +++ b/releasenotes/notes/add-port_type-8704295c6a56265d.yaml @@ -0,0 +1,5 @@ +--- +features: + - Integer and Float now support *min*, *max* and *choices*. Choices must + respect *min* and *max* (if provided). + - Added Port type as an Integer in the closed interval [0, 65535].