From 3623abaf001b0341a45b9d498e53dae299b1e648 Mon Sep 17 00:00:00 2001 From: Chris Dent Date: Tue, 9 Aug 2016 14:37:46 +0000 Subject: [PATCH] Fix handling of status in placement API json_error_formatter The API-WG standard for error responses[1] says that the value of the 'status' key should be an int. webob error formatters receive the status as a string (for example '400 Bad Request'). This change fixes that and adds unit test coverage for the json_error_formatter method. This was a problem that was highlighted during review of the basic framing of the placement API but was not resolved before that code merged. [1] http://specs.openstack.org/openstack/api-wg/guidelines/errors.html Change-Id: I243eeaf62f83417b77bc6fa4a7d6f898b1382ac1 Partially-Implements: blueprint generic-resource-pools --- nova/api/openstack/placement/util.py | 10 +- .../unit/api/openstack/placement/test_util.py | 95 +++++++++++++++++++ 2 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 nova/tests/unit/api/openstack/placement/test_util.py diff --git a/nova/api/openstack/placement/util.py b/nova/api/openstack/placement/util.py index 054a70f0af7b..4ce5c4431d20 100644 --- a/nova/api/openstack/placement/util.py +++ b/nova/api/openstack/placement/util.py @@ -27,8 +27,11 @@ def json_error_formatter(body, status, title, environ): """ # Clear out the html that webob sneaks in. body = webob.exc.strip_tags(body) + # Get status code out of status message. webob's error formatter + # only passes entire status string. + status_code = int(status.split(None, 1)[0]) error_dict = { - 'status': status, + 'status': status_code, 'title': title, 'detail': body } @@ -41,10 +44,7 @@ def json_error_formatter(body, status, title, environ): # microversion parsing failed so we need to include microversion # min and max information in the error response. microversion = nova.api.openstack.placement.microversion - # Get status code out of status message. webob's error formatter - # only passes entire status string. - code = status.split(None, 1)[0] - if code == '406' and microversion.MICROVERSION_ENVIRON not in environ: + if status_code == 406 and microversion.MICROVERSION_ENVIRON not in environ: error_dict['max_version'] = microversion.max_version_string() error_dict['min_version'] = microversion.max_version_string() diff --git a/nova/tests/unit/api/openstack/placement/test_util.py b/nova/tests/unit/api/openstack/placement/test_util.py new file mode 100644 index 000000000000..f83a764e515c --- /dev/null +++ b/nova/tests/unit/api/openstack/placement/test_util.py @@ -0,0 +1,95 @@ +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +"""Unit tests for the utility functions used by the placement API.""" + + +from oslo_middleware import request_id + +from nova.api.openstack.placement import microversion +from nova.api.openstack.placement import util +from nova import test + + +class TestJSONErrorFormatter(test.NoDBTestCase): + + def setUp(self): + super(TestJSONErrorFormatter, self).setUp() + self.environ = {} + + def test_status_to_int_code(self): + body = '' + status = '404 Not Found' + title = '' + + result = util.json_error_formatter( + body, status, title, self.environ) + self.assertEqual(404, result['errors'][0]['status']) + + def test_strip_body_tags(self): + body = '

Big Error!

' + status = '400 Bad Request' + title = '' + + result = util.json_error_formatter( + body, status, title, self.environ) + self.assertEqual('Big Error!', result['errors'][0]['detail']) + + def test_request_id_presence(self): + body = '' + status = '400 Bad Request' + title = '' + + # no request id in environ, none in error + result = util.json_error_formatter( + body, status, title, self.environ) + self.assertNotIn('request_id', result['errors'][0]) + + # request id in environ, request id in error + self.environ[request_id.ENV_REQUEST_ID] = 'stub-id' + + result = util.json_error_formatter( + body, status, title, self.environ) + self.assertEqual('stub-id', result['errors'][0]['request_id']) + + def test_microversion_406_handling(self): + body = '' + status = '400 Bad Request' + title = '' + + # Not a 406, no version info required. + result = util.json_error_formatter( + body, status, title, self.environ) + self.assertNotIn('max_version', result['errors'][0]) + self.assertNotIn('min_version', result['errors'][0]) + + # A 406 but not because of microversions (microversion + # parsing was successful), no version info + # required. + status = '406 Not Acceptable' + version_obj = microversion.parse_version_string('2.3') + self.environ[microversion.MICROVERSION_ENVIRON] = version_obj + + result = util.json_error_formatter( + body, status, title, self.environ) + self.assertNotIn('max_version', result['errors'][0]) + self.assertNotIn('min_version', result['errors'][0]) + + # Microversion parsing failed, status is 406, send version info. + del self.environ[microversion.MICROVERSION_ENVIRON] + + result = util.json_error_formatter( + body, status, title, self.environ) + self.assertEqual(microversion.max_version_string(), + result['errors'][0]['max_version']) + self.assertEqual(microversion.min_version_string(), + result['errors'][0]['min_version'])