Support address group in OVS firewall agent

Support security group rules with remote_address_group_id in openvswitch
firewall. This change reuses most of the firewall functions handling remote
security groups to also process remote address groups. The conjunctive flows
for a rule with remote_adress_group_id are similar to others with
remote_group_id but have different conj_ids.

Change-Id: I8c69e62ba56b0d3204e9c12df3133126071b92f7
Implements: blueprint address-groups-in-sg-rules

neutron/agent/linux/openvswitch_firewall/firewall.py

@@ -148,7 +148,7 @@ class SecurityGroup(object):
                 else:
                     rule['protocol'] = lib_const.IP_PROTOCOL_MAP.get(
                         protocol, protocol)
-            if 'remote_group_id' in rule:
+            if 'remote_group_id' in rule or 'remote_address_group_id' in rule:
                 self.remote_rules.append(rule)
             else:
                 self.raw_rules.append(rule)
@@ -269,8 +269,8 @@ class ConjIdMap(object):
     def __init__(self):
         self.id_map = collections.defaultdict(self._conj_id_factory)
         # Stores the set of conjuntion IDs used for each unique tuple
-        # (sg_id, remote_sg_id, direction, ethertype). Each tuple can have up
+        # (sg_id, remote_id, direction, ethertype). Each tuple
-        # to 8 conjuntion IDs (see ConjIPFlowManager.add()).
+        # can have up to 8 conjuntion IDs (see ConjIPFlowManager.add()).
         self.id_map_group = collections.defaultdict(set)
         self.id_free = collections.deque()
         self.max_id = 0
@@ -283,7 +283,7 @@ class ConjIdMap(object):
         self.max_id += 8
         return self.max_id
 
-    def get_conj_id(self, sg_id, remote_sg_id, direction, ethertype):
+    def get_conj_id(self, sg_id, remote_id, direction, ethertype):
         """Return a conjunction ID specified by the arguments.
         Allocate one if necessary.  The returned ID is divisible by 8,
         as there are 4 priority levels (see rules.flow_priority_offset)
@@ -295,7 +295,7 @@ class ConjIdMap(object):
         if ethertype not in [lib_const.IPv4, lib_const.IPv6]:
             raise ValueError(_("Invalid ethertype '%s'") % ethertype)
 
-        return self.id_map[(sg_id, remote_sg_id, direction, ethertype)]
+        return self.id_map[(sg_id, remote_id, direction, ethertype)]
 
     def delete_sg(self, sg_id):
         """Free all conj_ids associated with the sg_id and
@@ -344,18 +344,18 @@ class ConjIPFlowManager(object):
         self.flow_state = collections.defaultdict(
             lambda: collections.defaultdict(dict))
 
-    def _build_addr_conj_id_map(self, ethertype, sg_conj_id_map):
+    def _build_addr_conj_id_map(self, ethertype, sg_ag_conj_id_map):
         """Build a map of addr -> list of conj_ids."""
         addr_to_conj = collections.defaultdict(list)
-        for remote_id, conj_id_set in sg_conj_id_map.items():
+        for remote_id, conj_id_set in sg_ag_conj_id_map.items():
             remote_group = self.driver.sg_port_map.get_sg(remote_id)
             if not remote_group or not remote_group.members:
-                LOG.debug('No member for SG %s', remote_id)
+                LOG.debug('No member for security group or'
+                          'address group %s', remote_id)
                 continue
             for addr in remote_group.get_ethertype_filtered_addresses(
                     ethertype):
                 addr_to_conj[addr].extend(conj_id_set)
