FWaaS: Fix 500 INTERNAL error on too long name or description
The firewall interface was not validating string lengths, so passing in too long of a name (>255 char) or description (>1024) on a create returns a 500 INTERNAL SERVER error from a failed insert at the DB level. Solution is to validate the name and description strings in the firewall interfaces so a invalid input length user error returned instead. Used the standard attributes NAME_MAX_LEN and DESCRIPTION_MAX_LEN commonly used in other neutron interfaces for same validation. Fixes the 'firewall-create', 'firewall-policy-create' and 'firewall-rule-create' for both name and description variants. Added unit test cases for long names and descriptions. Merged with REJECT updates. Change-Id: I188140ae5e9624d2cc35ae980fefe35659ddaae2 Closes-Bug: 1489125
This commit is contained in:
James Arendt
parent
a287f4444f
commit
ed49720fd9
|
|
@ -244,10 +244,11 @@ RESOURCE_ATTRIBUTE_MAP = {
|
|||
'required_by_policy': True,
|
||||
'is_visible': True},
|
||||
'name': {'allow_post': True, 'allow_put': True,
|
||||
'validate': {'type:string': None},
|
||||
'validate': {'type:string': attr.NAME_MAX_LEN},
|
||||
'is_visible': True, 'default': ''},
|
||||
'description': {'allow_post': True, 'allow_put': True,
|
||||
'validate': {'type:string': None},
|
||||
'validate': {'type:string':
|
||||
attr.DESCRIPTION_MAX_LEN},
|
||||
'is_visible': True, 'default': ''},
|
||||
'firewall_policy_id': {'allow_post': False, 'allow_put': False,
|
||||
'validate': {'type:uuid_or_none': None},
|
||||
|
|
@ -298,10 +299,11 @@ RESOURCE_ATTRIBUTE_MAP = {
|
|||
'required_by_policy': True,
|
||||
'is_visible': True},
|
||||
'name': {'allow_post': True, 'allow_put': True,
|
||||
'validate': {'type:string': None},
|
||||
'validate': {'type:string': attr.NAME_MAX_LEN},
|
||||
'is_visible': True, 'default': ''},
|
||||
'description': {'allow_post': True, 'allow_put': True,
|
||||
'validate': {'type:string': None},
|
||||
'validate': {'type:string':
|
||||
attr.DESCRIPTION_MAX_LEN},
|
||||
'is_visible': True, 'default': ''},
|
||||
'shared': {'allow_post': True, 'allow_put': True,
|
||||
'default': False, 'convert_to': attr.convert_to_boolean,
|
||||
|
|
@ -324,10 +326,11 @@ RESOURCE_ATTRIBUTE_MAP = {
|
|||
'required_by_policy': True,
|
||||
'is_visible': True},
|
||||
'name': {'allow_post': True, 'allow_put': True,
|
||||
'validate': {'type:string': None},
|
||||
'validate': {'type:string': attr.NAME_MAX_LEN},
|
||||
'is_visible': True, 'default': ''},
|
||||
'description': {'allow_post': True, 'allow_put': True,
|
||||
'validate': {'type:string': None},
|
||||
'validate': {'type:string':
|
||||
attr.DESCRIPTION_MAX_LEN},
|
||||
'is_visible': True, 'default': ''},
|
||||
'admin_state_up': {'allow_post': True, 'allow_put': True,
|
||||
'default': True,
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@
|
|||
import copy
|
||||
|
||||
import mock
|
||||
from neutron.api.v2 import attributes as attr
|
||||
from neutron.plugins.common import constants
|
||||
from neutron.tests import base
|
||||
from neutron.tests.unit.api.v2 import test_base as test_api_v2
|
||||
|
|
@ -28,6 +29,8 @@ from neutron_fwaas.extensions import firewall
|
|||
|
||||
_uuid = uuidutils.generate_uuid
|
||||
_get_path = test_api_v2._get_path
|
||||
_long_name = 'x' * (attr.NAME_MAX_LEN + 1)
|
||||
_long_description = 'y' * (attr.DESCRIPTION_MAX_LEN + 1)
|
||||
|
||||
|
||||
class FirewallExtensionTestCase(test_api_v2_extension.ExtensionTestCase):
|
||||
|
|
@ -66,6 +69,32 @@ class FirewallExtensionTestCase(test_api_v2_extension.ExtensionTestCase):
|
|||
self.assertIn('firewall', res)
|
||||
self.assertEqual(res['firewall'], return_value)
|
||||
|
||||
def test_create_firewall_invalid_long_name(self):
|
||||
data = {'firewall': {'description': 'descr_firewall1',
|
||||
'name': _long_name,
|
||||
'admin_state_up': True,
|
||||
'firewall_policy_id': _uuid(),
|
||||
'shared': False,
|
||||
'tenant_id': _uuid()}}
|
||||
res = self.api.post(_get_path('fw/firewalls', fmt=self.fmt),
|
||||
self.serialize(data),
|
||||
content_type='application/%s' % self.fmt,
|
||||
status=exc.HTTPBadRequest.code)
|
||||
self.assertTrue('Invalid input for name' in res.body)
|
||||
|
||||
def test_create_firewall_invalid_long_description(self):
|
||||
data = {'firewall': {'description': _long_description,
|
||||
'name': 'firewall1',
|
||||
'admin_state_up': True,
|
||||
'firewall_policy_id': _uuid(),
|
||||
'shared': False,
|
||||
'tenant_id': _uuid()}}
|
||||
res = self.api.post(_get_path('fw/firewalls', fmt=self.fmt),
|
||||
self.serialize(data),
|
||||
content_type='application/%s' % self.fmt,
|
||||
status=exc.HTTPBadRequest.code)
|
||||
self.assertTrue('Invalid input for description' in res.body)
|
||||
|
||||
def test_firewall_list(self):
|
||||
fw_id = _uuid()
|
||||
return_value = [{'tenant_id': _uuid(),
|
||||
|
|
@ -164,6 +193,44 @@ class FirewallExtensionTestCase(test_api_v2_extension.ExtensionTestCase):
|
|||
def test_create_firewall_rule_with_port_range(self):
|
||||
self._test_create_firewall_rule('1:20', '30:40')
|
||||
|
||||
def test_create_firewall_rule_invalid_long_name(self):
|
||||
data = {'firewall_rule': {'description': 'descr_firewall_rule1',
|
||||
'name': _long_name,
|
||||
'shared': False,
|
||||
'protocol': 'tcp',
|
||||
'ip_version': 4,
|
||||
'source_ip_address': '192.168.0.1',
|
||||
'destination_ip_address': '127.0.0.1',
|
||||
'source_port': 1,
|
||||
'destination_port': 1,
|
||||
'action': 'allow',
|
||||
'enabled': True,
|
||||
'tenant_id': _uuid()}}
|
||||
res = self.api.post(_get_path('fw/firewall_rules', fmt=self.fmt),
|
||||
self.serialize(data),
|
||||
content_type='application/%s' % self.fmt,
|
||||
status=exc.HTTPBadRequest.code)
|
||||
self.assertTrue('Invalid input for name' in res.body)
|
||||
|
||||
def test_create_firewall_rule_invalid_long_description(self):
|
||||
data = {'firewall_rule': {'description': _long_description,
|
||||
'name': 'rule1',
|
||||
'shared': False,
|
||||
'protocol': 'tcp',
|
||||
'ip_version': 4,
|
||||
'source_ip_address': '192.168.0.1',
|
||||
'destination_ip_address': '127.0.0.1',
|
||||
'source_port': 1,
|
||||
'destination_port': 1,
|
||||
'action': 'allow',
|
||||
'enabled': True,
|
||||
'tenant_id': _uuid()}}
|
||||
res = self.api.post(_get_path('fw/firewall_rules', fmt=self.fmt),
|
||||
self.serialize(data),
|
||||
content_type='application/%s' % self.fmt,
|
||||
status=exc.HTTPBadRequest.code)
|
||||
self.assertTrue('Invalid input for description' in res.body)
|
||||
|
||||
def test_firewall_rule_list(self):
|
||||
rule_id = _uuid()
|
||||
return_value = [{'tenant_id': _uuid(),
|
||||
|
|
@ -248,6 +315,34 @@ class FirewallExtensionTestCase(test_api_v2_extension.ExtensionTestCase):
|
|||
self.assertIn('firewall_policy', res)
|
||||
self.assertEqual(res['firewall_policy'], return_value)
|
||||
|
||||
def test_create_firewall_policy_invalid_long_name(self):
|
||||
data = {'firewall_policy': {'description': 'descr_firewall_policy1',
|
||||
'name': _long_name,
|
||||
'shared': False,
|
||||
'firewall_rules': [_uuid(), _uuid()],
|
||||
'audited': False,
|
||||
'tenant_id': _uuid()}}
|
||||
res = self.api.post(_get_path('fw/firewall_policies',
|
||||
fmt=self.fmt),
|
||||
self.serialize(data),
|
||||
content_type='application/%s' % self.fmt,
|
||||
status=exc.HTTPBadRequest.code)
|
||||
self.assertTrue('Invalid input for name' in res.body)
|
||||
|
||||
def test_create_firewall_policy_invalid_long_description(self):
|
||||
data = {'firewall_policy': {'description': _long_description,
|
||||
'name': 'new_fw_policy1',
|
||||
'shared': False,
|
||||
'firewall_rules': [_uuid(), _uuid()],
|
||||
'audited': False,
|
||||
'tenant_id': _uuid()}}
|
||||
res = self.api.post(_get_path('fw/firewall_policies',
|
||||
fmt=self.fmt),
|
||||
self.serialize(data),
|
||||
content_type='application/%s' % self.fmt,
|
||||
status=exc.HTTPBadRequest.code)
|
||||
self.assertTrue('Invalid input for description' in res.body)
|
||||
|
||||
def test_firewall_policy_list(self):
|
||||
policy_id = _uuid()
|
||||
return_value = [{'tenant_id': _uuid(),
|
||||
|
|
|
|||
Loading…
Reference in New Issue