Improve Keypair error messages in osapi

The KeypairAPI code provides useful error messages that weren't being
percolated back through the OpenStack API. This patch addresses this by using
the original `message` when crafting the HTTPException objects.

References bug 1187952

Change-Id: I786a225010d912d7bfea5c01838ddcf43fd90d53

6 files changed, 69 insertions, 18 deletions

diff --git a/nova/api/openstack/compute/contrib/keypairs.py b/nova/api/openstack/compute/contrib/keypairs.py
index a79b39aae..4245355e5 100644
--- a/nova/api/openstack/compute/contrib/keypairs.py
+++ b/nova/api/openstack/compute/contrib/keypairs.py
@@ -94,12 +94,10 @@ class KeypairController(object):
             raise webob.exc.HTTPRequestEntityTooLarge(
                         explanation=msg,
                         headers={'Retry-After': 0})
-        except exception.InvalidKeypair:
-            msg = _("Keypair data is invalid")
-            raise webob.exc.HTTPBadRequest(explanation=msg)
-        except exception.KeyPairExists:
-            msg = _("Key pair '%s' already exists.") % name
-            raise webob.exc.HTTPConflict(explanation=msg)
+        except exception.InvalidKeypair as exc:
+            raise webob.exc.HTTPBadRequest(explanation=exc.format_message())
+        except exception.KeyPairExists as exc:
+            raise webob.exc.HTTPConflict(explanation=exc.format_message())
 
     def delete(self, req, id):
         """
diff --git a/nova/api/openstack/compute/plugins/v3/keypairs.py b/nova/api/openstack/compute/plugins/v3/keypairs.py
index 4051a3497..bf740641e 100644
--- a/nova/api/openstack/compute/plugins/v3/keypairs.py
+++ b/nova/api/openstack/compute/plugins/v3/keypairs.py
@@ -95,12 +95,10 @@ class KeypairController(object):
             raise webob.exc.HTTPRequestEntityTooLarge(
                         explanation=msg,
                         headers={'Retry-After': 0})
-        except exception.InvalidKeypair:
-            msg = _("Keypair data is invalid")
-            raise webob.exc.HTTPBadRequest(explanation=msg)
-        except exception.KeyPairExists:
-            msg = _("Key pair '%s' already exists.") % name
-            raise webob.exc.HTTPConflict(explanation=msg)
+        except exception.InvalidKeypair as exc:
+            raise webob.exc.HTTPBadRequest(explanation=exc.format_message())
+        except exception.KeyPairExists as exc:
+            raise webob.exc.HTTPConflict(explanation=exc.format_message())
 
     def delete(self, req, id):
         """
diff --git a/nova/exception.py b/nova/exception.py
index bbe5442f1..487eaae8e 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -895,7 +895,7 @@ class RotationRequiredForBackup(NovaException):
 
 
 class KeyPairExists(Duplicate):
-    message = _("Key pair %(key_name)s already exists.")
+    message = _("Key pair '%(key_name)s' already exists.")
 
 
 class InstanceExists(Duplicate):
diff --git a/nova/tests/api/openstack/compute/contrib/test_keypairs.py b/nova/tests/api/openstack/compute/contrib/test_keypairs.py
index e338cad69..56b9fe84b 100644
--- a/nova/tests/api/openstack/compute/contrib/test_keypairs.py
+++ b/nova/tests/api/openstack/compute/contrib/test_keypairs.py
@@ -100,8 +100,12 @@ class KeypairsTest(test.TestCase):
         req.headers['Content-Type'] = 'application/json'
         res = req.get_response(self.app)
         self.assertEqual(res.status_int, 400)
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual(
+            'Keypair name must be between 1 and 255 characters long',
+            res_dict['badRequest']['message'])
 