-
         return addr_to_conj
 
     def _update_flows_for_vlan_subr(self, direction, ethertype, vlan_tag,
@@ -368,16 +368,39 @@ class ConjIPFlowManager(object):
         self.driver.delete_flows_for_flow_state(
             flow_state, addr_to_conj, direction, ethertype, vlan_tag)
         for conj_id_set in conj_id_to_remove:
-            # Remove any remaining flow with remote SG ID conj_id_to_remove
+            # Remove any remaining flow with remote SG/AG ID conj_id_to_remove
             for current_ip, conj_ids in flow_state.items():
                 conj_ids_to_remove = conj_id_set & set(conj_ids)
                 self.driver.delete_flow_for_ip(
                     current_ip, direction, ethertype, vlan_tag,
                     conj_ids_to_remove)
 
+        # NOTE(hangyang): Handle add/delete overlapped IPs among
+        # remote security groups and remote address groups
+        removed_ips = set([str(netaddr.IPNetwork(addr[0]).cidr) for addr in (
+                set(flow_state.keys()) - set(addr_to_conj.keys()))])
+        ip_to_conj = collections.defaultdict(set)
         for addr, conj_ids in addr_to_conj.items():
+            # Addresses from remote security groups have mac addresses,
+            # others from remote address groups have not.
+            ip_to_conj[str(netaddr.IPNetwork(addr[0]).cidr)].update(conj_ids)
+
+        for addr in addr_to_conj.keys():
+            ip_cidr = str(netaddr.IPNetwork(addr[0]).cidr)
+            # When the overlapped IP in remote security group and remote
+            # address group have different conjunction ids but with the
+            # same priority offset, we need to combine the conj_ids together
+            # before create flows otherwise flows will be overridden in the
+            # creation sequence.
+            conj_ids = list(ip_to_conj[ip_cidr])
             conj_ids.sort()
-            if flow_state.get(addr) == conj_ids:
+            if flow_state.get(addr) == conj_ids and ip_cidr not in removed_ips:
+                # When there are IP overlaps among remote security groups
+                # and remote address groups, removal of the overlapped ips
+                # from one remote group will also delete the flows for the
+                # other groups because the non-strict delete method cannot
+                # match flow priority or actions for different conjunction
+                # ids, therefore we need to recreate the affected flows.
                 continue
             for flow in rules.create_flows_for_ip_address(
                     addr, direction, ethertype, vlan_tag, conj_ids):
@@ -385,40 +408,41 @@ class ConjIPFlowManager(object):
 
     def update_flows_for_vlan(self, vlan_tag, conj_id_to_remove=None):
         """Install action=conjunction(conj_id, 1/2) flows,
-        which depend on IP addresses of remote_group_id.
+        which depend on IP addresses of remote_group_id or
+        remote_address_group_id.
         """
-        for (direction, ethertype), sg_conj_id_map in (
+        for (direction, ethertype), sg_ag_conj_id_map in (
                 self.conj_ids[vlan_tag].items()):
             # TODO(toshii): optimize when remote_groups have
             # no address overlaps.
             addr_to_conj = self._build_addr_conj_id_map(
-                ethertype, sg_conj_id_map)
+                ethertype, sg_ag_conj_id_map)
             self._update_flows_for_vlan_subr(
                 direction, ethertype, vlan_tag,
                 self.flow_state[vlan_tag][(direction, ethertype)],
                 addr_to_conj, conj_id_to_remove)
             self.flow_state[vlan_tag][(direction, ethertype)] = addr_to_conj
 
-    def add(self, vlan_tag, sg_id, remote_sg_id, direction, ethertype,
+    def add(self, vlan_tag, sg_id, remote_id, direction, ethertype,
             priority_offset):
         """Get conj_id specified by the arguments
         and notify the manager that
-        (remote_sg_id, direction, ethertype, conj_id) flows need to be
+        (remote_id, direction, ethertype, conj_id) flows need
-        populated on the vlan_tag network.
+        to be populated on the vlan_tag network.
 
         A caller must call update_flows_for_vlan to have the change in effect.
 
         """
         conj_id = self.conj_id_map.get_conj_id(
-            sg_id, remote_sg_id, direction, ethertype) + priority_offset * 2
+            sg_id, remote_id, direction, ethertype) + priority_offset * 2
 
         if (direction, ethertype) not in self.conj_ids[vlan_tag]:
             self.conj_ids[vlan_tag][(direction, ethertype)] = (
                 collections.defaultdict(set))
-        self.conj_ids[vlan_tag][(direction, ethertype)][remote_sg_id].add(
+        self.conj_ids[vlan_tag][(direction, ethertype)][remote_id].add(
             conj_id)
 
-        conj_id_tuple = (sg_id, remote_sg_id, direction, ethertype)
+        conj_id_tuple = (sg_id, remote_id, direction, ethertype)
         self.conj_id_map.id_map_group[conj_id_tuple].add(conj_id)
         return conj_id
 
@@ -1390,20 +1414,28 @@ class OVSFirewallDriver(firewall.FirewallDriver):
             protocol = rule.get('protocol')
             priority_offset = rules.flow_priority_offset(rule)
 
+            if rule.get('remote_group_id'):
+                remote_type = 'security group'
+                remote_id = rule.get('remote_group_id')
+            else:
+                remote_type = 'address group'
+                remote_id = rule.get('remote_address_group_id')
             conj_id = self.conj_ip_manager.add(port.vlan_tag, sec_group_id,
-                                               rule['remote_group_id'],
+                                               remote_id,
                                                direction, ethertype,
                                                priority_offset)
             LOG.debug("Created conjunction %(conj_id)s for SG %(sg_id)s "
-                      "referencing remote SG ID %(remote_sg_id)s on port "
+                      "referencing remote %(remote_type)s ID %(remote_id)s "
-                      "%(port_id)s.",
+                      "on port %(port_id)s.",
                       {'conj_id': conj_id,
                        'sg_id': sec_group_id,
-                       'remote_sg_id': rule['remote_group_id'],
+                       'remote_type': remote_type,
+                       'remote_id': remote_id,
                        'port_id': port.id})
 
             rule1 = rule.copy()
-            del rule1['remote_group_id']
+            rule1.pop('remote_group_id', None)
+            rule1.pop('remote_address_group_id', None)
             port_rules_key = (direction, ethertype, protocol)
             port_rules[port_rules_key].append((rule1, conj_id))
 

neutron/agent/linux/openvswitch_firewall/rules.py

@@ -158,10 +158,13 @@ def merge_port_ranges(rule_conj_list):
 def flow_priority_offset(rule, conjunction=False):
     """Calculate flow priority offset from rule.
     Whether the rule belongs to conjunction flows or not is decided
-    upon existence of rule['remote_group_id'] but can be overridden
+    upon existence of rule['remote_group_id'] or
+    rule['remote_address_group_id'] but can be overridden
     to be True using the optional conjunction arg.
     """
-    conj_offset = 0 if 'remote_group_id' in rule or conjunction else 4
+    conj_offset = 0 if 'remote_group_id' in rule or \
+                       'remote_address_group_id' in rule or \
+                       conjunction else 4
     protocol = rule.get('protocol')
     if protocol is None:
         return conj_offset
@@ -295,7 +298,7 @@ def _flow_priority_offset_from_conj_id(conj_id):
 def create_flows_for_ip_address(ip_address, direction, ethertype,
                                 vlan_tag, conj_ids):
     """Create flows from a rule and an ip_address derived from
-    remote_group_id
+    remote_group_id or remote_address_group_id
     """
     ip_address, mac_address = ip_address
     net = netaddr.IPNetwork(str(ip_address))
@@ -324,7 +327,9 @@ def create_flows_for_ip_address(ip_address, direction, ethertype,
     flow_template[FLOW_FIELD_FOR_IPVER_AND_DIRECTION[(
         ip_ver, direction)]] = ip_prefix
 
-    if any_src_ip:
+    if any_src_ip and mac_address:
+        # A remote address group can contain an any_src_ip without
+        # mac_address and in that case we don't set the dl_src.
         flow_template['dl_src'] = mac_address
 
     result = []

neutron/agent/securitygroups_rpc.py

@@ -210,9 +210,12 @@ class SecurityGroupAgentRpc(object):
         LOG.debug("Update security group information")
         for sg_id, sg_rules in security_groups.items():
             self.firewall.update_security_group_rules(sg_id, sg_rules)
-        for remote_sg_id, member_ips in security_group_member_ips.items():
+        # NOTE(hangyang) security_group_member_ips contains IPs from remote
+        # security groups and remote address groups. Both are handled by
+        # the same firewall functions.
+        for remote_id, member_ips in security_group_member_ips.items():
             self.firewall.update_security_group_members(
-                remote_sg_id, member_ips)
+                remote_id, member_ips)
 
     def security_groups_rule_updated(self, security_groups):
         LOG.info("Security group "

neutron/api/rpc/handlers/securitygroups_rpc.py

@@ -357,6 +357,7 @@ class SecurityGroupServerAPIShim(sg_rpc_base.SecurityGroupInfoAPIMixin):
             port['security_groups'] = list(ovo.security_group_ids)
             port['security_group_rules'] = []
             port['security_group_source_groups'] = []
+            port['security_group_remote_address_groups'] = []
             port['fixed_ips'] = [str(f['ip_address'])
                                  for f in port['fixed_ips']]
             # NOTE(kevinbenton): this id==device is only safe for OVS. a lookup
@@ -384,6 +385,19 @@ class SecurityGroupServerAPIShim(sg_rpc_base.SecurityGroupInfoAPIMixin):
                     ips_by_group[sg_id].update(set(port_ips))
         return ips_by_group
 
+    def _select_ips_for_remote_address_group(self, context,
+                                             remote_address_group_ids):
+        if not remote_address_group_ids:
+            return {}
+        ips_by_group = {ag: set() for ag in remote_address_group_ids}
+        for ag_id in remote_address_group_ids:
+            ag = self.rcache.get_resource_by_id('AddressGroup', ag_id)
+            # NOTE(hangyang) Expected data structure is (ip, mac) tuple
+            ips = [(str(addr_assoc.address), None)
+                   for addr_assoc in ag.addresses]
+            ips_by_group[ag_id].update(set(ips))
+        return ips_by_group
+
     def _select_rules_for_ports(self, context, ports):
         if not ports:
             return []

neutron/db/securitygroups_rpc_base.py

@@ -22,6 +22,7 @@ from neutron_lib.db import api as db_api
 from neutron_lib.utils import helpers
 
 from neutron._i18n import _
+from neutron.db.models import address_group as ag_models
 from neutron.db.models import allowed_address_pair as aap_models
 from neutron.db.models import securitygroup as sg_models
 from neutron.db import models_v2
@@ -147,6 +148,7 @@ class SecurityGroupInfoAPIMixin(object):
         - security_groups
         - security_group_rules,
         - security_group_source_groups
+        - security_group_remote_address_groups
         - fixed_ips
         """
         raise NotImplementedError(_("%s must implement get_port_from_device "
@@ -168,14 +170,20 @@ class SecurityGroupInfoAPIMixin(object):
                    'sg_member_ips': {}}
         rules_in_db = self._select_rules_for_ports(context, ports)
         remote_security_group_info = {}
+        remote_address_group_info = {}
         for (port_id, rule_in_db) in rules_in_db:
             remote_gid = rule_in_db.get('remote_group_id')
+            remote_ag_id = rule_in_db.get('remote_address_group_id')
             security_group_id = rule_in_db.get('security_group_id')
             ethertype = rule_in_db['ethertype']
             if ('security_group_source_groups'
                     not in sg_info['devices'][port_id]):
                 sg_info['devices'][port_id][
                     'security_group_source_groups'] = []
+            if ('security_group_remote_address_groups'
+                    not in sg_info['devices'][port_id]):
+                sg_info['devices'][port_id][
+                    'security_group_remote_address_groups'] = []
 
             if remote_gid:
                 if (remote_gid
@@ -188,7 +196,18 @@ class SecurityGroupInfoAPIMixin(object):
                 if ethertype not in remote_security_group_info[remote_gid]:
                     # this set will be serialized into a list by rpc code
                     remote_security_group_info[remote_gid][ethertype] = set()
-
+            elif remote_ag_id:
+                if (remote_ag_id
+                    not in sg_info['devices'][port_id][
+                        'security_group_remote_address_groups']):
+                    sg_info['devices'][port_id][
+                        'security_group_remote_address_groups'].append(
+                        remote_ag_id)
+                if remote_ag_id not in remote_address_group_info:
+                    remote_address_group_info[remote_ag_id] = {}
+                if ethertype not in remote_address_group_info[remote_ag_id]:
+                    # this set will be serialized into a list by rpc code
+                    remote_address_group_info[remote_ag_id][ethertype] = set()
             direction = rule_in_db['direction']
             stateful = self._is_security_group_stateful(context,
                                                         security_group_id)
@@ -198,7 +217,8 @@ class SecurityGroupInfoAPIMixin(object):
                 'stateful': stateful}
 
             for key in ('protocol', 'port_range_min', 'port_range_max',
-                        'remote_ip_prefix', 'remote_group_id'):
+                        'remote_ip_prefix', 'remote_group_id',
+                        'remote_address_group_id'):
                 if rule_in_db.get(key) is not None:
                     if key == 'remote_ip_prefix':
                         direction_ip_prefix = DIRECTION_IP_PREFIX[direction]
@@ -221,7 +241,13 @@ class SecurityGroupInfoAPIMixin(object):
         # rules still reside in sg_info['devices'] [port_id]
         self._apply_provider_rule(context, sg_info['devices'])
 
-        return self._get_security_group_member_ips(context, sg_info)
+        self._get_security_group_member_ips(context, sg_info)
+        # NOTE(hangyang) Remote address group IP info are also stored in
+        # sg_info['sg_member_ips'] so that the two types of remote groups
+        # can be processed by the same firewall functions.
+        sg_info['sg_member_ips'].update(remote_address_group_info)
+        return self._get_address_group_ips(context, sg_info,
+                                           remote_address_group_info)
 
     def _get_security_group_member_ips(self, context, sg_info):
         ips = self._select_ips_for_remote_group(
@@ -233,6 +259,17 @@ class SecurityGroupInfoAPIMixin(object):
                     sg_info['sg_member_ips'][sg_id][ethertype].add(ip)
         return sg_info
 
+    def _get_address_group_ips(self, context, sg_info,
+                               remote_address_group_info):
+        ips = self._select_ips_for_remote_address_group(
+            context, remote_address_group_info.keys())
+        for ag_id, ag_ips in ips.items():
+            for ip in ag_ips:
+                ethertype = 'IPv%d' % netaddr.IPNetwork(ip[0]).version
+                if ethertype in remote_address_group_info[ag_id]:
+                    sg_info['sg_member_ips'][ag_id][ethertype].add(ip)
+        return sg_info
+
     def _select_remote_group_ids(self, ports):
         remote_group_ids = []
         for port in ports.values():
@@ -242,22 +279,41 @@ class SecurityGroupInfoAPIMixin(object):
                     remote_group_ids.append(remote_group_id)
         return remote_group_ids
 
-    def _convert_remote_group_id_to_ip_prefix(self, context, ports):
+    def _select_remote_address_group_ids(self, ports):
+        remote_address_group_ids = []
+        for port in ports.values():
+            for rule in port.get('security_group_rules'):
+                remote_address_group_id = rule.get('remote_address_group_id')
+                if remote_address_group_id:
+                    remote_address_group_ids.append(remote_address_group_id)
+        return remote_address_group_ids
+
+    def _convert_remote_id_to_ip_prefix(self, context, ports):
         remote_group_ids = self._select_remote_group_ids(ports)
+        remote_address_group_ids = self._select_remote_address_group_ids(ports)
         ips = self._select_ips_for_remote_group(context, remote_group_ids)
+        ips.update(self._select_ips_for_remote_address_group(
+            context, remote_address_group_ids))
         for port in ports.values():
             updated_rule = []
             for rule in port.get('security_group_rules'):
                 remote_group_id = rule.get('remote_group_id')
+                remote_address_group_id = rule.get('remote_address_group_id')
                 direction = rule.get('direction')
                 direction_ip_prefix = DIRECTION_IP_PREFIX[direction]
-                if not remote_group_id:
+                if not (remote_group_id or remote_address_group_id):
                     updated_rule.append(rule)
                     continue
 
-                port['security_group_source_groups'].append(remote_group_id)
                 base_rule = rule
-                ip_list = [ip[0] for ip in ips[remote_group_id]]
+                if remote_group_id:
+                    port['security_group_source_groups'].append(
+                        remote_group_id)
+                    ip_list = [ip[0] for ip in ips[remote_group_id]]
+                else:
+                    port['security_group_remote_address_groups'].append(
+                        remote_address_group_id)
+                    ip_list = [ip[0] for ip in ips[remote_address_group_id]]
                 for ip in ip_list:
                     if ip in port.get('fixed_ips', []):
                         continue
@@ -324,7 +380,8 @@ class SecurityGroupInfoAPIMixin(object):
                 'ethertype': rule_in_db['ethertype'],
             }
             for key in ('protocol', 'port_range_min', 'port_range_max',
-                        'remote_ip_prefix', 'remote_group_id'):
+                        'remote_ip_prefix', 'remote_group_id',
+                        'remote_address_group_id'):
                 if rule_in_db.get(key) is not None:
                     if key == 'remote_ip_prefix':
                         direction_ip_prefix = DIRECTION_IP_PREFIX[direction]
@@ -333,7 +390,7 @@ class SecurityGroupInfoAPIMixin(object):
                     rule_dict[key] = rule_in_db[key]
             port['security_group_rules'].append(rule_dict)
         self._apply_provider_rule(context, ports)
-        return self._convert_remote_group_id_to_ip_prefix(context, ports)
+        return self._convert_remote_id_to_ip_prefix(context, ports)
 
     def _select_ips_for_remote_group(self, context, remote_group_ids):
         """Get all IP addresses (including allowed addr pairs) for each sg.
@@ -342,6 +399,14 @@ class SecurityGroupInfoAPIMixin(object):
         """
         raise NotImplementedError()
 
+    def _select_ips_for_remote_address_group(self, context,
+                                             remote_address_group_ids):
+        """Get all IP addresses for each address group.
+
+        Return dict of lists of IPs keyed by address_group_id.
+        """
+        raise NotImplementedError()
+
     def _select_rules_for_ports(self, context, ports):
         """Get all security group rules associated with a list of ports.
 
@@ -435,6 +500,25 @@ class SecurityGroupServerRpcMixin(SecurityGroupInfoAPIMixin,
                     (allowed_addr_ip, mac))
         return ips_by_group
 
+    @db_api.retry_if_session_inactive()
+    def _select_ips_for_remote_address_group(self, context,
+                                             remote_address_group_ids):
+        ips_by_group = {}
+        if not remote_address_group_ids:
+            return ips_by_group
+        for remote_ag_id in remote_address_group_ids:
+            ips_by_group[remote_ag_id] = set()
+
+        ag_assoc_ag_id = ag_models.AddressAssociation.address_group_id
+        ag_assoc_addr = ag_models.AddressAssociation.address
+        query = context.session.query(ag_assoc_ag_id, ag_assoc_addr)
+        query = query.filter(ag_assoc_ag_id.in_(remote_address_group_ids))
+        for ag_id, addr in query:
+            # In order to align the data structure expected on firewall,
+            # we set the mac address as None
+            ips_by_group[ag_id].add((addr, None))
+        return ips_by_group
+
     @db_api.retry_if_session_inactive()
     def _is_security_group_stateful(self, context, sg_id):
         return sg_obj.SecurityGroup.get_sg_by_id(context, sg_id).stateful

neutron/plugins/ml2/db.py

@@ -206,6 +206,7 @@ def make_port_dict_with_security_groups(port, sec_groups):
     port_dict['security_groups'] = sec_groups
     port_dict['security_group_rules'] = []
     port_dict['security_group_source_groups'] = []
+    port_dict['security_group_remote_address_groups'] = []
     port_dict['fixed_ips'] = [ip['ip_address']
                               for ip in port['fixed_ips']]
     return port_dict

neutron/services/logapi/drivers/openvswitch/ovs_firewall_log.py

@@ -385,7 +385,8 @@ class OVSFirewallLoggingDriver(log_ext.LoggingDriver):
     def _create_conj_flows_log(self, remote_rule, port):
         ethertype = remote_rule['ethertype']
         direction = remote_rule['direction']
-        remote_sg_id = remote_rule['remote_group_id']
+        remote_sg_id = remote_rule.get('remote_group_id')
+        remote_ag_id = remote_rule.get('remote_address_group_id')
         secgroup_id = remote_rule['security_group_id']
         # we only want to log first accept packet, that means a packet with
         # ct_state=+new-est, reg_remote_group=conj_id + 1 will be logged
@@ -394,7 +395,8 @@ class OVSFirewallLoggingDriver(log_ext.LoggingDriver):
             'dl_type': ovsfw_consts.ethertype_to_dl_type_map[ethertype],
             'reg_port': port.ofport,
             'reg_remote_group': self.conj_id_map.get_conj_id(
-                secgroup_id, remote_sg_id, direction, ethertype) + 1,
+                secgroup_id, remote_sg_id or remote_ag_id,
+                direction, ethertype) + 1,
         }
         if direction == lib_const.INGRESS_DIRECTION:
             flow_template['table'] = ovs_consts.RULES_INGRESS_TABLE
@@ -406,7 +408,7 @@ class OVSFirewallLoggingDriver(log_ext.LoggingDriver):
         cookie = self.generate_cookie(port.id, log_const.ACCEPT_EVENT,
                                       log_id, project_id)
         for rule in self.create_rules_generator_for_port(port):
-            if 'remote_group_id' in rule:
+            if 'remote_group_id' in rule or 'remote_address_group_id' in rule:
                 flows = self._create_conj_flows_log(rule, port)
             else:
                 flows = rules.create_flows_from_rule_and_port(rule, port)
@@ -465,7 +467,7 @@ class OVSFirewallLoggingDriver(log_ext.LoggingDriver):
         if not cookie:
             return
         for rule in del_rules:
-            if 'remote_group_id' in rule:
+            if 'remote_group_id' in rule or 'remote_address_group_id' in rule:
                 flows = self._create_conj_flows_log(rule, port)
             else:
                 flows = rules.create_flows_from_rule_and_port(rule, port)

neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py

@@ -74,9 +74,12 @@ class TestSecurityGroup(base.BaseTestCase):
     def test_update_rules_split(self):
         rules = [
             {'foo': 'bar', 'rule': 'all'}, {'bar': 'foo'},
-            {'remote_group_id': '123456', 'foo': 'bar'}]
+            {'remote_group_id': '123456', 'foo': 'bar'},
+            {'remote_address_group_id': 'fake_ag_id', 'bar': 'foo'}]
         expected_raw_rules = [{'foo': 'bar', 'rule': 'all'}, {'bar': 'foo'}]
