Dedupe native and EC2 security group APIs.

Reduce the code duplication in the native openstack and EC2 APIs
related to security groups, by factoring commonality into a new
internal SecurityGroupAPI.

Also fixes bug lp 1005931

Change-Id: Ifb92bf5d0f07d5713818a3eee6175ef03e8c0b7c

2 files changed, 219 insertions, 585 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 6f0d605ed..037a84783 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -25,7 +25,6 @@ datastore.
 import base64
 import re
 import time
-import urllib
 
 from nova.api.ec2 import ec2utils
 from nova.api.ec2 import inst_state
@@ -41,7 +40,6 @@ from nova.image import s3
 from nova import log as logging
 from nova import network
 from nova.openstack.common import excutils
-from nova.openstack.common import importutils
 from nova import quota
 from nova import utils
 from nova import volume
@@ -190,10 +188,11 @@ class CloudController(object):
         self.image_service = s3.S3ImageService()
         self.network_api = network.API()
         self.volume_api = volume.API()
+        self.security_group_api = CloudSecurityGroupAPI()
         self.compute_api = compute.API(network_api=self.network_api,
-                                       volume_api=self.volume_api)
+                                   volume_api=self.volume_api,
+                                   security_group_api=self.security_group_api)
         self.keypair_api = compute.api.KeypairAPI()
-        self.sgh = importutils.import_object(FLAGS.security_group_handler)
 
     def __str__(self):
         return 'CloudController'
@@ -411,25 +410,12 @@ class CloudController(object):
 
     def describe_security_groups(self, context, group_name=None, group_id=None,
                                  **kwargs):
