Diff dot ns frames

docs/diff_output.md

@@ -34,26 +34,33 @@ added,0.0.0.0-255.255.255.255,default/backend[Deployment],No Connections,TCP 909
 ```
 
 Diff output in `dot` format:
+
+in dot output graphs, all the peers of the analyzed cluster are grouped by their namespaces.
 ```
 $ ./bin/k8snetpolicy diff --dir1 tests/netpol-analysis-example-minimal/ --dir2 tests/netpol-diff-example-minimal/ -o dot
 
 digraph {
+        subgraph cluster_default {
+                "default/backend[Deployment]" [label="backend[Deployment]" color="blue" fontcolor="blue"]    
+                "default/frontend[Deployment]" [label="frontend[Deployment]" color="blue" fontcolor="blue"]  
+                label="default"
+        }
         "0.0.0.0-255.255.255.255" [label="0.0.0.0-255.255.255.255" color="blue" fontcolor="blue"]
-        "default/backend[Deployment]" [label="default/backend[Deployment]" color="blue" fontcolor="blue"]
-        "default/frontend[Deployment]" [label="default/frontend[Deployment]" color="blue" fontcolor="blue"]
         "0.0.0.0-255.255.255.255" -> "default/backend[Deployment]" [label="TCP 9090" color="#008000" fontcolor="#008000"]
         "0.0.0.0-255.255.255.255" -> "default/frontend[Deployment]" [label="TCP 8080" color="grey" fontcolor="grey"]
         "default/frontend[Deployment]" -> "0.0.0.0-255.255.255.255" [label="UDP 53" color="grey" fontcolor="grey"]
-        "default/frontend[Deployment]" -> "default/backend[Deployment]" [label="TCP 9090,UDP 53 (old: TCP 9090)" color="magenta" fontcolor="magenta"]
+        "default/frontend[Deployment]" -> "default/backend[Deployment]" [label="TCP 9090,UDP 53 (dir1: TCP 9090)" color="magenta" fontcolor="magenta"]
 }
 ```
 
 `svg` graph from `dot` format output can be produced using `graphviz` as following:
 
 ```
