refactor security group api not to raise http exceptions

The SecurityGroupAPI had been raising http exceptions,
resulting in a difference in behavior depending on the
driver being used: nova or quantum. The nova driver
re-raises nova exceptions by default, while the quantum
driver was raising the built-in http exceptions. Since
servers.py handles nova exceptions, in some cases the
quantum driver's http exceptions weren't being handled
and resulted in a different http return code to the
user.

Both SecurityGroupAPI classes inherit from
NativeSecurityGroupExceptions and call its static
methods to raise exceptions.

This change refactors SecurityGroupAPI to raise nova
exceptions by modifying NativeSecurityGroupExceptions.
The nova exceptions are then translated to http
exceptions in the security group wsgi extension.

Fixes bug 1183347

Change-Id: I7177183154b4c41037623f443cf9958ac457a020

1 files changed, 75 insertions, 69 deletions

diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py
index b34a77cab..e2862be4e 100644
--- a/nova/api/openstack/compute/contrib/security_groups.py
+++ b/nova/api/openstack/compute/contrib/security_groups.py
@@ -16,6 +16,7 @@
 
 """The security groups extension."""
 
+import contextlib
 import json
 import webob
 from webob import exc
@@ -177,6 +178,25 @@ class SecurityGroupRulesXMLDeserializer(wsgi.MetadataXMLDeserializer):
         return sg_rule
 
 
+@contextlib.contextmanager
+def translate_exceptions():
+    """Translate nova exceptions to http exceptions."""
+    try:
+        yield
+    except exception.Invalid as exp:
+        msg = exp.format_message()
+        raise exc.HTTPBadRequest(explanation=msg)
+    except exception.SecurityGroupNotFound as exp:
+        msg = exp.format_message()
+        raise exc.HTTPNotFound(explanation=msg)
+    except exception.InstanceNotFound as exp:
+        msg = exp.format_message()
+        raise exc.HTTPNotFound(explanation=msg)
+    except exception.SecurityGroupLimitExceeded as exp:
+        msg = exp.format_message()
+        raise exc.HTTPRequestEntityTooLarge(explanation=msg)
+
+
 class SecurityGroupControllerBase(object):
     """Base class for Security Group controllers."""
 
@@ -196,8 +216,9 @@ class SecurityGroupControllerBase(object):
         sg_rule['group'] = {}
         sg_rule['ip_range'] = {}
         if rule['group_id']:
-            source_group = self.security_group_api.get(context,
-                                                       id=rule['group_id'])
+            with translate_exceptions():
+                source_group = self.security_group_api.get(context,
+                                                           id=rule['group_id'])
             sg_rule['group'] = {'name': source_group.get('name'),
                              'tenant_id': source_group.get('project_id')}
         else:
@@ -233,10 +254,10 @@ class SecurityGroupController(SecurityGroupControllerBase):
         """Return data about the given security group."""
         context = _authorize_context(req)
 
-        id = self.security_group_api.validate_id(id)
-
-        security_group = self.security_group_api.get(context, None, id,
-                                                     map_exception=True)
+        with translate_exceptions():
+            id = self.security_group_api.validate_id(id)
+            security_group = self.security_group_api.get(context, None, id,
+                                                         map_exception=True)
 
         return {'security_group': self._format_security_group(context,
                                                               security_group)}
@@ -245,12 +266,11 @@ class SecurityGroupController(SecurityGroupControllerBase):
         """Delete a security group."""
         context = _authorize_context(req)
 
-        id = self.security_group_api.validate_id(id)
-
-        security_group = self.security_group_api.get(context, None, id,
-                                                     map_exception=True)
-
-        self.security_group_api.destroy(context, security_group)
+        with translate_exceptions():
+            id = self.security_group_api.validate_id(id)
+            security_group = self.security_group_api.get(context, None, id,
+                                                         map_exception=True)
+            self.security_group_api.destroy(context, security_group)
 
         return webob.Response(status_int=202)
 
@@ -262,9 +282,11 @@ class SecurityGroupController(SecurityGroupControllerBase):
         search_opts = {}
         search_opts.update(req.GET)
 