-        self.compute_api.ensure_default_security_group(context)
-        if group_name or group_id:
-            groups = []
-            if group_name:
-                for name in group_name:
-                    group = db.security_group_get_by_name(context,
-                                                          context.project_id,
-                                                          name)
-                    groups.append(group)
-            if group_id:
-                for gid in group_id:
-                    group = db.security_group_get(context, gid)
-                    groups.append(group)
-        elif context.is_admin:
-            groups = db.security_group_get_all(context)
-        else:
-            groups = db.security_group_get_by_project(context,
-                                                      context.project_id)
-        groups = [self._format_security_group(context, g) for g in groups]
+        raw_groups = self.security_group_api.list(context,
+                                                  group_name,
+                                                  group_id,
+                                                  context.project_id)
+
+        groups = [self._format_security_group(context, g) for g in raw_groups]
 
         return {'securityGroupInfo':
                 list(sorted(groups,
@@ -536,146 +522,51 @@ class CloudController(object):
             notfound = exception.SecurityGroupNotFound
             if not source_security_group:
                 raise notfound(security_group_id=source_security_group_name)
-            values['group_id'] = source_security_group['id']
-        elif cidr_ip:
-            # If this fails, it throws an exception. This is what we want.
-            cidr_ip = urllib.unquote(cidr_ip).decode()
-
-            if not utils.is_valid_cidr(cidr_ip):
-                # Raise exception for non-valid address
-                raise exception.EC2APIError(_("Invalid CIDR"))
-
-            values['cidr'] = cidr_ip
-        else:
-            values['cidr'] = '0.0.0.0/0'
-
-        if source_security_group_name:
-            # Open everything if an explicit port range or type/code are not
-            # specified, but only if a source group was specified.
-            ip_proto_upper = ip_protocol.upper() if ip_protocol else ''
-            if (ip_proto_upper == 'ICMP' and
-                from_port is None and to_port is None):
-                from_port = -1
-                to_port = -1
-            elif (ip_proto_upper in ['TCP', 'UDP'] and from_port is None
-                  and to_port is None):
-                from_port = 1
-                to_port = 65535
-
-        if ip_protocol and from_port is not None and to_port is not None:
-
-            ip_protocol = str(ip_protocol)
-            try:
-                # Verify integer conversions
-                from_port = int(from_port)
-                to_port = int(to_port)
-            except ValueError:
-                if ip_protocol.upper() == 'ICMP':
-                    raise exception.InvalidInput(reason="Type and"
-                         " Code must be integers for ICMP protocol type")
-                else:
-                    raise exception.InvalidInput(reason="To and From ports "
-                          "must be integers")
-
-            if ip_protocol.upper() not in ['TCP', 'UDP', 'ICMP']:
-                raise exception.InvalidIpProtocol(protocol=ip_protocol)
-
-            # Verify that from_port must always be less than
-            # or equal to to_port
-            if (ip_protocol.upper() in ['TCP', 'UDP'] and
-                (from_port > to_port)):
-                raise exception.InvalidPortRange(from_port=from_port,
-                      to_port=to_port, msg="Former value cannot"
-                                            " be greater than the later")
-
-            # Verify valid TCP, UDP port ranges
-            if (ip_protocol.upper() in ['TCP', 'UDP'] and
-                (from_port < 1 or to_port > 65535)):
-                raise exception.InvalidPortRange(from_port=from_port,
-                      to_port=to_port, msg="Valid TCP ports should"
-                                           " be between 1-65535")
-
-            # Verify ICMP type and code
-            if (ip_protocol.upper() == "ICMP" and
-                (from_port < -1 or from_port > 255 or
-                to_port < -1 or to_port > 255)):
-                raise exception.InvalidPortRange(from_port=from_port,
-                      to_port=to_port, msg="For ICMP, the"
-                                           " type:code must be valid")
-
-            values['protocol'] = ip_protocol.lower()
-            values['from_port'] = from_port
-            values['to_port'] = to_port
+            group_id = source_security_group['id']
+            return self.security_group_api.new_group_ingress_rule(
+                                    group_id, ip_protocol, from_port, to_port)
         else:
-            # If cidr based filtering, protocol and ports are mandatory
-            if 'cidr' in values:
-                return None
-
-        return values
-
-    def _security_group_rule_exists(self, security_group, values):
-        """Indicates whether the specified rule values are already
-           defined in the given security group.
-        """
-        for rule in security_group.rules:
-            is_duplicate = True
-            keys = ('group_id', 'cidr', 'from_port', 'to_port', 'protocol')
-            for key in keys:
-                if rule.get(key) != values.get(key):
-                    is_duplicate = False
-                    break
-            if is_duplicate:
-                return rule['id']
-        return False
+            cidr = self.security_group_api.parse_cidr(cidr_ip)
+            return self.security_group_api.new_cidr_ingress_rule(
+                                        cidr, ip_protocol, from_port, to_port)
 
-    def revoke_security_group_ingress(self, context, group_name=None,
-                                      group_id=None, **kwargs):
+    def _validate_group_identifier(self, group_name, group_id):
         if not group_name and not group_id:
             err = _("Not enough parameters, need group_name or group_id")
             raise exception.EC2APIError(err)
-        self.compute_api.ensure_default_security_group(context)
-        notfound = exception.SecurityGroupNotFound
-        if group_name:
-            security_group = db.security_group_get_by_name(context,
-                                                           context.project_id,
-                                                           group_name)
-            if not security_group:
-                raise notfound(security_group_id=group_name)
-        if group_id:
-            security_group = db.security_group_get(context, group_id)
-            if not security_group:
-                raise notfound(security_group_id=group_id)
-
-        msg = _("Revoke security group ingress %s")
-        LOG.audit(msg, security_group['name'], context=context)
-        prevalues = []
-        try:
-            prevalues = kwargs['ip_permissions']
-        except KeyError:
-            prevalues.append(kwargs)
-        rule_id = None
+
+    def _validate_rulevalues(self, rulesvalues):
+        if not rulesvalues:
+            err = _("%s Not enough parameters to build a valid rule")
+            raise exception.EC2APIError(err % rulesvalues)
+
+    def revoke_security_group_ingress(self, context, group_name=None,
+                                      group_id=None, **kwargs):
+        self._validate_group_identifier(group_name, group_id)
+
+        security_group = self.security_group_api.get(context, group_name,
+                                                     group_id)
+
+        prevalues = kwargs.get('ip_permissions', [kwargs])
+
         rule_ids = []
         for values in prevalues:
             rulesvalues = self._rule_args_to_dict(context, values)
-            if not rulesvalues:
-                err = _("%s Not enough parameters to build a valid rule")
-                raise exception.EC2APIError(err % rulesvalues)
-
+            self._validate_rulevalues(rulesvalues)
             for values_for_rule in rulesvalues:
                 values_for_rule['parent_group_id'] = security_group.id
-                rule_id = self._security_group_rule_exists(security_group,
-                                                           values_for_rule)
-                if rule_id:
-                    db.security_group_rule_destroy(context, rule_id)
-                    rule_ids.append(rule_id)
-        if rule_id:
-            # NOTE(vish): we removed a rule, so refresh
-            self.compute_api.trigger_security_group_rules_refresh(
-                    context,
-                    security_group_id=security_group['id'])
-            self.sgh.trigger_security_group_rule_destroy_refresh(
-                    context, rule_ids)
+
+                rule_ids.append(self.security_group_api.rule_exists(
+                                             security_group, values_for_rule))
+
+        rule_ids = [id for id in rule_ids if id]
+
+        if rule_ids:
+            self.security_group_api.remove_rules(context, security_group,
+                                                 rule_ids)
+
             return True
+
         raise exception.EC2APIError(_("No rule for the specified parameters."))
 
     # TODO(soren): This has only been tested with Boto as the client.
@@ -684,64 +575,27 @@ class CloudController(object):
     #              is sketchy.
     def authorize_security_group_ingress(self, context, group_name=None,
                                          group_id=None, **kwargs):
-        if not group_name and not group_id:
-            err = _("Not enough parameters, need group_name or group_id")
-            raise exception.EC2APIError(err)
-        self.compute_api.ensure_default_security_group(context)
-        notfound = exception.SecurityGroupNotFound
-        if group_name:
-            security_group = db.security_group_get_by_name(context,
-                                                           context.project_id,
-                                                           group_name)
-            if not security_group:
-                raise notfound(security_group_id=group_name)
-        if group_id:
-            security_group = db.security_group_get(context, group_id)
-            if not security_group:
-                raise notfound(security_group_id=group_id)
-
-        msg = _("Authorize security group ingress %s")
-        LOG.audit(msg, security_group['name'], context=context)
-        prevalues = []
-        try:
-            prevalues = kwargs['ip_permissions']
-        except KeyError:
-            prevalues.append(kwargs)
+        self._validate_group_identifier(group_name, group_id)
+
+        security_group = self.security_group_api.get(context, group_name,
+                                                     group_id)
+
+        prevalues = kwargs.get('ip_permissions', [kwargs])
         postvalues = []
         for values in prevalues:
             rulesvalues = self._rule_args_to_dict(context, values)
-            if not rulesvalues:
-                err = _("%s Not enough parameters to build a valid rule")
-                raise exception.EC2APIError(err % rulesvalues)
+            self._validate_rulevalues(rulesvalues)
             for values_for_rule in rulesvalues:
                 values_for_rule['parent_group_id'] = security_group.id
-                if self._security_group_rule_exists(security_group,
-                                                    values_for_rule):
+                if self.security_group_api.rule_exists(security_group,
+                                                       values_for_rule):
                     err = _('%s - This rule already exists in group')
                     raise exception.EC2APIError(err % values_for_rule)
                 postvalues.append(values_for_rule)
 
-        count = QUOTAS.count(context, 'security_group_rules',
-                             security_group['id'])
-        try:
-            QUOTAS.limit_check(context, security_group_rules=count + 1)
-        except exception.OverQuota:
-            msg = _("Quota exceeded, too many security group rules.")
-            raise exception.EC2APIError(msg)
-
-        rule_ids = []
-        for values_for_rule in postvalues:
-            security_group_rule = db.security_group_rule_create(
-                    context,
-                    values_for_rule)
-            rule_ids.append(security_group_rule['id'])
-
         if postvalues:
-            self.compute_api.trigger_security_group_rules_refresh(
-                    context,
-                    security_group_id=security_group['id'])
-            self.sgh.trigger_security_group_rule_create_refresh(
-                    context, rule_ids)
+            self.security_group_api.add_rules(context, security_group['id'],
+                                           security_group['name'], postvalues)
             return True
 
         raise exception.EC2APIError(_("No rule for the specified parameters."))
@@ -766,64 +620,23 @@ class CloudController(object):
     def create_security_group(self, context, group_name, group_description):
         if isinstance(group_name, unicode):
             group_name = group_name.encode('utf-8')
-        # TODO(Daviey): LP: #813685 extend beyond group_name checking, and
-        # probably create a param validator that can be used elsewhere.
         if FLAGS.ec2_strict_validation:
             # EC2 specification gives constraints for name and description:
             # Accepts alphanumeric characters, spaces, dashes, and underscores
-            err = _("Value (%(value)s) for parameter %(param)s is invalid."
-                    " Content limited to Alphanumeric characters,"
-                    " spaces, dashes, and underscores.")
-            if not re.match('^[a-zA-Z0-9_\- ]+$', group_name):
-                raise exception.InvalidParameterValue(
-                    err=err % {"value": group_name,
-                               "param": "GroupName"})
-            if not re.match('^[a-zA-Z0-9_\- ]+$', group_description):
-                raise exception.InvalidParameterValue(
-                    err=err % {"value": group_description,
-                               "param": "GroupDescription"})
+            allowed = '^[a-zA-Z0-9_\- ]+$'
+            self.security_group_api.validate_property(group_name, 'name',
+                                                      allowed)
+            self.security_group_api.validate_property(group_description,
+                                                      'description', allowed)
         else:
             # Amazon accepts more symbols.
             # So, allow POSIX [:print:] characters.
-            if not re.match(r'^[\x20-\x7E]+$', group_name):
-                err = _("Value (%(value)s) for parameter %(param)s is invalid."
-                        " Content is limited to characters"
-                        " from the [:print:] class.")
-                raise exception.InvalidParameterValue(
-                    err=err % {"value": group_name,
-                               "param": "GroupName"})
-
-        if len(group_name) > 255:
-            err = _("Value (%s) for parameter GroupName is invalid."
-                    " Length exceeds maximum of 255.") % group_name
-            raise exception.InvalidParameterValue(err=err)
-
-        LOG.audit(_("Create Security Group %s"), group_name, context=context)
-        self.compute_api.ensure_default_security_group(context)
-        if db.security_group_exists(context, context.project_id, group_name):
-            msg = _('group %s already exists')
-            raise exception.EC2APIError(msg % group_name)
+            allowed = r'^[\x20-\x7E]+$'
+            self.security_group_api.validate_property(group_name, 'name',
+                                                      allowed)
 
-        try:
-            reservations = QUOTAS.reserve(context, security_groups=1)
-        except exception.OverQuota:
-            msg = _("Quota exceeded, too many security groups.")
-            raise exception.EC2APIError(msg)
-
-        try:
-            group = {'user_id': context.user_id,
-                     'project_id': context.project_id,
-                     'name': group_name,
-                     'description': group_description}
-            group_ref = db.security_group_create(context, group)
-
-            self.sgh.trigger_security_group_create_refresh(context, group)
-
-            # Commit the reservation
-            QUOTAS.commit(context, reservations)
-        except Exception:
-            with excutils.save_and_reraise_exception():
-                QUOTAS.rollback(context, reservations)
+        group_ref = self.security_group_api.create(context, group_name,
+                                                   group_description)
 
         return {'securityGroupSet': [self._format_security_group(context,
                                                                  group_ref)]}
@@ -833,37 +646,11 @@ class CloudController(object):
         if not group_name and not group_id:
             err = _("Not enough parameters, need group_name or group_id")
             raise exception.EC2APIError(err)
-        notfound = exception.SecurityGroupNotFound
-        if group_name:
-            security_group = db.security_group_get_by_name(context,
-                                                           context.project_id,
-                                                           group_name)
-            if not security_group:
-                raise notfound(security_group_id=group_name)
-        elif group_id:
-            security_group = db.security_group_get(context, group_id)
-            if not security_group:
-                raise notfound(security_group_id=group_id)
-        if db.security_group_in_use(context, security_group.id):
-            raise exception.InvalidGroup(reason="In Use")
-
-        # Get reservations
-        try:
-            reservations = QUOTAS.reserve(context, security_groups=-1)
-        except Exception:
-            reservations = None
-            LOG.exception(_("Failed to update usages deallocating "
-                            "security group"))
 
-        LOG.audit(_("Delete security group %s"), group_name, context=context)
-        db.security_group_destroy(context, security_group.id)
+        security_group = self.security_group_api.get(context, group_name,
+                                                     group_id)
 
-        self.sgh.trigger_security_group_destroy_refresh(context,
-                                                        security_group.id)
-
-        # Commit the reservations
-        if reservations:
-            QUOTAS.commit(context, reservations)
+        self.security_group_api.destroy(context, security_group)
 
         return True
 
@@ -1719,3 +1506,32 @@ class CloudController(object):
             self.compute_api.start(context, instance_id=instance_id)
 
         return {'imageId': image_id}
+
+
+class CloudSecurityGroupAPI(compute.api.SecurityGroupAPI):
+    @staticmethod
+    def raise_invalid_property(msg):
+        raise exception.InvalidParameterValue(err=msg)
+
+    @staticmethod
+    def raise_group_already_exists(msg):
+        raise exception.EC2APIError(message=msg)
+
+    @staticmethod
+    def raise_invalid_group(msg):
+        raise exception.InvalidGroup(reason=msg)
+
+    @staticmethod
+    def raise_invalid_cidr(cidr, decoding_exception=None):
+        if decoding_exception:
+            raise decoding_exception
+        else:
+            raise exception.EC2APIError(_("Invalid CIDR"))
+
+    @staticmethod
+    def raise_over_quota(msg):
+        raise exception.EC2APIError(message=msg)
+
+    @staticmethod
+    def raise_not_found(msg):
+        pass
diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py
index 4a69d392e..5e81347ec 100644
--- a/nova/api/openstack/compute/contrib/security_groups.py
+++ b/nova/api/openstack/compute/contrib/security_groups.py
@@ -16,7 +16,6 @@
 
 """The security groups extension."""
 
-import urllib
 from xml.dom import minidom
 
 import webob
@@ -32,14 +31,11 @@ from nova import exception
 from nova import flags
 from nova import log as logging
 from nova.openstack.common import excutils
-from nova.openstack.common import importutils
-from nova import quota
 from nova import utils
 
 
 LOG = logging.getLogger(__name__)
 FLAGS = flags.FLAGS
-QUOTAS = quota.QUOTAS
 authorize = extensions.extension_authorizer('compute', 'security_groups')
 
 
@@ -182,8 +178,9 @@ class SecurityGroupControllerBase(object):
     """Base class for Security Group controllers."""
 
     def __init__(self):
-        self.compute_api = compute.API()
-        self.sgh = importutils.import_object(FLAGS.security_group_handler)
+        self.security_group_api = NativeSecurityGroupAPI()
+        self.compute_api = compute.API(
+                                   security_group_api=self.security_group_api)
 
     def _format_security_group_rule(self, context, rule):
         sg_rule = {}
@@ -195,7 +192,8 @@ class SecurityGroupControllerBase(object):
         sg_rule['group'] = {}
         sg_rule['ip_range'] = {}
         if rule.group_id:
-            source_group = db.security_group_get(context, rule.group_id)
+            source_group = self.security_group_api.get(context,
+                                                       id=rule.group_id)
             sg_rule['group'] = {'name': source_group.name,
                              'tenant_id': source_group.project_id}
         else:
@@ -214,68 +212,65 @@ class SecurityGroupControllerBase(object):
                     context, rule)]
         return security_group
 
+    def _authorize_context(self, req):
+        context = req.environ['nova.context']
+        authorize(context)
+        return context
 
-class SecurityGroupController(SecurityGroupControllerBase):
-    """The Security group API controller for the OpenStack API."""
-
-    def _get_security_group(self, context, id):
+    def _validate_id(self, id):
         try:
-            id = int(id)
-            security_group = db.security_group_get(context, id)
+            return int(id)
         except ValueError:
             msg = _("Security group id should be integer")
             raise exc.HTTPBadRequest(explanation=msg)
-        except exception.NotFound as exp:
-            raise exc.HTTPNotFound(explanation=unicode(exp))
-        return security_group
+
+    def _from_body(self, body, key):
+        if not body:
+            raise exc.HTTPUnprocessableEntity()
+        value = body.get(key, None)
+        if value is None:
+            raise exc.HTTPUnprocessableEntity()
+        return value
+
+
+class SecurityGroupController(SecurityGroupControllerBase):
+    """The Security group API controller for the OpenStack API."""
 
     @wsgi.serializers(xml=SecurityGroupTemplate)
     def show(self, req, id):
         """Return data about the given security group."""
-        context = req.environ['nova.context']
-        authorize(context)
-        security_group = self._get_security_group(context, id)
+        context = self._authorize_context(req)
+
+        id = self._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)}
 
     def delete(self, req, id):
         """Delete a security group."""
