Correct VPNaaS limit checks in extension

For the following VPNaaS attributes, ensure that these validation
tests are performed in the extension:

    keepalive >= 60 (seconds)
    mtu >= 0 (octets)
    DPD interval > 0 (seconds)
    DPD timeout > 0 (seconds)

Currently, the units for keepalive can only be seconds. If this
changes in the future (e.g. to allow kilobytes units), then the
test for the value will need to be changed.

To correctly test the MTU limits for an IPSec connection, the
protocol must be taken into consideration, which is defined by
the vpnservice object. Because of this dependency, we cannot
validate this in the extension, and will instead, just make
sure the value is positive.

Likewise, the attribute validators cannot ensure that the
DPD timeout is greater than the interval (again, because the
validators can only check the individual attributes one at
a time).

The range validator was modified to allow single ended ranges
for some of these attributes. The range also ensures the value
is an integer.

Companion changes are being made in the CLI code for these limit
changes.

Update: Fixed typo causing tox failure.

bug 1216117

Change-Id: I85d6175d81fbe7d27231aed48fb4691351e0db4c

diff --git a/neutron/api/v2/attributes.py b/neutron/api/v2/attributes.py
index a180d201ebb2794775e92a0956a5b034c373bd16..d2e8c252218aa5644c66e25cfc11ae30a9528c0d 100644 (file)
--- a/neutron/api/v2/attributes.py
+++ b/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:
-        msg = _("'%(data)s' is not in range %(min_value)s through "
-                "%(max_value)s") % {'data': data,
-                                    'min_value': min_value,
-                                    'max_value': max_value}
+    try:
+        data = int(data)
+    except (ValueError, TypeError):
+        msg = _("'%s' is not an integer") % data
+        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
 

diff --git a/neutron/extensions/vpnaas.py b/neutron/extensions/vpnaas.py
index cb120fcb0e287d69ec557234a6ee8bb2da14c574..d607c8f4868f3a7e777722a6308f9ccb6bc2d65a 100644 (file)
--- a/neutron/extensions/vpnaas.py
+++ b/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,

diff --git a/neutron/tests/unit/test_attributes.py b/neutron/tests/unit/test_attributes.py
index a79a67e02cb6893e619959fd14316590922bac69..a763b54d17c71ea13ca663dcebeda53a138f2f74 100644 (file)
--- a/neutron/tests/unit/test_attributes.py
+++ b/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"