x/vmware-nsx
Code Issues Proposed changes

Correct VPNaaS limit checks in extension

Browse Source 
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
This commit is contained in:
Paul Michali 2013-08-23 16:16:01 -04:00
parent 0f3750ceb7
commit 0c4ba5e0b8
3 changed files with 56 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
neutron/api/v2/attributes.py
View File

@ -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

13
neutron/extensions/vpnaas.py
View File

@ -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,

25
neutron/tests/unit/test_attributes.py
View File

@ -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"