-        context = req.environ['nova.context']
-        authorize(context)
-        security_group = self._get_security_group(context, id)
-        if db.security_group_in_use(context, security_group.id):
-            msg = _("Security group is still in use")
-            raise exc.HTTPBadRequest(explanation=msg)
+        context = self._authorize_context(req)
 
-        # Get reservations
-        try:
-            reservations = QUOTAS.reserve(context, security_groups=-1)
-        except Exception:
-            reservations = None
-            LOG.exception(_("Failed to update usages deallocating "
-                            "security group"))
+        id = self._validate_id(id)
 
-        LOG.audit(_("Delete security group %s"), id, context=context)
-        db.security_group_destroy(context, security_group.id)
-        self.sgh.trigger_security_group_destroy_refresh(
-            context, security_group.id)
+        security_group = self.security_group_api.get(context, None, id,
+                                                     map_exception=True)
 
-        # Commit the reservations
-        if reservations:
-            QUOTAS.commit(context, reservations)
+        self.security_group_api.destroy(context, security_group)
 
         return webob.Response(status_int=202)
 
     @wsgi.serializers(xml=SecurityGroupsTemplate)
     def index(self, req):
         """Returns a list of security groups"""
-        context = req.environ['nova.context']
-        authorize(context)
+        context = self._authorize_context(req)
+
+        raw_groups = self.security_group_api.list(context,
+                                                  project=context.project_id)
 
