From 85c089eef2793d127698e524df74ccd512755ddf Mon Sep 17 00:00:00 2001
From: Hang Yang <hangyang@verizonmedia.com>
Date: Thu, 10 Sep 2020 16:26:29 -0500
Subject: [PATCH] Support remote address group in SG rules

- Add api extension and db model changes to support remote_address_group_id
  in SG rules.
- RPC and firewall agent changes will be in the follow-up patches.

Change-Id: I99681736d05eefd82bdba72b3866eab9468ef5dd
Implements: blueprint address-groups-in-sg-rules
---
 neutron/agent/securitygroups_rpc.py           |   3 +
 neutron/db/address_group_db.py                |  26 +++
 .../alembic_migrations/versions/EXPAND_HEAD   |   2 +-
 ...upport_remote_address_group_in_sg_rules.py |  41 ++++
 neutron/db/models/securitygroup.py            |   5 +
 neutron/db/securitygroups_db.py               |  22 +-
 .../security_groups_remote_address_group.py   |  21 ++
 neutron/extensions/securitygroup.py           |   6 +-
 neutron/objects/securitygroup.py              |  26 ++-
 neutron/plugins/ml2/plugin.py                 |   2 +
 .../tests/contrib/hooks/api_all_extensions    |   4 +-
 .../unit/db/test_ovn_revision_numbers_db.py   |   5 +-
 .../tests/unit/db/test_securitygroups_db.py   |   1 +
 .../unit/extensions/test_securitygroup.py     | 211 ++++++++++++++----
 neutron/tests/unit/objects/test_objects.py    |   4 +-
 .../tests/unit/objects/test_securitygroup.py  |  33 ++-
 .../tests/unit/plugins/ml2/test_ovo_rpc.py    |   1 +
 .../revisions/test_revision_plugin.py         |   1 +
 18 files changed, 353 insertions(+), 61 deletions(-)
 create mode 100644 neutron/db/migration/alembic_migrations/versions/wallaby/expand/a964d94b4677_support_remote_address_group_in_sg_rules.py
 create mode 100644 neutron/extensions/security_groups_remote_address_group.py

diff --git a/neutron/agent/securitygroups_rpc.py b/neutron/agent/securitygroups_rpc.py
index f3712efaa7c..3c73e1557b3 100644
--- a/neutron/agent/securitygroups_rpc.py
+++ b/neutron/agent/securitygroups_rpc.py
@@ -17,6 +17,8 @@
 import functools
 
 from neutron_lib.api.definitions import rbac_security_groups as rbac_sg_apidef
+from neutron_lib.api.definitions import security_groups_remote_address_group \
+    as sgag_def
 from neutron_lib.api.definitions import stateful_security_group as stateful_sg
 from oslo_concurrency import lockutils
 from oslo_config import cfg
@@ -49,6 +51,7 @@ def disable_security_group_extension_by_config(aliases):
         _disable_extension('security-group', aliases)
         _disable_extension(rbac_sg_apidef.ALIAS, aliases)
         _disable_extension(stateful_sg.ALIAS, aliases)
+        _disable_extension(sgag_def.ALIAS, aliases)
         LOG.info('Disabled allowed-address-pairs extension.')
         _disable_extension('allowed-address-pairs', aliases)
         LOG.info('Disabled address-group extension.')
diff --git a/neutron/db/address_group_db.py b/neutron/db/address_group_db.py
index 9a4fb279e98..93b2a84290e 100644
--- a/neutron/db/address_group_db.py
+++ b/neutron/db/address_group_db.py
@@ -16,12 +16,21 @@ from neutron_lib.callbacks import registry
 from neutron_lib import constants
 from neutron_lib.db import resource_extend
 from neutron_lib.db import utils as db_utils
+from neutron_lib import exceptions
 from neutron_lib.exceptions import address_group as ag_exc
 from oslo_utils import uuidutils
 
+from neutron._i18n import _
 from neutron.extensions import address_group as ag_ext
 from neutron.objects import address_group as ag_obj
 from neutron.objects import base as base_obj
+from neutron.objects import securitygroup as sg_obj
+
+
+# TODO(hangyang): Remove this exception once neutron_lib > 2.6.1 is released.
+class AddressGroupInUse(exceptions.InUse):
+    message = _("Address group %(address_group_id)s is in use on one or more "
+                "security group rules.")
 
 
 # TODO(mlavalle) Following line should be deleted when
@@ -44,6 +53,19 @@ class AddressGroupDbMixin(ag_ext.AddressGroupPluginBase):
                             for addr_assoc in address_group['addresses']]
         return db_utils.resource_fields(res, fields)
 
+    @staticmethod
+    def check_address_group(context, ag_id, project_id=None):
+        """Check if address group id exists for the given project"""
+        if project_id:
+            tmp_context_project_id = context.project_id
+            context.project_id = project_id
+        try:
+            if not ag_obj.AddressGroup.objects_exist(context, id=ag_id):
+                raise ag_exc.AddressGroupNotFound(address_group_id=ag_id)
+        finally:
+            if project_id:
+                context.project_id = tmp_context_project_id
+
     def _get_address_group(self, context, id):
         obj = ag_obj.AddressGroup.get_object(context, id=id)
         if obj is None:
