bgp: T3311: remove remote-as from address-family

When moving from Quagga to FRR the BGP address-family was extended by an
invalid peer-group statement. FRR always moved a configured peer-group
from the AFI level down to the neighbor level.

With the migration to FRR reload we must take care about this by ourselves.

2 files changed, 72 insertions, 25 deletions

diff --git a/src/conf_mode/protocols_bgp.py b/src/conf_mode/protocols_bgp.py
index 54352460c..b5bb018ae 100755
--- a/src/conf_mode/protocols_bgp.py
+++ b/src/conf_mode/protocols_bgp.py
@@ -54,6 +54,26 @@ def get_config(config=None):
 
     return bgp
 
+def verify_remote_as(peer_config, asn_config):
+    if 'remote_as' in peer_config:
+        return peer_config['remote_as']
+
+    if 'peer_group' in peer_config:
+        peer_group_name = peer_config['peer_group']
+        tmp = dict_search(f'peer_group.{peer_group_name}.remote_as', asn_config)
+        if tmp: return tmp
+
+    if 'interface' in peer_config:
+        if 'remote_as' in peer_config['interface']:
+            return peer_config['interface']['remote_as']
+
+        if 'peer_group' in peer_config['interface']:
+            peer_group_name = peer_config['interface']['peer_group']
+            tmp = dict_search(f'peer_group.{peer_group_name}.remote_as', asn_config)
+            if tmp: return tmp
+
+    return None
+
 def verify(bgp):
     if not bgp:
         return None
@@ -79,18 +99,13 @@ def verify(bgp):
                         raise ConfigError(f'Specified peer-group "{peer_group}" for '\
                                           f'neighbor "{neighbor}" does not exist!')
 
-                # Some checks can/must only be done on a neighbor and nor a peer-group
+
+                # Some checks can/must only be done on a neighbor and not a peer-group
                 if neighbor == 'neighbor':
                     # remote-as must be either set explicitly for the neighbor
                     # or for the entire peer-group
-                    if 'interface' in peer_config:
-                        if 'remote_as' not in peer_config['interface']:
-                            if 'peer_group' not in peer_config['interface'] or 'remote_as' not in asn_config['peer_group'][ peer_config['interface']['peer_group'] ]:
-                                raise ConfigError('Remote AS must be set for neighbor or peer-group!')
-
-                    elif 'remote_as' not in peer_config:
-                        if 'peer_group' not in peer_config or 'remote_as' not in asn_config['peer_group'][ peer_config['peer_group'] ]:
-                            raise ConfigError('Remote AS must be set for neighbor or peer-group!')
+                    if not verify_remote_as(peer_config, asn_config):
+                        raise ConfigError(f'Neighbor "{peer}" remote-as must be set!')
 
                 for afi in ['ipv4_unicast', 'ipv6_unicast', 'l2vpn_evpn']:
                     # Bail out early if address family is not configured
diff --git a/src/migration-scripts/quagga/6-to-7 b/src/migration-scripts/quagga/6-to-7
index f7aca0d2b..25cf5eebd 100755
--- a/src/migration-scripts/quagga/6-to-7
+++ b/src/migration-scripts/quagga/6-to-7
@@ -17,14 +17,17 @@
 # - T3037, BGP address-family ipv6-unicast capability dynamic does not exist in
 #   FRR, there is only a base, per neighbor dynamic capability, migrate config
 
-import sys
+from sys import argv
+from sys import exit
 from vyos.configtree import ConfigTree
+from vyos.template import is_ipv4
+from vyos.template import is_ipv6
 
-if (len(sys.argv) < 2):
+if (len(argv) < 2):
     print("Must specify file name!")
-    sys.exit(1)
+    exit(1)
 
-file_name = sys.argv[1]
+file_name = argv[1]
 
 with open(file_name, 'r') as f:
     config_file = f.read()
@@ -34,7 +37,7 @@ config = ConfigTree(config_file)
 
 if not config.exists(base):
     # Nothing to do
-    sys.exit(0)
+    exit(0)
 
 # Check if BGP is actually configured and obtain the ASN
 asn_list = config.list_nodes(base)
@@ -55,30 +58,59 @@ if asn_list:
                 config.delete(send_comm_path)
 
             cap_dynamic = False
+            peer_group = None
             for afi in ['ipv4-unicast', 'ipv6-unicast']:
-                afi_path = bgp_base + [neighbor_type, neighbor, 'address-family', afi, 'capability', 'dynamic']
-                if config.exists(afi_path):
+                afi_path = bgp_base + [neighbor_type, neighbor, 'address-family', afi]
+                # Exit loop early if AFI does not exist
+                if not config.exists(afi_path):
+                    continue
+
+                cap_path = afi_path + ['capability', 'dynamic']
+                if config.exists(cap_path):
                     cap_dynamic = True
-                    config.delete(afi_path)
+                    config.delete(cap_path)
+
+                    # We have now successfully migrated the address-family
+                    # specific dynamic capability to the neighbor/peer-group
+                    # level. If this has been the only option under the
+                    # address-family nodes, we can clean them up by checking if
+                    # no other nodes are left under that tree and if so, delete
+                    # the parent.
+                    #
+                    # We walk from the most inner node to the most outer one.
+                    cleanup = -1
+                    while len(config.list_nodes(cap_path[:cleanup])) == 0:
+                        config.delete(cap_path[:cleanup])
+                        cleanup -= 1
+
+                peer_group_path = afi_path + ['peer-group']
+                if config.exists(peer_group_path):
+                    if ((is_ipv4(neighbor) and afi == 'ipv4-unicast') or
+                        (is_ipv6(neighbor) and afi == 'ipv6-unicast')):
+                        peer_group = config.return_value(peer_group_path)
+
+                    config.delete(peer_group_path)
 
-                    # We have now successfully migrated the address-family specific
-                    # dynamic capability to the neighbor/peer-group level. If this
-                    # has been the only option under the address-family nodes, we
-                    # can clean them up by checking if no other nodes are left under
-                    # that tree and if so, delete the parent.
+                    # We have now successfully migrated the address-family
+                    # specific peer-group to the neighbor level. If this has
+                    # been the only option under the address-family nodes, we
+                    # can clean them up by checking if no other nodes are left
+                    # under that tree and if so, delete the parent.
                     #
                     # We walk from the most inner node to the most outer one.
                     cleanup = -1
-                    while len(config.list_nodes(afi_path[:cleanup])) == 0:
-                        config.delete(afi_path[:cleanup])
+                    while len(config.list_nodes(peer_group_path[:cleanup])) == 0:
+                        config.delete(peer_group_path[:cleanup])
                         cleanup -= 1
 
             if cap_dynamic:
                 config.set(bgp_base + [neighbor_type, neighbor, 'capability', 'dynamic'])
+            if peer_group:
+                config.set(bgp_base + [neighbor_type, neighbor, 'peer-group'], value=peer_group)
 
 try:
     with open(file_name, 'w') as f:
         f.write(config.to_string())
 except OSError as e:
     print("Failed to save the modified config: {}".format(e))
-    sys.exit(1)
+    exit(1)