-        self.compute_api.ensure_default_security_group(context)
-        groups = db.security_group_get_by_project(context,
-                                                  context.project_id)
-        limited_list = common.limited(groups, req)
+        limited_list = common.limited(raw_groups, req)
         result = [self._format_security_group(context, group)
                      for group in limited_list]
 
@@ -287,110 +282,43 @@ class SecurityGroupController(SecurityGroupControllerBase):
     @wsgi.deserializers(xml=SecurityGroupXMLDeserializer)
     def create(self, req, body):
         """Creates a new security group."""
-        context = req.environ['nova.context']
-        authorize(context)
-        if not body:
-            raise exc.HTTPUnprocessableEntity()
+        context = self._authorize_context(req)
 
-        security_group = body.get('security_group', None)
-
-        if security_group is None:
-            raise exc.HTTPUnprocessableEntity()
+        security_group = self._from_body(body, 'security_group')
 
         group_name = security_group.get('name', None)
         group_description = security_group.get('description', None)
 
-        self._validate_security_group_property(group_name, "name")
-        self._validate_security_group_property(group_description,
-                                               "description")
-        group_name = group_name.strip()
-        group_description = group_description.strip()
-
-        try:
-            reservations = QUOTAS.reserve(context, security_groups=1)
-        except exception.OverQuota:
-            msg = _("Quota exceeded, too many security groups.")
-            raise exc.HTTPBadRequest(explanation=msg)
+        self.security_group_api.validate_property(group_name, 'name', None)
+        self.security_group_api.validate_property(group_description,
+                                                  'description', None)
 