-        expected_remote_rules = [{'remote_group_id': '123456', 'foo': 'bar'}]
+        expected_remote_rules = [{'remote_group_id': '123456', 'foo': 'bar'},
+                                 {'remote_address_group_id': 'fake_ag_id',
+                                  'bar': 'foo'}]
         self.sg.update_rules(rules)
 
         self.assertEqual(expected_raw_rules, self.sg.raw_rules)
@@ -365,7 +368,7 @@ class TestConjIPFlowManager(base.BaseTestCase):
         self.assertTrue(remote_group.get_ethertype_filtered_addresses.called)
         self.assertTrue(self.driver._add_flow.called)
 
-    def test_update_flows_for_vlan(self):
+    def test_update_flows_for_vlan_remote_group(self):
         remote_group = self.driver.sg_port_map.get_sg.return_value
         remote_group.get_ethertype_filtered_addresses.return_value = [
             ('10.22.3.4', 'fa:16:3e:aa:bb:cc'), ]
@@ -461,7 +464,12 @@ class TestOVSFirewallDriver(base.BaseTestCase):
             {'ethertype': constants.IPv6,
              'protocol': constants.PROTO_NAME_TCP,
              'remote_group_id': 2,
-             'direction': constants.EGRESS_DIRECTION}]
+             'direction': constants.EGRESS_DIRECTION},
+            {'ethertype': constants.IPv4,
+             'protocol': constants.PROTO_NAME_TCP,
+             'remote_address_group_id': 3,
+             'direction': constants.EGRESS_DIRECTION}
+        ]
         self.firewall.update_security_group_rules(2, security_group_rules)
 
     @property
