From cd6d5c3d407355d2eca586407b14014f4c2043cd Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 8 Apr 2021 17:13:55 +0200 Subject: static: T3280: re-use common route-map XML building block --- interface-definitions/include/static/static-route-map.xml.i | 10 ---------- interface-definitions/protocols-static.xml.in | 2 +- 2 files changed, 1 insertion(+), 11 deletions(-) delete mode 100644 interface-definitions/include/static/static-route-map.xml.i diff --git a/interface-definitions/include/static/static-route-map.xml.i b/interface-definitions/include/static/static-route-map.xml.i deleted file mode 100644 index af825e043..000000000 --- a/interface-definitions/include/static/static-route-map.xml.i +++ /dev/null @@ -1,10 +0,0 @@ - - - - Filter routes installed in local route map - - policy route-map - - - - diff --git a/interface-definitions/protocols-static.xml.in b/interface-definitions/protocols-static.xml.in index baf13777a..3cc28e296 100644 --- a/interface-definitions/protocols-static.xml.in +++ b/interface-definitions/protocols-static.xml.in @@ -11,7 +11,7 @@ 480 - #include + #include #include #include -- cgit v1.2.3 From ee44b9ba6ed7b28f0ae05e65d193ccd95a3a3fe5 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 8 Apr 2021 17:14:44 +0200 Subject: isis: T1316: re-use common route-map XML building block --- .../include/isis/isis-redistribute-ipv4.xml.i | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/interface-definitions/include/isis/isis-redistribute-ipv4.xml.i b/interface-definitions/include/isis/isis-redistribute-ipv4.xml.i index df48b4d28..15c8c0b0b 100644 --- a/interface-definitions/include/isis/isis-redistribute-ipv4.xml.i +++ b/interface-definitions/include/isis/isis-redistribute-ipv4.xml.i @@ -16,14 +16,7 @@ - - - Route map reference - - policy route-map - - - + #include @@ -43,14 +36,7 @@ - - - Route map reference - - policy route-map - - - + #include -- cgit v1.2.3 From 9560d26a8fb726ab100e7978a64942291e53a731 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 8 Apr 2021 17:16:22 +0200 Subject: xml: route-map: add missing constraints Port over the regex used in the old node.def code-base and make this limitation also live on the new CLI interface. --- interface-definitions/include/route-map.xml.i | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/interface-definitions/include/route-map.xml.i b/interface-definitions/include/route-map.xml.i index 5a1c137b9..edbe76892 100644 --- a/interface-definitions/include/route-map.xml.i +++ b/interface-definitions/include/route-map.xml.i @@ -1,14 +1,18 @@ - Route map reference - - txt - Route map reference - + Specify route-map name to use policy route-map + + txt + Route map name + + + ^[-a-zA-Z0-9.]+$ + + Route-map name can only contain alpha-numeric letters and a hyphen -- cgit v1.2.3 From 4c3e1b1390e363e34b7e44e07b26e9bea355c659 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 8 Apr 2021 17:32:11 +0200 Subject: bgp: T2271: zebra route-map can only be set for default VRF --- interface-definitions/include/bgp/bgp-common-config.xml.i | 1 - interface-definitions/protocols-bgp.xml.in | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/interface-definitions/include/bgp/bgp-common-config.xml.i b/interface-definitions/include/bgp/bgp-common-config.xml.i index c89e2288e..14070e006 100644 --- a/interface-definitions/include/bgp/bgp-common-config.xml.i +++ b/interface-definitions/include/bgp/bgp-common-config.xml.i @@ -824,7 +824,6 @@ #include -#include BGP protocol timers diff --git a/interface-definitions/protocols-bgp.xml.in b/interface-definitions/protocols-bgp.xml.in index d610f8dff..f4ebddb42 100644 --- a/interface-definitions/protocols-bgp.xml.in +++ b/interface-definitions/protocols-bgp.xml.in @@ -9,6 +9,7 @@ #include + #include -- cgit v1.2.3 From 32ac0dcf88f2b451d713b6eff7ab58d45565f8c8 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 8 Apr 2021 17:33:04 +0200 Subject: ospf: T2271: zebra route-map can only be set for default VRF --- interface-definitions/include/ospf/ospf-common-config.xml.i | 1 - interface-definitions/protocols-ospf.xml.in | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/interface-definitions/include/ospf/ospf-common-config.xml.i b/interface-definitions/include/ospf/ospf-common-config.xml.i index 7316af670..a01d1c890 100644 --- a/interface-definitions/include/ospf/ospf-common-config.xml.i +++ b/interface-definitions/include/ospf/ospf-common-config.xml.i @@ -697,7 +697,6 @@ -#include Adjust routing timers diff --git a/interface-definitions/protocols-ospf.xml.in b/interface-definitions/protocols-ospf.xml.in index d9c3325ec..4431a1772 100644 --- a/interface-definitions/protocols-ospf.xml.in +++ b/interface-definitions/protocols-ospf.xml.in @@ -9,6 +9,7 @@ #include + #include -- cgit v1.2.3 From 3e6159fcad651f17591b99992c8283f65a59feec Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 8 Apr 2021 17:33:41 +0200 Subject: isis: T2271: zebra route-map can only be set for default VRF --- interface-definitions/protocols-isis.xml.in | 1 + 1 file changed, 1 insertion(+) diff --git a/interface-definitions/protocols-isis.xml.in b/interface-definitions/protocols-isis.xml.in index 1bc890446..42d5049cc 100644 --- a/interface-definitions/protocols-isis.xml.in +++ b/interface-definitions/protocols-isis.xml.in @@ -9,6 +9,7 @@ #include + #include -- cgit v1.2.3 From 421fa38445aea61ad9cc2a42699e679665ee971b Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 8 Apr 2021 19:24:16 +0200 Subject: protocols: T3464: proper handling of routing policy configuration The introduction of key_mangling=('-', '_') when working with get_config_dict() caused more harm then good. This commit extends common helpers and adds new helpers when verifying the existence of route-maps, access-lists or prefix-lists. --- python/vyos/configverify.py | 61 ++++++++++++++++++++++++++++----------- src/conf_mode/protocols_isis.py | 19 ++++++------ src/conf_mode/protocols_ospf.py | 19 ++++++++---- src/conf_mode/protocols_ospfv3.py | 14 +++++---- src/conf_mode/protocols_rip.py | 32 ++++++++++---------- src/conf_mode/protocols_ripng.py | 30 +++++++++---------- src/conf_mode/protocols_static.py | 4 +-- 7 files changed, 107 insertions(+), 72 deletions(-) diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py index 718b7445d..99c472582 100644 --- a/python/vyos/configverify.py +++ b/python/vyos/configverify.py @@ -337,18 +337,16 @@ def verify_accel_ppp_base_service(config): def verify_diffie_hellman_length(file, min_keysize): """ Verify Diffie-Hellamn keypair length given via file. It must be greater then or equal to min_keysize """ + import os + import re + from vyos.util import cmd try: keysize = str(min_keysize) except: return False - import os - import re - from vyos.util import cmd - if os.path.exists(file): - out = cmd(f'openssl dhparam -inform PEM -in {file} -text') prog = re.compile('\d+\s+bit') if prog.search(out): @@ -358,26 +356,55 @@ def verify_diffie_hellman_length(file, min_keysize): return False -def verify_route_maps(config): +def verify_common_route_maps(config): """ Common helper function used by routing protocol implementations to perform recurring validation if the specified route-map for either zebra to kernel installation exists (this is the top-level route_map key) or when a route is redistributed with a route-map that it exists! """ - if 'route_map' in config: - route_map = config['route_map'] + # XXX: This function is called in combination with a previous call to: + # tmp = conf.get_config_dict(['policy']) - see protocols_ospf.py as example. + # We should NOT call this with the key_mangling option as this would rename + # route-map hypens '-' to underscores '_' and one could no longer distinguish + # what should have been the "proper" route-map name, as foo-bar and foo_bar + # are two entire different route-map instances! + for route_map in ['route-map', 'route_map']: + if route_map not in config: + continue + tmp = config[route_map] # Check if the specified route-map exists, if not error out - if dict_search(f'policy.route_map.{route_map}', config) == None: - raise ConfigError(f'Specified route-map "{route_map}" does not exist!') + if dict_search(f'policy.route-map.{tmp}', config) == None: + raise ConfigError(f'Specified route-map "{tmp}" does not exist!') if 'redistribute' in config: for protocol, protocol_config in config['redistribute'].items(): if 'route_map' in protocol_config: - # A hyphen in a route-map name will be converted to _, take care - # about this effect during validation - route_map = protocol_config['route_map'].replace('-','_') - # Check if the specified route-map exists, if not error out - if dict_search(f'policy.route_map.{route_map}', config) == None: - raise ConfigError(f'Redistribution route-map "{route_map}" ' \ - f'for "{protocol}" does not exist!') + verify_route_map(protocol_config['route_map'], config) + +def verify_route_map(route_map_name, config): + """ + Common helper function used by routing protocol implementations to perform + recurring validation if a specified route-map exists! + """ + # Check if the specified route-map exists, if not error out + if dict_search(f'policy.route-map.{route_map_name}', config) == None: + raise ConfigError(f'Specified route-map "{route_map_name}" does not exist!') + +def verify_prefix_list(prefix_list, config, version=''): + """ + Common helper function used by routing protocol implementations to perform + recurring validation if a specified prefix-list exists! + """ + # Check if the specified prefix-list exists, if not error out + if dict_search(f'policy.prefix-list{version}.{prefix_list}', config) == None: + raise ConfigError(f'Specified prefix-list{version} "{prefix_list}" does not exist!') + +def verify_access_list(access_list, config, version=''): + """ + Common helper function used by routing protocol implementations to perform + recurring validation if a specified prefix-list exists! + """ + # Check if the specified ACL exists, if not error out + if dict_search(f'policy.access-list{version}.{access_list}', config) == None: + raise ConfigError(f'Specified access-list{version} "{access_list}" does not exist!') diff --git a/src/conf_mode/protocols_isis.py b/src/conf_mode/protocols_isis.py index 571520cfe..8b75f9873 100755 --- a/src/conf_mode/protocols_isis.py +++ b/src/conf_mode/protocols_isis.py @@ -22,6 +22,7 @@ from sys import argv from vyos.config import Config from vyos.configdict import dict_merge from vyos.configdict import node_changed +from vyos.configverify import verify_common_route_maps from vyos.configverify import verify_interface_exists from vyos.util import call from vyos.util import dict_search @@ -70,10 +71,12 @@ def get_config(config=None): return isis # We also need some additional information from the config, prefix-lists - # and route-maps for instance. They will be used in verify() - base = ['policy'] - tmp = conf.get_config_dict(base, key_mangling=('-', '_')) - # Merge policy dict into OSPF dict + # 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 isis = dict_merge(tmp, isis) return isis @@ -91,6 +94,8 @@ def verify(isis): if int(tmp[-1]) != 0: raise ConfigError('Last byte of IS-IS network entity title must always be 0!') + verify_common_route_maps(isis) + # If interface not set if 'interface' not in isis: raise ConfigError('Interface used for routing updates is mandatory!') @@ -141,12 +146,6 @@ def verify(isis): raise ConfigError(f'"protocols isis {process} redistribute {afi} {proto} {redistr_level}" ' \ f'can not be used with \"protocols isis {process} level {proc_level}\"') - if 'route_map' in redistr_config: - name = redistr_config['route_map'] - tmp = name.replace('-', '_') - if dict_search(f'policy.route_map.{tmp}', isis) == None: - raise ConfigError(f'Route-map {name} does not exist!') - # Segment routing checks if dict_search('segment_routing.global_block', isis): high_label_value = dict_search('segment_routing.global_block.high_label_value', isis) diff --git a/src/conf_mode/protocols_ospf.py b/src/conf_mode/protocols_ospf.py index 30cc33dcf..a97d5b5ed 100755 --- a/src/conf_mode/protocols_ospf.py +++ b/src/conf_mode/protocols_ospf.py @@ -22,7 +22,8 @@ from sys import argv from vyos.config import Config from vyos.configdict import dict_merge from vyos.configdict import node_changed -from vyos.configverify import verify_route_maps +from vyos.configverify import verify_common_route_maps +from vyos.configverify import verify_route_map from vyos.configverify import verify_interface_exists from vyos.template import render_to_string from vyos.util import call @@ -130,10 +131,12 @@ def get_config(config=None): ospf['interface'][interface]) # We also need some additional information from the config, prefix-lists - # and route-maps for instance. They will be used in verify() - base = ['policy'] - tmp = conf.get_config_dict(base, key_mangling=('-', '_')) - # Merge policy dict into OSPF dict + # 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 ospf = dict_merge(tmp, ospf) return ospf @@ -142,7 +145,11 @@ def verify(ospf): if not ospf: return None - verify_route_maps(ospf) + verify_common_route_maps(ospf) + + # As we can have a default-information route-map, we need to validate it! + route_map_name = dict_search('default_information.originate.route_map', ospf) + if route_map_name: verify_route_map(route_map_name, ospf) if 'interface' in ospf: for interface in ospf['interface']: diff --git a/src/conf_mode/protocols_ospfv3.py b/src/conf_mode/protocols_ospfv3.py index 42b6462e3..4ab7b65a3 100755 --- a/src/conf_mode/protocols_ospfv3.py +++ b/src/conf_mode/protocols_ospfv3.py @@ -20,7 +20,7 @@ from sys import exit from vyos.config import Config from vyos.configdict import dict_merge -from vyos.configverify import verify_route_maps +from vyos.configverify import verify_common_route_maps from vyos.template import render_to_string from vyos.util import call from vyos.ifconfig import Interface @@ -45,10 +45,12 @@ def get_config(config=None): return ospfv3 # We also need some additional information from the config, prefix-lists - # and route-maps for instance. They will be used in verify() - base = ['policy'] - tmp = conf.get_config_dict(base, key_mangling=('-', '_')) - # Merge policy dict into OSPF dict + # 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 ospfv3 = dict_merge(tmp, ospfv3) return ospfv3 @@ -57,7 +59,7 @@ def verify(ospfv3): if not ospfv3: return None - verify_route_maps(ospfv3) + verify_common_route_maps(ospfv3) if 'interface' in ospfv3: for ifname, if_config in ospfv3['interface'].items(): diff --git a/src/conf_mode/protocols_rip.py b/src/conf_mode/protocols_rip.py index e7eafd059..b48289dec 100755 --- a/src/conf_mode/protocols_rip.py +++ b/src/conf_mode/protocols_rip.py @@ -20,7 +20,9 @@ from sys import exit from vyos.config import Config from vyos.configdict import dict_merge -from vyos.configverify import verify_route_maps +from vyos.configverify import verify_common_route_maps +from vyos.configverify import verify_access_list +from vyos.configverify import verify_prefix_list from vyos.util import call from vyos.util import dict_search from vyos.xml import defaults @@ -51,10 +53,12 @@ def get_config(config=None): rip = dict_merge(default_values, rip) # We also need some additional information from the config, prefix-lists - # and route-maps for instance. They will be used in verify() - base = ['policy'] - tmp = conf.get_config_dict(base, key_mangling=('-', '_')) - # Merge policy dict into OSPF dict + # 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 rip = dict_merge(tmp, rip) return rip @@ -63,21 +67,19 @@ def verify(rip): if not rip: return None + verify_common_route_maps(rip) + acl_in = dict_search('distribute_list.access_list.in', rip) - if acl_in and acl_in not in (dict_search('policy.access_list', rip) or []): - raise ConfigError(f'Inbound ACL "{acl_in}" does not exist!') + if acl_in: verify_access_list(acl_in, rip) acl_out = dict_search('distribute_list.access_list.out', rip) - if acl_out and acl_out not in (dict_search('policy.access_list', rip) or []): - raise ConfigError(f'Outbound ACL "{acl_out}" does not exist!') + if acl_out: verify_access_list(acl_out, rip) - prefix_list_in = dict_search('distribute_list.prefix_list.in', rip) - if prefix_list_in and prefix_list_in.replace('-','_') not in (dict_search('policy.prefix_list', rip) or []): - raise ConfigError(f'Inbound prefix-list "{prefix_list_in}" does not exist!') + prefix_list_in = dict_search('distribute_list.prefix-list.in', rip) + if prefix_list_in: verify_prefix_list(prefix_list_in, rip) prefix_list_out = dict_search('distribute_list.prefix_list.out', rip) - if prefix_list_out and prefix_list_out.replace('-','_') not in (dict_search('policy.prefix_list', rip) or []): - raise ConfigError(f'Outbound prefix-list "{prefix_list_out}" does not exist!') + if prefix_list_out: verify_prefix_list(prefix_list_out, rip) if 'interface' in rip: for interface, interface_options in rip['interface'].items(): @@ -89,8 +91,6 @@ def verify(rip): raise ConfigError(f'You can not have "split-horizon poison-reverse" enabled ' \ f'with "split-horizon disable" for "{interface}"!') - verify_route_maps(rip) - def generate(rip): if not rip: rip['new_frr_config'] = '' diff --git a/src/conf_mode/protocols_ripng.py b/src/conf_mode/protocols_ripng.py index 140133bd0..06a9e97df 100755 --- a/src/conf_mode/protocols_ripng.py +++ b/src/conf_mode/protocols_ripng.py @@ -20,7 +20,9 @@ from sys import exit from vyos.config import Config from vyos.configdict import dict_merge -from vyos.configverify import verify_route_maps +from vyos.configverify import verify_common_route_maps +from vyos.configverify import verify_access_list +from vyos.configverify import verify_prefix_list from vyos.util import call from vyos.util import dict_search from vyos.xml import defaults @@ -51,10 +53,12 @@ def get_config(config=None): ripng = dict_merge(default_values, ripng) # We also need some additional information from the config, prefix-lists - # and route-maps for instance. They will be used in verify() - base = ['policy'] - tmp = conf.get_config_dict(base, key_mangling=('-', '_')) - # Merge policy dict into OSPF dict + # 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 ripng = dict_merge(tmp, ripng) return ripng @@ -63,21 +67,19 @@ def verify(ripng): if not ripng: return None + verify_common_route_maps(ripng) + acl_in = dict_search('distribute_list.access_list.in', ripng) - if acl_in and acl_in not in (dict_search('policy.access_list6', ripng) or []): - raise ConfigError(f'Inbound access-list6 "{acl_in}" does not exist!') + if acl_in: verify_access_list(acl_in, ripng, version='6') acl_out = dict_search('distribute_list.access_list.out', ripng) - if acl_out and acl_out not in (dict_search('policy.access_list6', ripng) or []): - raise ConfigError(f'Outbound access-list6 "{acl_out}" does not exist!') + if acl_out: verify_access_list(acl_out, ripng, version='6') prefix_list_in = dict_search('distribute_list.prefix_list.in', ripng) - if prefix_list_in and prefix_list_in.replace('-','_') not in (dict_search('policy.prefix_list6', ripng) or []): - raise ConfigError(f'Inbound prefix-list6 "{prefix_list_in}" does not exist!') + if prefix_list_in: verify_prefix_list(prefix_list_in, ripng, version='6') prefix_list_out = dict_search('distribute_list.prefix_list.out', ripng) - if prefix_list_out and prefix_list_out.replace('-','_') not in (dict_search('policy.prefix_list6', ripng) or []): - raise ConfigError(f'Outbound prefix-list6 "{prefix_list_out}" does not exist!') + if prefix_list_out: verify_prefix_list(prefix_list_out, ripng, version='6') if 'interface' in ripng: for interface, interface_options in ripng['interface'].items(): @@ -89,8 +91,6 @@ def verify(ripng): raise ConfigError(f'You can not have "split-horizon poison-reverse" enabled ' \ f'with "split-horizon disable" for "{interface}"!') - verify_route_maps(ripng) - def generate(ripng): if not ripng: ripng['new_frr_config'] = '' diff --git a/src/conf_mode/protocols_static.py b/src/conf_mode/protocols_static.py index 7ae952af8..b5b2d6641 100755 --- a/src/conf_mode/protocols_static.py +++ b/src/conf_mode/protocols_static.py @@ -20,7 +20,7 @@ from sys import exit from sys import argv from vyos.config import Config -from vyos.configverify import verify_route_maps +from vyos.configverify import verify_common_route_maps from vyos.configverify import verify_vrf from vyos.template import render_to_string from vyos.util import call @@ -52,7 +52,7 @@ def get_config(config=None): return static def verify(static): - verify_route_maps(static) + verify_common_route_maps(static) for route in ['route', 'route6']: # if there is no route(6) key in the dictionary we can immediately -- cgit v1.2.3 From c87376d1ff8919bd368af19f5b61b62e85ac2e9a Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 8 Apr 2021 19:43:33 +0200 Subject: bgp: T3464: use common helper functions to verify route-maps and prefix-lists --- smoketest/scripts/cli/test_protocols_bgp.py | 15 +++++---------- src/conf_mode/protocols_bgp.py | 29 ++++++++++++----------------- 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']: -- cgit v1.2.3