-        try:
-            LOG.audit(_("Create Security Group %s"), group_name,
-                      context=context)
-            self.compute_api.ensure_default_security_group(context)
-            if db.security_group_exists(context, context.project_id,
-                                        group_name):
-                msg = _('Security group %s already exists') % group_name
-                raise exc.HTTPBadRequest(explanation=msg)
-
-            group = {'user_id': context.user_id,
-                     'project_id': context.project_id,
-                     'name': group_name,
-                     'description': group_description}
-            group_ref = db.security_group_create(context, group)
-            self.sgh.trigger_security_group_create_refresh(context, group)
-
-            # Commit the reservation
-            QUOTAS.commit(context, reservations)
-        except Exception:
-            with excutils.save_and_reraise_exception():
-                QUOTAS.rollback(context, reservations)
+        group_ref = self.security_group_api.create(context, group_name,
+                                                   group_description)
 
         return {'security_group': self._format_security_group(context,
                                                                  group_ref)}
 
-    def _validate_security_group_property(self, value, typ):
-        """ typ will be either 'name' or 'description',
-            depending on the caller
-        """
-        try:
-            val = value.strip()
-        except AttributeError:
-            msg = _("Security group %s is not a string or unicode") % typ
-            raise exc.HTTPBadRequest(explanation=msg)
-        if not val:
-            msg = _("Security group %s cannot be empty.") % typ
-            raise exc.HTTPBadRequest(explanation=msg)
-        if len(val) > 255:
-            msg = _("Security group %s should not be greater "
-                            "than 255 characters.") % typ
-            raise exc.HTTPBadRequest(explanation=msg)
-
 
 class SecurityGroupRulesController(SecurityGroupControllerBase):
 
     @wsgi.serializers(xml=SecurityGroupRuleTemplate)
     @wsgi.deserializers(xml=SecurityGroupRulesXMLDeserializer)
     def create(self, req, body):