-        raw_groups = self.security_group_api.list(context,
-                                                  project=context.project_id,
-                                                  search_opts=search_opts)
+        with translate_exceptions():
+            project_id = context.project_id
+            raw_groups = self.security_group_api.list(context,
+                                                      project=project_id,
+                                                      search_opts=search_opts)
 
         limited_list = common.limited(raw_groups, req)
         result = [self._format_security_group(context, group)
@@ -285,16 +307,12 @@ class SecurityGroupController(SecurityGroupControllerBase):
         group_name = security_group.get('name', None)
         group_description = security_group.get('description', None)
 
-        self.security_group_api.validate_property(group_name, 'name', None)
-        self.security_group_api.validate_property(group_description,
-                                                  'description', None)
-
-        try:
+        with translate_exceptions():
+            self.security_group_api.validate_property(group_name, 'name', None)
+            self.security_group_api.validate_property(group_description,
+                                                      'description', None)
             group_ref = self.security_group_api.create_security_group(
                 context, group_name, group_description)
-        except exception.SecurityGroupLimitExceeded as err:
-            raise exc.HTTPRequestEntityTooLarge(
-                explanation=err.format_message())
 
         return {'security_group': self._format_security_group(context,
                                                               group_ref)}
@@ -304,21 +322,21 @@ class SecurityGroupController(SecurityGroupControllerBase):
         """Update a security group."""
         context = _authorize_context(req)
 
-        id = self.security_group_api.validate_id(id)
+        with translate_exceptions():
+            id = self.security_group_api.validate_id(id)
+            security_group = self.security_group_api.get(context, None, id,
+                                                         map_exception=True)
 
-        security_group = self.security_group_api.get(context, None, id,
-                                                     map_exception=True)
         security_group_data = self._from_body(body, 'security_group')
-
         group_name = security_group_data.get('name', None)
         group_description = security_group_data.get('description', None)
 
-        self.security_group_api.validate_property(group_name, 'name', None)
-        self.security_group_api.validate_property(group_description,
-                                                  'description', None)
-
-        group_ref = self.security_group_api.update_security_group(
-            context, security_group, group_name, group_description)
+        with translate_exceptions():
+            self.security_group_api.validate_property(group_name, 'name', None)
+            self.security_group_api.validate_property(group_description,
+                                                      'description', None)
+            group_ref = self.security_group_api.update_security_group(
+                context, security_group, group_name, group_description)
 
         return {'security_group': self._format_security_group(context,
                                                               group_ref)}
@@ -333,11 +351,12 @@ class SecurityGroupRulesController(SecurityGroupControllerBase):
 
         sg_rule = self._from_body(body, 'security_group_rule')
 
-        parent_group_id = self.security_group_api.validate_id(
-            sg_rule.get('parent_group_id', None))
-
-        security_group = self.security_group_api.get(context, None,
-                                          parent_group_id, map_exception=True)
+        with translate_exceptions():
+            parent_group_id = self.security_group_api.validate_id(
+                sg_rule.get('parent_group_id', None))
+            security_group = self.security_group_api.get(context, None,
+                                                         parent_group_id,
+                                                         map_exception=True)
         try:
             new_rule = self._rule_args_to_dict(context,
                               to_port=sg_rule.get('to_port'),
@@ -360,13 +379,10 @@ class SecurityGroupRulesController(SecurityGroupControllerBase):
                 msg = _("Bad prefix for network in cidr %s") % new_rule['cidr']
                 raise exc.HTTPBadRequest(explanation=msg)
 
-        try:
+        with translate_exceptions():
             security_group_rule = (
                 self.security_group_api.create_security_group_rule(
                     context, security_group, new_rule))
-        except exception.SecurityGroupLimitExceeded as err:
-            raise exc.HTTPRequestEntityTooLarge(
-                explanation=err.format_message())
 
         return {"security_group_rule": self._format_security_group_rule(
                                                         context,
@@ -390,17 +406,15 @@ class SecurityGroupRulesController(SecurityGroupControllerBase):
     def delete(self, req, id):
         context = _authorize_context(req)
 
-        id = self.security_group_api.validate_id(id)
-
-        rule = self.security_group_api.get_rule(context, id)
-
-        group_id = rule['parent_group_id']
-
-        security_group = self.security_group_api.get(context, None, group_id,
-                                                     map_exception=True)
-
-        self.security_group_api.remove_rules(context, security_group,
-                                             [rule['id']])
+        with translate_exceptions():
+            id = self.security_group_api.validate_id(id)
+            rule = self.security_group_api.get_rule(context, id)
+            group_id = rule['parent_group_id']
+            security_group = self.security_group_api.get(context, None,
+                                                         group_id,
+                                                         map_exception=True)
+            self.security_group_api.remove_rules(context, security_group,
+                                                 [rule['id']])
 
         return webob.Response(status_int=202)
 
@@ -414,13 +428,11 @@ class ServerSecurityGroupController(SecurityGroupControllerBase):
 
         self.security_group_api.ensure_default(context)
 
-        try:
+        with translate_exceptions():
             instance = self.compute_api.get(context, server_id)
-        except exception.InstanceNotFound as exp:
-            raise exc.HTTPNotFound(explanation=exp.format_message())
+            groups = self.security_group_api.get_instance_security_groups(
+                context, instance['uuid'], True)
 
-        groups = self.security_group_api.get_instance_security_groups(
-            context, instance['uuid'], True)
         result = [self._format_security_group(context, group)
                     for group in groups]
 
@@ -455,15 +467,9 @@ class SecurityGroupActionController(wsgi.Controller):
         return group_name
 
     def _invoke(self, method, context, id, group_name):
-        try:
+        with translate_exceptions():
             instance = self.compute_api.get(context, id)
             method(context, instance, group_name)
-        except exception.SecurityGroupNotFound as exp:
-            raise exc.HTTPNotFound(explanation=exp.format_message())
-        except exception.InstanceNotFound as exp:
-            raise exc.HTTPNotFound(explanation=exp.format_message())
-        except exception.Invalid as exp:
-            raise exc.HTTPBadRequest(explanation=exp.format_message())
 
         return webob.Response(status_int=202)
 
@@ -642,15 +648,15 @@ class Security_groups(extensions.ExtensionDescriptor):
 class NativeSecurityGroupExceptions(object):
     @staticmethod
     def raise_invalid_property(msg):
-        raise exc.HTTPBadRequest(explanation=msg)
+        raise exception.Invalid(msg)
 
     @staticmethod
     def raise_group_already_exists(msg):
-        raise exc.HTTPBadRequest(explanation=msg)
+        raise exception.Invalid(msg)
 
     @staticmethod
     def raise_invalid_group(msg):
-        raise exc.HTTPBadRequest(explanation=msg)
+        raise exception.Invalid(msg)
 
     @staticmethod
     def raise_invalid_cidr(cidr, decoding_exception=None):
@@ -662,7 +668,7 @@ class NativeSecurityGroupExceptions(object):
 
     @staticmethod
     def raise_not_found(msg):
-        raise exc.HTTPNotFound(explanation=msg)
+        raise exception.SecurityGroupNotFound(msg)
 
 
 class NativeNovaSecurityGroupAPI(NativeSecurityGroupExceptions,