@@ -813,8 +821,10 @@ class TestOVSFirewallDriver(base.BaseTestCase):
 
         self.firewall.update_port_filter(port_dict)
         self.assertTrue(self.mock_bridge.br.delete_flows.called)
-        conj_id = self.firewall.conj_ip_manager.conj_id_map.get_conj_id(
+        rsg_conj_id = self.firewall.conj_ip_manager.conj_id_map.get_conj_id(
             2, 2, constants.EGRESS_DIRECTION, constants.IPv6)
+        rag_conj_id = self.firewall.conj_ip_manager.conj_id_map.get_conj_id(
+            2, 3, constants.EGRESS_DIRECTION, constants.IPv4)
         filter_rules = [mock.call(
             actions='resubmit(,{:d})'.format(
                 ovs_consts.ACCEPT_OR_INGRESS_TABLE),
@@ -825,12 +835,20 @@ class TestOVSFirewallDriver(base.BaseTestCase):
             reg5=self.port_ofport,
             table=ovs_consts.RULES_EGRESS_TABLE),
                         mock.call(
-            actions='conjunction({:d},2/2)'.format(conj_id + 6),
+            actions='conjunction({:d},2/2)'.format(rsg_conj_id + 6),
             ct_state=ovsfw_consts.OF_STATE_ESTABLISHED_NOT_REPLY,
             dl_type=mock.ANY,
             nw_proto=6,
             priority=73, reg5=self.port_ofport,
-            table=ovs_consts.RULES_EGRESS_TABLE)]
+            table=ovs_consts.RULES_EGRESS_TABLE),
+                        mock.call(
+            actions='conjunction({:d},2/2)'.format(rag_conj_id + 6),
+            ct_state=ovsfw_consts.OF_STATE_ESTABLISHED_NOT_REPLY,
+            dl_type=mock.ANY,
+            nw_proto=6,
+            priority=73, reg5=self.port_ofport,
+            table=ovs_consts.RULES_EGRESS_TABLE)
+        ]
         self.mock_bridge.br.add_flow.assert_has_calls(
             filter_rules, any_order=True)
 

neutron/tests/unit/agent/test_securitygroups_rpc.py

@@ -156,6 +156,7 @@ class SecurityGroupRpcTestPlugin(test_sg.SecurityGroupTestPlugin,
         if device:
             device['security_group_rules'] = []
             device['security_group_source_groups'] = []
+            device['security_group_remote_address_groups'] = []
             device['fixed_ips'] = [ip['ip_address']
                                    for ip in device['fixed_ips']]
         return device
@@ -507,15 +508,23 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase):
         with self.network() as n,\
                 self.subnet(n),\
                 self.security_group() as sg1,\
-                self.security_group() as sg2:
+                self.security_group() as sg2,\
+                self.address_group(addresses=['10.0.1.0/24']) as ag1:
             sg1_id = sg1['security_group']['id']
             sg2_id = sg2['security_group']['id']
+            ag1_id = ag1['address_group']['id']
+            ag1_ip = ag1['address_group']['addresses'][0]
             rule1 = self._build_security_group_rule(
                 sg1_id,
                 'ingress', const.PROTO_NAME_TCP, '24',
                 '25', remote_group_id=sg2['security_group']['id'])
+            rule2 = self._build_security_group_rule(
+                sg1_id,
+                'ingress', const.PROTO_NAME_TCP, '26',
+                '27', remote_address_group_id=ag1_id)
             rules = {
-                'security_group_rules': [rule1['security_group_rule']]}
+                'security_group_rules': [rule1['security_group_rule'],
+                                         rule2['security_group_rule']]}
             res = self._create_security_group_rule(self.fmt, rules)
             self.deserialize(self.fmt, res)
             self.assertEqual(webob.exc.HTTPCreated.code, res.status_int)