-        context = req.environ['nova.context']
-        authorize(context)
+        context = self._authorize_context(req)
 
-        if not body:
-            raise exc.HTTPUnprocessableEntity()
+        sg_rule = self._from_body(body, 'security_group_rule')
 
-        if not 'security_group_rule' in body:
-            raise exc.HTTPUnprocessableEntity()
+        parent_group_id = self._validate_id(sg_rule.get('parent_group_id',
+                                                        None))
 
-        self.compute_api.ensure_default_security_group(context)
-
-        sg_rule = body['security_group_rule']
-        parent_group_id = sg_rule.get('parent_group_id', None)
-        try:
-            parent_group_id = int(parent_group_id)
-            security_group = db.security_group_get(context, parent_group_id)
-        except ValueError:
-            msg = _("Parent group id is not integer")
-            raise exc.HTTPBadRequest(explanation=msg)
-        except exception.NotFound as exp:
-            msg = _("Security group (%s) not found") % parent_group_id
-            raise exc.HTTPNotFound(explanation=msg)
-
-        msg = _("Authorize security group ingress %s")
-        LOG.audit(msg, security_group['name'], context=context)
+        security_group = self.security_group_api.get(context, None,
+                                          parent_group_id, map_exception=True)
 
         try:
             values = self._rule_args_to_dict(context,
                               to_port=sg_rule.get('to_port'),
                               from_port=sg_rule.get('from_port'),
-                              parent_group_id=sg_rule.get('parent_group_id'),
                               ip_protocol=sg_rule.get('ip_protocol'),
                               cidr=sg_rule.get('cidr'),
                               group_id=sg_rule.get('group_id'))
@@ -398,169 +326,50 @@ class SecurityGroupRulesController(SecurityGroupControllerBase):
             raise exc.HTTPBadRequest(explanation=unicode(exp))
 
         if values is None:
-            msg = _("Not enough parameters to build a "
-                                       "valid rule.")
+            msg = _("Not enough parameters to build a valid rule.")
             raise exc.HTTPBadRequest(explanation=msg)
 
         values['parent_group_id'] = security_group.id
 
-        if self._security_group_rule_exists(security_group, values):
+        if self.security_group_api.rule_exists(security_group, values):
             msg = _('This rule already exists in group %s') % parent_group_id
             raise exc.HTTPBadRequest(explanation=msg)
 
-        count = QUOTAS.count(context, 'security_group_rules', parent_group_id)
-        try:
-            QUOTAS.limit_check(context, security_group_rules=count + 1)
-        except exception.OverQuota:
-            msg = _("Quota exceeded, too many security group rules.")
-            raise exc.HTTPBadRequest(explanation=msg)
-
-        security_group_rule = db.security_group_rule_create(context, values)
-        self.sgh.trigger_security_group_rule_create_refresh(
-            context, [security_group_rule['id']])
-        self.compute_api.trigger_security_group_rules_refresh(context,
-                                    security_group_id=security_group['id'])
+        security_group_rule = self.security_group_api.add_rules(
+                context, parent_group_id, security_group['name'], [values])[0]
 
         return {"security_group_rule": self._format_security_group_rule(
                                                         context,
                                                         security_group_rule)}
 
-    def _security_group_rule_exists(self, security_group, values):
-        """Indicates whether the specified rule values are already
-           defined in the given security group.
-        """
-        for rule in security_group.rules:
-            is_duplicate = True
-            keys = ('group_id', 'cidr', 'from_port', 'to_port', 'protocol')
-            for key in keys:
-                if rule.get(key) != values.get(key):
-                    is_duplicate = False
-                    break
-            if is_duplicate:
-                return True
-        return False
-
     def _rule_args_to_dict(self, context, to_port=None, from_port=None,
-                                  parent_group_id=None, ip_protocol=None,
-                                  cidr=None, group_id=None):
-        values = {}
+                           ip_protocol=None, cidr=None, group_id=None):
 
         if group_id is not None:
-            try:
-                parent_group_id = int(parent_group_id)
-                group_id = int(group_id)
-            except ValueError:
-                msg = _("Parent or group id is not integer")
-                raise exception.InvalidInput(reason=msg)
-
-            values['group_id'] = group_id
+            group_id = self._validate_id(group_id)
             #check if groupId exists