-    def test_keypair_create_with_invalid_name(self):
+    def test_keypair_create_with_name_too_long(self):
         body = {
             'keypair': {
                 'name': 'a' * 256
@@ -113,6 +117,10 @@ class KeypairsTest(test.TestCase):
         req.headers['Content-Type'] = 'application/json'
         res = req.get_response(self.app)
         self.assertEqual(res.status_int, 400)
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual(
+            'Keypair name must be between 1 and 255 characters long',
+            res_dict['badRequest']['message'])
 
     def test_keypair_create_with_non_alphanumeric_name(self):
         body = {
@@ -127,6 +135,10 @@ class KeypairsTest(test.TestCase):
         res = req.get_response(self.app)
         res_dict = jsonutils.loads(res.body)
         self.assertEqual(res.status_int, 400)
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual(
+            "Keypair name contains unsafe characters",
+            res_dict['badRequest']['message'])
 
     def test_keypair_import(self):
         body = {
@@ -183,6 +195,10 @@ class KeypairsTest(test.TestCase):
         req.headers['Content-Type'] = 'application/json'
         res = req.get_response(self.app)
         self.assertEqual(res.status_int, 413)
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual(
+            "Quota exceeded, too many key pairs.",
+            res_dict['overLimit']['message'])
 
     def test_keypair_create_quota_limit(self):
 
@@ -203,6 +219,10 @@ class KeypairsTest(test.TestCase):
         req.headers['Content-Type'] = 'application/json'
         res = req.get_response(self.app)
         self.assertEqual(res.status_int, 413)
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual(
+            "Quota exceeded, too many key pairs.",
+            res_dict['overLimit']['message'])
 
     def test_keypair_create_duplicate(self):
         self.stubs.Set(db, "key_pair_create", db_key_pair_create_duplicate)
@@ -213,6 +233,10 @@ class KeypairsTest(test.TestCase):
         req.headers['Content-Type'] = 'application/json'
         res = req.get_response(self.app)
         self.assertEqual(res.status_int, 409)
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual(
+            "Key pair 'create_duplicate' already exists.",
+            res_dict['conflictingRequest']['message'])
 
     def test_keypair_import_bad_key(self):
         body = {
@@ -229,6 +253,10 @@ class KeypairsTest(test.TestCase):
         res = req.get_response(self.app)
         self.assertEqual(res.status_int, 400)
 
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual("Keypair data is invalid",
+                         res_dict['badRequest']['message'])
+
     def test_keypair_delete(self):
         req = webob.Request.blank('/v2/fake/os-keypairs/FAKE')
         req.method = 'DELETE'
@@ -305,7 +333,7 @@ class KeypairsTest(test.TestCase):
             self.assertTrue('key_name' in server_dict)
             self.assertEquals(server_dict['key_name'], '')
 
-    def test_keypair_create_with_invalid_keypairBody(self):
+    def test_keypair_create_with_invalid_keypair_body(self):
         body = {'alpha': {'name': 'create_test'}}
         req = webob.Request.blank('/v1.1/fake/os-keypairs')
         req.method = 'POST'
diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_keypairs.py b/nova/tests/api/openstack/compute/plugins/v3/test_keypairs.py
index 529c5eb71..bb74fdafc 100644
--- a/nova/tests/api/openstack/compute/plugins/v3/test_keypairs.py
+++ b/nova/tests/api/openstack/compute/plugins/v3/test_keypairs.py
@@ -102,8 +102,12 @@ class KeypairsTest(test.TestCase):
         req.headers['Content-Type'] = 'application/json'
         res = req.get_response(self.app)
         self.assertEqual(res.status_int, 400)
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual(
+            'Keypair name must be between 1 and 255 characters long',
+            res_dict['badRequest']['message'])
 
-    def test_keypair_create_with_invalid_name(self):
+    def test_keypair_create_with_name_too_long(self):
         body = {
             'keypair': {
                 'name': 'a' * 256
@@ -115,6 +119,10 @@ class KeypairsTest(test.TestCase):
         req.headers['Content-Type'] = 'application/json'
         res = req.get_response(self.app)
         self.assertEqual(res.status_int, 400)
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual(
+            'Keypair name must be between 1 and 255 characters long',
+            res_dict['badRequest']['message'])
 
     def test_keypair_create_with_non_alphanumeric_name(self):
         body = {
@@ -129,6 +137,10 @@ class KeypairsTest(test.TestCase):
         res = req.get_response(self.app)
         res_dict = jsonutils.loads(res.body)
         self.assertEqual(res.status_int, 400)
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual(
+            "Keypair name contains unsafe characters",
+            res_dict['badRequest']['message'])
 
     def test_keypair_import(self):
         body = {
@@ -185,6 +197,10 @@ class KeypairsTest(test.TestCase):
         req.headers['Content-Type'] = 'application/json'
         res = req.get_response(self.app)
         self.assertEqual(res.status_int, 413)
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual(
+            "Quota exceeded, too many key pairs.",
+            res_dict['overLimit']['message'])
 
     def test_keypair_create_quota_limit(self):
 
@@ -205,6 +221,10 @@ class KeypairsTest(test.TestCase):
         req.headers['Content-Type'] = 'application/json'
         res = req.get_response(self.app)
         self.assertEqual(res.status_int, 413)
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual(
+            "Quota exceeded, too many key pairs.",
+            res_dict['overLimit']['message'])
 
     def test_keypair_create_duplicate(self):
         self.stubs.Set(db, "key_pair_create", db_key_pair_create_duplicate)
@@ -215,6 +235,10 @@ class KeypairsTest(test.TestCase):
         req.headers['Content-Type'] = 'application/json'
         res = req.get_response(self.app)
         self.assertEqual(res.status_int, 409)
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual(
+            "Key pair 'create_duplicate' already exists.",
+            res_dict['conflictingRequest']['message'])
 
     def test_keypair_import_bad_key(self):
         body = {
@@ -230,6 +254,9 @@ class KeypairsTest(test.TestCase):
         req.headers['Content-Type'] = 'application/json'
         res = req.get_response(self.app)
         self.assertEqual(res.status_int, 400)
+        res_dict = jsonutils.loads(res.body)
+        self.assertEqual("Keypair data is invalid",
+                         res_dict['badRequest']['message'])
 
     def test_keypair_delete(self):
         req = webob.Request.blank('/v3/os-keypairs/FAKE')
@@ -307,7 +334,7 @@ class KeypairsTest(test.TestCase):
             self.assertTrue('key_name' in server_dict)
             self.assertEquals(server_dict['key_name'], '')
 
-    def test_keypair_create_with_invalid_keypairBody(self):
+    def test_keypair_create_with_invalid_keypair_body(self):
         body = {'alpha': {'name': 'create_test'}}
         req = webob.Request.blank('/v3/os-keypairs')
         req.method = 'POST'
diff --git a/nova/tests/compute/test_keypairs.py b/nova/tests/compute/test_keypairs.py
index a6fbda71b..fcb21b3e6 100644
--- a/nova/tests/compute/test_keypairs.py
+++ b/nova/tests/compute/test_keypairs.py
@@ -113,7 +113,7 @@ class CreateImportSharedTestMixIn(object):
 
         self.stubs.Set(db, "key_pair_create", db_key_pair_create_duplicate)
 
-        msg = (_("Key pair %(key_name)s already exists.") %
+        msg = (_("Key pair '%(key_name)s' already exists.") %
                {'key_name': self.existing_key_name})
         self.assertKeyNameRaises(exception.KeyPairExists, msg,
                                  self.existing_key_name)