bgp: T2174: refactor Jinja template and reduce redundant paths

The Jinja2 template contained a lot of redundant paths which only differed in
either the address-family or neighbor vs. peer-group. This paths have been
combined into for loops and a macro for generating a neighbor statement as
peer-groups and regular neighbors share ~95% of the config.

1 files changed, 44 insertions, 22 deletions

diff --git a/src/conf_mode/protocols_bgp.py b/src/conf_mode/protocols_bgp.py
index 654874232..981ff9fe9 100755
--- a/src/conf_mode/protocols_bgp.py
+++ b/src/conf_mode/protocols_bgp.py
@@ -14,16 +14,16 @@
 # 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 vyos.config import Config
 from vyos.util import call
+from vyos.util import dict_search
 from vyos.template import render
 from vyos.template import render_to_string
+from vyos import ConfigError
 from vyos import frr
-from vyos import ConfigError, airbag
+from vyos import airbag
 airbag.enable()
 
 config_file = r'/tmp/bgp.frr'
@@ -31,8 +31,10 @@ config_file = r'/tmp/bgp.frr'
 def get_config():
     conf = Config()
     base = ['protocols', 'nbgp']
-    bgp = conf.get_config_dict(base, key_mangling=('-', '_'))
+    bgp = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True)
 
+    # XXX: any reason we can not move this into the FRR template?
+    # we shall not call vtysh directly, especially not in get_config()
     if not conf.exists(base):
         bgp = {}
         call('vtysh -c \"conf t\" -c \"no ip protocol bgp\" ')
@@ -40,9 +42,6 @@ def get_config():
     if not conf.exists(base + ['route-map']):
         call('vtysh -c \"conf t\" -c \"no ip protocol bgp\" ')
 
-    from pprint import pprint
-    pprint(bgp)
-
     return bgp
 
 def verify(bgp):
@@ -50,9 +49,23 @@ def verify(bgp):
         return None
 
     # Check if declared more than one ASN
-    for asn in bgp['nbgp'].items():
-        if len(bgp['nbgp']) > 1:
-            raise ConfigError('Only one bgp ASN process can be definded')
+    if len(bgp) > 1:
+        raise ConfigError('Only one BGP AS can be defined!')
+
+    for asn, asn_config in bgp.items():
+        # Common verification for both peer-group and neighbor statements
+        for neigh in ['neighbor', 'peer_group']:
+            # bail out early if there is no neighbor or peer-group statement
+            # this also saves one indention level
+            if neigh not in asn_config:
+                continue
+
+            for neighbor, config in asn_config[neigh].items():
+                if 'remote_as' not in config and 'peer_group' not in config:
+                    raise ConfigError(f'BGP remote-as must be specified for "{neighbor}"!')
+
+                if 'remote_as' in config and 'peer_group' in config:
+                    raise ConfigError(f'BGP peer-group member "{neighbor}" cannot override remote-as of peer-group!')
 
     return None
 
@@ -61,33 +74,42 @@ def generate(bgp):
         bgp['new_frr_config'] = ''
         return None
 
+    # only one BGP AS is supported, so we can directly send the first key
+    # of the config dict
+    asn = list(bgp.keys())[0]
+    bgp[asn]['asn'] = asn
+
     # render(config) not needed, its only for debug
-    render(config_file, 'frr/bgp.frr.tmpl', bgp)
+    render(config_file, 'frr/bgp.frr.tmpl', bgp[asn], trim_blocks=True)
 
-    bgp['new_frr_config'] = render_to_string('frr/bgp.frr.tmpl', bgp)
+    bgp['new_frr_config'] = render_to_string('frr/bgp.frr.tmpl', bgp[asn],
+                                             trim_blocks=True)
 
     return None
 
 def apply(bgp):
-    # Save original configration prior to starting any commit actions
-    bgp['original_config'] = frr.get_configuration(daemon='bgpd')
-    bgp['modified_config'] = frr.replace_section(bgp['original_config'], bgp['new_frr_config'], from_re='router bgp .*')
+    # Save original configuration prior to starting any commit actions
+    frr_cfg = {}
+    frr_cfg['original_config'] = frr.get_configuration(daemon='bgpd')
+    frr_cfg['modified_config'] = frr.replace_section(frr_cfg['original_config'], bgp['new_frr_config'], from_re='router bgp .*')
 
     # Debugging
+    print('')
     print('--------- DEBUGGING ----------')
-    print(f'Existing config:\n{bgp["original_config"]}\n\n')
+    print(f'Existing config:\n{frr_cfg["original_config"]}\n\n')
     print(f'Replacement config:\n{bgp["new_frr_config"]}\n\n')
-    print(f'Modified config:\n{bgp["modified_config"]}\n\n')
+    print(f'Modified config:\n{frr_cfg["modified_config"]}\n\n')
 
-    # Frr Mark configuration will test for syntax errors and exception out if any syntax errors are detected
-    frr.mark_configuration(bgp['modified_config'])
+    # FRR mark configuration will test for syntax errors and throws an
+    # exception if any syntax errors is detected
+    frr.mark_configuration(frr_cfg['modified_config'])
 
-    # Commit the resulting new configuration to frr, this will render an frr.CommitError() Exception on fail
-    frr.reload_configuration(bgp['modified_config'], daemon='bgpd')
+    # Commit resulting configuration to FRR, this will throw CommitError
+    # on failure
+    frr.reload_configuration(frr_cfg['modified_config'], daemon='bgpd')
 
     return None
 
-
 if __name__ == '__main__':
     try:
         c = get_config()