-            db.security_group_get(context, group_id)
-        elif cidr:
-            # If this fails, it throws an exception. This is what we want.
-            try:
-                cidr = urllib.unquote(cidr).decode()
-            except Exception:
-                raise exception.InvalidCidr(cidr=cidr)
-
-            if not utils.is_valid_cidr(cidr):
-                # Raise exception for non-valid address
-                raise exception.InvalidCidr(cidr=cidr)
-
-            values['cidr'] = cidr
+            self.security_group_api.get(context, id=group_id)
+            return self.security_group_api.new_group_ingress_rule(
+                                    group_id, ip_protocol, from_port, to_port)
         else:
-            values['cidr'] = '0.0.0.0/0'
-
-        if group_id:
-            # Open everything if an explicit port range or type/code are not
-            # specified, but only if a source group was specified.
-            ip_proto_upper = ip_protocol.upper() if ip_protocol else ''
-            if (ip_proto_upper == 'ICMP' and
-                from_port is None and to_port is None):
-                from_port = -1
-                to_port = -1
-            elif (ip_proto_upper in ['TCP', 'UDP'] and from_port is None
-                  and to_port is None):
-                from_port = 1
-                to_port = 65535
-
-        if ip_protocol and from_port is not None and to_port is not None:
-
-            ip_protocol = str(ip_protocol)
-            try:
-                from_port = int(from_port)
-                to_port = int(to_port)
-            except ValueError:
-                if ip_protocol.upper() == 'ICMP':
-                    raise exception.InvalidInput(reason="Type and"
-                         " Code must be integers for ICMP protocol type")
-                else:
-                    raise exception.InvalidInput(reason="To and From ports "
-                          "must be integers")
-
-            if ip_protocol.upper() not in ['TCP', 'UDP', 'ICMP']:
-                raise exception.InvalidIpProtocol(protocol=ip_protocol)
-
-            # Verify that from_port must always be less than
-            # or equal to to_port
-            if (ip_protocol.upper() in ['TCP', 'UDP'] and
-                from_port > to_port):
-                raise exception.InvalidPortRange(from_port=from_port,
-                      to_port=to_port, msg="Former value cannot"
-                                            " be greater than the later")
-
-            # Verify valid TCP, UDP port ranges
-            if (ip_protocol.upper() in ['TCP', 'UDP'] and
-                (from_port < 1 or to_port > 65535)):
-                raise exception.InvalidPortRange(from_port=from_port,
-                      to_port=to_port, msg="Valid TCP ports should"
-                                           " be between 1-65535")
-
-            # Verify ICMP type and code
-            if (ip_protocol.upper() == "ICMP" and
-                (from_port < -1 or from_port > 255 or
-                to_port < -1 or to_port > 255)):
-                raise exception.InvalidPortRange(from_port=from_port,
-                      to_port=to_port, msg="For ICMP, the"
-                                           " type:code must be valid")
-
-            values['protocol'] = ip_protocol.lower()
-            values['from_port'] = from_port
-            values['to_port'] = to_port
-        else:
-            # If cidr based filtering, protocol and ports are mandatory
-            if 'cidr' in values:
-                return None
-
-        return values
+            cidr = self.security_group_api.parse_cidr(cidr)
+            return self.security_group_api.new_cidr_ingress_rule(
+                                        cidr, ip_protocol, from_port, to_port)
 
     def delete(self, req, id):
-        context = req.environ['nova.context']
-        authorize(context)
+        context = self._authorize_context(req)
 
-        self.compute_api.ensure_default_security_group(context)
-        try:
-            id = int(id)
-            rule = db.security_group_rule_get(context, id)
-        except ValueError:
-            msg = _("Rule id is not integer")
-            raise exc.HTTPBadRequest(explanation=msg)
-        except exception.NotFound:
-            msg = _("Rule (%s) not found") % id
-            raise exc.HTTPNotFound(explanation=msg)
+        id = self._validate_id(id)
+
+        rule = self.security_group_api.get_rule(context, id)
 
         group_id = rule.parent_group_id
-        self.compute_api.ensure_default_security_group(context)
-        security_group = db.security_group_get(context, group_id)
 
-        msg = _("Revoke security group ingress %s")
-        LOG.audit(msg, security_group['name'], context=context)
+        security_group = self.security_group_api.get(context, None, group_id,
+                                                     map_exception=True)
 
-        db.security_group_rule_destroy(context, rule['id'])
-        self.sgh.trigger_security_group_rule_destroy_refresh(
-            context, [rule['id']])
-        self.compute_api.trigger_security_group_rules_refresh(context,
-                                    security_group_id=security_group['id'])
+        self.security_group_api.remove_rules(context, security_group,
+                                             [rule['id']])
 
         return webob.Response(status_int=202)
 
@@ -570,10 +379,9 @@ class ServerSecurityGroupController(SecurityGroupControllerBase):
     @wsgi.serializers(xml=SecurityGroupsTemplate)
     def index(self, req, server_id):
         """Returns a list of security groups for the given instance."""
