diff options
| author | aapostoliuk <a.apostoliuk@vyos.io> | 2022-12-22 17:52:12 +0200 | 
|---|---|---|
| committer | aapostoliuk <a.apostoliuk@vyos.io> | 2023-01-06 15:33:05 +0200 | 
| commit | d072b88834fa3e7cb9d1f07d672c2da5f3825a24 (patch) | |
| tree | dbf386d29588194e53e41d74c6f427ea16c08d90 | |
| parent | 0c5416e9e629a6f9028c85fe5313f43d3484860a (diff) | |
| download | vyos-1x-d072b88834fa3e7cb9d1f07d672c2da5f3825a24.tar.gz vyos-1x-d072b88834fa3e7cb9d1f07d672c2da5f3825a24.zip | |
T4877: Added more checks if "import vrf" is used in bgp
1. Fixed: If rd and route-target are used in VRF, can not use "import vrf"
   in the same VRF in the same AFI/SAFI.
2. Fixed: If rd and route-target is used in VRF, this VRF can not be in
   the list of command "import vrf" in the same AFI/SAFI but in
   other VRFs.
3. Fixed: Do not allow to delete vrf if it is used in import list
   of other vrfs.
4. Added smoketests to check "import vrf" issues.
| -rwxr-xr-x | smoketest/scripts/cli/test_protocols_bgp.py | 108 | ||||
| -rwxr-xr-x | src/conf_mode/protocols_bgp.py | 164 | 
2 files changed, 254 insertions, 18 deletions
| diff --git a/smoketest/scripts/cli/test_protocols_bgp.py b/smoketest/scripts/cli/test_protocols_bgp.py index debc8270c..e33be6644 100755 --- a/smoketest/scripts/cli/test_protocols_bgp.py +++ b/smoketest/scripts/cli/test_protocols_bgp.py @@ -34,6 +34,10 @@ prefix_list_in6 = 'pfx-foo-in6'  prefix_list_out6 = 'pfx-foo-out6'  bfd_profile = 'foo-bar-baz' +import_afi = 'ipv4-unicast' +import_vrf = 'red' +import_rd = ASN + ':100' +import_vrf_base = ['vrf', 'name']  neighbor_config = {      '192.0.2.1' : {          'bfd'              : '', @@ -189,6 +193,15 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):          # Check for running process          self.assertTrue(process_named_running(PROCESS_NAME)) + +    def create_bgp_instances_for_import_test(self): +        table = '1000' +        self.cli_set(base_path + ['system-as', ASN]) +        # testing only one AFI is sufficient as it's generic code + +        self.cli_set(import_vrf_base + [import_vrf, 'table', table]) +        self.cli_set(import_vrf_base + [import_vrf, 'protocols', 'bgp', 'system-as', ASN]) +      def verify_frr_config(self, peer, peer_config, frrconfig):          # recurring patterns to verify for both a simple neighbor and a peer-group          if 'bfd' in peer_config: @@ -959,6 +972,101 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):          self.assertIn(f' neighbor {neighbor} remote-as {remote_asn}', frrconfig)          self.assertIn(f' neighbor {neighbor} local-as {local_asn}', frrconfig) +    def test_bgp_16_import_rd_rt_compatibility(self): +        # Verify if import vrf and rd vpn export +        # exist in the same address family +        self.create_bgp_instances_for_import_test() +        self.cli_set( +            base_path + ['address-family', import_afi, 'import', 'vrf', +                         import_vrf]) +        self.cli_set( +            base_path + ['address-family', import_afi, 'rd', 'vpn', 'export', +                         import_rd]) +        with self.assertRaises(ConfigSessionError): +            self.cli_commit() + +    def test_bgp_17_import_rd_rt_compatibility(self): +        # Verify if vrf that is in import vrf list contains rd vpn export +        self.create_bgp_instances_for_import_test() +        self.cli_set( +            base_path + ['address-family', import_afi, 'import', 'vrf', +                         import_vrf]) +        self.cli_commit() +        frrconfig = self.getFRRconfig(f'router bgp {ASN}') +        frrconfig_vrf = self.getFRRconfig(f'router bgp {ASN} vrf {import_vrf}') + +        self.assertIn(f'router bgp {ASN}', frrconfig) +        self.assertIn(f'address-family ipv4 unicast', frrconfig) +        self.assertIn(f'  import vrf {import_vrf}', frrconfig) +        self.assertIn(f'router bgp {ASN} vrf {import_vrf}', frrconfig_vrf) + +        self.cli_set( +            import_vrf_base + [import_vrf] + base_path + ['address-family', +                                                          import_afi, 'rd', +                                                          'vpn', 'export', +                                                          import_rd]) +        with self.assertRaises(ConfigSessionError): +            self.cli_commit() + +    def test_bgp_18_deleting_import_vrf(self): +        # Verify deleting vrf that is in import vrf list +        self.create_bgp_instances_for_import_test() +        self.cli_set( +            base_path + ['address-family', import_afi, 'import', 'vrf', +                         import_vrf]) +        self.cli_commit() +        frrconfig = self.getFRRconfig(f'router bgp {ASN}') +        frrconfig_vrf = self.getFRRconfig(f'router bgp {ASN} vrf {import_vrf}') +        self.assertIn(f'router bgp {ASN}', frrconfig) +        self.assertIn(f'address-family ipv4 unicast', frrconfig) +        self.assertIn(f'  import vrf {import_vrf}', frrconfig) +        self.assertIn(f'router bgp {ASN} vrf {import_vrf}', frrconfig_vrf) +        self.cli_delete(import_vrf_base + [import_vrf]) +        with self.assertRaises(ConfigSessionError): +            self.cli_commit() + +    def test_bgp_19_deleting_default_vrf(self): +        # Verify deleting existent vrf default if other vrfs were created +        self.create_bgp_instances_for_import_test() +        self.cli_commit() +        frrconfig = self.getFRRconfig(f'router bgp {ASN}') +        frrconfig_vrf = self.getFRRconfig(f'router bgp {ASN} vrf {import_vrf}') +        self.assertIn(f'router bgp {ASN}', frrconfig) +        self.assertIn(f'router bgp {ASN} vrf {import_vrf}', frrconfig_vrf) +        self.cli_delete(base_path) +        with self.assertRaises(ConfigSessionError): +            self.cli_commit() + +    def test_bgp_20_import_rd_rt_compatibility(self): +        # Verify if vrf that has rd vpn export is in import vrf of other vrfs +        self.create_bgp_instances_for_import_test() +        self.cli_set( +            import_vrf_base + [import_vrf] + base_path + ['address-family', +                                                          import_afi, 'rd', +                                                          'vpn', 'export', +                                                          import_rd]) +        self.cli_commit() +        frrconfig = self.getFRRconfig(f'router bgp {ASN}') +        frrconfig_vrf = self.getFRRconfig(f'router bgp {ASN} vrf {import_vrf}') +        self.assertIn(f'router bgp {ASN}', frrconfig) +        self.assertIn(f'router bgp {ASN} vrf {import_vrf}', frrconfig_vrf) +        self.assertIn(f'address-family ipv4 unicast', frrconfig_vrf) +        self.assertIn(f'  rd vpn export {import_rd}', frrconfig_vrf) + +        self.cli_set( +            base_path + ['address-family', import_afi, 'import', 'vrf', +                         import_vrf]) +        with self.assertRaises(ConfigSessionError): +            self.cli_commit() + +    def test_bgp_21_import_unspecified_vrf(self): +        # Verify if vrf that is in import is unspecified +        self.create_bgp_instances_for_import_test() +        self.cli_set( +            base_path + ['address-family', import_afi, 'import', 'vrf', +                         'test']) +        with self.assertRaises(ConfigSessionError): +            self.cli_commit()  if __name__ == '__main__':      unittest.main(verbosity=2) diff --git a/src/conf_mode/protocols_bgp.py b/src/conf_mode/protocols_bgp.py index ff568d470..c410258ee 100755 --- a/src/conf_mode/protocols_bgp.py +++ b/src/conf_mode/protocols_bgp.py @@ -14,8 +14,6 @@  # You should have received a copy of the GNU General Public License  # along with this program.  If not, see <http://www.gnu.org/licenses/>. -import os -  from sys import exit  from sys import argv @@ -57,13 +55,18 @@ def get_config(config=None):      # instead of the VRF instance.      if vrf: bgp.update({'vrf' : vrf}) +    bgp['dependent_vrfs'] = conf.get_config_dict(['vrf', 'name'], +                                                 key_mangling=('-', '_'), +                                                 get_first_key=True, +                                                 no_tag_node_value_mangle=True) + +    bgp['dependent_vrfs'].update({'default': {'protocols': { +        'bgp': conf.get_config_dict(base_path, key_mangling=('-', '_'), +                                    get_first_key=True, +                                    no_tag_node_value_mangle=True)}}})      if not conf.exists(base): +        # If bgp instance is deleted then mark it          bgp.update({'deleted' : ''}) -        if not vrf: -            # We are running in the default VRF context, thus we can not delete -            # our main BGP instance if there are dependent BGP VRF instances. -            bgp['dependent_vrfs'] = conf.get_config_dict(['vrf', 'name'], -                key_mangling=('-', '_'), get_first_key=True, no_tag_node_value_mangle=True)          return bgp      # We also need some additional information from the config, prefix-lists @@ -74,9 +77,91 @@ def get_config(config=None):      tmp = conf.get_config_dict(['policy'])      # Merge policy dict into "regular" config dict      bgp = dict_merge(tmp, bgp) -      return bgp + +def verify_vrf_as_import(search_vrf_name: str, afi_name: str, vrfs_config: dict) -> bool: +    """ +    :param search_vrf_name: search vrf name in import list +    :type search_vrf_name: str +    :param afi_name: afi/safi name +    :type afi_name: str +    :param vrfs_config: configuration dependents vrfs +    :type vrfs_config: dict +    :return: if vrf in import list retrun true else false +    :rtype: bool +    """ +    for vrf_name, vrf_config in vrfs_config.items(): +        import_list = dict_search( +            f'protocols.bgp.address_family.{afi_name}.import.vrf', +            vrf_config) +        if import_list: +            if search_vrf_name in import_list: +               return True +    return False + +def verify_vrf_import_options(afi_config: dict) -> bool: +    """ +    Search if afi contains one of options +    :param afi_config: afi/safi +    :type afi_config: dict +    :return: if vrf contains rd and route-target options return true else false +    :rtype: bool +    """ +    options = [ +        f'rd.vpn.export', +        f'route_target.vpn.import', +        f'route_target.vpn.export', +        f'route_target.vpn.both' +    ] +    for option in options: +        if dict_search(option, afi_config): +            return True +    return False + +def verify_vrf_import(vrf_name: str, vrfs_config: dict, afi_name: str) -> bool: +    """ +    Verify if vrf exists and contain options +    :param vrf_name: name of VRF +    :type vrf_name: str +    :param vrfs_config: dependent vrfs config +    :type vrfs_config: dict +    :param afi_name: afi/safi name +    :type afi_name: str +    :return: if vrf contains rd and route-target options return true else false +    :rtype: bool +    """ +    if vrf_name != 'default': +        verify_vrf({'vrf': vrf_name}) +    if dict_search(f'{vrf_name}.protocols.bgp.address_family.{afi_name}', +                   vrfs_config): +        afi_config = \ +        vrfs_config[vrf_name]['protocols']['bgp']['address_family'][ +            afi_name] +        if verify_vrf_import_options(afi_config): +            return True +    return False + +def verify_vrflist_import(afi_name: str, afi_config: dict, vrfs_config: dict) -> bool: +    """ +    Call function to verify +    if scpecific vrf contains rd and route-target +    options return true else false + +    :param afi_name: afi/safi name +    :type afi_name: str +    :param afi_config: afi/safi configuration +    :type afi_config: dict +    :param vrfs_config: dependent vrfs config +    :type vrfs_config:dict +    :return: if vrf contains rd and route-target options return true else false +    :rtype: bool +    """ +    for vrf_name in afi_config['import']['vrf']: +        if verify_vrf_import(vrf_name, vrfs_config, afi_name): +            return True +    return False +  def verify_remote_as(peer_config, bgp_config):      if 'remote_as' in peer_config:          return peer_config['remote_as'] @@ -113,12 +198,22 @@ def verify_afi(peer_config, bgp_config):      return False  def verify(bgp): -    if not bgp or 'deleted' in bgp: -        if 'dependent_vrfs' in bgp: -            for vrf, vrf_options in bgp['dependent_vrfs'].items(): -                if dict_search('protocols.bgp', vrf_options) != None: -                    raise ConfigError('Cannot delete default BGP instance, ' \ -                                      'dependent VRF instance(s) exist!') +    if 'deleted' in bgp: +        if 'vrf' in bgp: +            # Cannot delete vrf if it exists in import vrf list in other vrfs +            for tmp_afi in ['ipv4_unicast', 'ipv6_unicast']: +                if verify_vrf_as_import(bgp['vrf'],tmp_afi,bgp['dependent_vrfs']): +                    raise ConfigError(f'Cannot delete vrf {bgp["vrf"]} instance, ' \ +                                      'Please unconfigure import vrf commands!') +        else: +            # We are running in the default VRF context, thus we can not delete +            # our main BGP instance if there are dependent BGP VRF instances. +            if 'dependent_vrfs' in bgp: +                for vrf, vrf_options in bgp['dependent_vrfs'].items(): +                    if vrf != 'default': +                        if dict_search('protocols.bgp', vrf_options): +                            raise ConfigError('Cannot delete default BGP instance, ' \ +                                              'dependent VRF instance(s) exist!')          return None      if 'system_as' not in bgp: @@ -324,9 +419,43 @@ def verify(bgp):                                           f'{afi} administrative distance {key}!')              if afi in ['ipv4_unicast', 'ipv6_unicast']: -                if 'import' in afi_config and 'vrf' in afi_config['import']: -                    # Check if VRF exists -                    verify_vrf(afi_config['import']['vrf']) + +                vrf_name = bgp['vrf'] if dict_search('vrf', bgp) else 'default' +                # Verify if currant VRF contains rd and route-target options +                # and does not exist in import list in other VRFs +                if dict_search(f'rd.vpn.export', afi_config): +                    if verify_vrf_as_import(vrf_name, afi, bgp['dependent_vrfs']): +                        raise ConfigError( +                            'Command "import vrf" conflicts with "rd vpn export" command!') + +                if dict_search('route_target.vpn.both', afi_config): +                    if verify_vrf_as_import(vrf_name, afi, bgp['dependent_vrfs']): +                        raise ConfigError( +                            'Command "import vrf" conflicts with "route-target vpn both" command!') + +                if dict_search('route_target.vpn.import', afi_config): +                    if verify_vrf_as_import(vrf_name, afi, bgp['dependent_vrfs']): +                        raise ConfigError( +                            'Command "import vrf conflicts" with "route-target vpn import" command!') + +                if dict_search('route_target.vpn.export', afi_config): +                    if verify_vrf_as_import(vrf_name, afi, bgp['dependent_vrfs']): +                        raise ConfigError( +                            'Command "import vrf" conflicts with "route-target vpn export" command!') + +                # Verify if VRFs in import do not contain rd +                # and route-target options +                if dict_search('import.vrf', afi_config) is not None: +                    # Verify if VRF with import does not contain rd +                    # and route-target options +                    if verify_vrf_import_options(afi_config): +                        raise ConfigError( +                            'Please unconfigure "import vrf" commands before using vpn commands in the same VRF!') +                    # Verify if VRFs in import list do not contain rd +                    # and route-target options +                    if verify_vrflist_import(afi, afi_config, bgp['dependent_vrfs']): +                        raise ConfigError( +                            'Please unconfigure import vrf commands before using vpn commands in dependent VRFs!')                      # FRR error: please unconfigure vpn to vrf commands before                      # using import vrf commands @@ -339,7 +468,6 @@ def verify(bgp):                      tmp = dict_search(f'route_map.vpn.{export_import}', afi_config)                      if tmp: verify_route_map(tmp, bgp) -      return None  def generate(bgp): | 
