Merge pull request #12791 from taspelund/loc_rib_json_fix

bgpd: fix 'json detail' output structure

7 files changed, 158 insertions, 119 deletions

diff --git a/bgpd/bgp_evpn_vty.c b/bgpd/bgp_evpn_vty.c
index ceff7a46c4..33c202c3d7 100644
--- a/bgpd/bgp_evpn_vty.c
+++ b/bgpd/bgp_evpn_vty.c
@@ -2472,7 +2472,7 @@ static void evpn_show_route_vni_multicast(struct vty *vty, struct bgp *bgp,
 
 	/* Prefix and num paths displayed once per prefix. */
 	route_vty_out_detail_header(vty, bgp, dest, bgp_dest_get_prefix(dest),
-				    NULL, afi, safi, json);
+				    NULL, afi, safi, json, false);
 
 	/* Display each path for this prefix. */
 	for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) {
@@ -2574,7 +2574,7 @@ static void evpn_show_route_vni_macip(struct vty *vty, struct bgp *bgp,
 
 	/* Prefix and num paths displayed once per prefix. */
 	route_vty_out_detail_header(vty, bgp, dest, (struct prefix *)&p, NULL,
-				    afi, safi, json);
+				    afi, safi, json, false);
 
 	evp = (const struct prefix_evpn *)bgp_dest_get_prefix(dest);
 
@@ -2709,7 +2709,7 @@ static void evpn_show_route_rd_macip(struct vty *vty, struct bgp *bgp,
 
 	/* Prefix and num paths displayed once per prefix. */
 	route_vty_out_detail_header(vty, bgp, dest, bgp_dest_get_prefix(dest),
-				    prd, afi, safi, json);
+				    prd, afi, safi, json, false);
 
 	if (json)
 		json_paths = json_object_new_array();
@@ -2817,7 +2817,7 @@ static void evpn_show_route_rd(struct vty *vty, struct bgp *bgp,
 			/* Prefix and num paths displayed once per prefix. */
 			route_vty_out_detail_header(
 				vty, bgp, dest, bgp_dest_get_prefix(dest), prd,
-				afi, safi, json_prefix);
+				afi, safi, json_prefix, false);
 
 			prefix_cnt++;
 		}
@@ -2952,7 +2952,7 @@ static void evpn_show_route_rd_all_macip(struct vty *vty, struct bgp *bgp,
 			/* Prefix and num paths displayed once per prefix. */
 			route_vty_out_detail_header(
 				vty, bgp, dest, p, (struct prefix_rd *)rd_destp,
-				AFI_L2VPN, SAFI_EVPN, json_prefix);
+				AFI_L2VPN, SAFI_EVPN, json_prefix, false);
 
 		/* For EVPN, the prefix is displayed for each path (to
 		 * fit in with code that already exists).
@@ -3106,7 +3106,7 @@ static void evpn_show_all_routes(struct vty *vty, struct bgp *bgp, int type,
 					vty, bgp, dest,
 					bgp_dest_get_prefix(dest),
 					(struct prefix_rd *)rd_destp, AFI_L2VPN,
-					SAFI_EVPN, json_prefix);
+					SAFI_EVPN, json_prefix, false);
 
 			/* For EVPN, the prefix is displayed for each path (to
 			 * fit in
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 386c185cdb..0d93abc12f 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -11216,7 +11216,7 @@ static int bgp_show_table(struct vty *vty, struct bgp *bgp, safi_t safi,
 	for (dest = bgp_table_top(table); dest; dest = bgp_route_next(dest)) {
 		const struct prefix *dest_p = bgp_dest_get_prefix(dest);
 		enum rpki_states rpki_curr_state = RPKI_NOT_BEING_USED;
-		bool json_detail = json_detail_header;
+		bool json_detail_header_used = false;
 
 		pi = bgp_dest_get_bgp_path_info(dest);
 		if (pi == NULL)
@@ -11486,27 +11486,7 @@ static int bgp_show_table(struct vty *vty, struct bgp *bgp, safi_t safi,
 							   : BGP_SHOW_HEADER));
 				header = false;
 
-			} else if (json_detail && json_paths != NULL) {
-				const struct prefix_rd *prd;
-				json_object *jtemp;
-
-				/* Use common detail header, for most types;
-				 * need a json 'object'.
-				 */
-
-				jtemp = json_object_new_object();
-				prd = bgp_rd_from_dest(dest, safi);
-
-				route_vty_out_detail_header(
-					vty, bgp, dest,
-					bgp_dest_get_prefix(dest), prd,
-					table->afi, safi, jtemp);
-
-				json_object_array_add(json_paths, jtemp);
-
-				json_detail = false;
 			}
