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

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 @@ def _extract_security_group_rule(self, node):
         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 @@ def _format_security_group_rule(self, context, rule):
         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 @@ def show(self, req, id):
         """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 @@ def delete(self, req, id):
         """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 @@ def index(self, req):
         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 @@ def create(self, req, body):
         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 @@ def update(self, req, id, body):
         """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 @@ def create(self, req, body):
 
         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 @@ def create(self, req, body):
                 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 @@ def _rule_args_to_dict(self, context, to_port=None, from_port=None,
     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 @@ def index(self, req, server_id):
 
         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 @@ def _parse(self, body, action):
         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 @@ def get_resources(self):
 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 @@ def raise_over_quota(msg):
 
     @staticmethod
     def raise_not_found(msg):
-        raise exc.HTTPNotFound(explanation=msg)
+        raise exception.SecurityGroupNotFound(msg)
 
 
 class NativeNovaSecurityGroupAPI(NativeSecurityGroupExceptions,