diff --git a/quantum/api/v2/attributes.py b/quantum/api/v2/attributes.py index 8bf10a1f48..3d3e8581c2 100644 --- a/quantum/api/v2/attributes.py +++ b/quantum/api/v2/attributes.py @@ -33,13 +33,16 @@ SHARED = 'shared' def _verify_dict_keys(expected_keys, target_dict): if not isinstance(target_dict, dict): - msg = _("Invalid input. %s must be a dictionary.") % target_dict + msg = (_("Invalid input. '%(target_dict)s' must be a dictionary " + "with keys: %(expected_keys)s") % + dict(target_dict=target_dict, expected_keys=expected_keys)) return msg provided_keys = target_dict.keys() if set(expected_keys) != set(provided_keys): - msg = (_("Expected keys not found. Expected: %(expected_keys)s " - "Provided: %(provided_keys)s") % locals()) + msg = (_("Expected keys not found. Expected: '%(expected_keys)s', " + "Provided: '%(provided_keys)s'") % + dict(expected_keys=expected_keys, provided_keys=provided_keys)) return msg @@ -49,7 +52,8 @@ def is_attr_set(attribute): def _validate_values(data, valid_values=None): if data not in valid_values: - msg = _("'%(data)s' is not in %(valid_values)s") % locals() + msg = (_("'%(data)s' is not in %(valid_values)s") % + dict(data=data, valid_values=valid_values)) LOG.debug(msg) return msg @@ -61,7 +65,8 @@ def _validate_string(data, max_len=None): return msg if max_len is not None and len(data) > max_len: - msg = _("'%(data)s' exceeds maximum length of %(max_len)s") % locals() + msg = (_("'%(data)s' exceeds maximum length of %(max_len)s") % + dict(data=data, max_len=max_len)) LOG.debug(msg) return msg @@ -71,7 +76,9 @@ def _validate_range(data, valid_values=None): 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") % locals() + "%(max_value)s") % dict(data=data, + min_value=min_value, + max_value=max_value) LOG.debug(msg) return msg @@ -101,7 +108,7 @@ def _validate_ip_pools(data, valid_values=None): """ if not isinstance(data, list): - msg = _("'%s' is not a valid IP pool") % data + msg = _("Invalid data format for IP pool: '%s'") % data LOG.debug(msg) return msg @@ -120,18 +127,22 @@ def _validate_ip_pools(data, valid_values=None): def _validate_fixed_ips(data, valid_values=None): if not isinstance(data, list): - msg = _("'%s' is not a valid fixed IP") % data + msg = _("Invalid data format for fixed IP: '%s'") % data LOG.debug(msg) return msg ips = [] for fixed_ip in data: + if not isinstance(fixed_ip, dict): + msg = _("Invalid data format for fixed IP: '%s'") % fixed_ip + LOG.debug(msg) + return msg if 'ip_address' in fixed_ip: # Ensure that duplicate entries are not set - just checking IP # suffices. Duplicate subnet_id's are legitimate. fixed_ip_address = fixed_ip['ip_address'] if fixed_ip_address in ips: - msg = _("Duplicate entry %s") % fixed_ip + msg = _("Duplicate IP address '%s'") % fixed_ip_address else: msg = _validate_ip_address(fixed_ip_address) if msg: @@ -147,7 +158,7 @@ def _validate_fixed_ips(data, valid_values=None): def _validate_nameservers(data, valid_values=None): if not hasattr(data, '__iter__'): - msg = _("'%s' is not a valid nameserver") % data + msg = _("Invalid data format for nameserver: '%s'") % data LOG.debug(msg) return msg @@ -162,7 +173,7 @@ def _validate_nameservers(data, valid_values=None): LOG.debug(msg) return msg if ip in ips: - msg = _("Duplicate nameserver %s") % ip + msg = _("Duplicate nameserver '%s'") % ip LOG.debug(msg) return msg ips.append(ip) @@ -170,7 +181,7 @@ def _validate_nameservers(data, valid_values=None): def _validate_hostroutes(data, valid_values=None): if not isinstance(data, list): - msg = _("'%s' is not a valid hostroute") % data + msg = _("Invalid data format for hostroute: '%s'") % data LOG.debug(msg) return msg @@ -190,7 +201,7 @@ def _validate_hostroutes(data, valid_values=None): LOG.debug(msg) return msg if hostroute in hostroutes: - msg = _("Duplicate hostroute %s") % hostroute + msg = _("Duplicate hostroute '%s'") % hostroute LOG.debug(msg) return msg hostroutes.append(hostroute) @@ -252,7 +263,7 @@ def _validate_uuid_list(data, valid_values=None): return msg if len(set(data)) != len(data): - msg = _("Duplicate items in the list: %s") % ', '.join(data) + msg = _("Duplicate items in the list: '%s'") % ', '.join(data) LOG.debug(msg) return msg diff --git a/quantum/api/v2/base.py b/quantum/api/v2/base.py index 31c5fd63dc..e562004793 100644 --- a/quantum/api/v2/base.py +++ b/quantum/api/v2/base.py @@ -36,8 +36,6 @@ FAULT_MAP = {exceptions.NotFound: webob.exc.HTTPNotFound, exceptions.ServiceUnavailable: webob.exc.HTTPServiceUnavailable, exceptions.NotAuthorized: webob.exc.HTTPForbidden, netaddr.AddrFormatError: webob.exc.HTTPBadRequest, - AttributeError: webob.exc.HTTPBadRequest, - ValueError: webob.exc.HTTPBadRequest, } @@ -359,7 +357,11 @@ class Controller(object): def update(self, request, id, body=None, **kwargs): """Updates the specified entity's attributes""" parent_id = kwargs.get(self._parent_id_name) - payload = body.copy() + try: + payload = body.copy() + except AttributeError: + msg = _("Invalid format: %s") % request.body + raise exceptions.BadRequest(resource='body', msg=msg) payload['id'] = id notifier_api.notify(request.context, self._publisher_id, diff --git a/quantum/api/v2/resource.py b/quantum/api/v2/resource.py index 437e454b3c..144e1a9a8d 100644 --- a/quantum/api/v2/resource.py +++ b/quantum/api/v2/resource.py @@ -80,8 +80,7 @@ def Resource(controller, faults=None, deserializers=None, serializers=None): method = getattr(controller, action) result = method(request=request, **args) - except (ValueError, AttributeError, - exceptions.QuantumException, + except (exceptions.QuantumException, netaddr.AddrFormatError) as e: LOG.exception(_('%s failed'), action) body = serializer.serialize({'QuantumError': str(e)}) diff --git a/quantum/tests/unit/cisco/test_network_plugin.py b/quantum/tests/unit/cisco/test_network_plugin.py index 7efa198fd6..bd770b21e8 100644 --- a/quantum/tests/unit/cisco/test_network_plugin.py +++ b/quantum/tests/unit/cisco/test_network_plugin.py @@ -92,7 +92,7 @@ class TestCiscoPortsV2(CiscoNetworkPluginV2TestCase, 'test', True) # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'ports') + self._validate_behavior_on_bulk_failure(res, 'ports', 500) def test_create_ports_bulk_native_plugin_failure(self): if self._skip_native_bulk: @@ -112,7 +112,7 @@ class TestCiscoPortsV2(CiscoNetworkPluginV2TestCase, res = self._create_port_bulk(self.fmt, 2, net['network']['id'], 'test', True, context=ctx) # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'ports') + self._validate_behavior_on_bulk_failure(res, 'ports', 500) class TestCiscoNetworksV2(CiscoNetworkPluginV2TestCase, @@ -140,7 +140,7 @@ class TestCiscoNetworksV2(CiscoNetworkPluginV2TestCase, res = self._create_network_bulk(self.fmt, 2, 'test', True) LOG.debug("response is %s" % res) # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'networks') + self._validate_behavior_on_bulk_failure(res, 'networks', 500) def test_create_networks_bulk_native_plugin_failure(self): if self._skip_native_bulk: @@ -157,7 +157,7 @@ class TestCiscoNetworksV2(CiscoNetworkPluginV2TestCase, patched_plugin.side_effect = side_effect res = self._create_network_bulk(self.fmt, 2, 'test', True) # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'networks') + self._validate_behavior_on_bulk_failure(res, 'networks', 500) class TestCiscoSubnetsV2(CiscoNetworkPluginV2TestCase, @@ -189,7 +189,7 @@ class TestCiscoSubnetsV2(CiscoNetworkPluginV2TestCase, net['network']['id'], 'test') # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'subnets') + self._validate_behavior_on_bulk_failure(res, 'subnets', 500) def test_create_subnets_bulk_native_plugin_failure(self): if self._skip_native_bulk: @@ -209,7 +209,7 @@ class TestCiscoSubnetsV2(CiscoNetworkPluginV2TestCase, 'test') # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'subnets') + self._validate_behavior_on_bulk_failure(res, 'subnets', 500) class TestCiscoPortsV2XML(TestCiscoPortsV2): diff --git a/quantum/tests/unit/test_attributes.py b/quantum/tests/unit/test_attributes.py index a4e2d06b38..911b3e697b 100644 --- a/quantum/tests/unit/test_attributes.py +++ b/quantum/tests/unit/test_attributes.py @@ -139,17 +139,32 @@ class TestAttributes(unittest2.TestCase): self.assertIsNone(msg) def test_validate_fixed_ips(self): - fixed_ips = [[{'subnet_id': '00000000-ffff-ffff-ffff-000000000000', + fixed_ips = [ + {'data': [{'subnet_id': '00000000-ffff-ffff-ffff-000000000000', 'ip_address': '1111.1.1.1'}], - [{'subnet_id': 'invalid'}], - None, - [{'subnet_id': '00000000-0fff-ffff-ffff-000000000000', + 'error_msg': "'1111.1.1.1' is not a valid IP address"}, + {'data': [{'subnet_id': 'invalid', + 'ip_address': '1.1.1.1'}], + 'error_msg': "'invalid' is not a valid UUID"}, + {'data': None, + 'error_msg': "Invalid data format for fixed IP: 'None'"}, + {'data': "1.1.1.1", + 'error_msg': "Invalid data format for fixed IP: '1.1.1.1'"}, + {'data': ['00000000-ffff-ffff-ffff-000000000000', '1.1.1.1'], + 'error_msg': "Invalid data format for fixed IP: " + "'00000000-ffff-ffff-ffff-000000000000'"}, + {'data': [['00000000-ffff-ffff-ffff-000000000000', '1.1.1.1']], + 'error_msg': "Invalid data format for fixed IP: " + "'['00000000-ffff-ffff-ffff-000000000000', " + "'1.1.1.1']'"}, + {'data': [{'subnet_id': '00000000-0fff-ffff-ffff-000000000000', 'ip_address': '1.1.1.1'}, {'subnet_id': '00000000-ffff-ffff-ffff-000000000000', - 'ip_address': '1.1.1.1'}]] + 'ip_address': '1.1.1.1'}], + 'error_msg': "Duplicate IP address '1.1.1.1'"}] for fixed in fixed_ips: - msg = attributes._validate_fixed_ips(fixed) - self.assertIsNotNone(msg) + msg = attributes._validate_fixed_ips(fixed['data']) + self.assertEqual(msg, fixed['error_msg']) fixed_ips = [[{'subnet_id': '00000000-ffff-ffff-ffff-000000000000', 'ip_address': '1.1.1.1'}], @@ -402,7 +417,8 @@ class TestAttributes(unittest2.TestCase): 'f3eeab00-8367-4524-b662-55e64d4cacb5', 'e5069610-744b-42a7-8bd8-ceac1a229cd4'] msg = attributes._validate_uuid_list(duplicate_uuids) - error = "Duplicate items in the list: %s" % ', '.join(duplicate_uuids) + error = ("Duplicate items in the list: " + "'%s'" % ', '.join(duplicate_uuids)) self.assertEquals(msg, error) # check valid uuid lists diff --git a/quantum/tests/unit/test_db_plugin.py b/quantum/tests/unit/test_db_plugin.py index dffe31ca4b..7c6bd6765a 100644 --- a/quantum/tests/unit/test_db_plugin.py +++ b/quantum/tests/unit/test_db_plugin.py @@ -411,7 +411,7 @@ class QuantumDbPluginV2TestCase(testlib_api.WebTestCase): def _do_side_effect(self, patched_plugin, orig, *args, **kwargs): """ Invoked by test cases for injecting failures in plugin """ def second_call(*args, **kwargs): - raise AttributeError + raise q_exc.QuantumException patched_plugin.side_effect = second_call return orig(*args, **kwargs) @@ -740,7 +740,7 @@ class TestPortsV2(QuantumDbPluginV2TestCase): 'test', True) # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'ports') + self._validate_behavior_on_bulk_failure(res, 'ports', 500) def test_create_ports_bulk_native_plugin_failure(self): if self._skip_native_bulk: @@ -759,7 +759,7 @@ class TestPortsV2(QuantumDbPluginV2TestCase): res = self._create_port_bulk(self.fmt, 2, net['network']['id'], 'test', True, context=ctx) # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'ports') + self._validate_behavior_on_bulk_failure(res, 'ports', 500) def test_list_ports(self): # for this test we need to enable overlapping ips @@ -1731,7 +1731,7 @@ class TestNetworksV2(QuantumDbPluginV2TestCase): patched_plugin.side_effect = side_effect res = self._create_network_bulk(self.fmt, 2, 'test', True) # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'networks') + self._validate_behavior_on_bulk_failure(res, 'networks', 500) def test_create_networks_bulk_native_plugin_failure(self): if self._skip_native_bulk: @@ -1747,7 +1747,7 @@ class TestNetworksV2(QuantumDbPluginV2TestCase): patched_plugin.side_effect = side_effect res = self._create_network_bulk(self.fmt, 2, 'test', True) # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'networks') + self._validate_behavior_on_bulk_failure(res, 'networks', 500) def test_list_networks(self): with contextlib.nested(self.network(), @@ -1976,7 +1976,7 @@ class TestSubnetsV2(QuantumDbPluginV2TestCase): net['network']['id'], 'test') # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'subnets') + self._validate_behavior_on_bulk_failure(res, 'subnets', 500) def test_create_subnets_bulk_native_plugin_failure(self): if self._skip_native_bulk: @@ -1995,7 +1995,7 @@ class TestSubnetsV2(QuantumDbPluginV2TestCase): 'test') # We expect a 500 as we injected a fault in the plugin - self._validate_behavior_on_bulk_failure(res, 'subnets') + self._validate_behavior_on_bulk_failure(res, 'subnets', 500) def test_delete_subnet(self): gateway_ip = '10.0.0.1'