@@ -150,6 +172,10 @@ class AddressGroupDbMixin(ag_ext.AddressGroupPluginBase):
         ]
 
     def delete_address_group(self, context, id):
+        if sg_obj.SecurityGroupRule.get_objects(context.elevated(),
+                                                remote_address_group_id=id):
+            # TODO(hangyang): use exception from neutron_lib
+            raise AddressGroupInUse(address_group_id=id)
         address_group = self._get_address_group(context, id)
         address_group.delete()
         # TODO(mlavalle) this notification should be updated to publish when
diff --git a/neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD b/neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD
index 31ff56291e7..2df5d8a3747 100644
--- a/neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD
+++ b/neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD
@@ -1 +1 @@
-f010820fc498
+a964d94b4677
diff --git a/neutron/db/migration/alembic_migrations/versions/wallaby/expand/a964d94b4677_support_remote_address_group_in_sg_rules.py b/neutron/db/migration/alembic_migrations/versions/wallaby/expand/a964d94b4677_support_remote_address_group_in_sg_rules.py
new file mode 100644
index 00000000000..3b413b6b3e0
--- /dev/null
+++ b/neutron/db/migration/alembic_migrations/versions/wallaby/expand/a964d94b4677_support_remote_address_group_in_sg_rules.py
@@ -0,0 +1,41 @@
+# Copyright 2020 OpenStack Foundation
+#
+#    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.
+#
+
+from alembic import op
+from neutron_lib.db import constants as db_const
+import sqlalchemy as sa
+
+
+"""support remote address group in SG rules
+
+Revision ID: a964d94b4677
+Revises: f010820fc498
+Create Date: 2020-09-10 18:57:21.063935
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'a964d94b4677'
+down_revision = 'f010820fc498'
+
+
+def upgrade():
+    op.add_column('securitygrouprules',
+                  sa.Column('remote_address_group_id',
+                            sa.String(length=db_const.UUID_FIELD_SIZE),
+                            nullable=True))
+    op.create_foreign_key(None, 'securitygrouprules', 'address_groups',
+                          ['remote_address_group_id'],
+                          ['id'], ondelete='CASCADE')
diff --git a/neutron/db/models/securitygroup.py b/neutron/db/models/securitygroup.py
index 1f193ff0ebd..acd325bac48 100644
--- a/neutron/db/models/securitygroup.py
+++ b/neutron/db/models/securitygroup.py
@@ -87,6 +87,11 @@ class SecurityGroupRule(standard_attr.HasStandardAttributes, model_base.BASEV2,
                                 sa.ForeignKey("securitygroups.id",
                                               ondelete="CASCADE"),
                                 nullable=True)
+
+    remote_address_group_id = sa.Column(sa.String(db_const.UUID_FIELD_SIZE),
+                                        sa.ForeignKey("address_groups.id",
+                                                      ondelete="CASCADE"),
+                                        nullable=True)
     revises_on_change = ('security_group', )
     direction = sa.Column(sa.Enum('ingress', 'egress',
                                   name='securitygrouprules_direction'))
diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py
index 3a50379a3c3..a445f4dbf7f 100644
--- a/neutron/db/securitygroups_db.py
+++ b/neutron/db/securitygroups_db.py
@@ -35,6 +35,7 @@ from sqlalchemy.orm import scoped_session
 
 from neutron._i18n import _
 from neutron.common import _constants as const
+from neutron.db import address_group_db as ag_db
 from neutron.db.models import securitygroup as sg_models
 from neutron.db import rbac_db_mixin as rbac_mixin
 from neutron.extensions import securitygroup as ext_sg
@@ -426,6 +427,8 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase,
             'security_group_id': rule_dict['security_group_id'],
             'direction': rule_dict['direction'],
             'remote_group_id': rule_dict.get('remote_group_id'),
+            'remote_address_group_id': rule_dict.get(
+                'remote_address_group_id'),
             'ethertype': rule_dict['ethertype'],
             'protocol': protocol,
             'remote_ip_prefix': remote_ip_prefix,
@@ -621,8 +624,12 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase,
         self._validate_ip_prefix(rule)
         self._validate_ethertype_and_protocol(rule)
 
-        if rule['remote_ip_prefix'] and rule['remote_group_id']:
-            raise ext_sg.SecurityGroupRemoteGroupAndRemoteIpPrefix()
+        remote = None
+        for key in ['remote_ip_prefix', 'remote_group_id',
+                    'remote_address_group_id']:
+            if remote and rule.get(key):
+                raise ext_sg.SecurityGroupMultipleRemoteEntites()
+            remote = rule.get(key) or remote
 
         remote_group_id = rule['remote_group_id']
         # Check that remote_group_id exists for tenant
@@ -630,8 +637,14 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase,
             self._check_security_group(context, remote_group_id,
                                        tenant_id=rule['tenant_id'])
 
-        security_group_id = rule['security_group_id']
+        remote_address_group_id = rule['remote_address_group_id']
+        # Check that remote_address_group_id exists for project
+        if remote_address_group_id:
+            ag_db.AddressGroupDbMixin.check_address_group(
+                context, remote_address_group_id,
+                project_id=rule['project_id'])
 
+        security_group_id = rule['security_group_id']
         # Confirm that the tenant has permission
         # to add rules to this security group.
         self._check_security_group(context, security_group_id,
@@ -663,6 +676,8 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase,
                'port_range_min': security_group_rule['port_range_min'],
                'port_range_max': security_group_rule['port_range_max'],
                'remote_ip_prefix': security_group_rule['remote_ip_prefix'],
+               'remote_address_group_id': security_group_rule[
+                   'remote_address_group_id'],
                'remote_group_id': security_group_rule['remote_group_id'],
                'standard_attr_id': security_group_rule.standard_attr.id,
                }
@@ -694,6 +709,7 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase,
             'port_range_min',
             'protocol',
             'remote_group_id',
+            'remote_address_group_id',
             'remote_ip_prefix',
             'security_group_id'
         ]
diff --git a/neutron/extensions/security_groups_remote_address_group.py b/neutron/extensions/security_groups_remote_address_group.py
new file mode 100644
index 00000000000..e83843311ec
--- /dev/null
+++ b/neutron/extensions/security_groups_remote_address_group.py
@@ -0,0 +1,21 @@
+#    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.
+
+from neutron_lib.api.definitions import security_groups_remote_address_group
+from neutron_lib.api import extensions
+
+
+class Security_groups_remote_address_group(extensions.APIExtensionDescriptor):
+    """Extension class supporting remote address group in security
+    group rules.
+    """
+    api_definition = security_groups_remote_address_group
diff --git a/neutron/extensions/securitygroup.py b/neutron/extensions/securitygroup.py
index 48b3b5f4c0b..e212f4baf1e 100644
--- a/neutron/extensions/securitygroup.py
+++ b/neutron/extensions/securitygroup.py
@@ -95,9 +95,9 @@ class SecurityGroupRulesNotSingleTenant(exceptions.InvalidInput):
                 " not allowed")
 
 
-class SecurityGroupRemoteGroupAndRemoteIpPrefix(exceptions.InvalidInput):
-    message = _("Only remote_ip_prefix or remote_group_id may "
-                "be provided.")
+class SecurityGroupMultipleRemoteEntites(exceptions.InvalidInput):
+    message = _("Only one of remote_ip_prefix or remote_group_id or "
+                "remote_address_group_id may be provided.")
 
 
 class SecurityGroupProtocolRequiredWithPorts(exceptions.InvalidInput):
diff --git a/neutron/objects/securitygroup.py b/neutron/objects/securitygroup.py
index 5edfd80b988..7a7986509dd 100644
--- a/neutron/objects/securitygroup.py
+++ b/neutron/objects/securitygroup.py
@@ -38,7 +38,8 @@ class SecurityGroup(rbac_db.NeutronRbacObject):
     # Version 1.0: Initial version
     # Version 1.1: Add RBAC support
     # Version 1.2: Added stateful support
-    VERSION = '1.2'
+    # Version 1.3: Added support for remote_address_group_id in rules
+    VERSION = '1.3'
 
     # required by RbacNeutronMetaclass
     rbac_db_cls = SecurityGroupRBAC
@@ -93,10 +94,21 @@ class SecurityGroup(rbac_db.NeutronRbacObject):
 
     def obj_make_compatible(self, primitive, target_version):
         _target_version = versionutils.convert_version_to_tuple(target_version)
+
+        def filter_remote_address_group_id_from_rules(rules):
+            sg_rule = SecurityGroupRule()
+            for rule in rules:
+                sg_rule.obj_make_compatible(
+                    rule['versioned_object.data'], '1.0')
+                rule['versioned_object.version'] = '1.0'
+
         if _target_version < (1, 1):
             primitive.pop('shared')
         if _target_version < (1, 2):
             primitive.pop('stateful')
+        if _target_version < (1, 3):
+            if 'rules' in primitive:
+                filter_remote_address_group_id_from_rules(primitive['rules'])
 
     @classmethod
     def get_bound_tenant_ids(cls, context, obj_id):
@@ -125,7 +137,8 @@ class DefaultSecurityGroup(base.NeutronDbObject):
 @base.NeutronObjectRegistry.register
 class SecurityGroupRule(base.NeutronDbObject):
     # Version 1.0: Initial version
-    VERSION = '1.0'
+    # Version 1.1: Add remote address group support
+    VERSION = '1.1'
 
     db_model = sg_models.SecurityGroupRule
 
@@ -140,11 +153,18 @@ class SecurityGroupRule(base.NeutronDbObject):
         'port_range_min': common_types.PortRangeWith0Field(nullable=True),
         'port_range_max': common_types.PortRangeWith0Field(nullable=True),
         'remote_ip_prefix': common_types.IPNetworkField(nullable=True),
+        'remote_address_group_id': common_types.UUIDField(nullable=True),
     }
 
     foreign_keys = {'SecurityGroup': {'security_group_id': 'id'}}
 
-    fields_no_update = ['project_id', 'security_group_id', 'remote_group_id']
+    fields_no_update = ['project_id', 'security_group_id', 'remote_group_id',
+                        'remote_address_group_id']
+
+    def obj_make_compatible(self, primitive, target_version):
+        _target_version = versionutils.convert_version_to_tuple(target_version)
+        if _target_version < (1, 1):
+            primitive.pop('remote_address_group_id', None)
 
     # TODO(sayalilunkad): get rid of it once we switch the db model to using
     # custom types.
diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py
index 09a29b31824..eb0e579dbc1 100644
--- a/neutron/plugins/ml2/plugin.py
+++ b/neutron/plugins/ml2/plugin.py
@@ -49,6 +49,7 @@ from neutron_lib.api.definitions import rbac_address_scope
 from neutron_lib.api.definitions import rbac_security_groups as rbac_sg_apidef
 from neutron_lib.api.definitions import rbac_subnetpool
 from neutron_lib.api.definitions import security_groups_port_filtering
+from neutron_lib.api.definitions import security_groups_remote_address_group
 from neutron_lib.api.definitions import stateful_security_group
 from neutron_lib.api.definitions import subnet as subnet_def
 from neutron_lib.api.definitions import subnet_onboard as subnet_onboard_def
@@ -206,6 +207,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                                     "subnet-service-types",
                                     ip_substring_port_filtering.ALIAS,
                                     security_groups_port_filtering.ALIAS,
+                                    security_groups_remote_address_group.ALIAS,
                                     empty_string_filtering.ALIAS,
                                     filter_apidef.ALIAS,
                                     port_mac_address_regenerate.ALIAS,
diff --git a/neutron/tests/contrib/hooks/api_all_extensions b/neutron/tests/contrib/hooks/api_all_extensions
index 01646bb1538..773f6646095 100644
--- a/neutron/tests/contrib/hooks/api_all_extensions
+++ b/neutron/tests/contrib/hooks/api_all_extensions
@@ -1,6 +1,7 @@
 # Keep entries alphabetically
 # NOTE: The first entry should not use '+=' and a comma.
-NETWORK_API_EXTENSIONS="address-scope"
+NETWORK_API_EXTENSIONS="address-group"
+NETWORK_API_EXTENSIONS+=",address-scope"
 NETWORK_API_EXTENSIONS+=",agent"
 NETWORK_API_EXTENSIONS+=",allowed-address-pairs"
 NETWORK_API_EXTENSIONS+=",auto-allocated-topology"
@@ -52,6 +53,7 @@ NETWORK_API_EXTENSIONS+=",router"
 NETWORK_API_EXTENSIONS+=",router-admin-state-down-before-update"
 NETWORK_API_EXTENSIONS+=",router_availability_zone"
 NETWORK_API_EXTENSIONS+=",security-group"
+NETWORK_API_EXTENSIONS+=",security-groups-remote-address-group"
 NETWORK_API_EXTENSIONS+=",port-mac-address-regenerate"
 NETWORK_API_EXTENSIONS+=",port-numa-affinity-policy"
 NETWORK_API_EXTENSIONS+=",port-security-groups-filtering"
diff --git a/neutron/tests/unit/db/test_ovn_revision_numbers_db.py b/neutron/tests/unit/db/test_ovn_revision_numbers_db.py
index d1692c84e82..00d1834e006 100644
--- a/neutron/tests/unit/db/test_ovn_revision_numbers_db.py
+++ b/neutron/tests/unit/db/test_ovn_revision_numbers_db.py
@@ -15,6 +15,8 @@
 
 from unittest import mock
 
+from neutron_lib.api.definitions import security_groups_remote_address_group \
+    as sgag_def
 from neutron_lib import constants as n_const
 from neutron_lib import context
 from neutron_lib.db import api as db_api
@@ -113,7 +115,8 @@ class TestMaintenancePlugin(test_securitygroup.SecurityGroupTestPlugin,
     __native_pagination_support = True
     __native_sorting_support = True
 
-    supported_extension_aliases = ['external-net', 'security-group']
+    supported_extension_aliases = ['external-net', 'security-group',
+                                   sgag_def.ALIAS]
 
 
 class TestRevisionNumberMaintenance(test_securitygroup.SecurityGroupsTestCase,
diff --git a/neutron/tests/unit/db/test_securitygroups_db.py b/neutron/tests/unit/db/test_securitygroups_db.py
index 96a548bfe79..a064a8beaaf 100644
--- a/neutron/tests/unit/db/test_securitygroups_db.py
+++ b/neutron/tests/unit/db/test_securitygroups_db.py
@@ -50,6 +50,7 @@ FAKE_SECGROUP_RULE = {
         'remote_ip_prefix': '10.0.0.1',
         'ethertype': 'IPv4',
         'remote_group_id': None,
+        'remote_address_group_id': None,
         'security_group_id': 'None',
         'direction': 'ingress'
     }
diff --git a/neutron/tests/unit/extensions/test_securitygroup.py b/neutron/tests/unit/extensions/test_securitygroup.py
index 68a3c337b32..2d0bb60a8ab 100644
--- a/neutron/tests/unit/extensions/test_securitygroup.py
+++ b/neutron/tests/unit/extensions/test_securitygroup.py
@@ -17,6 +17,8 @@ import contextlib
 import copy
 from unittest import mock
 
+from neutron_lib.api.definitions import security_groups_remote_address_group \
+    as sgag_def
 from neutron_lib.api import validators
 from neutron_lib import constants as const
 from neutron_lib import context
@@ -29,12 +31,15 @@ import oslo_db.exception as exc
 import testtools
 import webob.exc
 
+from neutron.db import address_group_db
 from neutron.db import db_base_plugin_v2
 from neutron.db import securitygroups_db
+from neutron.extensions import address_group as ext_ag
 from neutron.extensions import securitygroup as ext_sg
 from neutron.extensions import standardattrdescription
 from neutron.tests import base
 from neutron.tests.unit.db import test_db_base_plugin_v2
+from neutron.tests.unit.extensions import test_address_group
 
 DB_PLUGIN_KLASS = ('neutron.tests.unit.extensions.test_securitygroup.'
                    'SecurityGroupTestPlugin')
@@ -60,7 +65,11 @@ class SecurityGroupTestExtensionManager(object):
                 ext_sg.RESOURCE_ATTRIBUTE_MAP[ext_sg.SECURITYGROUPS])
             sg_attr_desc = ext_res[ext_sg.SECURITYGROUPS]
             existing_sg_attr_map.update(sg_attr_desc)
-        return ext_sg.Securitygroup.get_resources()
+        # update with the remote address group api definition
+        ext_sg.Securitygroup().update_attributes_map(
+            sgag_def.RESOURCE_ATTRIBUTE_MAP)
+        return (ext_sg.Securitygroup.get_resources() +
+                ext_ag.Address_group().get_resources())
 
     def get_actions(self):
         return []
@@ -97,6 +106,7 @@ class SecurityGroupsTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
             self, security_group_id, direction, proto,
             port_range_min=None, port_range_max=None,
             remote_ip_prefix=None, remote_group_id=None,
+            remote_address_group_id=None,
             tenant_id=test_db_base_plugin_v2.TEST_TENANT_ID,
             ethertype=const.IPv4):
 
@@ -117,6 +127,10 @@ class SecurityGroupsTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
         if remote_group_id:
             data['security_group_rule']['remote_group_id'] = remote_group_id
 
+        if remote_address_group_id:
+            data['security_group_rule']['remote_address_group_id'] = \
+                remote_address_group_id
+
         return data
 
     def _create_security_group_rule(self, fmt, rules, **kwargs):
@@ -160,6 +174,7 @@ class SecurityGroupsTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
                             direction='ingress', protocol=const.PROTO_NAME_TCP,
                             port_range_min='22', port_range_max='22',
                             remote_ip_prefix=None, remote_group_id=None,
+                            remote_address_group_id=None,
                             fmt=None, ethertype=const.IPv4):
         if not fmt:
             fmt = self.fmt
@@ -169,6 +184,7 @@ class SecurityGroupsTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
                                                port_range_max,
                                                remote_ip_prefix,
                                                remote_group_id,
+                                               remote_address_group_id,
                                                ethertype=ethertype)
         security_group_rule = self._make_security_group_rule(self.fmt, rule)
         yield security_group_rule
@@ -194,7 +210,8 @@ class SecurityGroupsTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
 
 
 class SecurityGroupTestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
-                              securitygroups_db.SecurityGroupDbMixin):
+                              securitygroups_db.SecurityGroupDbMixin,
+                              address_group_db.AddressGroupDbMixin):
     """Test plugin that implements necessary calls on create/delete port for
     associating ports with security groups.
     """
@@ -248,7 +265,8 @@ class SecurityGroupTestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
         return neutron_lports
 
 
-class SecurityGroupDBTestCase(SecurityGroupsTestCase):
+class SecurityGroupDBTestCase(SecurityGroupsTestCase,
+                              test_address_group.AddressGroupTestCase):
     def setUp(self, plugin=None, ext_mgr=None):
         self._backup = copy.deepcopy(ext_sg.RESOURCE_ATTRIBUTE_MAP)
         self.addCleanup(self._restore)
@@ -659,10 +677,12 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
             port_range_min = 22
             port_range_max = 22
             ethertype = 'ipV4'
-            with self.security_group_rule(security_group_id, direction,
-                                          protocol, port_range_min,
-                                          port_range_max,
-                                          remote_ip_prefix,
+            with self.security_group_rule(security_group_id,
+                                          direction=direction,
+                                          protocol=protocol,
+                                          port_range_min=port_range_min,
+                                          port_range_max=port_range_max,
+                                          remote_ip_prefix=remote_ip_prefix,
                                           ethertype=ethertype) as rule:
 
                 # the lower case value will be return
@@ -689,10 +709,12 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                     ('protocol', protocol),
                     ('port_range_min', port_range_min),
                     ('port_range_max', port_range_max)]
-            with self.security_group_rule(security_group_id, direction,
-                                          protocol, port_range_min,
-                                          port_range_max,
-                                          remote_ip_prefix):
+            with self.security_group_rule(security_group_id,
+                                          direction=direction,
+                                          protocol=protocol,
+                                          port_range_min=port_range_min,
+                                          port_range_max=port_range_max,
+                                          remote_ip_prefix=remote_ip_prefix):
 
                 group = self.deserialize(
                     self.fmt, res.get_response(self.ext_api))
@@ -951,10 +973,13 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                     ('protocol', protocol),
                     ('port_range_min', port_range_min),
                     ('port_range_max', port_range_max)]
-            with self.security_group_rule(security_group_id, direction,
-                                          protocol, port_range_min,
-                                          port_range_max,
-                                          remote_ip_prefix) as rule:
+            with self.security_group_rule(security_group_id,
+                                          direction=direction,
+                                          protocol=protocol,
+                                          port_range_min=port_range_min,
+                                          port_range_max=port_range_max,
+                                          remote_ip_prefix=remote_ip_prefix
+                                          ) as rule:
                 for k, v, in keys:
                     self.assertEqual(v, rule['security_group_rule'][k])
 
@@ -975,14 +1000,76 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                         ('protocol', protocol),
                         ('port_range_min', port_range_min),
                         ('port_range_max', port_range_max)]
-                with self.security_group_rule(security_group_id, direction,
-                                              protocol, port_range_min,
-                                              port_range_max,
+                with self.security_group_rule(security_group_id,
+                                              direction=direction,
+                                              protocol=protocol,
+                                              port_range_min=port_range_min,
+                                              port_range_max=port_range_max,
                                               remote_group_id=remote_group_id
                                               ) as rule:
                     for k, v, in keys:
                         self.assertEqual(v, rule['security_group_rule'][k])
 
+    def test_create_security_group_rule_remote_address_group_id(self):
+        name = 'webservers'
+        description = 'my webservers'
+        ag = self._test_create_address_group(name='foo')
+        ag_id = ag['address_group']['id']
+        with self.security_group(name, description) as sg:
+            security_group_id = sg['security_group']['id']
+            direction = "ingress"
+            remote_address_group_id = ag_id
+            protocol = const.PROTO_NAME_TCP
+            port_range_min = 22
+            port_range_max = 22
+            keys = [('remote_address_group_id', remote_address_group_id),
+                    ('security_group_id', security_group_id),
+                    ('direction', direction),
+                    ('protocol', protocol),
+                    ('port_range_min', port_range_min),
+                    ('port_range_max', port_range_max)]
+            with self.security_group_rule(security_group_id,
+                                          direction=direction,
+                                          protocol=protocol,
+                                          port_range_min=port_range_min,
+                                          port_range_max=port_range_max,
+                                          remote_address_group_id=(
+                                                  remote_address_group_id)
+                                          ) as rule:
+                for k, v, in keys:
+                    self.assertEqual(v, rule['security_group_rule'][k])
+
+    def test_delete_address_group_in_use(self):
+        ag = self._test_create_address_group(name='foo')
+        ag_id = ag['address_group']['id']
+        with self.security_group() as sg:
+            security_group_id = sg['security_group']['id']
+            with self.security_group_rule(security_group_id,
+                                          remote_address_group_id=ag_id):
+                self._delete('address-groups', ag['address_group']['id'],
+                             expected_code=webob.exc.HTTPConflict.code)
+
+    def test_create_security_group_rule_multiple_remotes(self):
+        name = 'webservers'
+        description = 'my webservers'
+        ag = self._test_create_address_group(name='foo')
+        ag_id = ag['address_group']['id']
+        with self.security_group(name, description) as sg:
+            sg_id = sg['security_group']['id']
+            for remote in [
+                {'remote_ip_prefix': '10.0.0.0/8', 'remote_group_id': sg_id},
+                {'remote_group_id': sg_id, 'remote_address_group_id': ag_id},
+                {'remote_ip_prefix': '10.0.0.0/8',
+                 'remote_address_group_id': ag_id},
+                {'remote_ip_prefix': '10.0.0.0/8', 'remote_group_id': sg_id,
+                 'remote_address_group_id': ag_id}
+            ]:
+                rule = self._build_security_group_rule(sg_id, "ingress",
+                                                       const.PROTO_NAME_TCP,
+                                                       **remote)
+                res = self._create_security_group_rule(self.fmt, rule)
+                self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int)
+
     def test_create_security_group_rule_port_range_min_max_limits(self):
         name = 'webservers'
         description = 'my webservers'
@@ -998,9 +1085,12 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                     ('protocol', protocol),
                     ('port_range_min', None),
                     ('port_range_max', None)]
-            with self.security_group_rule(security_group_id, direction,
-                                          protocol, port_range_min,
-                                          port_range_max) as rule:
+            with self.security_group_rule(security_group_id,
+                                          direction=direction,
+                                          protocol=protocol,
+                                          port_range_min=port_range_min,
+                                          port_range_max=port_range_max
+                                          ) as rule:
                 for k, v, in keys:
                     self.assertEqual(v, rule['security_group_rule'][k])
 
@@ -1023,10 +1113,13 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                     ('protocol', protocol),
                     ('port_range_min', port_range_min),
                     ('port_range_max', port_range_max)]
-            with self.security_group_rule(security_group_id, direction,
-                                          protocol, port_range_min,
-                                          port_range_max,
-                                          remote_ip_prefix) as rule:
+            with self.security_group_rule(security_group_id,
+                                          direction=direction,
+                                          protocol=protocol,
+                                          port_range_min=port_range_min,
+                                          port_range_max=port_range_max,
+                                          remote_ip_prefix=remote_ip_prefix
+                                          ) as rule:
                 for k, v, in keys:
                     self.assertEqual(v, rule['security_group_rule'][k])
 
@@ -1048,10 +1141,13 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                     ('protocol', protocol),
                     ('port_range_min', port_range_min),
                     ('port_range_max', port_range_max)]
-            with self.security_group_rule(security_group_id, direction,
-                                          protocol, port_range_min,
-                                          port_range_max,
-                                          remote_ip_prefix) as rule:
+            with self.security_group_rule(security_group_id,
+                                          direction=direction,
+                                          protocol=protocol,
+                                          port_range_min=port_range_min,
+                                          port_range_max=port_range_max,
+                                          remote_ip_prefix=remote_ip_prefix
+                                          ) as rule:
                 for k, v, in keys:
                     self.assertEqual(v, rule['security_group_rule'][k])
 
@@ -1075,12 +1171,13 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                     ('protocol', protocol),
                     ('port_range_min', port_range_min),
                     ('port_range_max', port_range_max)]
-            with self.security_group_rule(security_group_id, direction,
-                                          protocol, port_range_min,
-                                          port_range_max,
-                                          remote_ip_prefix,
-                                          None, None,
-                                          ethertype) as rule:
+            with self.security_group_rule(security_group_id,
+                                          direction=direction,
+                                          protocol=protocol,
+                                          port_range_min=port_range_min,
+                                          port_range_max=port_range_max,
+                                          remote_ip_prefix=remote_ip_prefix,
+                                          ethertype=ethertype) as rule:
                 for k, v, in keys:
                     self.assertEqual(v, rule['security_group_rule'][k])
 
@@ -1097,11 +1194,11 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                     ('direction', direction),
                     ('ethertype', ethertype),
                     ('protocol', protocol)]
-            with self.security_group_rule(security_group_id, direction,
-                                          protocol, None, None,
-                                          remote_ip_prefix,
-                                          None, None,
-                                          ethertype) as rule:
+            with self.security_group_rule(security_group_id,
+                                          direction=direction,
+                                          protocol=protocol,
+                                          remote_ip_prefix=remote_ip_prefix,
+                                          ethertype=ethertype) as rule:
                 for k, v, in keys:
                     # IPv6 ICMP protocol will always be 'ipv6-icmp'
                     if k == 'protocol':
@@ -1129,11 +1226,11 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                     ('direction', direction),
                     ('ethertype', ethertype),
                     ('protocol', protocol)]
-            with self.security_group_rule(security_group_id, direction,
-                                          protocol, None, None,
-                                          remote_ip_prefix,
-                                          None, None,
-                                          ethertype) as rule:
+            with self.security_group_rule(security_group_id,
+                                          direction=direction,
+                                          protocol=protocol,
+                                          remote_ip_prefix=remote_ip_prefix,
+                                          ethertype=ethertype) as rule:
                 for k, v, in keys:
                     # IPv6 ICMP protocol will always be '58'
                     if k == 'protocol':
@@ -1255,6 +1352,26 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
         self.deserialize(self.fmt, res)
         self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int)
 
+    def test_create_security_group_rule_bad_remote_address_group_id(self):
+        name = 'webservers'
+        description = 'my webservers'
+        with self.security_group(name, description) as sg:
+            security_group_id = sg['security_group']['id']
+            remote_address_group_id = "4cd70774-cc67-4a87-9b39-7d1db38eb087"
+            direction = "ingress"
+            protocol = const.PROTO_NAME_TCP
+            port_range_min = 22
+            port_range_max = 22
+        rule = self._build_security_group_rule(security_group_id, direction,
+                                               protocol, port_range_min,
+                                               port_range_max,
+                                               remote_address_group_id=(
+                                                   remote_address_group_id
+                                               ))
+        res = self._create_security_group_rule(self.fmt, rule)
+        self.deserialize(self.fmt, res)
+        self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int)
+
     def test_create_security_group_rule_duplicate_rules(self):
         name = 'webservers'
         description = 'my webservers'
@@ -1961,7 +2078,9 @@ class TestSecurityGroups(SecurityGroupDBTestCase):
                                                 'port_range_min': None,
                                                 'port_range_max': None,
                                                 'remote_ip_prefix': None,
-                                                'remote_group_id': None})
+                                                'remote_group_id': None,
+                                                'remote_address_group_id':
+                                                    None})
             result = self.plugin.create_security_group_rule(
                 neutron_context, rule)
             self.assertEqual(specified_id, result['id'])
diff --git a/neutron/tests/unit/objects/test_objects.py b/neutron/tests/unit/objects/test_objects.py
index 4ccbd5455f8..a6022d39fb6 100644
--- a/neutron/tests/unit/objects/test_objects.py
+++ b/neutron/tests/unit/objects/test_objects.py
@@ -102,10 +102,10 @@ object_data = {
     'RouterL3AgentBinding': '1.0-c5ba6c95e3a4c1236a55f490cd67da82',
     'RouterPort': '1.0-c8c8f499bcdd59186fcd83f323106908',
     'RouterRoute': '1.0-07fc5337c801fb8c6ccfbcc5afb45907',
-    'SecurityGroup': '1.2-7b63b834e511856f54a09282d6843ecc',
+    'SecurityGroup': '1.3-7b63b834e511856f54a09282d6843ecc',
     'SecurityGroupPortBinding': '1.0-6879d5c0af80396ef5a72934b6a6ef20',
     'SecurityGroupRBAC': '1.0-192845c5ed0718e1c54fac36936fcd7d',
-    'SecurityGroupRule': '1.0-e9b8dace9d48b936c62ad40fe1f339d5',
+    'SecurityGroupRule': '1.1-0a8614633901e353dd32948dc2f8708f',
     'SegmentHostMapping': '1.0-521597cf82ead26217c3bd10738f00f0',
     'ServiceProfile': '1.0-9beafc9e7d081b8258f3c5cb66ac5eed',
     'StandardAttribute': '1.0-617d4f46524c4ce734a6fc1cc0ac6a0b',
diff --git a/neutron/tests/unit/objects/test_securitygroup.py b/neutron/tests/unit/objects/test_securitygroup.py
index 86accc9bbce..bbfe14e3fef 100644
--- a/neutron/tests/unit/objects/test_securitygroup.py
+++ b/neutron/tests/unit/objects/test_securitygroup.py
@@ -76,6 +76,25 @@ class SecurityGroupDbObjTestCase(test_base.BaseDbObjectTestCase,
         self.objs[0].create()
         return self.objs[0]
 
+    def _create_test_security_group_with_rule(self):
+        sg_obj = self._create_test_security_group()
+        rule_params = {
+            'project_id': sg_obj.project_id,
+            'security_group_id': sg_obj.id,
+            'remote_address_group_id': None}
+        sg_rule = securitygroup.SecurityGroupRule(
+            self.context, **rule_params)
+        sg_obj.rules = [sg_rule]
+        return sg_obj
+
+    def test_object_version_degradation_1_3_to_1_2_no_remote_ag(self):
+        sg_obj = self._create_test_security_group_with_rule()
+        sg_obj_1_2 = sg_obj.obj_to_primitive('1.2')
+        for rule in sg_obj_1_2['versioned_object.data']['rules']:
+            self.assertEqual('1.0', rule['versioned_object.version'])
+            self.assertNotIn('remote_address_group_id',
+                             rule['versioned_object.data'])
+
     def test_object_version_degradation_1_2_to_1_1_no_stateful(self):
         sg_stateful_obj = self._create_test_security_group()
         sg_no_stateful_obj = sg_stateful_obj.obj_to_primitive('1.1')
@@ -213,9 +232,15 @@ class SecurityGroupRuleDbObjTestCase(test_base.BaseDbObjectTestCase,
                 'security_group_id':
                     lambda: self._create_test_security_group_id(),
                 'remote_group_id':
-                    lambda: self._create_test_security_group_id()
+                    lambda: self._create_test_security_group_id(),
+                'remote_address_group_id':
+                    lambda: self._create_test_address_group_id()
             })
 
+    def _create_test_security_group_rule(self):
+        self.objs[0].create()
+        return self.objs[0]
+
     def test_get_security_group_rule_ids(self):
         """Retrieve the SG rules associated to a project (see method desc.)
 