-
 			if (rd != NULL && !display && !output_count) {
 				if (!use_json)
 					vty_out(vty,
@@ -11537,7 +11517,7 @@ static int bgp_show_table(struct vty *vty, struct bgp *bgp, safi_t safi,
 							bgp_dest_get_prefix(
 								dest),
 							prd, table->afi, safi,
-							NULL);
+							NULL, false);
 
 					route_vty_out_detail(
 						vty, bgp, dest, dest_p, pi,
@@ -11583,6 +11563,23 @@ static int bgp_show_table(struct vty *vty, struct bgp *bgp, safi_t safi,
 				else
 					vty_out(vty, ",\"%pFX\": ", dest_p);
 			}
+
+			if (json_detail_header && json_paths != NULL) {
+				const struct prefix_rd *prd;
+
+				vty_out(vty, "{\n");
+
+				prd = bgp_rd_from_dest(dest, safi);
+
+				route_vty_out_detail_header(
+					vty, bgp, dest,
+					bgp_dest_get_prefix(dest), prd,
+					table->afi, safi, json_paths, true);
+
+				vty_out(vty, "\"paths\": ");
+				json_detail_header_used = true;
+			}
+
 			/*
 			 * We are using no_pretty here because under
 			 * extremely high settings( say lots and lots of
@@ -11593,6 +11590,10 @@ static int bgp_show_table(struct vty *vty, struct bgp *bgp, safi_t safi,
 			 * routers out there
 			 */
 			vty_json_no_pretty(vty, json_paths);
+
+			if (json_detail_header_used)
+				vty_out(vty, "} ");
+
 			json_paths = NULL;
 			first = 0;
 		} else
@@ -11777,7 +11778,8 @@ static void bgp_show_all_instances_routes_vty(struct vty *vty, afi_t afi,
 void route_vty_out_detail_header(struct vty *vty, struct bgp *bgp,
 				 struct bgp_dest *dest, const struct prefix *p,
 				 const struct prefix_rd *prd, afi_t afi,
-				 safi_t safi, json_object *json)
+				 safi_t safi, json_object *json,
+				 bool incremental_print)
 {
 	struct bgp_path_info *pi;
 	struct peer *peer;
@@ -11836,16 +11838,27 @@ void route_vty_out_detail_header(struct vty *vty, struct bgp *bgp,
 				dest->version);
 
 		} else {
-			json_object_string_addf(json, "prefix", "%pFX", p);
-			json_object_int_add(json, "version", dest->version);
-
+			if (incremental_print) {
+				vty_out(vty, "\"prefix\": \"%pFX\",\n", p);
+				vty_out(vty, "\"version\": \"%" PRIu64 "\",\n",
+					dest->version);
+			} else {
+				json_object_string_addf(json, "prefix", "%pFX",
+							p);
+				json_object_int_add(json, "version",
+						    dest->version);
+			}
 		}
 	}
 
 	if (has_valid_label) {
-		if (json)
-			json_object_int_add(json, "localLabel", label);
-		else
+		if (json) {
+			if (incremental_print)
+				vty_out(vty, "\"localLabel\": \"%u\",\n",
+					label);
+			else
+				json_object_int_add(json, "localLabel", label);
+		} else
 			vty_out(vty, "Local label: %d\n", label);
 	}
 
@@ -11969,13 +11982,16 @@ void route_vty_out_detail_header(struct vty *vty, struct bgp *bgp,
 			}
 		}
 
-		if (json) {
-			if (json_adv_to) {
+		if (json && json_adv_to) {
+			if (incremental_print) {
+				vty_out(vty, "\"advertisedTo\": ");
+				vty_json(vty, json_adv_to);
+				vty_out(vty, ",");
+			} else
 				json_object_object_add(json, "advertisedTo",
 						       json_adv_to);
-			}
 		} else {
-			if (first)
+			if (!json && first)
 				vty_out(vty, "  Not advertised to any peer");
 			vty_out(vty, "\n");
 		}
@@ -12018,7 +12034,7 @@ static void bgp_show_path_info(const struct prefix_rd *pfx_rd,
 			route_vty_out_detail_header(
 				vty, bgp, bgp_node,
 				bgp_dest_get_prefix(bgp_node), pfx_rd, AFI_IP,
-				safi, json_header);
+				safi, json_header, false);
 			header = 0;
 		}
 		(*display)++;
diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h
index 11ec722f19..d449152c21 100644
--- a/bgpd/bgp_route.h
+++ b/bgpd/bgp_route.h
@@ -842,7 +842,8 @@ extern void route_vty_out_detail_header(struct vty *vty, struct bgp *bgp,
 					struct bgp_dest *dest,
 					const struct prefix *p,
 					const struct prefix_rd *prd, afi_t afi,
-					safi_t safi, json_object *json);
+					safi_t safi, json_object *json,
+					bool incremental_print);
 extern void route_vty_out_detail(struct vty *vty, struct bgp *bgp,
 				 struct bgp_dest *bn, const struct prefix *p,
 				 struct bgp_path_info *path, afi_t afi,
diff --git a/tests/topotests/bgp_aigp/test_bgp_aigp.py b/tests/topotests/bgp_aigp/test_bgp_aigp.py
index b7ea40dd46..6fda459405 100644
--- a/tests/topotests/bgp_aigp/test_bgp_aigp.py
+++ b/tests/topotests/bgp_aigp/test_bgp_aigp.py
@@ -138,30 +138,34 @@ def test_bgp_aigp():
         )
         expected = {
             "routes": {
-                "10.0.0.71/32": [
-                    {
-                        "aigpMetric": 101,
-                        "valid": True,
-                    },
-                    {
-                        "aigpMetric": 91,
-                        "valid": True,
-                        "bestpath": {"selectionReason": "AIGP"},
-                        "nexthops": [{"hostname": "r3", "accessible": True}],
-                    },
-                ],
-                "10.0.0.72/32": [
-                    {
-                        "aigpMetric": 102,
-                        "valid": True,
-                    },
-                    {
-                        "aigpMetric": 92,
-                        "valid": True,
-                        "bestpath": {"selectionReason": "AIGP"},
-                        "nexthops": [{"hostname": "r3", "accessible": True}],
-                    },
-                ],
+                "10.0.0.71/32": {
+                    "paths": [
+                        {
+                            "aigpMetric": 101,
+                            "valid": True,
+                        },
+                        {
+                            "aigpMetric": 91,
+                            "valid": True,
+                            "bestpath": {"selectionReason": "AIGP"},
+                            "nexthops": [{"hostname": "r3", "accessible": True}],
+                        },
+                    ],
+                },
+                "10.0.0.72/32": {
+                    "paths": [
+                        {
+                            "aigpMetric": 102,
+                            "valid": True,
+                        },
+                        {
+                            "aigpMetric": 92,
+                            "valid": True,
+                            "bestpath": {"selectionReason": "AIGP"},
+                            "nexthops": [{"hostname": "r3", "accessible": True}],
+                        },
+                    ],
+                },
             }
         }
         return topotest.json_cmp(output, expected)
diff --git a/tests/topotests/bgp_community_alias/test_bgp-community-alias.py b/tests/topotests/bgp_community_alias/test_bgp-community-alias.py
index 5b7974c11a..000ea6075b 100644
--- a/tests/topotests/bgp_community_alias/test_bgp-community-alias.py
+++ b/tests/topotests/bgp_community_alias/test_bgp-community-alias.py
@@ -105,12 +105,16 @@ def test_bgp_community_alias():
         )
         expected = {
             "routes": {
-                "172.16.16.1/32": [
-                    {
-                        "community": {"string": "community-r2-1 65001:2"},
-                        "largeCommunity": {"string": "large-community-r2-1 65001:1:2"},
-                    }
-                ]
+                "172.16.16.1/32": {
+                    "paths": [
+                        {
+                            "community": {"string": "community-r2-1 65001:2"},
+                            "largeCommunity": {
+                                "string": "large-community-r2-1 65001:1:2"
+                            },
+                        }
+                    ]
+                }
             }
         }
         return topotest.json_cmp(output, expected)
