From 0c4ba5e0b8da2e87d72314198ee3feca52c0031f Mon Sep 17 00:00:00 2001 From: Paul Michali Date: Fri, 23 Aug 2013 16:16:01 -0400 Subject: [PATCH] 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 --- neutron/api/v2/attributes.py | 30 ++++++++++++++++++++++----- neutron/extensions/vpnaas.py | 13 +++++++----- neutron/tests/unit/test_attributes.py | 25 ++++++++++++++++++++-- 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/neutron/api/v2/attributes.py b/neutron/api/v2/attributes.py index a180d201eb..d2e8c25221 100644 --- 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 cb120fcb0e..d607c8f486 100644 --- 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 a79a67e02c..a763b54d17 100644 --- 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"