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

@@ -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):
+    def __init__(self, name, min=None, max=None, choices=None, **kwargs):
-
+        type = types.Port(min=min, max=max, choices=choices,
-        # choices and min/max are mutally exclusive for Integer type. So check
+                          type_name='port value')
-        # 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')
         super(PortOpt, self).__init__(name, type=type, **kwargs)
 
 

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):

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,
+                          max=10,
-                          choices=[50, 60])
-        self.assertRaises(ValueError,
-                          types.Integer,
-                          min=10, max=100,
                           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)

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
+    :param choices: Optional sequence of valid values.
-                    with 'min/max'.
     :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 min is not None and max is not None and max < min:
-        if choices is not None:
+            raise ValueError('Max value is less than min value')
-            if min is not None or max is not None:
+        invalid_choices = [c for c in choices or []
-                raise ValueError("'choices' and 'min/max' cannot both be "
+                           if (min is not None and min > c)
-                                 "specified")
+                           or (max is not None and max < c)]
-        else:
+        if invalid_choices:
-            if min is not None and max is not None and max < min:
+            raise ValueError("Choices %s are out of bounds [%s..%s]"
-                raise ValueError('Max value is less than min value')
+                             % (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
+                return None
-            else:
+            value = self.num_type(value)
-                value = self.num_type(value)
 
-        if value is not None:
+        if self.choices is None:
-            if self.choices is not None:
+            if self.min is not None and value < self.min:
-                self._check_choices(value)
+                raise ValueError('Should be greater than or equal to %g' %
-            else:
+                                 self.min)
-                self._check_range(value)
+            if self.max is not None and value > self.max:
-
+                raise ValueError('Should be less than or equal to %g' %
-        return value
+                                 self.max)
-
-    def _check_choices(self, value):
-        if value in self.choices:
-            return
         else:
-            raise ValueError('Valid values are %r, but found %g' % (
+            if value not in self.choices:
-                             self.choices, value))
+                raise ValueError('Valid values are %r, but found %g' % (
-
+                                 self.choices, value))
-    def _check_range(self, value):
+        return 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)
 
     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
+    :param choices: Optional sequence of valid values.
-                    with 'min/max'.
 
     .. 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.

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].