diff --git a/tests/topotests/bgp_path_attribute_discard/test_bgp_path_attribute_discard.py b/tests/topotests/bgp_path_attribute_discard/test_bgp_path_attribute_discard.py
index 38fb4c25d0..ebaab60341 100644
--- a/tests/topotests/bgp_path_attribute_discard/test_bgp_path_attribute_discard.py
+++ b/tests/topotests/bgp_path_attribute_discard/test_bgp_path_attribute_discard.py
@@ -68,24 +68,28 @@ def test_bgp_path_attribute_discard():
         output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast json detail"))
         expected = {
             "routes": {
-                "192.168.100.101/32": [
-                    {
-                        "valid": True,
-                        "atomicAggregate": True,
-                        "community": {
-                            "string": "65001:101",
-                        },
-                    }
-                ],
-                "192.168.100.102/32": [
-                    {
-                        "valid": True,
-                        "originatorId": "10.0.0.2",
-                        "community": {
-                            "string": "65001:102",
-                        },
-                    }
-                ],
+                "192.168.100.101/32": {
+                    "paths": [
+                        {
+                            "valid": True,
+                            "atomicAggregate": True,
+                            "community": {
+                                "string": "65001:101",
+                            },
+                        }
+                    ],
+                },
+                "192.168.100.102/32": {
+                    "paths": [
+                        {
+                            "valid": True,
+                            "originatorId": "10.0.0.2",
+                            "community": {
+                                "string": "65001:102",
+                            },
+                        }
+                    ],
+                },
             }
         }
         return topotest.json_cmp(output, expected)
@@ -107,20 +111,24 @@ def test_bgp_path_attribute_discard():
         output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast json detail"))
         expected = {
             "routes": {
-                "192.168.100.101/32": [
-                    {
-                        "valid": True,
-                        "atomicAggregate": None,
-                        "community": None,
-                    }
-                ],
-                "192.168.100.102/32": [
-                    {
-                        "valid": True,
-                        "originatorId": None,
-                        "community": None,
-                    }
-                ],
+                "192.168.100.101/32": {
+                    "paths": [
+                        {
+                            "valid": True,
+                            "atomicAggregate": None,
+                            "community": None,
+                        }
+                    ],
+                },
+                "192.168.100.102/32": {
+                    "paths": [
+                        {
+                            "valid": True,
+                            "originatorId": None,
+                            "community": None,
+                        }
+                    ],
+                },
             }
         }
         return topotest.json_cmp(output, expected)
diff --git a/tests/topotests/bgp_path_attribute_treat_as_withdraw/test_bgp_path_attribute_treat_as_withdraw.py b/tests/topotests/bgp_path_attribute_treat_as_withdraw/test_bgp_path_attribute_treat_as_withdraw.py
index 4aa297511a..a9d678a42d 100644
--- a/tests/topotests/bgp_path_attribute_treat_as_withdraw/test_bgp_path_attribute_treat_as_withdraw.py
+++ b/tests/topotests/bgp_path_attribute_treat_as_withdraw/test_bgp_path_attribute_treat_as_withdraw.py
@@ -82,17 +82,21 @@ def test_bgp_path_attribute_treat_as_withdraw():
         output = json.loads(r2.vtysh_cmd("show bgp ipv4 unicast json detail"))
         expected = {
             "routes": {
-                "10.10.10.10/32": [
-                    {
-                        "valid": True,
-                        "atomicAggregate": True,
-                    }
-                ],
-                "10.10.10.20/32": [
-                    {
-                        "valid": True,
-                    }
-                ],
+                "10.10.10.10/32": {
+                    "paths": [
+                        {
+                            "valid": True,
+                            "atomicAggregate": True,
+                        }
+                    ],
+                },
+                "10.10.10.20/32": {
+                    "paths": [
+                        {
+                            "valid": True,
+                        }
+                    ],
+                },
             }
         }
         return topotest.json_cmp(output, expected)
@@ -115,11 +119,13 @@ def test_bgp_path_attribute_treat_as_withdraw():
         expected = {
             "routes": {
                 "10.10.10.10/32": None,
-                "10.10.10.20/32": [
-                    {
-                        "valid": True,
-                    }
-                ],
+                "10.10.10.20/32": {
+                    "paths": [
+                        {
+                            "valid": True,
+                        }
+                    ],
+                },
             }
         }
         return topotest.json_cmp(output, expected)