Make nova security groups more pluggable

This patch moves the nova security group code out of nova/compute/api.py
into nova/network/security_group. It also removes any query to the database
from security group api into the nova security group driver. This allows
security group drivers the ability to decouple themselves from storing
security group information in the nova_db.

Change-Id: Ib183515a0418203c8bcc88176e3a1498d7333300

10 files changed, 419 insertions, 238 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 03bf9f890..b3f9bd099 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -214,7 +214,7 @@ class CloudController(object):
         self.image_service = s3.S3ImageService()
         self.network_api = network.API()
         self.volume_api = volume.API()
-        self.security_group_api = CloudSecurityGroupAPI()
+        self.security_group_api = get_cloud_security_group_api()
         self.compute_api = compute.API(network_api=self.network_api,
                                    volume_api=self.volume_api,
                                    security_group_api=self.security_group_api)
@@ -712,8 +712,8 @@ class CloudController(object):
             self.security_group_api.validate_property(group_name, 'name',
                                                       allowed)
 
-        group_ref = self.security_group_api.create(context, group_name,
-                                                   group_description)
+        group_ref = self.security_group_api.create_security_group(
+            context, group_name, group_description)
 
         return {'securityGroupSet': [self._format_security_group(context,
                                                                  group_ref)]}
@@ -1662,7 +1662,7 @@ class CloudController(object):
         return {'imageId': ec2_id}
 
 
-class CloudSecurityGroupAPI(compute_api.SecurityGroupAPI):
+class EC2SecurityGroupExceptions(object):
     @staticmethod
     def raise_invalid_property(msg):
         raise exception.InvalidParameterValue(err=msg)
@@ -1689,3 +1689,13 @@ class CloudSecurityGroupAPI(compute_api.SecurityGroupAPI):
     @staticmethod
     def raise_not_found(msg):
         pass
+
+
+class CloudSecurityGroupNovaAPI(compute_api.SecurityGroupAPI,
+                                EC2SecurityGroupExceptions):
+    pass
+
+
+def get_cloud_security_group_api():
+    if cfg.CONF.security_group_api.lower() == 'nova':
+        return CloudSecurityGroupNovaAPI()
diff --git a/nova/api/openstack/compute/contrib/security_group_default_rules.py b/nova/api/openstack/compute/contrib/security_group_default_rules.py
index fed1468a8..e2bba8127 100644
--- a/nova/api/openstack/compute/contrib/security_group_default_rules.py
+++ b/nova/api/openstack/compute/contrib/security_group_default_rules.py
@@ -24,6 +24,7 @@ from nova.api.openstack import extensions
 from nova.api.openstack import wsgi
 from nova.api.openstack import xmlutil
 from nova import exception
+from nova.network.security_group import openstack_driver
 from nova.openstack.common import log as logging
 
 
@@ -104,6 +105,10 @@ class SecurityGroupDefaultRulesXMLDeserializer(wsgi.MetadataXMLDeserializer):
 
 class SecurityGroupDefaultRulesController(sg.SecurityGroupControllerBase):
 
+    def __init__(self):
+        self.security_group_api = (
+            openstack_driver.get_openstack_security_group_driver())
+
     @wsgi.serializers(xml=SecurityGroupDefaultRuleTemplate)
     @wsgi.deserializers(xml=SecurityGroupDefaultRulesXMLDeserializer)
     def create(self, req, body):
@@ -144,7 +149,8 @@ class SecurityGroupDefaultRulesController(sg.SecurityGroupControllerBase):
         context = self._authorize_context(req)
         authorize(context)
 
-        id = self._validate_id(id)
+        id = self.security_group_api.validate_id(id)
+
         LOG.debug(_("Showing security_group_default_rule with id %s") % id)
         try:
             rule = self.security_group_api.get_default_rule(context, id)
@@ -158,7 +164,7 @@ class SecurityGroupDefaultRulesController(sg.SecurityGroupControllerBase):
         context = self._authorize_context(req)
         authorize(context)
 