-$ dot -Tsvg tests/netpol-diff-example-minimal/diff_output_from_netpol-analysis-example-minimal.dot -o tests/netpol-diff-example-minimal/diff_output_from_netpol-analysis-example-minimal.svg
+$ dot -Tsvg test_outputs/diff/diff_between_netpol-diff-example-minimal_and_netpol-analysis-example-minimal.dot -o test_outputs/diff/diff_between_netpol-diff-example-minimal_and_netpol-analysis-example-minimal.dot.svg
 
 ```
+Reminder: frames in the graph represent namespaces of the analyzed cluster
+
 ![svg graph](./diff_example_svg.svg)
 
 ### Understanding the output

pkg/netpol/connlist/conns_formatter_dot.go

@@ -20,52 +20,50 @@ type formatDOT struct {
 // formats an edge line from a singleConnFields struct , to be used for dot graph
 func getEdgeLine(c Peer2PeerConnection) string {
 	connStr := common.ConnStrFromConnProperties(c.AllProtocolsAndPorts(), c.ProtocolsAndPorts())
-	srcName, _, _ := peerNameAndColorByType(c.Src())
-	dstName, _, _ := peerNameAndColorByType(c.Dst())
-	return fmt.Sprintf("\t%q -> %q [label=%q color=\"gold2\" fontcolor=\"darkgreen\"]", srcName, dstName, connStr)
+	return fmt.Sprintf("\t%q -> %q [label=%q color=\"gold2\" fontcolor=\"darkgreen\"]", c.Src().String(), c.Dst().String(), connStr)
 }
 
-// returns the peer name and color to be represented in the graph, and whether the peer is external to cluster's namespaces
-func peerNameAndColorByType(peer Peer) (name, color string, isExternal bool) {
+// returns the peer label and color to be represented in the graph, and whether the peer is external to cluster's namespaces
+func peerNameAndColorByType(peer Peer) (nameLabel, color string, isExternal bool) {
 	if peer.IsPeerIPType() {
 		return peer.String(), ipColor, true
 	} else if peer.Name() == common.IngressPodName {
 		return peer.String(), nonIPPeerColor, true
 	}
-	return peer.Name() + "[" + peer.Kind() + "]", nonIPPeerColor, false
+	return common.NodeClusterPeerLabel(peer.Name(), peer.Kind()), nonIPPeerColor, false
 }
 
 // formats a peer line for dot graph
 func getPeerLine(peer Peer) (string, bool) {
-	peerName, peerColor, isExternalPeer := peerNameAndColorByType(peer)
-	linePrefix := "\t\t"
-	if isExternalPeer {
-		linePrefix = "\t"
-	}
-	return fmt.Sprintf("%s%q [label=%q color=%q fontcolor=%q]", linePrefix, peerName, peerName, peerColor, peerColor), isExternalPeer
+	peerNameLabel, peerColor, isExternalPeer := peerNameAndColorByType(peer)
+	return fmt.Sprintf("\t%q [label=%q color=%q fontcolor=%q]", peer.String(), peerNameLabel, peerColor, peerColor), isExternalPeer
 }
 
 // returns a dot string form of connections from list of Peer2PeerConnection objects
 func (d formatDOT) writeOutput(conns []Peer2PeerConnection) (string, error) {
-	nsPeers := make(map[string][]string)         // map from namespace to its peers (grouping peers by namespaces)
-	externalPeersLines := make([]string, 0)      // list of peers which are not in a cluster's namespace (will not be grouped)
-	edgeLines := make([]string, len(conns))      // list of edges lines
-	peersVisited := make(map[string]struct{}, 0) // acts as a set
+	nsPeers := make(map[string][]string)     // map from namespace to its peers (grouping peers by namespaces)
+	externalPeersLines := make([]string, 0)  // list of peers which are not in a cluster's namespace (will not be grouped)
+	edgeLines := make([]string, len(conns))  // list of edges lines
+	peersVisited := make(map[string]bool, 0) // acts as a set
 	for index := range conns {
 		srcStr, dstStr := conns[index].Src().String(), conns[index].Dst().String()
 		edgeLines[index] = getEdgeLine(conns[index])
-		if _, ok := peersVisited[srcStr]; !ok {
-			peersVisited[srcStr] = struct{}{}
-			externalSrcLine := checkAndAddPeerToNsGroup(nsPeers, conns[index].Src())
-			if externalSrcLine != "" {
-				externalPeersLines = append(externalPeersLines, externalSrcLine)
+		if !peersVisited[srcStr] {
+			peersVisited[srcStr] = true
+			peerLine, isExternalPeer := getPeerLine(conns[index].Src())
+			if isExternalPeer { // peer that does not belong to a cluster's namespace (i.e. ip/ ingress-controller)
+				externalPeersLines = append(externalPeersLines, peerLine)
+			} else { // add to Ns group
+				common.AddPeerToNsGroup(conns[index].Src().Namespace(), peerLine, nsPeers)
 			}
 		}
-		if _, ok := peersVisited[dstStr]; !ok {
-			peersVisited[dstStr] = struct{}{}
-			externalDstLine := checkAndAddPeerToNsGroup(nsPeers, conns[index].Dst())
-			if externalDstLine != "" {
-				externalPeersLines = append(externalPeersLines, externalDstLine)
+		if !peersVisited[dstStr] {
+			peersVisited[dstStr] = true
+			peerLine, isExternalPeer := getPeerLine(conns[index].Dst())
+			if isExternalPeer {
+				externalPeersLines = append(externalPeersLines, peerLine)
+			} else {
+				common.AddPeerToNsGroup(conns[index].Dst().Namespace(), peerLine, nsPeers)
 			}
 		}
 	}
@@ -74,52 +72,9 @@ func (d formatDOT) writeOutput(conns []Peer2PeerConnection) (string, error) {
 	sort.Strings(externalPeersLines)
 	// collect all lines by order
 	allLines := []string{common.DotHeader}
-	allLines = append(allLines, addNsGroups(nsPeers)...)
+	allLines = append(allLines, common.AddNsGroups(nsPeers)...)
 	allLines = append(allLines, externalPeersLines...)
 	allLines = append(allLines, edgeLines...)
 	allLines = append(allLines, common.DotClosing)
 	return strings.Join(allLines, newLineChar), nil
 }
-
-// checks if the peer is in cluster's namespace, then adds its line to the namespace list in the given map.
-// else, returns its line to be added to the external peers lines
-func checkAndAddPeerToNsGroup(mapNsToPeers map[string][]string, peer Peer) string {
-	peerLine, isExternalPeer := getPeerLine(peer)
-	if !isExternalPeer { // belongs to a cluster's namespace
-		if _, ok := mapNsToPeers[peer.Namespace()]; !ok {
-			mapNsToPeers[peer.Namespace()] = []string{}
-		}
-		mapNsToPeers[peer.Namespace()] = append(mapNsToPeers[peer.Namespace()], peerLine)
-		return ""
-	}
-	// else case - an external (ip/ ingress-controller) peer
-	return peerLine
-}
-
-func addNsGroups(nsPeersMap map[string][]string) []string {
-	res := []string{}
-	// sort namespaces (map's keys) to ensure same output always
-	nsKeys := sortMapKeys(nsPeersMap)
-	// write ns groups
-	for _, ns := range nsKeys {
-		peersLines := nsPeersMap[ns]
-		sort.Strings(peersLines)
-		// create ns  subgraph cluster
-		nsLabel := strings.ReplaceAll(ns, "-", "_")                 // dot format does not accept "-" in its sub-graphs names (headers)
-		nsLines := []string{"\tsubgraph cluster_" + nsLabel + " {"} // subgraph header
-		nsLines = append(nsLines, peersLines...)
-		nsLines = append(nsLines, "\t\tlabel=\""+ns+"\"", "\t}")
-		// add ns section to the res
-		res = append(res, nsLines...)
-	}
-	return res
-}
-
-func sortMapKeys(nsPeersMap map[string][]string) []string {
-	keys := make([]string, 0, len(nsPeersMap))
-	for k := range nsPeersMap {
-		keys = append(keys, k)
-	}
-	sort.Strings(keys)
-	return keys
-}