@@ -548,17 +557,31 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase):
                      'ethertype': const.IPv4,
                      'port_range_max': 25, 'port_range_min': 24,
                      'remote_group_id': sg2_id,
-                     'stateful': True}
+                     'stateful': True},
+                    {'direction': u'ingress',
+                     'protocol': const.PROTO_NAME_TCP,
+                     'ethertype': const.IPv4,
+                     'port_range_max': 27, 'port_range_min': 26,
+                     'remote_address_group_id': ag1_id,
+                     'stateful': True},
                 ]},
-                'sg_member_ips': {sg2_id: {
+                'sg_member_ips': {
-                    'IPv4': set([(port_ip2, None), ]),
+                    sg2_id: {
-                    'IPv6': set(),
+                        'IPv4': set([(port_ip2, None), ]),
-                }}
+                        'IPv6': set()
+                    },
+                    ag1_id: {
+                        'IPv4': set([(ag1_ip, None), ]),
+                        'IPv6': set()
+                    },
+                },
             }
             self.assertEqual(expected['security_groups'],
                              ports_rpc['security_groups'])
             self.assertEqual(expected['sg_member_ips'][sg2_id]['IPv4'],
                              ports_rpc['sg_member_ips'][sg2_id]['IPv4'])
+            self.assertEqual(expected['sg_member_ips'][ag1_id]['IPv4'],
+                             ports_rpc['sg_member_ips'][ag1_id]['IPv4'])
             self._delete('ports', port_id1)
             self._delete('ports', port_id2)
 
@@ -633,15 +656,24 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase):
     def test_security_group_info_for_devices_only_ipv6_rule(self):
         with self.network() as n,\
                 self.subnet(n),\
-                self.security_group() as sg1:
+                self.security_group() as sg1,\
+                self.address_group(addresses=['2001::/16']) as ag1:
             sg1_id = sg1['security_group']['id']
+            ag1_id = ag1['address_group']['id']
+            ag1_ip = ag1['address_group']['addresses'][0]
             rule1 = self._build_security_group_rule(
                 sg1_id,
                 'ingress', const.PROTO_NAME_TCP, '22',
                 '22', remote_group_id=sg1_id,
                 ethertype=const.IPv6)