-        context = req.environ['nova.context']
-        authorize(context)
+        context = self._authorize_context(req)
 
-        self.compute_api.ensure_default_security_group(context)
+        self.security_group_api.ensure_default(context)
 
         try:
             instance = self.compute_api.get(context, server_id)
@@ -595,16 +403,13 @@ class ServerSecurityGroupController(SecurityGroupControllerBase):
 class SecurityGroupActionController(wsgi.Controller):
     def __init__(self, *args, **kwargs):
         super(SecurityGroupActionController, self).__init__(*args, **kwargs)
-        self.compute_api = compute.API()
-        self.sgh = importutils.import_object(FLAGS.security_group_handler)
-
-    @wsgi.action('addSecurityGroup')
-    def _addSecurityGroup(self, req, id, body):
-        context = req.environ['nova.context']
-        authorize(context)
+        self.security_group_api = NativeSecurityGroupAPI()
+        self.compute_api = compute.API(
+                                   security_group_api=self.security_group_api)
 
+    def _parse(self, body, action):
         try:
-            body = body['addSecurityGroup']
+            body = body[action]
             group_name = body['name']
         except TypeError:
             msg = _("Missing parameter dict")
@@ -617,11 +422,12 @@ class SecurityGroupActionController(wsgi.Controller):
             msg = _("Security group name cannot be empty")
             raise webob.exc.HTTPBadRequest(explanation=msg)
 
+        return group_name
+
+    def _invoke(self, method, context, id, group_name):
         try:
             instance = self.compute_api.get(context, id)
-            self.compute_api.add_security_group(context, instance, group_name)
-            self.sgh.trigger_instance_add_security_group_refresh(
-                context, instance, group_name)
+            method(context, instance, group_name)
         except exception.SecurityGroupNotFound as exp:
             raise exc.HTTPNotFound(explanation=unicode(exp))
         except exception.InstanceNotFound as exp:
@@ -631,39 +437,25 @@ class SecurityGroupActionController(wsgi.Controller):
 
         return webob.Response(status_int=202)
 
-    @wsgi.action('removeSecurityGroup')
-    def _removeSecurityGroup(self, req, id, body):
+    @wsgi.action('addSecurityGroup')
+    def _addSecurityGroup(self, req, id, body):
         context = req.environ['nova.context']
         authorize(context)
 
-        try:
-            body = body['removeSecurityGroup']
-            group_name = body['name']
-        except TypeError:
-            msg = _("Missing parameter dict")
-            raise webob.exc.HTTPBadRequest(explanation=msg)
-        except KeyError:
-            msg = _("Security group not specified")
-            raise webob.exc.HTTPBadRequest(explanation=msg)
+        group_name = self._parse(body, 'addSecurityGroup')
 
-        if not group_name or group_name.strip() == '':
-            msg = _("Security group name cannot be empty")
-            raise webob.exc.HTTPBadRequest(explanation=msg)
+        return self._invoke(self.security_group_api.add_to_instance,
+                            context, id, group_name)
 
-        try:
-            instance = self.compute_api.get(context, id)
-            self.compute_api.remove_security_group(context, instance,
-                                                   group_name)
-            self.sgh.trigger_instance_remove_security_group_refresh(
-                context, instance, group_name)
-        except exception.SecurityGroupNotFound as exp:
-            raise exc.HTTPNotFound(explanation=unicode(exp))
-        except exception.InstanceNotFound as exp:
-            raise exc.HTTPNotFound(explanation=unicode(exp))
-        except exception.Invalid as exp:
-            raise exc.HTTPBadRequest(explanation=unicode(exp))
+    @wsgi.action('removeSecurityGroup')
+    def _removeSecurityGroup(self, req, id, body):
+        context = req.environ['nova.context']
+        authorize(context)
 
-        return webob.Response(status_int=202)
+        group_name = self._parse(body, 'removeSecurityGroup')
+
+        return self._invoke(self.security_group_api.remove_from_instance,
+                            context, id, group_name)
 
 
 class Security_groups(extensions.ExtensionDescriptor):
@@ -698,3 +490,29 @@ class Security_groups(extensions.ExtensionDescriptor):
         resources.append(res)
 
         return resources
+
+
+class NativeSecurityGroupAPI(compute.api.SecurityGroupAPI):
+    @staticmethod
+    def raise_invalid_property(msg):
+        raise exc.HTTPBadRequest(explanation=msg)
+
+    @staticmethod
+    def raise_group_already_exists(msg):
+        raise exc.HTTPBadRequest(explanation=msg)
+
+    @staticmethod
+    def raise_invalid_group(msg):
+        raise exc.HTTPBadRequest(explanation=msg)
+
+    @staticmethod
+    def raise_invalid_cidr(cidr, decoding_exception=None):
+        raise exception.InvalidCidr(cidr=cidr)
+
+    @staticmethod
+    def raise_over_quota(msg):
+        raise exc.HTTPBadRequest(explanation=msg)
+
+    @staticmethod
+    def raise_not_found(msg):
+        raise exc.HTTPNotFound(explanation=msg)