@@ -250,3 +275,9 @@ class SecurityGroupRuleDbObjTestCase(test_base.BaseDbObjectTestCase,
             rule_ids_ref = set(rules_per_project[projects[idx]])
             rule_ids_ref.update(set(rules_per_sg[sgs[idx]]))
             self.assertEqual(rule_ids_ref, set(rule_ids))
+
+    def test_object_version_degradation_1_1_to_1_0_no_remote_ag(self):
+        rule_remote_ag_obj = self._create_test_security_group_rule()
+        rule_no_remote_ag_obj = rule_remote_ag_obj.obj_to_primitive('1.0')
+        self.assertNotIn('remote_address_group_id',
+                         rule_no_remote_ag_obj['versioned_object.data'])
diff --git a/neutron/tests/unit/plugins/ml2/test_ovo_rpc.py b/neutron/tests/unit/plugins/ml2/test_ovo_rpc.py
index c19fb296b5a..3373d334e4f 100644
--- a/neutron/tests/unit/plugins/ml2/test_ovo_rpc.py
+++ b/neutron/tests/unit/plugins/ml2/test_ovo_rpc.py
@@ -81,6 +81,7 @@ class OVOServerRpcInterfaceTestCase(test_plugin.Ml2PluginV2TestCase):
                                          'port_range_max': None,
                                          'remote_ip_prefix': None,
                                          'remote_group_id': None,
+                                         'remote_address_group_id': None,
                                          'protocol': None,
                                          'direction': None,
                                          'ethertype': 'IPv4'}})
diff --git a/neutron/tests/unit/services/revisions/test_revision_plugin.py b/neutron/tests/unit/services/revisions/test_revision_plugin.py
index ac08758d600..48ed29e91c8 100644
--- a/neutron/tests/unit/services/revisions/test_revision_plugin.py
+++ b/neutron/tests/unit/services/revisions/test_revision_plugin.py
@@ -192,6 +192,7 @@ class TestRevisionPlugin(test_plugin.Ml2PluginV2TestCase):
                                      'remote_ip_prefix': '0.0.0.0/0',
                                      'ethertype': 'IPv4',
                                      'remote_group_id': None,
+                                     'remote_address_group_id': None,
                                      'direction': 'ingress',
                                      'security_group_id': sg['id']}}
         rule = self.cp.create_security_group_rule(self.ctx, r)