-        id = self._validate_id(id)
+        id = self.security_group_api.validate_id(id)
 
         rule = self.security_group_api.get_default_rule(context, id)
 
diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py
index d42dc1b0a..3f48176cc 100644
--- a/nova/api/openstack/compute/contrib/security_groups.py
+++ b/nova/api/openstack/compute/contrib/security_groups.py
@@ -27,10 +27,12 @@ from nova import compute
 from nova.compute import api as compute_api
 from nova import db
 from nova import exception
+from nova.network.security_group import openstack_driver
 from nova.openstack.common import log as logging
 from nova import utils
 from nova.virt import netutils
 
+
 LOG = logging.getLogger(__name__)
 authorize = extensions.extension_authorizer('compute', 'security_groups')
 softauth = extensions.soft_extension_authorizer('compute', 'security_groups')
@@ -175,7 +177,8 @@ class SecurityGroupControllerBase(object):
     """Base class for Security Group controllers."""
 
     def __init__(self):
-        self.security_group_api = NativeSecurityGroupAPI()
+        self.security_group_api = (
+            openstack_driver.get_openstack_security_group_driver())
         self.compute_api = compute.API(
                                    security_group_api=self.security_group_api)
 
@@ -214,13 +217,6 @@ class SecurityGroupControllerBase(object):
         authorize(context)
         return context
 
-    def _validate_id(self, id):
-        try:
-            return int(id)
-        except ValueError:
-            msg = _("Security group id should be integer")
-            raise exc.HTTPBadRequest(explanation=msg)
-
     def _from_body(self, body, key):
         if not body:
             raise exc.HTTPUnprocessableEntity()
@@ -238,7 +234,7 @@ class SecurityGroupController(SecurityGroupControllerBase):
         """Return data about the given security group."""
         context = self._authorize_context(req)
 
-        id = self._validate_id(id)
+        id = self.security_group_api.validate_id(id)
 
         security_group = self.security_group_api.get(context, None, id,
                                                      map_exception=True)
@@ -250,7 +246,7 @@ class SecurityGroupController(SecurityGroupControllerBase):
         """Delete a security group."""
         context = self._authorize_context(req)
 
-        id = self._validate_id(id)
+        id = self.security_group_api.validate_id(id)
 
         security_group = self.security_group_api.get(context, None, id,
                                                      map_exception=True)
