Change the way error are returned from API to JSON

Previously we used just strings. It's not a breaking change in terms
of API, but it will lead to less readable errors for older clients.

Nice side effect of this change is that exceptions are no longer
reported as HTML.

In-tree client was updated, as separate one is not set up yet.

Change-Id: Ice3111e08f8716d904efc4869c6d2ded25e23e2e
Closes-Bug: #1465347

ironic_inspector/client.py

@@ -11,6 +11,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import json
+import logging
+
 from oslo_utils import netutils
 import requests
 import six
@@ -20,6 +23,7 @@ from ironic_inspector.common.i18n import _
 
 _DEFAULT_URL = 'http://' + netutils.get_my_ipv4() + ':5050/v1'
 _ERROR_ENCODING = 'utf-8'
+LOG = logging.getLogger('ironic_inspector_client')
 
 
 def _prepare(base_url, auth_token):
@@ -35,6 +39,13 @@ class ClientError(requests.HTTPError):
     def __init__(self, response):
         # inspector returns error message in body
         msg = response.content.decode(_ERROR_ENCODING)
+        try:
+            msg = json.loads(msg)['error']['message']
+        except ValueError:
+            LOG.debug('Old style error response returned, assuming '
+                      'ironic-discoverd')
+        except (KeyError, TypeError):
+            LOG.exception('Bad error response from ironic-inspector')
         super(ClientError, self).__init__(msg, response=response)
 
     @classmethod

ironic_inspector/main.py

@@ -41,13 +41,21 @@ app = flask.Flask(__name__)
 LOG = logging.getLogger('ironic_inspector.main')
 
 
+def error_response(exc, code=500):
+    res = flask.jsonify(error={'message': str(exc)})
+    res.status_code = code
+    return res
+
+
 def convert_exceptions(func):
     @functools.wraps(func)
     def wrapper(*args, **kwargs):
         try:
             return func(*args, **kwargs)
         except utils.Error as exc:
-            return str(exc), exc.http_code
+            return error_response(exc, exc.http_code)
+        except Exception as exc:
+            return error_response(exc)
 
     return wrapper
 

ironic_inspector/test/test_client.py

@@ -80,11 +80,23 @@ class TestIntrospect(unittest.TestCase):
         )
 
     def test_failed(self, mock_post):
+        mock_post.return_value.status_code = 404
+        mock_post.return_value.content = b'{"error":{"message":"boom"}}'
+        self.assertRaisesRegexp(client.ClientError, "boom",
+                                client.introspect, self.uuid)
+
+    def test_failed_discoverd_style(self, mock_post):
         mock_post.return_value.status_code = 404
         mock_post.return_value.content = b"boom"
         self.assertRaisesRegexp(client.ClientError, "boom",
                                 client.introspect, self.uuid)
 
+    def test_failed_bad_json(self, mock_post):
+        mock_post.return_value.status_code = 404
+        mock_post.return_value.content = b'42'
+        self.assertRaisesRegexp(client.ClientError, "42",
+                                client.introspect, self.uuid)
+
 
 @mock.patch.object(client.requests, 'get', autospec=True,
                    **{'return_value.status_code': 200})
@@ -109,7 +121,19 @@ class TestGetStatus(unittest.TestCase):
         self.assertRaises(TypeError, client.get_status, 42)
 
     def test_failed(self, mock_post):
+        mock_post.return_value.status_code = 404
+        mock_post.return_value.content = b'{"error":{"message":"boom"}}'
+        self.assertRaisesRegexp(client.ClientError, "boom",
+                                client.get_status, self.uuid)
+
+    def test_failed_discoverd_style(self, mock_post):
         mock_post.return_value.status_code = 404
         mock_post.return_value.content = b"boom"
         self.assertRaisesRegexp(client.ClientError, "boom",
                                 client.get_status, self.uuid)
+
+    def test_failed_bad_json(self, mock_post):
+        mock_post.return_value.status_code = 404
+        mock_post.return_value.content = b'42'
+        self.assertRaisesRegexp(client.ClientError, "42",
+                                client.get_status, self.uuid)

ironic_inspector/test/test_main.py

@@ -75,7 +75,9 @@ class TestApi(test_base.BaseTest):
         introspect_mock.side_effect = utils.Error("boom")
         res = self.app.post('/v1/introspection/%s' % self.uuid)
         self.assertEqual(400, res.status_code)
-        self.assertEqual(b"boom", res.data)
+        self.assertEqual(
+            'boom',
+            json.loads(res.data.decode('utf-8'))['error']['message'])
         introspect_mock.assert_called_once_with(
             self.uuid,
             new_ipmi_credentials=None)
@@ -113,7 +115,9 @@ class TestApi(test_base.BaseTest):
         res = self.app.post('/v1/continue', data='"JSON"')
         self.assertEqual(400, res.status_code)
         process_mock.assert_called_once_with("JSON")
-        self.assertEqual(b'boom', res.data)
+        self.assertEqual(
+            'boom',
+            json.loads(res.data.decode('utf-8'))['error']['message'])
 
     @mock.patch.object(node_cache, 'get_node', autospec=True)
     def test_get_introspection_in_progress(self, get_mock):