+            rule2 = self._build_security_group_rule(
+                sg1_id,
+                'ingress', const.PROTO_NAME_TCP, '23',
+                '23', remote_address_group_id=ag1_id,
+                ethertype=const.IPv6)
             rules = {
-                'security_group_rules': [rule1['security_group_rule']]}
+                'security_group_rules': [rule1['security_group_rule'],
+                                         rule2['security_group_rule']]}
             self._make_security_group_rule(self.fmt, rules)
 
             res1 = self._create_port(
@@ -666,16 +698,27 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase):
                      'ethertype': const.IPv6,
                      'port_range_max': 22, 'port_range_min': 22,
                      'remote_group_id': sg1_id,
-                     'stateful': True}
+                     'stateful': True},
+                    {'direction': u'ingress',
+                     'protocol': const.PROTO_NAME_TCP,
+                     'ethertype': const.IPv6,
+                     'port_range_max': 23, 'port_range_min': 23,
+                     'remote_address_group_id': ag1_id,
+                     'stateful': True},
                 ]},
-                'sg_member_ips': {sg1_id: {
+                'sg_member_ips': {
-                    'IPv6': set(),
+                    sg1_id: {'IPv6': set()},
-                }}
+                    # This test uses RPC version 1.2 which gets compatible
+                    # member ips without mac address
+                    ag1_id: {'IPv6': set([ag1_ip])}
+                },
             }
             self.assertEqual(expected['security_groups'],
                              ports_rpc['security_groups'])
             self.assertEqual(expected['sg_member_ips'][sg1_id]['IPv6'],
                              ports_rpc['sg_member_ips'][sg1_id]['IPv6'])
+            self.assertEqual(expected['sg_member_ips'][ag1_id]['IPv6'],
+                             ports_rpc['sg_member_ips'][ag1_id]['IPv6'])
             self._delete('ports', port_id1)
 
     def test_security_group_rules_for_devices_ipv6_egress(self):
@@ -947,6 +990,34 @@ class SecurityGroupAgentRpcTestCase(BaseSecurityGroupAgentRpcTestCase):
         self.assertFalse(self.agent.refresh_firewall.called)
         self.assertFalse(self.firewall.security_group_updated.called)
 
+    def test_address_group_updated(self):
+        ag_list = ['fake_agid1', 'fake_agid2']
+        sg_list = ['fake_sgid1', 'fake_sgid2']
+        self.agent.plugin_rpc.get_secgroup_ids_for_address_group = mock.Mock(
+            return_value=sg_list
+        )
+        self.agent.security_groups_rule_updated = mock.Mock()
+        self.agent.address_group_updated(ag_list)
+        self.agent.plugin_rpc.get_secgroup_ids_for_address_group.\
+            assert_called_once_with(ag_list)
+        self.agent.security_groups_rule_updated.assert_called_once_with(
+            sg_list
+        )
+
+    def test_address_group_deleted(self):
+        ag_list = ['fake_agid1', 'fake_agid2']
+        sg_list = []
+        self.agent.plugin_rpc.get_secgroup_ids_for_address_group = mock.Mock(
+            return_value=sg_list
+        )
+        self.agent.security_groups_rule_updated = mock.Mock()
+        self.agent.address_group_updated(ag_list)
+        self.agent.plugin_rpc.get_secgroup_ids_for_address_group.\
+            assert_called_once_with(ag_list)
+        self.agent.security_groups_rule_updated.assert_called_once_with(
+            sg_list
+        )
+
     def test_refresh_firewall(self):
         self.agent.prepare_devices_filter(['fake_port_id'])
         self.agent.refresh_firewall()
@@ -1006,8 +1077,10 @@ class SecurityGroupAgentEnhancedRpcTestCase(BaseSecurityGroupAgentRpcTestCase):
         fake_sg_info = {
             'security_groups': collections.OrderedDict([
                 ('fake_sgid2', []),
-                ('fake_sgid1', [{'remote_group_id': 'fake_sgid2'}])]),
+                ('fake_sgid1', [{'remote_group_id': 'fake_sgid2'},
-            'sg_member_ips': {'fake_sgid2': {'IPv4': [], 'IPv6': []}},
+                                {'remote_address_group_id': 'fake_agid1'}])]),
+            'sg_member_ips': {'fake_sgid2': {'IPv4': [], 'IPv6': []},
+                              'fake_agid1': {'IPv4': [], 'IPv6': []}},
             'devices': self.firewall.ports}
         self.agent.plugin_rpc.security_group_info_for_devices.return_value = (
             fake_sg_info)
@@ -1017,15 +1090,19 @@ class SecurityGroupAgentEnhancedRpcTestCase(BaseSecurityGroupAgentRpcTestCase):
         self.agent.remove_devices_filter(['fake_device'])
         # these two mocks are too long, just use tmp_mock to replace them
         tmp_mock1 = mock.call.update_security_group_rules(
-            'fake_sgid1', [{'remote_group_id': 'fake_sgid2'}])
+            'fake_sgid1', [{'remote_group_id': 'fake_sgid2'},
+                           {'remote_address_group_id': 'fake_agid1'}])
         tmp_mock2 = mock.call.update_security_group_members(
             'fake_sgid2', {'IPv4': [], 'IPv6': []})
