diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index 119a994572f..48694f74808 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/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 - # to 8 conjuntion IDs (see ConjIPFlowManager.add()). + # (sg_id, remote_id, direction, ethertype). Each tuple + # 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 - populated on the vlan_tag network. + (remote_id, direction, ethertype, conj_id) flows need + 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 " - "%(port_id)s.", + "referencing remote %(remote_type)s ID %(remote_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)) diff --git a/neutron/agent/linux/openvswitch_firewall/rules.py b/neutron/agent/linux/openvswitch_firewall/rules.py index 94bbd874de3..ef59be5b3dd 100644 --- a/neutron/agent/linux/openvswitch_firewall/rules.py +++ b/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 = [] diff --git a/neutron/agent/securitygroups_rpc.py b/neutron/agent/securitygroups_rpc.py index e9d9d5b6d7e..e2e88b86c0b 100644 --- a/neutron/agent/securitygroups_rpc.py +++ b/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 " diff --git a/neutron/api/rpc/handlers/securitygroups_rpc.py b/neutron/api/rpc/handlers/securitygroups_rpc.py index 03c53a59603..6618625ec12 100644 --- a/neutron/api/rpc/handlers/securitygroups_rpc.py +++ b/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 [] diff --git a/neutron/db/securitygroups_rpc_base.py b/neutron/db/securitygroups_rpc_base.py index 534db6cf017..27644852c47 100644 --- a/neutron/db/securitygroups_rpc_base.py +++ b/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 diff --git a/neutron/plugins/ml2/db.py b/neutron/plugins/ml2/db.py index d61c70dbdc6..7dd5b0bcccb 100644 --- a/neutron/plugins/ml2/db.py +++ b/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 diff --git a/neutron/services/logapi/drivers/openvswitch/ovs_firewall_log.py b/neutron/services/logapi/drivers/openvswitch/ovs_firewall_log.py index cbd8afda7be..e0dccb79782 100644 --- a/neutron/services/logapi/drivers/openvswitch/ovs_firewall_log.py +++ b/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) diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py index 9b7c24909a3..f1f0ffd680d 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/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) diff --git a/neutron/tests/unit/agent/test_securitygroups_rpc.py b/neutron/tests/unit/agent/test_securitygroups_rpc.py index c5f2df50897..f39f0d45b8b 100644 --- a/neutron/tests/unit/agent/test_securitygroups_rpc.py +++ b/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: { - 'IPv4': set([(port_ip2, None), ]), - 'IPv6': set(), - }} + 'sg_member_ips': { + sg2_id: { + '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: { - 'IPv6': set(), - }} + 'sg_member_ips': { + 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'}])]), - 'sg_member_ips': {'fake_sgid2': {'IPv4': [], 'IPv6': []}}, + ('fake_sgid1', [{'remote_group_id': 'fake_sgid2'}, + {'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] diff --git a/neutron/tests/unit/extensions/test_address_group.py b/neutron/tests/unit/extensions/test_address_group.py index 82ce4131edd..ff37ba684f0 100644 --- a/neutron/tests/unit/extensions/test_address_group.py +++ b/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): diff --git a/releasenotes/notes/support-address-groups-in-security-group-rules-ce3d9864068c1131.yaml b/releasenotes/notes/support-address-groups-in-security-group-rules-ce3d9864068c1131.yaml new file mode 100644 index 00000000000..6c3c0aa9f90 --- /dev/null +++ b/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 `_