From ffae82f8be6b8024784cd868882e9b1362522092 Mon Sep 17 00:00:00 2001 From: pavel-shirshov Date: Fri, 2 Oct 2020 10:06:04 -0700 Subject: [PATCH] [bgp] Add 'allow list' manager feature (#5513) implements a new feature: "BGP Allow list." This feature allows us to control which IP prefixes are going to be advertised via ebgp from the routes received from EBGP neighbors. --- .../bgpd/templates/general/peer-group.conf.j2 | 2 +- .../bgpd/templates/general/policies.conf.j2 | 27 + files/image_config/constants/constants.yml | 12 + rules/sonic_bgpcfgd.mk | 2 +- src/sonic-bgpcfgd/.gitignore | 1 + src/sonic-bgpcfgd/app/allow_list.py | 632 ++++++++++++++++++ src/sonic-bgpcfgd/app/config.py | 20 +- src/sonic-bgpcfgd/app/directory.py | 159 +++++ src/sonic-bgpcfgd/app/manager.py | 71 ++ src/sonic-bgpcfgd/app/vars.py | 2 +- src/sonic-bgpcfgd/bgpcfgd | 7 +- src/sonic-bgpcfgd/pytest.ini | 2 + src/sonic-bgpcfgd/setup.py | 3 +- .../data/general/policies.conf/param_all.json | 13 +- .../general/policies.conf/param_base.json | 11 +- .../general/policies.conf/param_deny.json | 17 + .../general/policies.conf/result_all.conf | 14 + .../general/policies.conf/result_deny.conf | 39 ++ src/sonic-bgpcfgd/tests/test_allow_list.py | 482 +++++++++++++ .../tests/test_ipv6_nexthop_global.py | 29 +- src/sonic-bgpcfgd/tests/util.py | 4 +- 21 files changed, 1527 insertions(+), 22 deletions(-) create mode 100644 src/sonic-bgpcfgd/app/allow_list.py create mode 100644 src/sonic-bgpcfgd/app/directory.py create mode 100644 src/sonic-bgpcfgd/app/manager.py create mode 100644 src/sonic-bgpcfgd/pytest.ini create mode 100644 src/sonic-bgpcfgd/tests/data/general/policies.conf/param_deny.json create mode 100644 src/sonic-bgpcfgd/tests/data/general/policies.conf/result_deny.conf create mode 100644 src/sonic-bgpcfgd/tests/test_allow_list.py diff --git a/dockers/docker-fpm-frr/frr/bgpd/templates/general/peer-group.conf.j2 b/dockers/docker-fpm-frr/frr/bgpd/templates/general/peer-group.conf.j2 index b0acd1b2a..5790d47a5 100644 --- a/dockers/docker-fpm-frr/frr/bgpd/templates/general/peer-group.conf.j2 +++ b/dockers/docker-fpm-frr/frr/bgpd/templates/general/peer-group.conf.j2 @@ -24,7 +24,7 @@ {% if CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'ToRRouter' %} neighbor PEER_V6 allowas-in 1 neighbor PEER_V6_INT allowas-in 1 - {% endif %} +{% endif %} {% if CONFIG_DB__DEVICE_METADATA['localhost']['sub_role'] == 'BackEnd' %} neighbor PEER_V6_INT route-reflector-client {% endif %} diff --git a/dockers/docker-fpm-frr/frr/bgpd/templates/general/policies.conf.j2 b/dockers/docker-fpm-frr/frr/bgpd/templates/general/policies.conf.j2 index c545cf272..4c27db4a4 100644 --- a/dockers/docker-fpm-frr/frr/bgpd/templates/general/policies.conf.j2 +++ b/dockers/docker-fpm-frr/frr/bgpd/templates/general/policies.conf.j2 @@ -3,6 +3,33 @@ ! ! ! +{% if constants.bgp.allow_list is defined and constants.bgp.allow_list.enabled is defined and constants.bgp.allow_list.enabled %} +{% if constants.bgp.allow_list.default_action is defined and constants.bgp.allow_list.default_action.strip() == 'deny' %} +route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 65535 + set community no-export additive +! +route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 65535 + set community no-export additive +{% else %} +route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 65535 + set community {{ constants.bgp.allow_list.drop_community }} additive +! +route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 65535 + set community {{ constants.bgp.allow_list.drop_community }} additive +{% endif %} +! +route-map FROM_BGP_PEER_V4 permit 2 + call ALLOW_LIST_DEPLOYMENT_ID_0_V4 + on-match next +! +route-map FROM_BGP_PEER_V6 permit 2 + call ALLOW_LIST_DEPLOYMENT_ID_0_V6 + on-match next +! +{% endif %} +! +! +! route-map FROM_BGP_PEER_V4 permit 100 ! route-map TO_BGP_PEER_V4 permit 100 diff --git a/files/image_config/constants/constants.yml b/files/image_config/constants/constants.yml index 3e1b76be0..074956ff8 100644 --- a/files/image_config/constants/constants.yml +++ b/files/image_config/constants/constants.yml @@ -18,6 +18,18 @@ constants: enabled: true ipv4: 64 ipv6: 64 + allow_list: + enabled: true + default_action: "permit" # or "deny" + drop_community: 5060:12345 # value of the community to identify a prefix to drop. Make sense only with allow_list_default_action equal to 'permit' + default_pl_rules: + v4: + - "deny 0.0.0.0/0 le 17" + - "permit 127.0.0.1/32" + v6: + - "deny 0::/0 le 59" + - "deny 0::/0 ge 65" + - "permit fe80::/64" peers: general: # peer_type db_table: "BGP_NEIGHBOR" diff --git a/rules/sonic_bgpcfgd.mk b/rules/sonic_bgpcfgd.mk index 32abbd5af..17e2c7b3f 100644 --- a/rules/sonic_bgpcfgd.mk +++ b/rules/sonic_bgpcfgd.mk @@ -6,6 +6,6 @@ $(SONIC_BGPCFGD)_SRC_PATH = $(SRC_PATH)/sonic-bgpcfgd # of sonic-config-engine and bgpcfgd explicitly calls sonic-cfggen # as part of its unit tests. # TODO: Refactor unit tests so that these dependencies are not needed -$(SONIC_BGPCFGD)_DEPENDS += $(SWSSSDK_PY2) $(SONIC_PY_COMMON_PY2) +$(SONIC_BGPCFGD)_DEPENDS += $(SONIC_PY_COMMON_PY2) $(SONIC_BGPCFGD)_PYTHON_VERSION = 2 SONIC_PYTHON_WHEELS += $(SONIC_BGPCFGD) diff --git a/src/sonic-bgpcfgd/.gitignore b/src/sonic-bgpcfgd/.gitignore index bb1ba531d..920a1b3ae 100644 --- a/src/sonic-bgpcfgd/.gitignore +++ b/src/sonic-bgpcfgd/.gitignore @@ -6,3 +6,4 @@ app/*.pyc tests/*.pyc tests/__pycache__/ .idea +.coverage diff --git a/src/sonic-bgpcfgd/app/allow_list.py b/src/sonic-bgpcfgd/app/allow_list.py new file mode 100644 index 000000000..2637d6999 --- /dev/null +++ b/src/sonic-bgpcfgd/app/allow_list.py @@ -0,0 +1,632 @@ +""" +Implementation of "allow-list" feature +""" +import re + +from app.log import log_debug, log_info, log_err, log_warn +from app.template import TemplateFabric +from app.manager import Manager +from app.util import run_command + +class BGPAllowListMgr(Manager): + """ This class initialize "AllowList" settings """ + ALLOW_ADDRESS_PL_NAME_TMPL = "ALLOW_ADDRESS_%d_%s" # template for a name for the ALLOW_ADDRESS prefix-list ??? + EMPTY_COMMUNITY = "empty" + PL_NAME_TMPL = "PL_ALLOW_LIST_DEPLOYMENT_ID_%d_COMMUNITY_%s_V%s" + COMMUNITY_NAME_TMPL = "COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_%d_COMMUNITY_%s" + RM_NAME_TMPL = "ALLOW_LIST_DEPLOYMENT_ID_%d_V%s" + ROUTE_MAP_ENTRY_WITH_COMMUNITY_START = 10 + ROUTE_MAP_ENTRY_WITH_COMMUNITY_END = 29990 + ROUTE_MAP_ENTRY_WITHOUT_COMMUNITY_START = 30000 + ROUTE_MAP_ENTRY_WITHOUT_COMMUNITY_END = 65530 + + V4 = "v4" # constant for af enum: V4 + V6 = "v6" # constant for af enum: V6 + + def __init__(self, common_objs, db, table): + """ + Initialize the object + :param common_objs: common object dictionary + :param db: name of the db + :param table: name of the table in the db + """ + super(BGPAllowListMgr, self).__init__( + common_objs, + [], + db, + table, + ) + self.cfg_mgr = common_objs["cfg_mgr"] + self.constants = common_objs["constants"] + self.key_re = re.compile(r"^DEPLOYMENT_ID\|\d+\|\S+$|^DEPLOYMENT_ID\|\d+$") + self.enabled = self.__get_enabled() + self.__load_constant_lists() + + def set_handler(self, key, data): + """ + Manager method which runs on receiving 'SET' message + :param key: ket of the 'SET' message + :param data: data of the 'SET' message + :return: True if the message was executed, False - the message should be postponed. + """ + if not self.enabled: + log_warn("BGPAllowListMgr::Received 'SET' command, but this feature is disabled in constants") + return True + if not self.__set_handler_validate(key, data): + return True + key = key.replace("DEPLOYMENT_ID|", "") + deployment_id, community_value = key.split('|', 1) if '|' in key else (key, BGPAllowListMgr.EMPTY_COMMUNITY) + deployment_id = int(deployment_id) + prefixes_v4 = [] + prefixes_v6 = [] + if "prefixes_v4" in data: + prefixes_v4 = str(data['prefixes_v4']).split(",") + if "prefixes_v6" in data: + prefixes_v6 = str(data['prefixes_v6']).split(",") + self.__update_policy(deployment_id, community_value, prefixes_v4, prefixes_v6) + return True + + def __set_handler_validate(self, key, data): + """ + Validate parameters of a "Set" message + :param key: ket of the 'SET' message + :param data: data of the 'SET' message + :return: True if parameters are valid, False if parameters are invalid + """ + if data is None: + log_err("BGPAllowListMgr::Received BGP ALLOWED 'SET' message without data") + return False + if not self.key_re.match(key): + log_err("BGPAllowListMgr::Received BGP ALLOWED 'SET' message with invalid key: '%s'" % key) + return False + prefixes_v4 = [] + prefixes_v6 = [] + if "prefixes_v4" in data: + prefixes_v4 = str(data["prefixes_v4"]).split(",") + if not all(TemplateFabric.is_ipv4(prefix) for prefix in prefixes_v4): + arguments = "prefixes_v4", str(data["prefixes_v4"]) + log_err("BGPAllowListMgr::Received BGP ALLOWED 'SET' message with invalid input[%s]:'%s'" % arguments) + return False + if "prefixes_v6" in data: + prefixes_v6 = str(data["prefixes_v6"]).split(",") + if not all(TemplateFabric.is_ipv6(prefix) for prefix in prefixes_v6): + arguments = "prefixes_v6", str(data["prefixes_v6"]) + log_err("BGPAllowListMgr::Received BGP ALLOWED 'SET' message with invalid input[%s]:'%s'" % arguments) + return False + if not prefixes_v4 and not prefixes_v6: + log_err("BGPAllowListMgr::Received BGP ALLOWED 'SET' message with no prefixes specified: %s" % str(data)) + return False + return True + + def del_handler(self, key): + """ + Manager method which runs on "DEL" message + :param key: a key of "DEL" message + """ + if not self.enabled: + log_warn("BGPAllowListMgr::Received 'DEL' command, but this feature is disabled in constants") + return + if not self.__del_handler_validate(key): + return + key = key.replace('DEPLOYMENT_ID|', '') + deployment_id, community = key.split('|', 1) if '|' in key else (key, BGPAllowListMgr.EMPTY_COMMUNITY) + deployment_id = int(deployment_id) + self.__remove_policy(deployment_id, community) + + def __del_handler_validate(self, key): + """ + Validate "DEL" method parameters + :param key: a key of "DEL" message + :return: True if parameters are valid, False if parameters are invalid + """ + if not self.key_re.match(key): + log_err("BGPAllowListMgr::Received BGP ALLOWED 'DEL' message with invalid key: '$s'" % key) + return False + return True + + def __update_policy(self, deployment_id, community_value, prefixes_v4, prefixes_v6): + """ + Update "allow list" policy with parameters + :param deployment_id: deployment id which policy will be changed + :param community_value: community value to match for the updated policy + :param prefixes_v4: a list of v4 prefixes for the updated policy + :param prefixes_v6: a list of v6 prefixes for the updated policy + """ + # update all related entries with the information + info = deployment_id, community_value, str(prefixes_v4), str(prefixes_v6) + msg = "BGPAllowListMgr::Updating 'Allow list' policy." + msg += " deployment_id '%s'. community: '%s'" + msg += " prefix_v4 '%s'. prefix_v6: '%s'" + log_info(msg % info) + names = self.__generate_names(deployment_id, community_value) + self.cfg_mgr.update() + cmds = [] + cmds += self.__update_prefix_list(self.V4, names['pl_v4'], prefixes_v4) + cmds += self.__update_prefix_list(self.V6, names['pl_v6'], prefixes_v6) + cmds += self.__update_community(names['community'], community_value) + cmds += self.__update_allow_route_map_entry(self.V4, names['pl_v4'], names['community'], names['rm_v4']) + cmds += self.__update_allow_route_map_entry(self.V6, names['pl_v6'], names['community'], names['rm_v6']) + if cmds: + rc = self.cfg_mgr.push_list(cmds) + rc = rc and self.__restart_peers(deployment_id) + log_debug("BGPAllowListMgr::__update_policy. The peers were updated: rc=%s" % rc) + else: + log_debug("BGPAllowListMgr::__update_policy. Nothing to update") + log_info("BGPAllowListMgr::Done") + + def __remove_policy(self, deployment_id, community_value): + """ + Remove "allow list" policy for given deployment_id and community_value + :param deployment_id: deployment id which policy will be removed + :param community_value: community value to match for the removed policy + """ + # remove all related entries from the configuration + # put default rule to the route-map + info = deployment_id, community_value + msg = "BGPAllowListMgr::Removing 'Allow list' policy." + msg += " deployment_id '%s'. community: '%s'" + log_info(msg % info) + + names = self.__generate_names(deployment_id, community_value) + self.cfg_mgr.update() + cmds = [] + cmds += self.__remove_allow_route_map_entry(self.V4, names['pl_v4'], names['community'], names['rm_v4']) + cmds += self.__remove_allow_route_map_entry(self.V6, names['pl_v6'], names['community'], names['rm_v6']) + cmds += self.__remove_prefix_list(self.V4, names['pl_v4']) + cmds += self.__remove_prefix_list(self.V6, names['pl_v6']) + cmds += self.__remove_community(names['community']) + if cmds: + rc = self.cfg_mgr.push_list(cmds) + rc = rc and self.__restart_peers(deployment_id) + log_debug("BGPAllowListMgr::__remove_policy. 'Allow list' policy was removed. rc:%s" % rc) + else: + log_debug("BGPAllowListMgr::__remove_policy. Nothing to remove") + log_info('BGPAllowListMgr::Done') + + @staticmethod + def __generate_names(deployment_id, community_value): + """ + Generate prefix-list names for a given peer_ip and community value + :param deployment_id: deployment_id for which we're going to filter prefixes + :param community_value: community, which we want to use to filter prefixes + :return: a dictionary with names + """ + if community_value == BGPAllowListMgr.EMPTY_COMMUNITY: + community_name = BGPAllowListMgr.EMPTY_COMMUNITY + else: + community_name = BGPAllowListMgr.COMMUNITY_NAME_TMPL % (deployment_id, community_value) + names = { + "pl_v4": BGPAllowListMgr.PL_NAME_TMPL % (deployment_id, community_value, '4'), + "pl_v6": BGPAllowListMgr.PL_NAME_TMPL % (deployment_id, community_value, '6'), + "rm_v4": BGPAllowListMgr.RM_NAME_TMPL % (deployment_id, '4'), + "rm_v6": BGPAllowListMgr.RM_NAME_TMPL % (deployment_id, '6'), + "community": community_name, + } + arguments = deployment_id, community_value, str(names) + log_debug("BGPAllowListMgr::__generate_names. deployment_id: %d, community: %s. names: %s" % arguments) + return names + + def __update_prefix_list(self, af, pl_name, allow_list): + """ + Create or update a prefix-list with name pl_name. + :param af: "v4" to create ipv4 prefix-list, "v6" to create ipv6 prefix-list + :param pl_name: prefix-list name + :param allow_list: prefix-list entries + :return: True if updating was successful, False otherwise + """ + assert af == self.V4 or af == self.V6 + constant_list = self.__get_constant_list(af) + allow_list = self.__to_prefix_list(allow_list) + log_debug("BGPAllowListMgr::__update_prefix_list. af='%s' prefix-list name=%s" % (af, pl_name)) + exist, correct = self.__is_prefix_list_valid(af, pl_name, allow_list, constant_list) + if correct: + log_debug("BGPAllowListMgr::__update_prefix_list. the prefix-list '%s' exists and correct" % pl_name) + return [] + family = self.__af_to_family(af) + cmds = [] + seq_no = 10 + if exist: + cmds.append('no %s prefix-list %s' % (family, pl_name)) + for entry in constant_list + allow_list: + cmds.append('%s prefix-list %s seq %d %s' % (family, pl_name, seq_no, entry)) + seq_no += 10 + return cmds + + def __remove_prefix_list(self, af, pl_name): + """ + Remove prefix-list in the address-family af. + :param af: "v4" to create ipv4 prefix-list, "v6" to create ipv6 prefix-list + :param pl_name: list of prefix-list names + :return: True if operation was successful, False otherwise + """ + assert af == self.V4 or af == self.V6 + log_debug("BGPAllowListMgr::__remove_prefix_lists. af='%s' pl_names='%s'" % (af, pl_name)) + exist, _ = self.__is_prefix_list_valid(af, pl_name, [], []) + if not exist: + log_debug("BGPAllowListMgr::__remove_prefix_lists: prefix_list '%s' not found" % pl_name) + return [] + family = self.__af_to_family(af) + return ["no %s prefix-list %s" % (family, pl_name)] + + def __is_prefix_list_valid(self, af, pl_name, allow_list, constant_list): + """ + Check that a prefix list exists and it has valid entries + :param af: address family of the checked prefix-list + :param pl_name: prefix-list name + :param allow_list: a prefix-list which must be a part of the valid prefix list + :param constant_list: a constant list which must be on top of each "allow" prefix list on the device + :return: a tuple. The first element of the tuple has True if the prefix-list exists, False otherwise, + The second element of the tuple has True if the prefix-list contains correct entries, False if not + """ + assert af == self.V4 or af == self.V6 + family = self.__af_to_family(af) + match_string = '%s prefix-list %s seq ' % (family, pl_name) + conf = self.cfg_mgr.get_text() + if not any(line.strip().startswith(match_string) for line in conf): + return False, False # if the prefix list is not exists, it is not correct + constant_set = set(constant_list) + allow_set = set(allow_list) + for line in conf: + if line.startswith(match_string): + found = line[len(match_string):].strip().split(' ') + rule = " ".join(found[1:]) + if rule in constant_set: + constant_set.discard(rule) + elif rule in allow_set: + if constant_set: + return True, False # Not everything from constant set is presented + else: + allow_set.discard(rule) + return True, len(allow_set) == 0 # allow_set should be presented all + + def __update_community(self, community_name, community_value): + """ + Update community for a peer + :param community_name: name of the community to update + :param community_value: community value for the peer + :return: True if operation was successful, False otherwise + """ + log_debug("BGPAllowListMgr::__update_community. community_name='%s' community='%s'" % (community_name, community_value)) + if community_value == self.EMPTY_COMMUNITY: # we don't need to do anything for EMPTY community + log_debug("BGPAllowListMgr::__update_community. Empty community. exiting") + return [] + cmds = [] + exists, found_community_value = self.__is_community_presented(community_name) + if exists: + if community_value == found_community_value: + log_debug("BGPAllowListMgr::__update_community. community '%s' is already presented" % community_name) + return [] + else: + msg = "BGPAllowListMgr::__update_community. " + msg += "community '%s' is already presented, but community value should be updated" % community_name + log_debug(msg) + cmds.append("no bgp community-list standard %s" % community_name) + cmds.append('bgp community-list standard %s permit %s' % (community_name, community_value)) + return cmds + + def __remove_community(self, community_name): + """ + Remove community for a peer + :param community_name: community value for the peer + :return: True if operation was successful, False otherwise + """ + log_debug("BGPAllowListMgr::__remove_community. community='%s'" % community_name) + if community_name == self.EMPTY_COMMUNITY: # we don't need to do anything for EMPTY community + log_debug("BGPAllowListMgr::__remove_community. There is nothing to remove in empty community") + return [] + exists, _ = self.__is_community_presented(community_name) + if not exists: + log_debug("BGPAllowListMgr::__remove_community. Community is already removed.") + return [] + return ['no bgp community-list standard %s' % community_name] + + def __is_community_presented(self, community_name): + """ + Return True if community for the peer_ip exists + :param community_name: community value for the peer + :return: A tuple. First element: True if operation was successful, False otherwise + Second element: community value if the first element is True no value otherwise + """ + log_debug("BGPAllowListMgr::__is_community_presented. community='%s'" % community_name) + match_string = 'bgp community-list standard %s permit ' % community_name + conf = self.cfg_mgr.get_text() + found = [line.strip() for line in conf if line.strip().startswith(match_string)] + if not found: + return False, None + community_value = found[0].replace(match_string, '') + return True, community_value + + def __update_allow_route_map_entry(self, af, allow_address_pl_name, community_name, route_map_name): + """ + Add or update a "Allow address" route-map entry with the parameters + :param af: "v4" to create ipv4 prefix-list, "v6" to create ipv6 prefix-list + :return: True if operation was successful, False otherwise + """ + assert af == self.V4 or af == self.V6 + info = af, route_map_name, allow_address_pl_name, community_name + log_debug("BGPAllowListMgr::__update_allow_route_map_entry. af='%s' Allow rm='%s' pl='%s' cl='%s'" % info) + entries = self.__parse_allow_route_map_entries(af, route_map_name) + found, _ = self.__find_route_map_entry(entries, allow_address_pl_name, community_name) + if found: + log_debug("BGPAllowListMgr::__update_allow_route_map_entry. route-map='%s' is already found" % route_map_name) + return [] + seq_number = self.__find_next_seq_number(entries.keys(), community_name != self.EMPTY_COMMUNITY, route_map_name) + info = af, seq_number, allow_address_pl_name, community_name + out = "af='%s' seqno='%d' Allow pl='%s' cl='%s'" % info + log_debug("BGPAllowListMgr::__update_allow_route_map_entry. %s" % out) + ip_version = "" if af == self.V4 else "v6" + cmds = [ + 'route-map %s permit %d' % (route_map_name, seq_number), + ' match ip%s address prefix-list %s' % (ip_version, allow_address_pl_name) + ] + if not community_name.endswith(self.EMPTY_COMMUNITY): + cmds.append(" match community %s" % community_name) + return cmds + + def __remove_allow_route_map_entry(self, af, allow_address_pl_name, community_name, route_map_name): + """ + Add or update a "Allow address" route-map entry with the parameters + :param af: "v4" to create ipv4 prefix-list, "v6" to create ipv6 prefix-list + :return: True if operation was successful, False otherwise + """ + assert af == self.V4 or af == self.V6 + info = af, route_map_name, allow_address_pl_name, community_name + log_debug("BGPAllowListMgr::__update_allow_route_map_entry. af='%s' Allow rm='%s' pl='%s' cl='%s'" % info) + entries = self.__parse_allow_route_map_entries(af, route_map_name) + found, seq_number = self.__find_route_map_entry(entries, allow_address_pl_name, community_name) + if not found: + log_debug("BGPAllowListMgr::__update_allow_route_map_entry. Not found route-map '%s' entry" % allow_address_pl_name) + return [] + return ['no route-map %s permit %d' % (route_map_name, seq_number)] + + @staticmethod + def __find_route_map_entry(entries, allow_address_pl_name, community_name): + """ + Find route-map entry with given allow_address prefix list name and community name in the parsed route-map. + :param entries: entries of parsed route-map + :param allow_address_pl_name: name of the "allow address" prefix-list + :param community_name: name of the "allow address" community name + :return: a tuple. The first element of the tuple is True, if the route-map entry was found, False otherwise. + The second element of the tuple has a sequence number of the entry. + """ + for sequence_number, values in entries.items(): + if sequence_number == 65535: + continue + allow_list_presented = values['pl_allow_list'] == allow_address_pl_name + community_presented = values['community'] == community_name + if allow_list_presented and community_presented: + log_debug("BGPAllowListMgr::__find_route_map_entry. found route-map '%s' entry" % allow_address_pl_name) + return True, sequence_number + return False, None + + def __parse_allow_route_map_entries(self, af, route_map_name): + """ + Parse "Allow list" route-map entries. + :param af: "v4" to create ipv4 prefix-list, "v6" to create ipv6 prefix-list + :return: A tuple, First element: True if operation was successful, False otherwise + Second element: list of object with parsed route-map entries + """ + assert af == self.V4 or af == self.V6 + log_debug("BGPAllowListMgr::__parse_allow_route_map_entries. af='%s', rm='%s'" % (af, route_map_name)) + match_string = 'route-map %s permit ' % route_map_name + entries = {} + inside_route_map = False + route_map_seq_number = None + pl_allow_list_name = None + community_name = self.EMPTY_COMMUNITY + if af == self.V4: + match_pl_allow_list = 'match ip address prefix-list ' + else: # self.V6 + match_pl_allow_list = 'match ipv6 address prefix-list ' + match_community = 'match community ' + conf = self.cfg_mgr.get_text() + for line in conf + [""]: + if inside_route_map: + if line.strip().startswith(match_pl_allow_list): + pl_allow_list_name = line.strip()[len(match_pl_allow_list):] + continue + elif line.strip().startswith(match_community): + community_name = line.strip()[len(match_community):] + continue + else: + if pl_allow_list_name is not None: + entries[route_map_seq_number] = { + 'pl_allow_list': pl_allow_list_name, + 'community': community_name, + } + else: + if route_map_seq_number != 65535: + log_warn("BGPAllowListMgr::Found incomplete route-map '%s' entry. seq_no=%d" % (route_map_name, route_map_seq_number)) + inside_route_map = False + pl_allow_list_name = None + community_name = self.EMPTY_COMMUNITY + route_map_seq_number = None + if line.startswith(match_string): + found = line[len(match_string):] + assert found.isdigit() + route_map_seq_number = int(found) + inside_route_map = True + return entries + + @staticmethod + def __find_next_seq_number(seq_numbers, has_community, route_map_name): + """ + Find a next available "Allow list" route-map entry number + :param seq_numbers: a list of already used sequence numbers + :param has_community: True, if the route-map entry has community + :return: next available route-map sequence number + """ + used_sequence_numbers = set(seq_numbers) + sequence_number = None + if has_community: # put entries without communities after 29999 + start_seq = BGPAllowListMgr.ROUTE_MAP_ENTRY_WITH_COMMUNITY_START + end_seq = BGPAllowListMgr.ROUTE_MAP_ENTRY_WITH_COMMUNITY_END + else: + start_seq = BGPAllowListMgr.ROUTE_MAP_ENTRY_WITHOUT_COMMUNITY_START + end_seq = BGPAllowListMgr.ROUTE_MAP_ENTRY_WITHOUT_COMMUNITY_END + for i in range(start_seq, end_seq, 10): + if i not in used_sequence_numbers: + sequence_number = i + break + if sequence_number is None: + raise RuntimeError("No free sequence numbers for '%s'" % route_map_name) + info = sequence_number, "yes" if has_community else "no" + log_debug("BGPAllowListMgr::__find_next_seq_number '%d' has_community='%s'" % info) + return sequence_number + + def __extract_peer_group_names(self): + """ + Extract names of all peer-groups defined in the config + :return: list of peer-group names + """ + # Find all peer-groups entries + re_peer_group = re.compile(r'^\s*neighbor (\S+) peer-group$') + peer_groups = [] + for line in self.cfg_mgr.get_text(): + result = re_peer_group.match(line) + if result: + peer_groups.append(result.group(1)) + return peer_groups + + def __get_peer_group_to_route_map(self, peer_groups): + """ + Extract names of route-maps which is connected to peer-groups defines as peer_groups + :peer_groups: a list of peer-group names + :return: dictionary where key is a peer-group, value is a route-map name which is defined as route-map in + for the peer_group. + """ + pg_2_rm = {} + for pg in peer_groups: + re_peer_group_rm = re.compile(r'^\s*neighbor %s route-map (\S+) in$' % pg) + for line in self.cfg_mgr.get_text(): + result = re_peer_group_rm.match(line) + if result: + pg_2_rm[pg] = result.group(1) + break + return pg_2_rm + + def __get_route_map_calls(self, rms): + """ + Find mapping between route-maps and route-map call names, defined for the route-maps + :rms: a set with route-map names + :return: a dictionary: key - name of a route-map, value - name of a route-map call defined for the route-map + """ + rm_2_call = {} + re_rm = re.compile(r'^route-map (\S+) permit \d+$') + re_call = re.compile(r'^\s*call (\S+)$') + inside_name = None + for line in self.cfg_mgr.get_text(): + if inside_name: + inside_result = re_call.match(line) + if inside_result: + rm_2_call[inside_name] = inside_result.group(1) + inside_name = None + continue + result = re_rm.match(line) + if not result: + continue + inside_name = None + if result.group(1) not in rms: + continue + inside_name = result.group(1) + return rm_2_call + + def __get_peer_group_to_restart(self, deployment_id, pg_2_rm, rm_2_call): + """ + Get peer_groups which are assigned to deployment_id + :deployment_id: deployment_id number + :pg_2_rm: a dictionary where key is a peer-group, value is a route-map name which is defined as route-map in + for the peer_group. + :rm_2_call: a dictionary: key - name of a route-map, value - name of a route-map call defined for the route-map + """ + ret = set() + target_allow_list_prefix = 'ALLOW_LIST_DEPLOYMENT_ID_%d_V' % deployment_id + for peer_group, route_map in pg_2_rm.items(): + if route_map in rm_2_call: + if rm_2_call[route_map].startswith(target_allow_list_prefix): + ret.add(peer_group) + return list(ret) + + def __find_peer_group_by_deployment_id(self, deployment_id): + """ + Deduce peer-group names which are connected to devices with requested deployment_id + :param deployment_id: deployment_id number + :return: a list of peer-groups which a used by devices with requested deployment_id number + """ + self.cfg_mgr.update() + peer_groups = self.__extract_peer_group_names() + pg_2_rm = self.__get_peer_group_to_route_map(peer_groups) + rm_2_call = self.__get_route_map_calls(set(pg_2_rm.values())) + ret = self.__get_peer_group_to_restart(deployment_id, pg_2_rm, rm_2_call) + return list(ret) + + def __restart_peers(self, deployment_id): + """ + Restart peer-groups with requested deployment_id + :param deployment_id: deployment_id number + """ + log_info("BGPAllowListMgr::Restart peers with deployment_id=%d" % deployment_id) + peer_groups = self.__find_peer_group_by_deployment_id(deployment_id) + rv = True + if peer_groups: + for peer_group in peer_groups: + no_error, _, _ = run_command(["vtysh", "-c", "clear bgp peer-group %s soft in" % peer_group]) + rv = no_error == 0 and rv + else: + no_error, _, _ = run_command(["vtysh", "-c", "clear bgp * soft in"]) + rv = no_error == 0 + return rv + + def __get_enabled(self): + """ + Load enable/disabled property from constants + :return: True if enabled, False otherwise + """ + return 'bgp' in self.constants \ + and 'allow_list' in self.constants["bgp"] \ + and "enabled" in self.constants["bgp"]["allow_list"] \ + and self.constants["bgp"]["allow_list"]["enabled"] + + def __load_constant_lists(self): + """ + Load default prefix-list entries from constants.yml file + """ + if 'bgp' in self.constants and 'allow_list' in self.constants["bgp"] \ + and "default_pl_rules" in self.constants["bgp"]["allow_list"]: + obj = self.constants["bgp"]["allow_list"]["default_pl_rules"] + if "v4" in obj: + self.constants_v4 = obj["v4"] + else: + self.constants_v4 = [] + if "v6" in obj: + self.constants_v6 = obj["v6"] + else: + self.constants_v6 = [] + + def __get_constant_list(self, af): + """ + Return loaded default prefix-list entries bases on address family + :param af: address family + :return: default prefix-list entries + """ + if af == self.V4: + return self.constants_v4 + else: + return self.constants_v6 + + @staticmethod + def __to_prefix_list(allow_list): + """ + Convert "allow list" prefix list, to a prefix-list rules + :param allow_list: "allow list" prefix list + :return: prefix-list rules + """ + return ["permit %s ge %d" % (prefix, int(prefix.split("/")[1])+1) for prefix in allow_list] + + def __af_to_family(self, af): + """ + Convert address family into prefix list family + :param af: address family + :return: prefix list ip family + """ + return 'ip' if af == self.V4 else 'ipv6' diff --git a/src/sonic-bgpcfgd/app/config.py b/src/sonic-bgpcfgd/app/config.py index 26639fe75..a0c1329f8 100644 --- a/src/sonic-bgpcfgd/app/config.py +++ b/src/sonic-bgpcfgd/app/config.py @@ -10,19 +10,33 @@ class ConfigMgr(object): """ The class represents frr configuration """ def __init__(self): self.current_config = None + self.current_config_raw = None def reset(self): """ Reset stored config """ self.current_config = None + self.current_config_raw = None def update(self): """ Read current config from FRR """ self.current_config = None + self.current_config_raw = None ret_code, out, err = run_command(["vtysh", "-c", "show running-config"]) if ret_code != 0: + # FIXME: should we throw exception here? log_crit("can't update running config: rc=%d out='%s' err='%s'" % (ret_code, out, err)) return - self.current_config = self.to_canonical(out) + text = [] + for line in out.split('\n'): + if line.lstrip().startswith('!'): + continue + text.append(line) + text += [" "] # Add empty line to have something to work on, if there is no text + self.current_config_raw = text + self.current_config = self.to_canonical(out) # FIXME: use test as an input + + def push_list(self, cmdlist): + return self.push("\n".join(cmdlist)) def push(self, cmd): """ @@ -51,8 +65,12 @@ class ConfigMgr(object): log_err("ConfigMgr::push(): can't push configuration '%s', rc='%d', stdout='%s', stderr='%s'" % err_tuple) if ret_code == 0: self.current_config = None # invalidate config + self.current_config_raw = None return ret_code == 0 + def get_text(self): + return self.current_config_raw + @staticmethod def to_canonical(raw_config): """ diff --git a/src/sonic-bgpcfgd/app/directory.py b/src/sonic-bgpcfgd/app/directory.py new file mode 100644 index 000000000..d27ec6425 --- /dev/null +++ b/src/sonic-bgpcfgd/app/directory.py @@ -0,0 +1,159 @@ +from collections import defaultdict + +from app.log import log_err + + +class Directory(object): + """ This class stores values and notifies callbacks which were registered to be executed as soon + as some value is changed. This class works as DB cache mostly """ + def __init__(self): + self.data = defaultdict(dict) # storage. A key is a slot name, a value is a dictionary with data + self.notify = defaultdict(lambda: defaultdict(list)) # registered callbacks: slot -> path -> handlers[] + + @staticmethod + def get_slot_name(db, table): + """ Convert db, table pair into a slot name """ + return db + "__" + table + + def path_traverse(self, slot, path): + """ + Traverse a path in the storage. + If the path is an empty string, it returns a value as it is. + If the path is not an empty string, the method will traverse through the dictionary value. + Example: + self.data["key_1"] = { "abc": { "cde": { "fgh": "val_1", "ijk": "val_2" } } } + self.path_traverse("key_1", "abc/cde") will return True, { "fgh": "val_1", "ijk": "val_2" } + :param slot: storage key + :param path: storage path as a string where each internal key is separated by '/' + :return: a pair: True if the path was found, object if it was found + """ + if slot not in self.data: + return False, None + elif path == '': + return True, self.data[slot] + d = self.data[slot] + for p in path.split("/"): + if p not in d: + return False, None + d = d[p] + return True, d + + def path_exist(self, db, table, path): + """ + Check if the path exists in the storage + :param db: db name + :param table: table name + :param path: requested path + :return: True if the path is available, False otherwise + """ + slot = self.get_slot_name(db, table) + return self.path_traverse(slot, path)[0] + + def get_path(self, db, table, path): + """ + Return the requested path from the storage + :param db: db name + :param table: table name + :param path: requested path + :return: object if the path was found, None otherwise + """ + slot = self.get_slot_name(db, table) + return self.path_traverse(slot, path)[1] + + def put(self, db, table, key, value): + """ + Put information into the storage. Notify handlers which are dependant to the information + :param db: db name + :param table: table name + :param key: key to change + :param value: value to put + :return: + """ + slot = self.get_slot_name(db, table) + self.data[slot][key] = value + if slot in self.notify: + for path in self.notify[slot].keys(): + if self.path_exist(db, table, path): + for handler in self.notify[slot][path]: + handler() + + def get(self, db, table, key): + """ + Get a value from the storage + :param db: db name + :param table: table name + :param key: ket to get + :return: value for the key + """ + slot = self.get_slot_name(db, table) + return self.data[slot][key] + + def get_slot(self, db, table): + """ + Get an object from the storage + :param db: db name + :param table: table name + :return: object for the slot + """ + slot = self.get_slot_name(db, table) + return self.data[slot] + + def remove(self, db, table, key): + """ + Remove a value from the storage + :param db: db name + :param table: table name + :param key: key to remove + """ + slot = self.get_slot_name(db, table) + if slot in self.data: + if key in self.data[slot]: + del self.data[slot][key] + else: + log_err("Directory: Can't remove key '%s' from slot '%s'. The key doesn't exist" % (key, slot)) + else: + log_err("Directory: Can't remove key '%s' from slot '%s'. The slot doesn't exist" % (key, slot)) + + def remove_slot(self, db, table): + """ + Remove an object from the storage + :param db: db name + :param table: table name + """ + slot = self.get_slot_name(db, table) + if slot in self.data: + del self.data[slot] + else: + log_err("Directory: Can't remove slot '%s'. The slot doesn't exist" % slot) + + def available(self, db, table): + """ + Check if the table is available + :param db: db name + :param table: table name + :return: True if the slot is available, False if not + """ + slot = self.get_slot_name(db, table) + return slot in self.data + + def available_deps(self, deps): + """ + Check if all items from the deps list is available in the storage + :param deps: list of dependencies + :return: True if all dependencies are presented, False otherwise + """ + res = True + for db, table, path in deps: + res = res and self.path_exist(db, table, path) + return res + + def subscribe(self, deps, handler): + """ + Subscribe the handler to be run as soon as all dependencies are presented + :param deps: + :param handler: + :return: + """ + for db, table, path in deps: + slot = self.get_slot_name(db, table) + self.notify[slot][path].append(handler) \ No newline at end of file diff --git a/src/sonic-bgpcfgd/app/manager.py b/src/sonic-bgpcfgd/app/manager.py new file mode 100644 index 000000000..ba45029b5 --- /dev/null +++ b/src/sonic-bgpcfgd/app/manager.py @@ -0,0 +1,71 @@ +from swsscommon import swsscommon + +from app.log import log_debug, log_err + + +class Manager(object): + """ This class represents a SONiC DB table """ + def __init__(self, common_objs, deps, database, table_name): + """ + Initialize class + :param common_objs: common object dictionary + :param deps: dependencies list + :param database: database name + :param table_name: table name + """ + self.directory = common_objs['directory'] + self.cfg_mgr = common_objs['cfg_mgr'] + self.constants = common_objs['constants'] + self.deps = deps + self.db_name = database + self.table_name = table_name + self.set_queue = [] + self.directory.subscribe(deps, self.on_deps_change) # subscribe this class method on directory changes + + def get_database(self): + """ Return associated database """ + return self.db_name + + def get_table_name(self): + """ Return associated table name""" + return self.table_name + + def handler(self, key, op, data): + """ + This method is executed on each add/remove event on the table. + :param key: key of the table entry + :param op: operation on the table entry. Could be either 'SET' or 'DEL' + :param data: associated data of the event. Empty for 'DEL' operation. + """ + if op == swsscommon.SET_COMMAND: + if self.directory.available_deps(self.deps): # all required dependencies are set in the Directory? + res = self.set_handler(key, data) + if not res: # set handler returned False, which means it is not ready to process is. Save it for later. + log_debug("'SET' handler returned NOT_READY for the Manager: %s" % self.__class__) + self.set_queue.append((key, data)) + else: + log_debug("Not all dependencies are met for the Manager: %s" % self.__class__) + self.set_queue.append((key, data)) + elif op == swsscommon.DEL_COMMAND: + self.del_handler(key) + else: + log_err("Invalid operation '%s' for key '%s'" % (op, key)) + + def on_deps_change(self): + """ This method is being executed on every dependency change """ + if not self.directory.available_deps(self.deps): + return + new_queue = [] + for key, data in self.set_queue: + res = self.set_handler(key, data) + if not res: + new_queue.append((key, data)) + self.set_queue = new_queue + + def set_handler(self, key, data): + """ Placeholder for 'SET' command """ + log_err("set_handler() wasn't implemented for %s" % self.__class__.__name__) + + def del_handler(self, key): + """ Placeholder for 'DEL' command """ + log_err("del_handler wasn't implemented for %s" % self.__class__.__name__) \ No newline at end of file diff --git a/src/sonic-bgpcfgd/app/vars.py b/src/sonic-bgpcfgd/app/vars.py index 11377fc87..18bee5578 100644 --- a/src/sonic-bgpcfgd/app/vars.py +++ b/src/sonic-bgpcfgd/app/vars.py @@ -1 +1 @@ -g_debug = False +g_debug = True # FIXME: read from env variable, or from constants diff --git a/src/sonic-bgpcfgd/bgpcfgd b/src/sonic-bgpcfgd/bgpcfgd index 6f52ac162..419fd41fb 100755 --- a/src/sonic-bgpcfgd/bgpcfgd +++ b/src/sonic-bgpcfgd/bgpcfgd @@ -15,10 +15,13 @@ import jinja2 import netaddr from swsscommon import swsscommon +from app.directory import Directory +from app.manager import Manager from app.vars import g_debug from app.log import log_debug, log_notice, log_info, log_warn, log_err, log_crit from app.template import TemplateFabric from app.config import ConfigMgr +from app.allow_list import BGPAllowListMgr from app.util import run_command g_run = True @@ -846,7 +849,7 @@ def wait_for_daemons(daemons, seconds): def read_constants(): """ Read file with constants values from /etc/sonic/constants.yml """ with open('/etc/sonic/constants.yml') as fp: - content = yaml.load(fp) + content = yaml.load(fp) # FIXME: , Loader=yaml.FullLoader) if "constants" not in content: log_crit("/etc/sonic/constants.yml doesn't have 'constants' key") raise Exception("/etc/sonic/constants.yml doesn't have 'constants' key") @@ -878,6 +881,8 @@ def main(): BGPPeerMgrBase(common_objs, "CONFIG_DB", swsscommon.CFG_BGP_NEIGHBOR_TABLE_NAME, "general", True), BGPPeerMgrBase(common_objs, "CONFIG_DB", "BGP_MONITORS", "monitors", False), BGPPeerMgrBase(common_objs, "CONFIG_DB", "BGP_PEER_RANGE", "dynamic", False), + # AllowList Managers + BGPAllowListMgr(common_objs, "CONFIG_DB", "BGP_ALLOWED_PREFIXES"), ] runner = Runner() for mgr in managers: diff --git a/src/sonic-bgpcfgd/pytest.ini b/src/sonic-bgpcfgd/pytest.ini new file mode 100644 index 000000000..639ceb636 --- /dev/null +++ b/src/sonic-bgpcfgd/pytest.ini @@ -0,0 +1,2 @@ +[pytest] +addopts = --cov=app --cov-report term diff --git a/src/sonic-bgpcfgd/setup.py b/src/sonic-bgpcfgd/setup.py index 29d441e09..fc0839dff 100755 --- a/src/sonic-bgpcfgd/setup.py +++ b/src/sonic-bgpcfgd/setup.py @@ -16,5 +16,6 @@ setuptools.setup(name='sonic-bgpcfgd', ] }, install_requires=['jinja2>=2.10', 'netaddr', 'pyyaml'], - setup_requires=['pytest-runner', 'pytest'], + setup_requires=['pytest-runner'], + test_requires=['pytest', 'pytest-cov'], ) diff --git a/src/sonic-bgpcfgd/tests/data/general/policies.conf/param_all.json b/src/sonic-bgpcfgd/tests/data/general/policies.conf/param_all.json index 148456fe9..08f1eef63 100644 --- a/src/sonic-bgpcfgd/tests/data/general/policies.conf/param_all.json +++ b/src/sonic-bgpcfgd/tests/data/general/policies.conf/param_all.json @@ -4,5 +4,14 @@ "sub_role": "BackEnd" } }, - "loopback0_ipv4": "10.10.10.10/32" -} \ No newline at end of file + "loopback0_ipv4": "10.10.10.10/32", + "constants": { + "bgp": { + "allow_list": { + "enabled": true, + "default_action": "permit", + "drop_community": "12345:12345" + } + } + } +} diff --git a/src/sonic-bgpcfgd/tests/data/general/policies.conf/param_base.json b/src/sonic-bgpcfgd/tests/data/general/policies.conf/param_base.json index 53bf5572e..958c9b0fb 100644 --- a/src/sonic-bgpcfgd/tests/data/general/policies.conf/param_base.json +++ b/src/sonic-bgpcfgd/tests/data/general/policies.conf/param_base.json @@ -4,5 +4,12 @@ "sub_role": "NotBackEnd" } }, - "loopback0_ipv4": "10.10.10.10/32" -} \ No newline at end of file + "loopback0_ipv4": "10.10.10.10/32", + "constants": { + "bgp": { + "allow_list": { + "enabled": false + } + } + } +} diff --git a/src/sonic-bgpcfgd/tests/data/general/policies.conf/param_deny.json b/src/sonic-bgpcfgd/tests/data/general/policies.conf/param_deny.json new file mode 100644 index 000000000..669810960 --- /dev/null +++ b/src/sonic-bgpcfgd/tests/data/general/policies.conf/param_deny.json @@ -0,0 +1,17 @@ +{ + "CONFIG_DB__DEVICE_METADATA": { + "localhost": { + "sub_role": "BackEnd" + } + }, + "loopback0_ipv4": "10.10.10.10/32", + "constants": { + "bgp": { + "allow_list": { + "enabled": true, + "default_action": "deny", + "drop_community": "12345:12345" + } + } + } +} diff --git a/src/sonic-bgpcfgd/tests/data/general/policies.conf/result_all.conf b/src/sonic-bgpcfgd/tests/data/general/policies.conf/result_all.conf index 1e3288b9a..9e6c32b17 100644 --- a/src/sonic-bgpcfgd/tests/data/general/policies.conf/result_all.conf +++ b/src/sonic-bgpcfgd/tests/data/general/policies.conf/result_all.conf @@ -1,6 +1,20 @@ ! ! template: bgpd/templates/general/policies.conf.j2 ! +route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 65535 + set community 12345:12345 additive +! +route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 65535 + set community 12345:12345 additive +! +route-map FROM_BGP_PEER_V4 permit 2 + call ALLOW_LIST_DEPLOYMENT_ID_0_V4 + on-match next +! +route-map FROM_BGP_PEER_V6 permit 2 + call ALLOW_LIST_DEPLOYMENT_ID_0_V6 + on-match next +! route-map FROM_BGP_PEER_V4 permit 100 ! route-map TO_BGP_PEER_V4 permit 100 diff --git a/src/sonic-bgpcfgd/tests/data/general/policies.conf/result_deny.conf b/src/sonic-bgpcfgd/tests/data/general/policies.conf/result_deny.conf new file mode 100644 index 000000000..6e0389fc1 --- /dev/null +++ b/src/sonic-bgpcfgd/tests/data/general/policies.conf/result_deny.conf @@ -0,0 +1,39 @@ +! +! template: bgpd/templates/general/policies.conf.j2 +! +route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 65535 + set community no-export additive +! +route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 65535 + set community no-export additive +! +route-map FROM_BGP_PEER_V4 permit 2 + call ALLOW_LIST_DEPLOYMENT_ID_0_V4 + on-match next +! +route-map FROM_BGP_PEER_V6 permit 2 + call ALLOW_LIST_DEPLOYMENT_ID_0_V6 + on-match next +! +route-map FROM_BGP_PEER_V4 permit 100 +! +route-map TO_BGP_PEER_V4 permit 100 +! +route-map FROM_BGP_PEER_V6 permit 1 + set ipv6 next-hop prefer-global +! +route-map FROM_BGP_PEER_V6 permit 100 +! +route-map TO_BGP_PEER_V6 permit 100 +! +route-map FROM_BGP_PEER_V4_INT permit 2 + set originator-id 10.10.10.10 +! +route-map FROM_BGP_PEER_V6_INT permit 1 + set ipv6 next-hop prefer-global +! +route-map FROM_BGP_PEER_V6_INT permit 2 + set originator-id 10.10.10.10 +! +! end of template: bgpd/templates/general/policies.conf.j2 +! diff --git a/src/sonic-bgpcfgd/tests/test_allow_list.py b/src/sonic-bgpcfgd/tests/test_allow_list.py new file mode 100644 index 000000000..f3cf2f534 --- /dev/null +++ b/src/sonic-bgpcfgd/tests/test_allow_list.py @@ -0,0 +1,482 @@ +from app.directory import Directory +from app.template import TemplateFabric +import app +from mock import MagicMock, patch + +swsscommon_module_mock = MagicMock() + +global_constants = { + "bgp": { + "allow_list": { + "enabled": True, + "default_pl_rules": { + "v4": [ "deny 0.0.0.0/0 le 17" ], + "v6": [ + "deny 0::/0 le 59", + "deny 0::/0 ge 65" + ] + } + } + } +} + +@patch.dict("sys.modules", swsscommon=swsscommon_module_mock) +def set_del_test(op, args, currect_config, expected_config): + from app.allow_list import BGPAllowListMgr + set_del_test.push_list_called = False + def push_list(args): + set_del_test.push_list_called = True + assert args == expected_config + return True + # + app.allow_list.run_command = lambda cmd: (0, "", "") + # + cfg_mgr = MagicMock() + cfg_mgr.update.return_value = None + cfg_mgr.push_list = push_list + cfg_mgr.get_text.return_value = currect_config + common_objs = { + 'directory': Directory(), + 'cfg_mgr': cfg_mgr, + 'tf': TemplateFabric(), + 'constants': global_constants, + } + + mgr = BGPAllowListMgr(common_objs, "CONFIG_DB", "BGP_ALLOWED_PREFIXES") + if op == "SET": + mgr.set_handler(*args) + elif op == "DEL": + mgr.del_handler(*args) + else: + assert False, "Wrong operation" + if expected_config: + assert set_del_test.push_list_called, "cfg_mgr.push_list wasn't called" + else: + assert not set_del_test.push_list_called, "cfg_mgr.push_list was called" + +def test_set_handler_with_community(): + set_del_test( + "SET", + ("DEPLOYMENT_ID|5|1010:2020", { + "prefixes_v4": "10.20.30.0/24,30.50.0.0/16", + "prefixes_v6": "fc00:20::/64,fc00:30::/64", + }), + [], + [ + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 10 deny 0.0.0.0/0 le 17', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 20 permit 10.20.30.0/24 ge 25', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 30 permit 30.50.0.0/16 ge 17', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 10 deny 0::/0 le 59', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 20 deny 0::/0 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 30 permit fc00:20::/64 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 40 permit fc00:30::/64 ge 65', + 'bgp community-list standard COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020 permit 1010:2020', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 10', + ' match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4', + ' match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 10', + ' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6', + ' match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020', + ] + ) + +def test_set_handler_no_community(): + set_del_test( + "SET", + ("DEPLOYMENT_ID|5", { + "prefixes_v4": "20.20.30.0/24,40.50.0.0/16", + "prefixes_v6": "fc01:20::/64,fc01:30::/64", + }), + [], + [ + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 10 deny 0.0.0.0/0 le 17', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 20 permit 20.20.30.0/24 ge 25', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 30 permit 40.50.0.0/16 ge 17', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 10 deny 0::/0 le 59', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 20 deny 0::/0 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 30 permit fc01:20::/64 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 40 permit fc01:30::/64 ge 65', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 30000', + ' match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 30000', + ' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6', + ] + ) + +def test_del_handler_with_community(): + set_del_test( + "DEL", + ("DEPLOYMENT_ID|5|1010:2020",), + [ + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 10 deny 0.0.0.0/0 le 17', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 20 permit 10.20.30.0/24 ge 25', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 30 permit 30.50.0.0/16 ge 17', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 10 deny 0::/0 le 59', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 20 deny 0::/0 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 30 permit fc00:20::/64 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 40 permit fc00:30::/64 ge 65', + 'bgp community-list standard COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020 permit 1010:2020', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 10', + ' match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4', + ' match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 10', + ' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6', + ' match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020', + "" + ], + [ + 'no route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 10', + 'no route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 10', + 'no ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4', + 'no ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6', + 'no bgp community-list standard COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020', + ] + ) + +def test_del_handler_no_community(): + set_del_test( + "DEL", + ("DEPLOYMENT_ID|5",), + [ + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 10 deny 0.0.0.0/0 le 17', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 20 permit 20.20.30.0/24 ge 25', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 30 permit 40.50.0.0/16 ge 17', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 10 deny 0::/0 le 59', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 20 deny 0::/0 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 30 permit fc01:20::/64 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 40 permit fc01:30::/64 ge 65', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 30000', + ' match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 30000', + ' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6', + " " + ], + [ + 'no route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 30000', + 'no route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 30000', + 'no ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4', + 'no ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6', + ] + ) + +def test_set_handler_with_community_data_is_already_presented(): + set_del_test( + "SET", + ("DEPLOYMENT_ID|5|1010:2020", { + "prefixes_v4": "10.20.30.0/24,30.50.0.0/16", + "prefixes_v6": "fc00:20::/64,fc00:30::/64", + }), + [ + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 10 deny 0.0.0.0/0 le 17', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 20 permit 10.20.30.0/24 ge 25', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 30 permit 30.50.0.0/16 ge 17', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 10 deny 0::/0 le 59', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 20 deny 0::/0 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 30 permit fc00:20::/64 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 40 permit fc00:30::/64 ge 65', + 'bgp community-list standard COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020 permit 1010:2020', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 10', + ' match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4', + ' match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 10', + ' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6', + ' match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020', + "" + ], + [] + ) + +@patch.dict("sys.modules", swsscommon=swsscommon_module_mock) +def test_set_handler_no_community_data_is_already_presented(): + from app.allow_list import BGPAllowListMgr + cfg_mgr = MagicMock() + cfg_mgr.update.return_value = None + cfg_mgr.get_text.return_value = [ + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 10 deny 0.0.0.0/0 le 17', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 20 permit 20.20.30.0/24 ge 25', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 30 permit 40.50.0.0/16 ge 17', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 10 deny 0::/0 le 59', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 20 deny 0::/0 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 30 permit fc01:20::/64 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 40 permit fc01:30::/64 ge 65', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 30000', + ' match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 30000', + ' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6', + "" + ] + common_objs = { + 'directory': Directory(), + 'cfg_mgr': cfg_mgr, + 'tf': TemplateFabric(), + 'constants': global_constants, + } + mgr = BGPAllowListMgr(common_objs, "CONFIG_DB", "BGP_ALLOWED_PREFIXES") + mgr.set_handler("DEPLOYMENT_ID|5", { + "prefixes_v4": "20.20.30.0/24,40.50.0.0/16", + "prefixes_v6": "fc01:20::/64,fc01:30::/64", + }) + assert not cfg_mgr.push_list.called, "cfg_mgr.push_list was called, but it shouldn't have been" + +def test_del_handler_with_community_no_data(): + set_del_test( + "DEL", + ("DEPLOYMENT_ID|5|1010:2020",), + [""], + [] + ) + +def test_del_handler_no_community_no_data(): + set_del_test( + "DEL", + ("DEPLOYMENT_ID|5",), + [""], + [] + ) + +def test_set_handler_with_community_update_prefixes_add(): + set_del_test( + "SET", + ("DEPLOYMENT_ID|5|1010:2020", { + "prefixes_v4": "10.20.30.0/24,30.50.0.0/16,80.90.0.0/16", + "prefixes_v6": "fc00:20::/64,fc00:30::/64,fc02::/64", + }), + [ + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 10 deny 0.0.0.0/0 le 17', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 20 permit 10.20.30.0/24 ge 25', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 30 permit 30.50.0.0/16 ge 17', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 10 deny 0::/0 le 59', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 20 deny 0::/0 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 30 permit fc00:20::/64 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 40 permit fc00:30::/64 ge 65', + 'bgp community-list standard COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020 permit 1010:2020', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 10', + ' match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4', + ' match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 10', + ' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6', + ' match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020', + "" + ], + [ + 'no ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 10 deny 0.0.0.0/0 le 17', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 20 permit 10.20.30.0/24 ge 25', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 30 permit 30.50.0.0/16 ge 17', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 40 permit 80.90.0.0/16 ge 17', + 'no ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 10 deny 0::/0 le 59', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 20 deny 0::/0 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 30 permit fc00:20::/64 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 40 permit fc00:30::/64 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6 seq 50 permit fc02::/64 ge 65', + ] + ) + +def test_set_handler_no_community_update_prefixes_add(): + set_del_test( + "SET", + ("DEPLOYMENT_ID|5", { + "prefixes_v4": "20.20.30.0/24,40.50.0.0/16,80.90.0.0/16", + "prefixes_v6": "fc01:20::/64,fc01:30::/64,fc02::/64", + }), + [ + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 10 deny 0.0.0.0/0 le 17', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 20 permit 20.20.30.0/24 ge 25', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 30 permit 40.50.0.0/16 ge 17', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 10 deny 0::/0 le 59', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 20 deny 0::/0 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 30 permit fc01:20::/64 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 40 permit fc01:30::/64 ge 65', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 30000', + ' match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 30000', + ' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6', + "" + ], + [ + 'no ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 10 deny 0.0.0.0/0 le 17', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 20 permit 20.20.30.0/24 ge 25', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 30 permit 40.50.0.0/16 ge 17', + 'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 40 permit 80.90.0.0/16 ge 17', + 'no ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 10 deny 0::/0 le 59', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 20 deny 0::/0 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 30 permit fc01:20::/64 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 40 permit fc01:30::/64 ge 65', + 'ipv6 prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6 seq 50 permit fc02::/64 ge 65', + ] + ) + +@patch.dict("sys.modules", swsscommon=swsscommon_module_mock) +def test___set_handler_validate(): + from app.allow_list import BGPAllowListMgr + cfg_mgr = MagicMock() + common_objs = { + 'directory': Directory(), + 'cfg_mgr': cfg_mgr, + 'tf': TemplateFabric(), + 'constants': global_constants, + } + mgr = BGPAllowListMgr(common_objs, "CONFIG_DB", "BGP_ALLOWED_PREFIXES") + data = { + "prefixes_v4": "20.20.30.0/24,40.50.0.0/16", + "prefixes_v6": "fc01:20::/64,fc01:30::/64", + } + assert not mgr._BGPAllowListMgr__set_handler_validate("DEPLOYMENT_ID|5|1010:2020", None) + assert not mgr._BGPAllowListMgr__set_handler_validate("DEPLOYMENT_ID1|5|1010:2020", data) + assert not mgr._BGPAllowListMgr__set_handler_validate("DEPLOYMENT_ID|z|1010:2020", data) + assert not mgr._BGPAllowListMgr__set_handler_validate("DEPLOYMENT_ID|5|1010:2020", { + "prefixes_v4": "20.20.30.0/24,40.50.0.0/16", + "prefixes_v6": "20.20.30.0/24,40.50.0.0/16", + }) + assert not mgr._BGPAllowListMgr__set_handler_validate("DEPLOYMENT_ID|5|1010:2020", { + "prefixes_v4": "fc01:20::/64,fc01:30::/64", + "prefixes_v6": "fc01:20::/64,fc01:30::/64", + }) + +@patch.dict("sys.modules", swsscommon=swsscommon_module_mock) +def test___find_peer_group_by_deployment_id(): + from app.allow_list import BGPAllowListMgr + cfg_mgr = MagicMock() + cfg_mgr.update.return_value = None + cfg_mgr.get_text.return_value = [ + 'router bgp 64601', + ' neighbor BGPSLBPassive peer-group', + ' neighbor BGPSLBPassive remote-as 65432', + ' neighbor BGPSLBPassive passive', + ' neighbor BGPSLBPassive ebgp-multihop 255', + ' neighbor BGPSLBPassive update-source 10.1.0.32', + ' neighbor PEER_V4 peer-group', + ' neighbor PEER_V4_INT peer-group', + ' neighbor PEER_V6 peer-group', + ' neighbor PEER_V6_INT peer-group', + ' neighbor 10.0.0.1 remote-as 64802', + ' neighbor 10.0.0.1 peer-group PEER_V4', + ' neighbor 10.0.0.1 description ARISTA01T1', + ' neighbor 10.0.0.1 timers 3 10', + ' neighbor fc00::2 remote-as 64802', + ' neighbor fc00::2 peer-group PEER_V6', + ' neighbor fc00::2 description ARISTA01T1', + ' neighbor fc00::2 timers 3 10', + ' address-family ipv4 unicast', + ' neighbor BGPSLBPassive activate', + ' neighbor BGPSLBPassive soft-reconfiguration inbound', + ' neighbor BGPSLBPassive route-map FROM_BGP_SPEAKER in', + ' neighbor BGPSLBPassive route-map TO_BGP_SPEAKER out', + ' neighbor PEER_V4 soft-reconfiguration inbound', + ' neighbor PEER_V4 allowas-in 1', + ' neighbor PEER_V4 route-map FROM_BGP_PEER_V4 in', + ' neighbor PEER_V4 route-map TO_BGP_PEER_V4 out', + ' neighbor PEER_V4_INT soft-reconfiguration inbound', + ' neighbor PEER_V4_INT allowas-in 1', + ' neighbor PEER_V4_INT route-map FROM_BGP_PEER_V4 in', + ' neighbor PEER_V4_INT route-map TO_BGP_PEER_V4 out', + ' neighbor 10.0.0.1 activate', + ' exit-address-family', + ' address-family ipv6 unicast', + ' neighbor BGPSLBPassive activate', + ' neighbor PEER_V6 soft-reconfiguration inbound', + ' neighbor PEER_V6 allowas-in 1', + ' neighbor PEER_V6 route-map FROM_BGP_PEER_V6 in', + ' neighbor PEER_V6 route-map TO_BGP_PEER_V6 out', + ' neighbor PEER_V6_INT soft-reconfiguration inbound', + ' neighbor PEER_V6_INT allowas-in 1', + ' neighbor PEER_V6_INT route-map FROM_BGP_PEER_V6 in', + ' neighbor PEER_V6_INT route-map TO_BGP_PEER_V6 out', + ' neighbor fc00::2 activate', + ' exit-address-family', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 10', + ' match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010', + ' match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010_V4', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 30000', + ' match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_empty_V4', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 65535', + ' set community 5060:12345 additive', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 10', + ' match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010', + ' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_1010:1010_V6', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 30000', + ' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_0_COMMUNITY_empty_V6', + 'route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 65535', + ' set community 5060:12345 additive', + 'route-map FROM_BGP_PEER_V4 permit 100', + 'route-map FROM_BGP_PEER_V4 permit 2', + ' call ALLOW_LIST_DEPLOYMENT_ID_0_V4', + ' on-match next', + 'route-map FROM_BGP_PEER_V6 permit 1', + ' set ipv6 next-hop prefer-global ', + 'route-map FROM_BGP_PEER_V6 permit 100', + 'route-map FROM_BGP_PEER_V6 permit 2', + ' call ALLOW_LIST_DEPLOYMENT_ID_0_V6', + ' on-match next', + 'route-map FROM_BGP_SPEAKER permit 10', + 'route-map RM_SET_SRC permit 10', + ' set src 10.1.0.32', + 'route-map RM_SET_SRC6 permit 10', + ' set src FC00:1::32', + 'route-map TO_BGP_PEER_V4 permit 100', + 'route-map TO_BGP_PEER_V6 permit 100', + 'route-map TO_BGP_SPEAKER deny 1', + ] + common_objs = { + 'directory': Directory(), + 'cfg_mgr': cfg_mgr, + 'tf': TemplateFabric(), + 'constants': global_constants, + } + mgr = BGPAllowListMgr(common_objs, "CONFIG_DB", "BGP_ALLOWED_PREFIXES") + values = mgr._BGPAllowListMgr__find_peer_group_by_deployment_id(0) + assert values == ['PEER_V4_INT', 'PEER_V6_INT', 'PEER_V6', 'PEER_V4'] + +@patch.dict("sys.modules", swsscommon=swsscommon_module_mock) +def test___restart_peers_found_deployment_id(): + from app.allow_list import BGPAllowListMgr + test___restart_peers_found_deployment_id.run_command_counter = 0 + def run_command(cmd): + output = [ + ['vtysh', '-c', 'clear bgp peer-group BGP_TEST_PEER_GROUP_1 soft in'], + ['vtysh', '-c', 'clear bgp peer-group BGP_TEST_PEER_GROUP_2 soft in'], + ] + desired_value = output[test___restart_peers_found_deployment_id.run_command_counter] + assert cmd == desired_value + test___restart_peers_found_deployment_id.run_command_counter += 1 + return 0, "", "" + cfg_mgr = MagicMock() + common_objs = { + 'directory': Directory(), + 'cfg_mgr': cfg_mgr, + 'tf': TemplateFabric(), + 'constants': global_constants, + } + mgr = BGPAllowListMgr(common_objs, "CONFIG_DB", "BGP_ALLOWED_PREFIXES") + mocked = MagicMock(name='_BGPAllowListMgr__find_peer_group_by_deployment_id') + mocked.return_value = ["BGP_TEST_PEER_GROUP_1", "BGP_TEST_PEER_GROUP_2"] + mgr._BGPAllowListMgr__find_peer_group_by_deployment_id = mocked + app.allow_list.run_command = run_command + rc = mgr._BGPAllowListMgr__restart_peers(5) + assert rc + +@patch.dict("sys.modules", swsscommon=swsscommon_module_mock) +def test___restart_peers_not_found_deployment_id(): + from app.allow_list import BGPAllowListMgr + def run_command(cmd): + assert cmd == ['vtysh', '-c', 'clear bgp * soft in'] + return 0, "", "" + cfg_mgr = MagicMock() + common_objs = { + 'directory': Directory(), + 'cfg_mgr': cfg_mgr, + 'tf': TemplateFabric(), + 'constants': global_constants, + } + mgr = BGPAllowListMgr(common_objs, "CONFIG_DB", "BGP_ALLOWED_PREFIXES") + mocked = MagicMock(name='_BGPAllowListMgr__find_peer_group_by_deployment_id') + mocked.return_value = [] + mgr._BGPAllowListMgr__find_peer_group_by_deployment_id = mocked + app.allow_list.run_command = run_command + rc = mgr._BGPAllowListMgr__restart_peers(5) + assert rc + +# FIXME: more testcases for coverage diff --git a/src/sonic-bgpcfgd/tests/test_ipv6_nexthop_global.py b/src/sonic-bgpcfgd/tests/test_ipv6_nexthop_global.py index bc6d01cdb..686b1ade6 100644 --- a/src/sonic-bgpcfgd/tests/test_ipv6_nexthop_global.py +++ b/src/sonic-bgpcfgd/tests/test_ipv6_nexthop_global.py @@ -77,21 +77,30 @@ def extract_rm_from_peer_group(path, peer_group_name): return list(rm_set) def check_routemap_in_file(filename, route_map_name): - route_map_re = re.compile(r'^route-map\s+%s\s+(\S+)' % route_map_name) + route_map_re = re.compile(r'^route-map\s+%s\s+permit\s+(\d+)' % route_map_name) set_re = re.compile(r'set ipv6 next-hop prefer-global') with open(filename) as fp: lines = [line.strip() for line in fp if not line.strip().startswith('!') and line.strip() != ''] - found_first_entry = False + found_entry = False + found_seq_no = None + route_map_entries = {} for line in lines: - err_msg = "route-map %s doesn't have mandatory 'set ipv6 next-hop prefer-global' entry as the first rule" % route_map_name - assert not (found_first_entry and line.startswith("route-map")), err_msg - if found_first_entry and set_re.match(line): - break # We're good + if found_entry: + route_map_entries[found_seq_no] = set_re.match(line) is not None + found_entry = False + found_seq_no = None if route_map_re.match(line): - err_msg = "route-map %s doesn't have mandatory permit entry for 'set ipv6 next-hop prefer-global" % route_map_name - assert route_map_re.match(line).group(1) == 'permit', err_msg - found_first_entry = True - return found_first_entry + found_seq_no = None + seq_n_txt = route_map_re.match(line).group(1) + assert seq_n_txt.isdigit(), "wrong sequence number for line '%s'" % line + found_seq_no = int(seq_n_txt) + assert found_seq_no not in route_map_entries, "Route-map has duplicate entries: %s - %d" % (route_map_name, found_seq_no) + found_entry = True + results = [route_map_entries[seq] for seq in sorted(route_map_entries.keys())] + if (len(results)): + err_msg = "route-map %s doesn't have mandatory permit entry for 'set ipv6 next-hop prefer-global" % route_map_name + assert results[0], err_msg + return len(results) > 0 def check_routemap(path, route_map_name): result_files = load_results(path, "policies.conf") diff --git a/src/sonic-bgpcfgd/tests/util.py b/src/sonic-bgpcfgd/tests/util.py index 0bc12b060..aa6c62a56 100644 --- a/src/sonic-bgpcfgd/tests/util.py +++ b/src/sonic-bgpcfgd/tests/util.py @@ -5,7 +5,7 @@ CONSTANTS_PATH = os.path.abspath('../../files/image_config/constants/constants.y def load_constants(): with open(CONSTANTS_PATH) as f: - data = yaml.load(f) + data = yaml.load(f) # FIXME" , Loader=yaml.FullLoader) result = {} assert "constants" in data, "'constants' key not found in constants.yml" assert "bgp" in data["constants"], "'bgp' key not found in constants.yml" @@ -13,4 +13,4 @@ def load_constants(): for name, value in data["constants"]["bgp"]["peers"].items(): assert "template_dir" in value, "'template_dir' key not found for peer '%s'" % name result[name] = value["template_dir"] - return result \ No newline at end of file + return result