Merge "Correct VPNaaS limit checks in extension"

neutron/api/v2/attributes.py

@@ -30,6 +30,9 @@ ATTR_NOT_SPECIFIED = object()
 # Defining a constant to avoid repeating string literal in several modules
 SHARED = 'shared'
 
+# Used by range check to indicate no limit for a bound.
+UNLIMITED = None
+
 
 def _verify_dict_keys(expected_keys, target_dict, strict=True):
     """Allows to verify keys in a dictionary.
@@ -94,13 +97,30 @@ def _validate_boolean(data, valid_values=None):
 
 
 def _validate_range(data, valid_values=None):
+    """Check that integer value is within a range provided.
+
+    Test is inclusive. Allows either limit to be ignored, to allow
+    checking ranges where only the lower or upper limit matter.
+    It is expected that the limits provided are valid integers or
+    the value None.
+    """
+
     min_value = valid_values[0]
     max_value = valid_values[1]
-    if not min_value <= data <= max_value:
+    try:
-        msg = _("'%(data)s' is not in range %(min_value)s through "
+        data = int(data)
-                "%(max_value)s") % {'data': data,
+    except (ValueError, TypeError):
-                                    'min_value': min_value,
+        msg = _("'%s' is not an integer") % data
-                                    'max_value': max_value}
+        LOG.debug(msg)
+        return msg
+    if min_value is not UNLIMITED and data < min_value:
+        msg = _("'%(data)s' is too small - must be at least "
+                "'%(limit)d'") % {'data': data, 'limit': min_value}
+        LOG.debug(msg)
+        return msg
+    if max_value is not UNLIMITED and data > max_value:
+        msg = _("'%(data)s' is too large - must be no larger than "
+                "'%(limit)d'") % {'data': data, 'limit': max_value}
         LOG.debug(msg)
         return msg
 

neutron/extensions/vpnaas.py

@@ -82,6 +82,8 @@ vpn_supported_auth_mode = ['psk']
 vpn_supported_auth_algorithms = ['sha1']
 vpn_supported_phase1_negotiation_mode = ['main']
 
+vpn_lifetime_limits = (60, attr.UNLIMITED)
+positive_int = (0, attr.UNLIMITED)
 
 RESOURCE_ATTRIBUTE_MAP = {
 
@@ -144,7 +146,7 @@ RESOURCE_ATTRIBUTE_MAP = {
                        'is_visible': True},
         'mtu': {'allow_post': True, 'allow_put': True,
                 'default': '1500',
-                'validate': {'type:non_negative': None},
+                'validate': {'type:range': positive_int},
                 'convert_to': attr.convert_to_int,
                 'is_visible': True},
         'initiator': {'allow_post': True, 'allow_put': True,
@@ -168,10 +170,10 @@ RESOURCE_ATTRIBUTE_MAP = {
                             'type:values': vpn_dpd_supported_actions,
                         },
                         'interval': {
-                            'type:non_negative': None
+                            'type:range': positive_int
                         },
                         'timeout': {
-                            'type:non_negative': None
+                            'type:range': positive_int
                         }}}},
         'admin_state_up': {'allow_post': True, 'allow_put': True,
                            'default': True,
@@ -245,7 +247,8 @@ RESOURCE_ATTRIBUTE_MAP = {
                                  'type:values': vpn_supported_lifetime_units,
                              },
                              'value': {
-                                 'type:non_negative': None}}},
+                                 'type:range': vpn_lifetime_limits
+                             }}},
                      'is_visible': True},
         'pfs': {'allow_post': True, 'allow_put': True,
                 'default': 'group5',
@@ -294,7 +297,7 @@ RESOURCE_ATTRIBUTE_MAP = {
                                  'type:values': vpn_supported_lifetime_units,
                              },
                              'value': {
-                                 'type:non_negative': None,
+                                 'type:range': vpn_lifetime_limits,
                              }}},
                      'is_visible': True},
         'ike_version': {'allow_post': True, 'allow_put': True,

neutron/tests/unit/test_attributes.py

@@ -125,10 +125,31 @@ class TestAttributes(base.BaseTestCase):
         self.assertIsNone(msg)
 
         msg = attributes._validate_range(0, [1, 9])
-        self.assertEqual(msg, "'0' is not in range 1 through 9")
+        self.assertEqual(msg, "'0' is too small - must be at least '1'")
 
         msg = attributes._validate_range(10, (1, 9))
-        self.assertEqual(msg, "'10' is not in range 1 through 9")
+        self.assertEqual(msg,
+                         "'10' is too large - must be no larger than '9'")
+
+        msg = attributes._validate_range("bogus", (1, 9))
+        self.assertEqual(msg, "'bogus' is not an integer")
+
+        msg = attributes._validate_range(10, (attributes.UNLIMITED,
+                                              attributes.UNLIMITED))
+        self.assertIsNone(msg)
+
+        msg = attributes._validate_range(10, (1, attributes.UNLIMITED))
+        self.assertIsNone(msg)
+
+        msg = attributes._validate_range(1, (attributes.UNLIMITED, 9))
+        self.assertIsNone(msg)
+
+        msg = attributes._validate_range(-1, (0, attributes.UNLIMITED))
+        self.assertEqual(msg, "'-1' is too small - must be at least '0'")
+
+        msg = attributes._validate_range(10, (attributes.UNLIMITED, 9))
+        self.assertEqual(msg,
+                         "'10' is too large - must be no larger than '9'")
 
     def test_validate_mac_address(self):
         mac_addr = "ff:16:3e:4f:00:00"