bgp: T3464: use common helper functions to verify route-maps and prefix-lists

2 files changed, 17 insertions, 27 deletions

diff --git a/smoketest/scripts/cli/test_protocols_bgp.py b/smoketest/scripts/cli/test_protocols_bgp.py
index 4f39948c0..8f0856f91 100755
--- a/smoketest/scripts/cli/test_protocols_bgp.py
+++ b/smoketest/scripts/cli/test_protocols_bgp.py
@@ -142,6 +142,8 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):
         self.cli_set(['policy', 'prefix-list6', prefix_list_out6, 'rule', '10', 'action', 'deny'])
         self.cli_set(['policy', 'prefix-list6', prefix_list_out6, 'rule', '10', 'prefix', '2001:db8:2000::/64'])
 
+        self.cli_set(base_path + ['local-as', ASN])
+
     def tearDown(self):
         self.cli_delete(['policy', 'route-map', route_map_in])
         self.cli_delete(['policy', 'route-map', route_map_out])
@@ -214,7 +216,9 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):
         self.cli_set(base_path + ['parameters', 'router-id', router_id])
         self.cli_set(base_path + ['parameters', 'log-neighbor-changes'])
 
-        # Local AS number MUST be defined
+        # Local AS number MUST be defined - as this is set in setUp() we remove
+        # this once for testing of the proper error
+        self.cli_delete(base_path + ['local-as'])
         with self.assertRaises(ConfigSessionError):
             self.cli_commit()
         self.cli_set(base_path + ['local-as', ASN])
@@ -257,7 +261,6 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):
 
 
     def test_bgp_02_neighbors(self):
-        self.cli_set(base_path + ['local-as', ASN])
         # Test out individual neighbor configuration items, not all of them are
         # also available to a peer-group!
         for peer, peer_config in neighbor_config.items():
@@ -332,7 +335,6 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):
             self.verify_frr_config(peer, peer_config, frrconfig)
 
     def test_bgp_03_peer_groups(self):
-        self.cli_set(base_path + ['local-as', ASN])
         # Test out individual peer-group configuration items
         for peer_group, config in peer_group_config.items():
             if 'cap_dynamic' in config:
@@ -403,8 +405,6 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):
                 },
         }
 
-        self.cli_set(base_path + ['local-as', ASN])
-
         # We want to redistribute ...
         redistributes = ['connected', 'isis', 'kernel', 'ospf', 'rip', 'static']
         for redistribute in redistributes:
@@ -451,8 +451,6 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):
             },
         }
 
-        self.cli_set(base_path + ['local-as', ASN])
-
         # We want to redistribute ...
         redistributes = ['connected', 'kernel', 'ospfv3', 'ripng', 'static']
         for redistribute in redistributes:
@@ -495,7 +493,6 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):
         listen_ranges = ['192.0.2.0/25', '192.0.2.128/25']
         peer_group = 'listenfoobar'
 
-        self.cli_set(base_path + ['local-as', ASN])
         self.cli_set(base_path + ['listen', 'limit', limit])
 
         for prefix in listen_ranges:
@@ -527,8 +524,6 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):
         vnis = ['10010', '10020', '10030']
         neighbors = ['192.0.2.10', '192.0.2.20', '192.0.2.30']
 
-        self.cli_set(base_path + ['local-as', ASN])
-
         self.cli_set(base_path + ['address-family', 'l2vpn-evpn', 'advertise-all-vni'])
         self.cli_set(base_path + ['address-family', 'l2vpn-evpn', 'advertise-default-gw'])
         self.cli_set(base_path + ['address-family', 'l2vpn-evpn', 'advertise-svi-ip'])
diff --git a/src/conf_mode/protocols_bgp.py b/src/conf_mode/protocols_bgp.py
index 8304df2e5..a76aec30b 100755
--- a/src/conf_mode/protocols_bgp.py
+++ b/src/conf_mode/protocols_bgp.py
@@ -21,6 +21,8 @@ from sys import argv
 
 from vyos.config import Config
 from vyos.configdict import dict_merge
+from vyos.configverify import verify_prefix_list
+from vyos.configverify import verify_route_map
 from vyos.template import is_ip
 from vyos.template import is_interface
 from vyos.template import render_to_string
@@ -59,11 +61,13 @@ def get_config(config=None):
         bgp.update({'deleted' : ''})
         return bgp
 
-    # We also need some additional information from the config,
-    # prefix-lists and route-maps for instance.
-    base = ['policy']
-    tmp = conf.get_config_dict(base, key_mangling=('-', '_'))
-    # Merge policy dict into bgp dict
+    # We also need some additional information from the config, prefix-lists
+    # and route-maps for instance. They will be used in verify().
+    #
+    # XXX: one MUST always call this without the key_mangling() option! See
+    # vyos.configverify.verify_common_route_maps() for more information.
+    tmp = conf.get_config_dict(['policy'])
+    # Merge policy dict into "regular" config dict
     bgp = dict_merge(tmp, bgp)
 
     return bgp
@@ -148,24 +152,15 @@ def verify(bgp):
                         if tmp not in afi_config['prefix_list']:
                             # bail out early
                             continue
-                        # get_config_dict() mangles all '-' characters to '_' this is legitimate, thus all our
-                        # compares will run on '_' as also '_' is a valid name for a prefix-list
-                        prefix_list = afi_config['prefix_list'][tmp].replace('-', '_')
                         if afi == 'ipv4_unicast':
-                            if dict_search(f'policy.prefix_list.{prefix_list}', bgp) == None:
-                                raise ConfigError(f'prefix-list "{prefix_list}" used for "{tmp}" does not exist!')
+                            verify_prefix_list(afi_config['prefix_list'][tmp], bgp)
                         elif afi == 'ipv6_unicast':
-                            if dict_search(f'policy.prefix_list6.{prefix_list}', bgp) == None:
-                                raise ConfigError(f'prefix-list6 "{prefix_list}" used for "{tmp}" does not exist!')
+                            verify_prefix_list(afi_config['prefix_list'][tmp], bgp, version='6')
 
                 if 'route_map' in afi_config:
                     for tmp in ['import', 'export']:
                         if tmp in afi_config['route_map']:
-                            # get_config_dict() mangles all '-' characters to '_' this is legitim, thus all our
-                            # compares will run on '_' as also '_' is a valid name for a route-map
-                            route_map = afi_config['route_map'][tmp].replace('-', '_')
-                            if dict_search(f'policy.route_map.{route_map}', bgp) == None:
-                                raise ConfigError(f'route-map "{route_map}" used for "{tmp}" does not exist!')
+                            verify_route_map(afi_config['route_map'][tmp], bgp)
 
                 if 'route_reflector_client' in afi_config:
                     if 'remote_as' in peer_config and bgp['local_as'] != peer_config['remote_as']: