Implement no BGP config behavior for Junos (get_bgp_config) (#1890)

napalm-automation · Apr 3, 2023 · 483efae · 483efae
1 parent 322f208
commit 483efae
Show file tree

Hide file tree

Showing 8 changed files with 72 additions and 64 deletions.
diff --git a/napalm/base/test/getters.py b/napalm/base/test/getters.py
@@ -244,7 +244,7 @@ def test_get_lldp_neighbors_detail(self, test_case):
     def test_get_bgp_config(self, test_case):
         """Test get_bgp_config."""
         get_bgp_config = self.device.get_bgp_config()
-        assert len(get_bgp_config) > 0
+        assert get_bgp_config == {} or len(get_bgp_config) > 0
 
         for bgp_group in get_bgp_config.values():
             assert helpers.test_model(models.BPGConfigGroupDict, bgp_group)

diff --git a/napalm/junos/junos.py b/napalm/junos/junos.py
@@ -1256,30 +1256,34 @@ def build_prefix_limit(**args):
         bgp_asn_obj = routing_options.xml.find(
             "./routing-options/autonomous-system/as-number"
         )
-        bgp_asn = int(bgp_asn_obj.text) if bgp_asn_obj is not None else 0
+        system_bgp_asn = int(bgp_asn_obj.text) if bgp_asn_obj is not None else 0
 
-        bgp_config["_"] = {
-            "apply_groups": [],
-            "description": "",
-            "local_as": bgp_asn,
-            "type": "",
-            "import_policy": "",
-            "export_policy": "",
-            "local_address": "",
-            "multipath": False,
-            "multihop_ttl": 0,
-            "remote_as": 0,
-            "remove_private_as": False,
-            "prefix_limit": {},
-            "neighbors": {},
-        }
-        if group:
+        # No BGP peer-group i.e. "_" key is a special case.
+        if group and group != "_":
             bgp = junos_views.junos_bgp_config_group_table(self.device)
             bgp.get(group=group, options=self.junos_config_options)
         else:
             bgp = junos_views.junos_bgp_config_table(self.device)
             bgp.get(options=self.junos_config_options)
             neighbor = ""  # if no group is set, no neighbor should be set either
+
+            # Only set no peer-group if BGP is actually configured.
+            if bgp.items() or system_bgp_asn:
+                bgp_config["_"] = {
+                    "apply_groups": [],
+                    "description": "",
+                    "local_as": system_bgp_asn,
+                    "type": "",
+                    "import_policy": "",
+                    "export_policy": "",
+                    "local_address": "",
+                    "multipath": False,
+                    "multihop_ttl": 0,
+                    "remote_as": 0,
+                    "remove_private_as": False,
+                    "prefix_limit": {},
+                    "neighbors": {},
+                }
         bgp_items = bgp.items()
 
         if neighbor:
@@ -1306,36 +1310,44 @@ def build_prefix_limit(**args):
                 for field, datatype in _GROUP_FIELDS_DATATYPE_MAP_.items()
                 if "_prefix_limit" not in field
             }
-            for elem in bgp_group_details:
-                if not ("_prefix_limit" not in elem[0] and elem[1] is not None):
+
+            # Always overwrite with the system local_as (this will either be
+            # valid or will be zero i.e. the same as the default value).
+            bgp_config[bgp_group_name]["local_as"] = system_bgp_asn
+
+            for key, value in bgp_group_details:
+                if "_prefix_limit" in key or value is None:
                     continue
-                datatype = _GROUP_FIELDS_DATATYPE_MAP_.get(elem[0])
+                datatype = _GROUP_FIELDS_DATATYPE_MAP_.get(key)
                 default = _DATATYPE_DEFAULT_.get(datatype)
-                key = elem[0]
-                value = elem[1]
+
                 if key in ["export_policy", "import_policy"]:
                     if isinstance(value, list):
                         value = " ".join(value)
                 if key == "local_address":
                     value = napalm.base.helpers.convert(
                         napalm.base.helpers.ip, value, value
                     )
+                if key == "apply_groups":
+                    # Ensure apply_groups value is wrapped in a list
+                    if isinstance(value, str):
+                        value = [value]
                 if key == "neighbors":
                     bgp_group_peers = value
                     continue
                 bgp_config[bgp_group_name].update(
                     {key: napalm.base.helpers.convert(datatype, value, default)}
                 )
             prefix_limit_fields = {}
-            for elem in bgp_group_details:
-                if "_prefix_limit" in elem[0] and elem[1] is not None:
-                    datatype = _GROUP_FIELDS_DATATYPE_MAP_.get(elem[0])
+            for key, value in bgp_group_details:
+                if "_prefix_limit" in key and value is not None:
+                    datatype = _GROUP_FIELDS_DATATYPE_MAP_.get(key)
                     default = _DATATYPE_DEFAULT_.get(datatype)
                     prefix_limit_fields.update(
                         {
-                            elem[0].replace(
+                            key.replace(
                                 "_prefix_limit", ""
-                            ): napalm.base.helpers.convert(datatype, elem[1], default)
+                            ): napalm.base.helpers.convert(datatype, value, default)
                         }
                     )
             bgp_config[bgp_group_name]["prefix_limit"] = build_prefix_limit(
@@ -1349,23 +1361,31 @@ def build_prefix_limit(**args):
                     bgp_config[bgp_group_name]["multihop_ttl"] = 64
 
             bgp_config[bgp_group_name]["neighbors"] = {}
+            bgp_group_remote_as = bgp_config[bgp_group_name]["remote_as"]
             for bgp_group_neighbor in bgp_group_peers.items():
                 bgp_peer_address = napalm.base.helpers.ip(bgp_group_neighbor[0])
                 if neighbor and bgp_peer_address != neighbor:
                     continue  # if filters applied, jump over all other neighbors
                 bgp_group_details = bgp_group_neighbor[1]
+
+                # Set defaults for this BGP peer
                 bgp_peer_details = {
                     field: _DATATYPE_DEFAULT_.get(datatype)
                     for field, datatype in _PEER_FIELDS_DATATYPE_MAP_.items()
                     if "_prefix_limit" not in field
                 }
-                for elem in bgp_group_details:
-                    if not ("_prefix_limit" not in elem[0] and elem[1] is not None):
+
+                # Always overwrite with the system local_as (this will either be
+                # valid or will be zero i.e. the same as the default value).
+                bgp_peer_details["local_as"] = system_bgp_asn
+                # Always set the default remote-as as the Peer-Group remote-as
+                bgp_peer_details["remote_as"] = bgp_group_remote_as
+
+                for key, value in bgp_group_details:
+                    if "_prefix_limit" in key or value is None:
                         continue
-                    datatype = _PEER_FIELDS_DATATYPE_MAP_.get(elem[0])
+                    datatype = _PEER_FIELDS_DATATYPE_MAP_.get(key)
                     default = _DATATYPE_DEFAULT_.get(datatype)
-                    key = elem[0]
-                    value = elem[1]
                     if key in ["export_policy"]:
                         # next-hop self is applied on export IBGP sessions
                         bgp_peer_details["nhs"] = _check_nhs(value, nhs_policies)
@@ -1393,17 +1413,15 @@ def build_prefix_limit(**args):
                 if "cluster" in bgp_config[bgp_group_name].keys():
                     bgp_peer_details["route_reflector_client"] = True
                 prefix_limit_fields = {}
-                for elem in bgp_group_details:
-                    if "_prefix_limit" in elem[0] and elem[1] is not None:
-                        datatype = _PEER_FIELDS_DATATYPE_MAP_.get(elem[0])
+                for key, value in bgp_group_details:
+                    if "_prefix_limit" in key and value is not None:
+                        datatype = _PEER_FIELDS_DATATYPE_MAP_.get(key)
                         default = _DATATYPE_DEFAULT_.get(datatype)
                         prefix_limit_fields.update(
                             {
-                                elem[0].replace(
+                                key.replace(
                                     "_prefix_limit", ""
-                                ): napalm.base.helpers.convert(
-                                    datatype, elem[1], default
-                                )
+                                ): napalm.base.helpers.convert(datatype, value, default)
                             }
                         )
                 bgp_peer_details["prefix_limit"] = build_prefix_limit(

diff --git a/test/junos/mocked_data/test_get_bgp_config/nhs/expected_result.json b/test/junos/mocked_data/test_get_bgp_config/nhs/expected_result.json
@@ -20,7 +20,7 @@
     "export_policy": "",
     "import_policy": "",
     "local_address": "",
-    "local_as": 0,
+    "local_as": 65001,
     "multihop_ttl": 0,
     "multipath": false,
     "neighbors": {
@@ -30,7 +30,7 @@
         "export_policy": "nhs",
         "import_policy": "",
         "local_address": "",
-        "local_as": 0,
+        "local_as": 65001,
         "nhs": true,
         "prefix_limit": {},
         "remote_as": 0,
@@ -48,7 +48,7 @@
     "export_policy": "",
     "import_policy": "",
     "local_address": "",
-    "local_as": 0,
+    "local_as": 65001,
     "multihop_ttl": 0,
     "multipath": false,
     "neighbors": {
@@ -58,7 +58,7 @@
         "export_policy": "static",
         "import_policy": "",
         "local_address": "",
-        "local_as": 0,
+        "local_as": 65001,
         "nhs": false,
         "prefix_limit": {},
         "remote_as": 0,

diff --git a/...p/_configuration__policy_options__policy_statement____policy_options___configuration_.xml b/...p/_configuration__policy_options__policy_statement____policy_options___configuration_.xml
@@ -0,0 +1,2 @@
+<configuration>
+</configuration>
diff --git a/...nfig/no_bgp/_configuration__protocols__bgp__group____bgp___protocols___configuration_.xml b/...nfig/no_bgp/_configuration__protocols__bgp__group____bgp___protocols___configuration_.xml
@@ -0,0 +1,2 @@
+<configuration>
+</configuration>
diff --git a/...configuration__routing_options__autonomous_system____routing_options___configuration_.xml b/...configuration__routing_options__autonomous_system____routing_options___configuration_.xml
@@ -0,0 +1,2 @@
+<configuration>
+</configuration>
diff --git a/test/junos/mocked_data/test_get_bgp_config/no_bgp/expected_result.json b/test/junos/mocked_data/test_get_bgp_config/no_bgp/expected_result.json
@@ -0,0 +1 @@
+{}
diff --git a/test/junos/mocked_data/test_get_bgp_config/normal/expected_result.json b/test/junos/mocked_data/test_get_bgp_config/normal/expected_result.json
@@ -27,7 +27,7 @@
         },
         "route_reflector_client": false,
         "description": "Facebook [CDN]",
-        "local_as": 0,
+        "local_as": 13335,
         "nhs": false,
         "local_address": "",
         "remote_as": 32934,
@@ -45,7 +45,7 @@
         },
         "route_reflector_client": false,
         "description": "Twitter [CDN]",
-        "local_as": 0,
+        "local_as": 13335,
         "nhs": false,
         "local_address": "",
         "remote_as": 13414,
@@ -59,24 +59,7 @@
     "description": "",
     "local_as": 13335,
     "multihop_ttl": 0,
-    "apply_groups": [
-      "B",
-      "G",
-      "P",
-      "-",
-      "P",
-      "R",
-      "E",
-      "F",
-      "I",
-      "X",
-      "-",
-      "L",
-      "I",
-      "M",
-      "I",
-      "T"
-    ],
+    "apply_groups": ["BGP-PREFIX-LIMIT"],
     "remote_as": 0,
     "remove_private_as": true,
     "local_address": "",