@@ -273,7 +269,7 @@ class SecurityGroupController(SecurityGroupControllerBase):
 
         limited_list = common.limited(raw_groups, req)
         result = [self._format_security_group(context, group)
-                     for group in limited_list]
+                    for group in limited_list]
 
         return {'security_groups':
                 list(sorted(result,
@@ -294,11 +290,11 @@ class SecurityGroupController(SecurityGroupControllerBase):
         self.security_group_api.validate_property(group_description,
                                                   'description', None)
 
-        group_ref = self.security_group_api.create(context, group_name,
-                                                   group_description)
+        group_ref = self.security_group_api.create_security_group(
+            context, group_name, group_description)
 
         return {'security_group': self._format_security_group(context,
-                                                                 group_ref)}
+                                                              group_ref)}
 
 
 class SecurityGroupRulesController(SecurityGroupControllerBase):
@@ -310,14 +306,13 @@ class SecurityGroupRulesController(SecurityGroupControllerBase):
 
         sg_rule = self._from_body(body, 'security_group_rule')
 
-        parent_group_id = self._validate_id(sg_rule.get('parent_group_id',
-                                                        None))
+        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:
-            values = self._rule_args_to_dict(context,
+            new_rule = self._rule_args_to_dict(context,
                               to_port=sg_rule.get('to_port'),
                               from_port=sg_rule.get('from_port'),
                               ip_protocol=sg_rule.get('ip_protocol'),
@@ -326,24 +321,21 @@ class SecurityGroupRulesController(SecurityGroupControllerBase):
         except Exception as exp:
             raise exc.HTTPBadRequest(explanation=unicode(exp))
 
-        if values is None:
+        if new_rule is None:
             msg = _("Not enough parameters to build a valid rule.")
             raise exc.HTTPBadRequest(explanation=msg)
 
-        values['parent_group_id'] = security_group.id
+        new_rule['parent_group_id'] = security_group['id']
 
-        if 'cidr' in values:
-            net, prefixlen = netutils.get_net_and_prefixlen(values['cidr'])
+        if 'cidr' in new_rule:
+            net, prefixlen = netutils.get_net_and_prefixlen(new_rule['cidr'])
             if net != '0.0.0.0' and prefixlen == '0':
-                msg = _("Bad prefix for network in cidr %s") % values['cidr']
+                msg = _("Bad prefix for network in cidr %s") % new_rule['cidr']
                 raise exc.HTTPBadRequest(explanation=msg)
 
-        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)
-
-        security_group_rule = self.security_group_api.add_rules(
-                context, parent_group_id, security_group['name'], [values])[0]
+        security_group_rule = (
+            self.security_group_api.create_security_group_rule(
+                context, security_group, new_rule))
 
         return {"security_group_rule": self._format_security_group_rule(
                                                         context,
@@ -353,8 +345,9 @@ class SecurityGroupRulesController(SecurityGroupControllerBase):
                            ip_protocol=None, cidr=None, group_id=None):
 
         if group_id is not None:
-            group_id = self._validate_id(group_id)
-            #check if groupId exists
+            group_id = self.security_group_api.validate_id(group_id)
+
+            # check if groupId exists
             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)
@@ -366,11 +359,11 @@ class SecurityGroupRulesController(SecurityGroupControllerBase):
     def delete(self, req, id):
         context = self._authorize_context(req)
 
-        id = self._validate_id(id)
+        id = self.security_group_api.validate_id(id)
 
         rule = self.security_group_api.get_rule(context, id)
 
-        group_id = rule.parent_group_id
+        group_id = rule['parent_group_id']
 
         security_group = self.security_group_api.get(context, None, group_id,
                                                      map_exception=True)
@@ -408,7 +401,8 @@ class ServerSecurityGroupController(SecurityGroupControllerBase):
 class SecurityGroupActionController(wsgi.Controller):
     def __init__(self, *args, **kwargs):
         super(SecurityGroupActionController, self).__init__(*args, **kwargs)
-        self.security_group_api = NativeSecurityGroupAPI()
+        self.security_group_api = (
+            openstack_driver.get_openstack_security_group_driver())
         self.compute_api = compute.API(
                                    security_group_api=self.security_group_api)
 
@@ -467,6 +461,8 @@ class SecurityGroupsOutputController(wsgi.Controller):
     def __init__(self, *args, **kwargs):
         super(SecurityGroupsOutputController, self).__init__(*args, **kwargs)
         self.compute_api = compute.API()
+        self.security_group_api = (
+            openstack_driver.get_openstack_security_group_driver())
 
     def _extend_servers(self, req, servers):
         key = "security_groups"
@@ -562,7 +558,7 @@ class Security_groups(extensions.ExtensionDescriptor):
         return resources
 
 
-class NativeSecurityGroupAPI(compute_api.SecurityGroupAPI):
+class NativeSecurityGroupExceptions(object):
     @staticmethod
     def raise_invalid_property(msg):
         raise exc.HTTPBadRequest(explanation=msg)
@@ -586,3 +582,8 @@ class NativeSecurityGroupAPI(compute_api.SecurityGroupAPI):
     @staticmethod
     def raise_not_found(msg):
         raise exc.HTTPNotFound(explanation=msg)
+
+
+class NativeNovaSecurityGroupAPI(compute_api.SecurityGroupAPI,
+                                 NativeSecurityGroupExceptions):
+    pass
diff --git a/nova/compute/api.py b/nova/compute/api.py
index 86e10bc03..90b1e9176 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -26,7 +26,6 @@ import functools
 import re
 import string
 import time
-import urllib
 import uuid
 
 from oslo.config import cfg
@@ -47,9 +46,10 @@ from nova import exception
 from nova import hooks
 from nova.image import glance
 from nova import network
+from nova.network.security_group import openstack_driver
+from nova.network.security_group import security_group_base
 from nova import notifications
 from nova.openstack.common import excutils
-from nova.openstack.common import importutils
 from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
 from nova.openstack.common import timeutils
@@ -61,7 +61,6 @@ from nova import servicegroup
 from nova import utils
 from nova import volume
 
-
 LOG = logging.getLogger(__name__)
 
 compute_opts = [
@@ -81,12 +80,6 @@ compute_opts = [
                default='nokernel',
                help='kernel image that indicates not to use a kernel, but to '
                     'use a raw disk image instead'),
-    cfg.StrOpt('security_group_handler',
-               default='nova.network.sg.NullSecurityGroupHandler',
-               help='The full class name of the security group handler class'),
-    cfg.StrOpt('security_group_api',
-               default='nova.compute.api.SecurityGroupAPI',
-               help='The full class name of the security API class'),
     cfg.StrOpt('multi_instance_display_name_template',
                default='%(name)s-%(uuid)s',
                help='When creating multiple instances with a single request '
@@ -191,9 +184,8 @@ class API(base.Base):
         self.network_api = network_api or network.API()
         self.volume_api = volume_api or volume.API()
         self.security_group_api = (security_group_api or
-                                   importutils.import_object(
-                                   CONF.security_group_api))
-        self.sgh = importutils.import_object(CONF.security_group_handler)
+            openstack_driver.get_openstack_security_group_driver())
+        self.sgh = openstack_driver.get_security_group_handler()
         self.consoleauth_rpcapi = consoleauth_rpcapi.ConsoleAuthAPI()
         self.scheduler_rpcapi = scheduler_rpcapi.SchedulerAPI()
         self.compute_rpcapi = compute_rpcapi.ComputeAPI()
@@ -864,13 +856,8 @@ class API(base.Base):
             base_image_ref = base_options['image_ref']
 
         instance['system_metadata']['image_base_image_ref'] = base_image_ref
-
-        # Use 'default' security_group if none specified.
-        if security_groups is None:
-            security_groups = ['default']
-        elif not isinstance(security_groups, list):
-            security_groups = [security_groups]
-        instance['security_groups'] = security_groups
+        self.security_group_api.populate_security_groups(instance,
+                                                         security_groups)
 
         return instance
 
@@ -2755,7 +2742,7 @@ class KeypairAPI(base.Base):
                 'fingerprint': key_pair['fingerprint']}
 
 
-class SecurityGroupAPI(base.Base):
+class SecurityGroupAPI(base.Base, security_group_base.SecurityGroupBase):
     """
     Sub-set of the Compute API related to managing security groups
     and security group rules
@@ -2763,7 +2750,7 @@ class SecurityGroupAPI(base.Base):
     def __init__(self, **kwargs):
         super(SecurityGroupAPI, self).__init__(**kwargs)
         self.security_group_rpcapi = compute_rpcapi.SecurityGroupAPI()
-        self.sgh = importutils.import_object(CONF.security_group_handler)
+        self.sgh = openstack_driver.get_security_group_handler()
 
     def validate_property(self, value, property, allowed):
         """
@@ -2810,7 +2797,7 @@ class SecurityGroupAPI(base.Base):
         if not existed:
             self.sgh.trigger_security_group_create_refresh(context, group)
 
-    def create(self, context, name, description):
+    def create_security_group(self, context, name, description):
         try:
             reservations = QUOTAS.reserve(context, security_groups=1)
         except exception.OverQuota:
@@ -2989,164 +2976,15 @@ class SecurityGroupAPI(base.Base):
         self.trigger_handler('instance_remove_security_group',
                 context, instance, security_group_name)
 
-    def trigger_handler(self, event, *args):
-        handle = getattr(self.sgh, 'trigger_%s_refresh' % event)
-        handle(*args)
-
-    def trigger_rules_refresh(self, context, id):
-        """Called when a rule is added to or removed from a security_group."""
-
-        security_group = self.db.security_group_get(context, id)
-
-        for instance in security_group['instances']:
-            if instance['host'] is not None:
-                self.security_group_rpcapi.refresh_instance_security_rules(
-                        context, instance['host'], instance)
-
-    def trigger_members_refresh(self, context, group_ids):
-        """Called when a security group gains a new or loses a member.
-
-        Sends an update request to each compute node for each instance for
-        which this is relevant.
-        """
-        # First, we get the security group rules that reference these groups as
-        # the grantee..
-        security_group_rules = set()
-        for group_id in group_ids:
-            security_group_rules.update(
-                self.db.security_group_rule_get_by_security_group_grantee(
-                                                                     context,
-                                                                     group_id))
-
-        # ..then we distill the rules into the groups to which they belong..
-        security_groups = set()
-        for rule in security_group_rules:
-            security_group = self.db.security_group_get(
-                                                    context,
-                                                    rule['parent_group_id'])
-            security_groups.add(security_group)
-
-        # ..then we find the instances that are members of these groups..
-        instances = {}
-        for security_group in security_groups:
-            for instance in security_group['instances']:
-                if instance['uuid'] not in instances:
-                    instances[instance['uuid']] = instance
-
-        # ..then we send a request to refresh the rules for each instance.
-        for instance in instances.values():
-            if instance['host']:
-                self.security_group_rpcapi.refresh_instance_security_rules(
-                        context, instance['host'], instance)
-
-    def parse_cidr(self, cidr):
-        if cidr:
-            try:
-                cidr = urllib.unquote(cidr).decode()
-            except Exception as e:
-                self.raise_invalid_cidr(cidr, e)
-
-            if not utils.is_valid_cidr(cidr):
-                self.raise_invalid_cidr(cidr)
-
-            return cidr
-        else:
-            return '0.0.0.0/0'
-
-    @staticmethod
-    def new_group_ingress_rule(grantee_group_id, protocol, from_port,
-                               to_port):
-        return SecurityGroupAPI._new_ingress_rule(protocol, from_port,
-                                to_port, group_id=grantee_group_id)
-
-    @staticmethod
-    def new_cidr_ingress_rule(grantee_cidr, protocol, from_port, to_port):
-        return SecurityGroupAPI._new_ingress_rule(protocol, from_port,
-                                to_port, cidr=grantee_cidr)
-
-    @staticmethod
-    def _new_ingress_rule(ip_protocol, from_port, to_port,
-                          group_id=None, cidr=None):
-        values = {}
-
-        if group_id:
-            values['group_id'] = 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
-
-        elif cidr:
-            values['cidr'] = cidr
-
-        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
-            values['from_port'] = from_port
-            values['to_port'] = to_port
-
-        else:
-            # If cidr based filtering, protocol and ports are mandatory
-            if cidr:
-                return None
-
-        return values
-
-    def rule_exists(self, security_group, values):
-        """Indicates whether the specified rule values are already
+    def rule_exists(self, security_group, new_rule):
+        """Indicates whether the specified rule is 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):
+                if rule.get(key) != new_rule.get(key):
                     is_duplicate = False
                     break
             if is_duplicate:
@@ -3162,6 +3000,13 @@ class SecurityGroupAPI(base.Base):
             self.raise_not_found(msg)
 
     def add_rules(self, context, id, name, vals):
+        """Add security group rule(s) to security group.
+
+        Note: the Nova security group API doesn't support adding muliple
+        security group rules at once but the EC2 one does. Therefore,
+        this function is writen to support both.
+        """
+
         count = QUOTAS.count(context, 'security_group_rules', id)
         try:
             projected = count + len(vals)
@@ -3231,26 +3076,82 @@ class SecurityGroupAPI(base.Base):
             msg = _("Rule (%s) not found") % id
             self.raise_not_found(msg)
 
-    @staticmethod
-    def raise_invalid_property(msg):
-        raise NotImplementedError()
+    def validate_id(self, id):
+        try:
+            return int(id)
+        except ValueError:
+            msg = _("Security group id should be integer")
+            self.raise_invalid_property(msg)
 
-    @staticmethod
-    def raise_group_already_exists(msg):
-        raise NotImplementedError()
+    def create_security_group_rule(self, context, security_group, new_rule):
+        if self.rule_exists(security_group, new_rule):
+            msg = (_('This rule already exists in group %s') %
+                   new_rule['parent_group_id'])
+            self.raise_group_already_exists(msg)
+        return self.add_rules(context, new_rule['parent_group_id'],
+                             security_group['name'],
+                             [new_rule])[0]
 
-    @staticmethod
-    def raise_invalid_group(msg):
-        raise NotImplementedError()
+    def trigger_handler(self, event, *args):
+        handle = getattr(self.sgh, 'trigger_%s_refresh' % event)
+        handle(*args)
 
-    @staticmethod
-    def raise_invalid_cidr(cidr, decoding_exception=None):
-        raise NotImplementedError()
+    def trigger_rules_refresh(self, context, id):
+        """Called when a rule is added to or removed from a security_group."""
 
-    @staticmethod
-    def raise_over_quota(msg):
-        raise NotImplementedError()
+        security_group = self.db.security_group_get(context, id)
 
-    @staticmethod
-    def raise_not_found(msg):
-        raise NotImplementedError()
+        for instance in security_group['instances']:
+            if instance['host'] is not None:
+                self.security_group_rpcapi.refresh_instance_security_rules(
+                        context, instance['host'], instance)
+
+    def trigger_members_refresh(self, context, group_ids):
+        """Called when a security group gains a new or loses a member.
+
+        Sends an update request to each compute node for each instance for
+        which this is relevant.
+        """
+        # First, we get the security group rules that reference these groups as
+        # the grantee..
+        security_group_rules = set()
+        for group_id in group_ids:
+            security_group_rules.update(
+                self.db.security_group_rule_get_by_security_group_grantee(
+                                                                     context,
+                                                                     group_id))
+
+        # ..then we distill the rules into the groups to which they belong..
+        security_groups = set()
+        for rule in security_group_rules:
+            security_group = self.db.security_group_get(
+                                                    context,
+                                                    rule['parent_group_id'])
+            security_groups.add(security_group)
+
+        # ..then we find the instances that are members of these groups..
+        instances = {}
+        for security_group in security_groups:
+            for instance in security_group['instances']:
+                if instance['uuid'] not in instances:
+                    instances[instance['uuid']] = instance
+
+        # ..then we send a request to refresh the rules for each instance.
+        for instance in instances.values():
+            if instance['host']:
+                self.security_group_rpcapi.refresh_instance_security_rules(
+                        context, instance['host'], instance)
+
+    def get_instance_security_groups(self, req, instance_id):
+        instance = req.get_db_instance(instance_id)
+        groups = instance.get('security_groups')
+        if groups:
+            return [{'name': group['name']} for group in groups]
+
+    def populate_security_groups(self, instance, security_groups):
+        # Use 'default' security_group if none specified.
+        if security_groups is None:
+            security_groups = ['default']
+        elif not isinstance(security_groups, list):
+            security_groups = [security_groups]
+        instance['security_groups'] = security_groups
diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py
index 6202cb8b6..1c54b1f0b 100644
--- a/nova/conductor/manager.py
+++ b/nova/conductor/manager.py
@@ -20,6 +20,7 @@ from nova.compute import utils as compute_utils
 from nova import exception
 from nova import manager
 from nova import network
+from nova.network.security_group import openstack_driver
 from nova import notifications
 from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
@@ -53,7 +54,8 @@ class ConductorManager(manager.SchedulerDependentManager):
     def __init__(self, *args, **kwargs):
         super(ConductorManager, self).__init__(service_name='conductor',
                                                *args, **kwargs)
-        self.security_group_api = compute_api.SecurityGroupAPI()
+        self.security_group_api = (
+            openstack_driver.get_openstack_security_group_driver())
         self._network_api = None
         self._compute_api = None
         self.quotas = quota.QUOTAS
diff --git a/nova/network/manager.py b/nova/network/manager.py
index d7f80dbda..a4669def0 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -52,7 +52,6 @@ from eventlet import greenpool
 import netaddr
 from oslo.config import cfg
 
-from nova.compute import api as compute_api
 from nova import context
 from nova import exception
 from nova import ipv6
@@ -62,6 +61,7 @@ from nova.network import driver
 from nova.network import floating_ips
 from nova.network import model as network_model
 from nova.network import rpcapi as network_rpcapi
+from nova.network.security_group import openstack_driver
 from nova.openstack.common import excutils
 from nova.openstack.common import importutils
 from nova.openstack.common import jsonutils
@@ -286,7 +286,9 @@ class NetworkManager(manager.SchedulerDependentManager):
                 CONF.floating_ip_dns_manager)
         self.network_api = network_api.API()
         self.network_rpcapi = network_rpcapi.NetworkAPI()
