From 4e7ce772ea6af872a2234e981b152753c3187cec Mon Sep 17 00:00:00 2001 From: khramshinr Date: Wed, 3 Apr 2024 16:32:09 +0800 Subject: bgp: T5943: BGP Peer-group members must be all internal or all external (cherry picked from commit d403117cdb5e7718c8590cfeb79a336cb5b67aac) --- smoketest/scripts/cli/test_protocols_bgp.py | 71 +++++++++++++++++++++++++++++ src/conf_mode/protocols_bgp.py | 13 ++++++ 2 files changed, 84 insertions(+) diff --git a/smoketest/scripts/cli/test_protocols_bgp.py b/smoketest/scripts/cli/test_protocols_bgp.py index 60c49b8b4..03daa34aa 100755 --- a/smoketest/scripts/cli/test_protocols_bgp.py +++ b/smoketest/scripts/cli/test_protocols_bgp.py @@ -1259,6 +1259,77 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase): self.assertIn('neighbor peer1 route-reflector-client', conf) + def test_bgp_28_peer_group_member_all_internal_or_external(self): + def _common_config_check(conf, include_ras=True): + if include_ras: + self.assertIn(f'neighbor {int_neighbors[0]} remote-as {ASN}', conf) + self.assertIn(f'neighbor {int_neighbors[1]} remote-as {ASN}', conf) + self.assertIn(f'neighbor {ext_neighbors[0]} remote-as {int(ASN) + 1}',conf) + + self.assertIn(f'neighbor {int_neighbors[0]} peer-group {int_pg_name}', conf) + self.assertIn(f'neighbor {int_neighbors[1]} peer-group {int_pg_name}', conf) + self.assertIn(f'neighbor {ext_neighbors[0]} peer-group {ext_pg_name}', conf) + + int_neighbors = ['192.0.2.2', '192.0.2.3'] + ext_neighbors = ['192.122.2.2', '192.122.2.3'] + int_pg_name, ext_pg_name = 'SMOKETESTINT', 'SMOKETESTEXT' + + self.cli_set(base_path + ['neighbor', int_neighbors[0], 'peer-group', int_pg_name]) + self.cli_set(base_path + ['neighbor', int_neighbors[0], 'remote-as', ASN]) + self.cli_set(base_path + ['peer-group', int_pg_name, 'address-family', 'ipv4-unicast']) + self.cli_set(base_path + ['neighbor', ext_neighbors[0], 'peer-group', ext_pg_name]) + self.cli_set(base_path + ['neighbor', ext_neighbors[0], 'remote-as', f'{int(ASN) + 1}']) + self.cli_set(base_path + ['peer-group', ext_pg_name, 'address-family', 'ipv4-unicast']) + self.cli_commit() + + # test add external remote-as to internal group + self.cli_set(base_path + ['neighbor', int_neighbors[1], 'peer-group', int_pg_name]) + self.cli_set(base_path + ['neighbor', int_neighbors[1], 'remote-as', f'{int(ASN) + 1}']) + + with self.assertRaises(ConfigSessionError) as e: + self.cli_commit() + # self.assertIn('\nPeer-group members must be all internal or all external\n', str(e.exception)) + + # test add internal remote-as to internal group + self.cli_set(base_path + ['neighbor', int_neighbors[1], 'remote-as', ASN]) + self.cli_commit() + + conf = self.getFRRconfig(f'router bgp {ASN}') + _common_config_check(conf) + + # test add internal remote-as to external group + self.cli_set(base_path + ['neighbor', ext_neighbors[1], 'peer-group', ext_pg_name]) + self.cli_set(base_path + ['neighbor', ext_neighbors[1], 'remote-as', ASN]) + + with self.assertRaises(ConfigSessionError) as e: + self.cli_commit() + # self.assertIn('\nPeer-group members must be all internal or all external\n', str(e.exception)) + + # test add external remote-as to external group + self.cli_set(base_path + ['neighbor', ext_neighbors[1], 'remote-as', f'{int(ASN) + 2}']) + self.cli_commit() + + conf = self.getFRRconfig(f'router bgp {ASN}') + _common_config_check(conf) + self.assertIn(f'neighbor {ext_neighbors[1]} remote-as {int(ASN) + 2}', conf) + self.assertIn(f'neighbor {ext_neighbors[1]} peer-group {ext_pg_name}', conf) + + # test named remote-as + self.cli_set(base_path + ['neighbor', int_neighbors[0], 'remote-as', 'internal']) + self.cli_set(base_path + ['neighbor', int_neighbors[1], 'remote-as', 'internal']) + self.cli_set(base_path + ['neighbor', ext_neighbors[0], 'remote-as', 'external']) + self.cli_set(base_path + ['neighbor', ext_neighbors[1], 'remote-as', 'external']) + self.cli_commit() + + conf = self.getFRRconfig(f'router bgp {ASN}') + _common_config_check(conf, include_ras=False) + + self.assertIn(f'neighbor {int_neighbors[0]} remote-as internal', conf) + self.assertIn(f'neighbor {int_neighbors[1]} remote-as internal', conf) + self.assertIn(f'neighbor {ext_neighbors[0]} remote-as external', conf) + self.assertIn(f'neighbor {ext_neighbors[1]} remote-as external', conf) + self.assertIn(f'neighbor {ext_neighbors[1]} peer-group {ext_pg_name}', conf) + def test_bgp_99_bmp(self): target_name = 'instance-bmp' target_address = '127.0.0.1' diff --git a/src/conf_mode/protocols_bgp.py b/src/conf_mode/protocols_bgp.py index 512fa26e9..2b16de775 100755 --- a/src/conf_mode/protocols_bgp.py +++ b/src/conf_mode/protocols_bgp.py @@ -285,6 +285,7 @@ def verify(bgp): elif tmp != 'default': raise ConfigError(f'{error_msg} "{tmp}"!') + peer_groups_context = dict() # Common verification for both peer-group and neighbor statements for neighbor in ['neighbor', 'peer_group']: # bail out early if there is no neighbor or peer-group statement @@ -301,6 +302,18 @@ def verify(bgp): raise ConfigError(f'Specified peer-group "{peer_group}" for '\ f'neighbor "{neighbor}" does not exist!') + if 'remote_as' in peer_config: + is_ibgp = True + if peer_config['remote_as'] != 'internal' and \ + peer_config['remote_as'] != bgp['system_as']: + is_ibgp = False + + if peer_group not in peer_groups_context: + peer_groups_context[peer_group] = is_ibgp + elif peer_groups_context[peer_group] != is_ibgp: + raise ConfigError(f'Peer-group members must be ' + f'all internal or all external') + if 'local_role' in peer_config: #Ensure Local Role has only one value. if len(peer_config['local_role']) > 1: -- cgit v1.2.3