+        tmp_mock3 = mock.call.update_security_group_members(
+            'fake_agid1', {'IPv4': [], 'IPv6': []})
         # ignore device which is not filtered
         self.firewall.assert_has_calls([mock.call.defer_apply(),
                                         mock.call.update_security_group_rules(
                                             'fake_sgid2', []),
                                         tmp_mock1,
                                         tmp_mock2,
+                                        tmp_mock3,
                                         mock.call.prepare_port_filter(
                                             self.fake_device),
                                         mock.call.process_trusted_ports([]),
@@ -1075,17 +1152,25 @@ class SecurityGroupAgentEnhancedRpcTestCase(BaseSecurityGroupAgentRpcTestCase):
         calls = [mock.call.defer_apply(),
                  mock.call.update_security_group_rules('fake_sgid2', []),
                  mock.call.update_security_group_rules(
-                     'fake_sgid1', [{'remote_group_id': 'fake_sgid2'}]),
+                     'fake_sgid1', [{'remote_group_id': 'fake_sgid2'},
+                                    {'remote_address_group_id': 'fake_agid1'}
+                                    ]),
                  mock.call.update_security_group_members(
                      'fake_sgid2', {'IPv4': [], 'IPv6': []}),
+                 mock.call.update_security_group_members('fake_agid1', {
+                     'IPv4': [], 'IPv6': []}),
                  mock.call.prepare_port_filter(self.fake_device),
                  mock.call.process_trusted_ports(['fake_port_id']),
                  mock.call.defer_apply(),
                  mock.call.update_security_group_rules('fake_sgid2', []),
                  mock.call.update_security_group_rules(
-                     'fake_sgid1', [{'remote_group_id': 'fake_sgid2'}]),
+                     'fake_sgid1', [{'remote_group_id': 'fake_sgid2'},
+                                    {'remote_address_group_id': 'fake_agid1'}
+                                    ]),
                  mock.call.update_security_group_members(
                      'fake_sgid2', {'IPv4': [], 'IPv6': []}),
+                 mock.call.update_security_group_members('fake_agid1', {
+                     'IPv4': [], 'IPv6': []}),
                  mock.call.update_port_filter(self.fake_device),
                  mock.call.process_trusted_ports([])]
 
@@ -1097,18 +1182,24 @@ class SecurityGroupAgentEnhancedRpcTestCase(BaseSecurityGroupAgentRpcTestCase):
         calls = [mock.call.defer_apply(),
                  mock.call.update_security_group_rules('fake_sgid2', []),
                  mock.call.update_security_group_rules('fake_sgid1', [
-                     {'remote_group_id': 'fake_sgid2'}]),
+                     {'remote_group_id': 'fake_sgid2'},
+                     {'remote_address_group_id': 'fake_agid1'}]),
                  mock.call.update_security_group_members('fake_sgid2', {
                      'IPv4': [], 'IPv6': []
                  }),
+                 mock.call.update_security_group_members('fake_agid1', {
+                     'IPv4': [], 'IPv6': []}),
                  mock.call.prepare_port_filter(self.fake_device),
                  mock.call.process_trusted_ports([]),
                  mock.call.defer_apply(),
                  mock.call.update_security_group_rules('fake_sgid2', []),
                  mock.call.update_security_group_rules('fake_sgid1', [
-                     {'remote_group_id': 'fake_sgid2'}]),
+                     {'remote_group_id': 'fake_sgid2'},
+                     {'remote_address_group_id': 'fake_agid1'}]),
                  mock.call.update_security_group_members('fake_sgid2', {
                      'IPv4': [], 'IPv6': []}),
+                 mock.call.update_security_group_members('fake_agid1', {
+                     'IPv4': [], 'IPv6': []}),
                  mock.call.update_port_filter(self.fake_device),
                  mock.call.process_trusted_ports([]),
                  ]
@@ -1127,14 +1218,22 @@ class SecurityGroupAgentRpcWithDeferredRefreshTestCase(
             defer_refresh_firewall=True)
 
     @contextlib.contextmanager
-    def add_fake_device(self, device, sec_groups, source_sec_groups=None):
+    def add_fake_device(self, device, sec_groups, source_sec_groups=None,
+                        remote_address_groups=None):
         fake_device = {'device': device,
                        'security_groups': sec_groups,
                        'security_group_source_groups': source_sec_groups or [],
+                       'security_group_remote_address_groups':
+                           remote_address_groups or [],
                        'security_group_rules': [{'security_group_id':
                                                  'fake_sgid1',
                                                  'remote_group_id':
-                                                 'fake_sgid2'}]}
+                                                 'fake_sgid2'},
+                                                {'security_group_id':
+                                                     'fake_sgid1',
+                                                 'remote_address_group_id':
+                                                     'fake_agid1'},
+                                                ]}
         self.firewall.ports[device] = fake_device
         yield
         del self.firewall.ports[device]

neutron/tests/unit/extensions/test_address_group.py

@@ -10,6 +10,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import contextlib
+
 from neutron_lib.api.definitions import address_group as apidef
 from neutron_lib import context
 import webob.exc
@@ -94,6 +96,15 @@ class AddressGroupTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
 
         return act_res
 
+    @contextlib.contextmanager
+    def address_group(self, name='test_ag', description='test_ag',
+                      addresses=None, fmt=None):
+        if not fmt:
+            fmt = self.fmt
+        res = self._create_address_group(name=name, description=description,
+                                         addresses=addresses)
+        yield self.deserialize(fmt, res)
+
 
 class AddressGroupTestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
                              address_group_db.AddressGroupDbMixin):

releasenotes/notes/support-address-groups-in-security-group-rules-ce3d9864068c1131.yaml

@@ -0,0 +1,12 @@
+---
+features:
+  - |
+    A new API resource ``address group`` and its CRUD operations are introduced
+    to represent a group of IPv4 and IPv6 address blocks. A new option
+    ``--remote-address-group`` is added to the ``security group rule create``
+    command to allow network connectivity with a group of address blocks. And
+    the backend support is added to the ``openvswitch`` firewall.  When IP
+    addresses are updated in the address groups, changes will also be reflected
+    in the firewall rules of the associated security group rules.
+    For more information, see RFE:
+    `1592028 <https://bugs.launchpad.net/neutron/+bug/1592028>`_