[bgpcfgd] to support removal part of configuration of bgp allowed prefix list (#10165)

* fix allow list issue

Signed-off-by: stormliang <[email protected]>

* add the ipaddress in the install list

* add unit test

Co-authored-by: Ubuntu <azureuser@SONIC-SH-STORM-02.5pu3m0fajw1edcfltykk1gauxa.gx.internal.cloudapp.net>

Why I did it
Failed to remove part of configuration of bgp allowed prefix list. The details in #10141

How I did it
There are two issues:

In FRR, ipv6 default route is ::/0, but in the configuration, it is 0::/0, string comparison would be false, but why ipv4 failed to remove the allowed prefix list, ipv6 works? Looks into next one for the answer.

The current managers_allow_list doesn’t support removal part of the prefix list. But why IPv6 works in 1? It is because the bug for the IPv6 default route comparison, it would do the update no matter what is the operation (the code will compare the prefix list in the FRR and configuration db, if all configurations in db are presented in FRR, it do nothing, otherwise it will update the prefix list based on the configuration from db).

How to verify it
Follow the step in #10141

Author: StormLiangMS

src/sonic-bgpcfgd/bgpcfgd/managers_allow_list.py

@@ -2,6 +2,7 @@
 Implementation of "allow-list" feature
 """
 import re
+import ipaddress
 
 from .log import log_debug, log_info, log_err, log_warn
 from .template import TemplateFabric
@@ -19,6 +20,7 @@ class BGPAllowListMgr(Manager):
     ROUTE_MAP_ENTRY_WITH_COMMUNITY_END = 29990
     ROUTE_MAP_ENTRY_WITHOUT_COMMUNITY_START = 30000
     ROUTE_MAP_ENTRY_WITHOUT_COMMUNITY_END = 65530
+    PREFIX_LIST_POS = 1 # the position of the ip prefix in the permit string.
 
     V4 = "v4"  # constant for af enum: V4
     V6 = "v6"  # constant for af enum: V6
@@ -228,6 +230,12 @@ def __update_prefix_list(self, af, pl_name, allow_list):
         constant_list = self.__get_constant_list(af)
         allow_list = self.__to_prefix_list(af, allow_list)
         log_debug("BGPAllowListMgr::__update_prefix_list. af='%s' prefix-list name=%s" % (af, pl_name))
+        '''
+            Need to check exist and equality of the allowed prefix list.
+            A. If exist and equal, no operation needed. 
+            B. If exist but not equal, first delete then add prefix based on the data from condig db and constants. 
+            C. If non-exist, directly add prefix based on the data from condig db and constants.
+        '''
         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)
@@ -237,7 +245,7 @@ def __update_prefix_list(self, af, pl_name, allow_list):
         seq_no = 10
         if exist:
             cmds.append('no %s prefix-list %s' % (family, pl_name))
-        for entry in constant_list + allow_list:
+        for entry in self.__normalize_ipnetwork(af, constant_list + allow_list):
             cmds.append('%s prefix-list %s seq %d %s' % (family, pl_name, seq_no, entry))
             seq_no += 10
         return cmds
@@ -258,6 +266,24 @@ def __remove_prefix_list(self, af, pl_name):
         family = self.__af_to_family(af)
         return ["no %s prefix-list %s" % (family, pl_name)]
 
+    def __normalize_ipnetwork(self, af, allow_prefix_list):
+        '''
+            Normalize IPv6 addresses
+                for example:
+                    2001:cdba:0000:0000:0000:0000:3257:9652
+                    2001:cdba:0:0:0:0:3257:9652
+                    2001:cdba::3257:9652
+                after normalize, all would be normalized to 
+                    2001:cdba::3257:9652
+        '''
+        normalize_list = []
+        for allow_item in allow_prefix_list:
+            tmp_list = allow_item.split(' ')
+            if af == self.V6:
+                tmp_list[self.PREFIX_LIST_POS] = str(ipaddress.IPv6Network(tmp_list[self.PREFIX_LIST_POS]))
+            normalize_list.append(' '.join(tmp_list))
+        return normalize_list
+
     def __is_prefix_list_valid(self, af, pl_name, allow_list, constant_list):
         """
         Check that a prefix list exists and it has valid entries
@@ -266,28 +292,27 @@ def __is_prefix_list_valid(self, af, pl_name, allow_list, constant_list):
         :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
+                 The second element of the tuple has True if allow prefix list in running configuraiton is
+                 equal with ones in config db + constants, 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)
+        expect_set = set(self.__normalize_ipnetwork(af, constant_list))
+        expect_set.update(set(self.__normalize_ipnetwork(af, allow_list)))
+
+        config_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
+                config_list.append(rule)
+
+        # Return double Ture, when running configuraiton is identical with config db + constants.
+        return True, expect_set == set(self.__normalize_ipnetwork(af, config_list))  
 
     def __update_community(self, community_name, community_value):
         """

src/sonic-bgpcfgd/setup.py

@@ -18,6 +18,7 @@
         'jinja2>=2.10',
         'netaddr==0.8.0',
         'pyyaml==5.4.1',
+        'ipaddress==1.0.23'
     ],
     setup_requires = [
         'pytest-runner',