-        self.security_group_api = compute_api.SecurityGroupAPI()
+        self.security_group_api = (
+            openstack_driver.get_openstack_security_group_driver())
+
         self.servicegroup_api = servicegroup.API()
 
         # NOTE(tr3buchet: unless manager subclassing NetworkManager has
diff --git a/nova/network/security_group/__init__.py b/nova/network/security_group/__init__.py
new file mode 100644
index 000000000..5f67d7881
--- /dev/null
+++ b/nova/network/security_group/__init__.py
@@ -0,0 +1,18 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2013 Nicira, Inc.
+# All Rights Reserved
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+# @author: Aaron Rosen, Nicira Networks, Inc.
diff --git a/nova/network/security_group/openstack_driver.py b/nova/network/security_group/openstack_driver.py
new file mode 100644
index 000000000..91ce69891
--- /dev/null
+++ b/nova/network/security_group/openstack_driver.py
@@ -0,0 +1,46 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2013 Nicira, Inc.
+# All Rights Reserved
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+# @author: Aaron Rosen, Nicira Networks, Inc.
+
+from oslo.config import cfg
+
+from nova.openstack.common import importutils
+
+security_group_opts = [
+    cfg.StrOpt('security_group_api',
+               default='nova',
+               help='The full class name of the security API class'),
+    cfg.StrOpt('security_group_handler',
+               default='nova.network.sg.NullSecurityGroupHandler',
+               help='The full class name of the security group handler class'),
+]
+
+CONF = cfg.CONF
+CONF.register_opts(security_group_opts)
+
+NOVA_DRIVER = ('nova.api.openstack.compute.contrib.security_groups.'
+               'NativeNovaSecurityGroupAPI')
+
+
+def get_openstack_security_group_driver():
+    if CONF.security_group_api.lower() == 'nova':
+        return importutils.import_object(NOVA_DRIVER)
+
+
+def get_security_group_handler():
+    return importutils.import_object(CONF.security_group_handler)
diff --git a/nova/network/security_group/security_group_base.py b/nova/network/security_group/security_group_base.py
new file mode 100644
index 000000000..e6abf988a
--- /dev/null
+++ b/nova/network/security_group/security_group_base.py
@@ -0,0 +1,193 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2010 United States Government as represented by the
+# Administrator of the National Aeronautics and Space Administration.
+# Copyright 2011 Piston Cloud Computing, Inc.
+# Copyright 2012 Red Hat, Inc.
+# Copyright 2013 Nicira, Inc.
+# All Rights Reserved
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+# @author: Aaron Rosen, Nicira Networks, Inc.
+
+import urllib
+
+from oslo.config import cfg
+
+from nova import exception
+from nova import utils
+
+CONF = cfg.CONF
+
+
+class SecurityGroupBase(object):
+
+    def parse_cidr(self, cidr):
+        if cidr:
+            try:
+                cidr = urllib.unquote(cidr).decode()
+            except Exception as e:
+                self.raise_invalid_cidr(cidr, e)
+
+            if not utils.is_valid_cidr(cidr):
+                self.raise_invalid_cidr(cidr)
+
+            return cidr
+        else:
+            return '0.0.0.0/0'
+
+    @staticmethod
+    def new_group_ingress_rule(grantee_group_id, protocol, from_port,
+                               to_port):
+        return SecurityGroupBase._new_ingress_rule(
+            protocol, from_port, to_port, group_id=grantee_group_id)
+
+    @staticmethod
+    def new_cidr_ingress_rule(grantee_cidr, protocol, from_port, to_port):
+        return SecurityGroupBase._new_ingress_rule(
+            protocol, from_port, to_port, cidr=grantee_cidr)
+
+    @staticmethod
+    def _new_ingress_rule(ip_protocol, from_port, to_port,
+                          group_id=None, cidr=None):
+        values = {}
+
+        if group_id:
+            values['group_id'] = 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
+
+        elif cidr:
+            values['cidr'] = cidr
+
+        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
+            values['from_port'] = from_port
+            values['to_port'] = to_port
+
+        else:
+            # If cidr based filtering, protocol and ports are mandatory
+            if cidr:
+                return None
+
+        return values
+
+    def validate_property(self, value, property, allowed):
+        pass
+
+    def ensure_default(self, context):
+        pass
+
+    def trigger_handler(self, event, *args):
+        pass
+
+    def trigger_rules_refresh(self, context, id):
+        """Called when a rule is added to or removed from a security_group."""
+        pass
+
+    def trigger_members_refresh(self, context, group_ids):
+        """Called when a security group gains a new or loses a member.
+
+        Sends an update request to each compute node for each instance for
+        which this is relevant.
+        """
+        pass
+
+    def populate_security_groups(self, instance, security_groups):
+        """Called when populating the database for an instances
+        security groups."""
+        raise NotImplementedError()
+
+    def create_security_group(self, context, name, description):
+        raise NotImplementedError()
+
+    def get(self, context, name=None, id=None, map_exception=False):
+        raise NotImplementedError()
+
+    def list(self, context, names=None, ids=None, project=None,
+             search_opts=None):
+        raise NotImplementedError()
+
+    def destroy(self, context, security_group):
+        raise NotImplementedError()
+
+    def add_rules(self, context, id, name, vals):
+        raise NotImplementedError()
+
+    def create_security_group_rule(self, context, security_group, new_rule):
+        raise NotImplementedError()
+
+    def remove_rules(self, context, security_group, rule_ids):
+        raise NotImplementedError()
+
+    def get_rule(self, context, id):
+        raise NotImplementedError()
+
+    def get_instance_security_groups(self, req, instance_id):
+        raise NotImplementedError()
+
+    def add_to_instance(self, context, instance, security_group_name):
+        raise NotImplementedError()
+
+    def remove_from_instance(self, context, instance, security_group_name):
+        raise NotImplementedError()
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index 6eb5feb85..542a7e162 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -46,6 +46,7 @@ from nova import exception
 from nova.image import glance
 from nova.network import api as network_api
 from nova.network import model as network_model
+from nova.network.security_group import openstack_driver
 from nova.openstack.common import importutils
 from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
@@ -69,7 +70,6 @@ from nova import utils
 from nova.virt import fake
 from nova.volume import cinder
 
-
 QUOTAS = quota.QUOTAS
 LOG = logging.getLogger(__name__)
 CONF = cfg.CONF
@@ -3649,7 +3649,9 @@ class ComputeAPITestCase(BaseTestCase):
         super(ComputeAPITestCase, self).setUp()
         self.stubs.Set(network_api.API, 'get_instance_nw_info',
                        fake_get_nw_info)
-        self.security_group_api = compute_api.SecurityGroupAPI()
+        self.security_group_api = (
+            openstack_driver.get_openstack_security_group_driver())
+
         self.compute_api = compute.API(
                                    security_group_api=self.security_group_api)
         self.fake_image = {