Fixed a bug in semantic diff query (#565)

* Fixed a bug in semantic diff query:
when skipping the computation of allowed conns between peer1 and peer2, should not automatically skip the reverse direction (computation of allowed connections between peer2 and peer1).

Signed-off-by: Tanya <[email protected]>

* Fixed a bug in semantic diff query:
when skipping the computation of allowed conns between peer1 and peer2, should not automatically skip the reverse direction (computation of allowed connections between peer2 and peer1).
Fixed expected result of a test following the above change.

Signed-off-by: Tanya <[email protected]>

---------

Signed-off-by: Tanya <[email protected]>

Author: tanyaveksler

nca/NetworkConfig/NetworkConfigQuery.py

@@ -1530,35 +1530,31 @@ def compute_diff(self):  # noqa: C901
         conn_graph_removed_per_key[key] = self.get_conn_graph_changed_conns(key, old_ip_blocks, False)
         conn_graph_added_per_key[key] = None
         for pair in itertools.product(removed_peers, old_ip_blocks):
-            if not self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[0], pair[1]):
-                continue
-            lost_conns, _, _, _ = self.config1.allowed_connections(pair[0], pair[1])
-            if lost_conns:
-                conn_graph_removed_per_key[key].add_edge(pair[0], pair[1], lost_conns)
+            if self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[0], pair[1]):
+                lost_conns, _, _, _ = self.config1.allowed_connections(pair[0], pair[1])
+                if lost_conns:
+                    conn_graph_removed_per_key[key].add_edge(pair[0], pair[1], lost_conns)
 
-            if not self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[1], pair[0]):
-                continue
-            lost_conns, _, _, _ = self.config1.allowed_connections(pair[1], pair[0])
-            if lost_conns:
-                conn_graph_removed_per_key[key].add_edge(pair[1], pair[0], lost_conns)
+            if self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[1], pair[0]):
+                lost_conns, _, _, _ = self.config1.allowed_connections(pair[1], pair[0])
+                if lost_conns:
+                    conn_graph_removed_per_key[key].add_edge(pair[1], pair[0], lost_conns)
 
         # 2.1. lost connections between removed peers and intersected peers
         key = 'Lost connections between removed peers and persistent peers'
         keys_list.append(key)
         conn_graph_removed_per_key[key] = self.get_conn_graph_changed_conns(key, PeerSet(), False)
         conn_graph_added_per_key[key] = None
         for pair in itertools.product(removed_peers, intersected_peers):
-            if not self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[0], pair[1]):
-                continue
-            lost_conns, _, _, _ = self.config1.allowed_connections(pair[0], pair[1])
-            if lost_conns:
-                conn_graph_removed_per_key[key].add_edge(pair[0], pair[1], lost_conns)
+            if self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[0], pair[1]):
+                lost_conns, _, _, _ = self.config1.allowed_connections(pair[0], pair[1])
+                if lost_conns:
+                    conn_graph_removed_per_key[key].add_edge(pair[0], pair[1], lost_conns)
 
-            if not self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[1], pair[0]):
-                continue
-            lost_conns, _, _, _ = self.config1.allowed_connections(pair[1], pair[0])
-            if lost_conns:
-                conn_graph_removed_per_key[key].add_edge(pair[1], pair[0], lost_conns)
+            if self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[1], pair[0]):
+                lost_conns, _, _, _ = self.config1.allowed_connections(pair[1], pair[0])
+                if lost_conns:
+                    conn_graph_removed_per_key[key].add_edge(pair[1], pair[0], lost_conns)
 
         # 3.1. lost/new connections between intersected peers due to changes in policies and labels of pods/namespaces
         key = 'Changed connections between persistent peers'
@@ -1600,17 +1596,15 @@ def compute_diff(self):  # noqa: C901
         conn_graph_removed_per_key[key] = None
         conn_graph_added_per_key[key] = self.get_conn_graph_changed_conns(key, PeerSet(), True)
         for pair in itertools.product(intersected_peers, added_peers):
-            if not self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[0], pair[1]):
-                continue
-            new_conns, _, _, _ = self.config2.allowed_connections(pair[0], pair[1])
-            if new_conns:
-                conn_graph_added_per_key[key].add_edge(pair[0], pair[1], new_conns)
+            if self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[0], pair[1]):
+                new_conns, _, _, _ = self.config2.allowed_connections(pair[0], pair[1])
+                if new_conns:
+                    conn_graph_added_per_key[key].add_edge(pair[0], pair[1], new_conns)
 
-            if not self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[1], pair[0]):
-                continue
-            new_conns, _, _, _ = self.config2.allowed_connections(pair[1], pair[0])
-            if new_conns:
-                conn_graph_added_per_key[key].add_edge(pair[1], pair[0], new_conns)
+            if self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[1], pair[0]):
+                new_conns, _, _, _ = self.config2.allowed_connections(pair[1], pair[0])
+                if new_conns:
+                    conn_graph_added_per_key[key].add_edge(pair[1], pair[0], new_conns)
 
         # 5.1. new connections between added peers
         key = 'New connections between added peers'
@@ -1631,17 +1625,15 @@ def compute_diff(self):  # noqa: C901
         conn_graph_added_per_key[key] = self.get_conn_graph_changed_conns(key, new_ip_blocks, True)
 
         for pair in itertools.product(added_peers, new_ip_blocks):
-            if not self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[0], pair[1]):
-                continue
-            new_conns, _, _, _ = self.config2.allowed_connections(pair[0], pair[1])
-            if new_conns:
-                conn_graph_added_per_key[key].add_edge(pair[0], pair[1], new_conns)
-
-            if not self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[1], pair[0]):
-                continue
-            new_conns, _, _, _ = self.config2.allowed_connections(pair[1], pair[0])
-            if new_conns:
-                conn_graph_added_per_key[key].add_edge(pair[1], pair[0], new_conns)
+            if self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[0], pair[1]):
+                new_conns, _, _, _ = self.config2.allowed_connections(pair[0], pair[1])
+                if new_conns:
+                    conn_graph_added_per_key[key].add_edge(pair[0], pair[1], new_conns)
+
+            if self.determine_whether_to_compute_allowed_conns_for_peer_types(pair[1], pair[0]):
+                new_conns, _, _, _ = self.config2.allowed_connections(pair[1], pair[0])
+                if new_conns:
+                    conn_graph_added_per_key[key].add_edge(pair[1], pair[0], new_conns)
 
         return self.get_results_for_computed_fw_rules(keys_list, conn_graph_removed_per_key,
                                                       conn_graph_added_per_key)

tests/k8s_testcases/expected_output/semantic_diff_online_boutique_new_synthesized_vs_orig_synthesized.txt

@@ -2,6 +2,9 @@ orig_online_boutique_synthesis_res and new_online_synthesis_res are not semantic
 
 Lost connections between removed peers and persistent peers (based on topology from config: orig_online_boutique_synthesis_res) :
 src_ns: [default] src_pods: [cartservice] dst_ns: [default] dst_pods: [redis-cart] conn: TCP 6379
+src_ns: [kube-system] src_pods: [*] dst: *.googleapis.com conn: All connections
+src_ns: [kube-system] src_pods: [*] dst: accounts.google.com conn: All connections
+src_ns: [kube-system] src_pods: [*] dst: metadata.google.internal conn: All connections
 
 Removed connections between persistent peers (based on topology from config: orig_online_boutique_synthesis_res) :
 src_ns: [default] src_pods: [cartservice] dst_ns: [kube-system] dst_pods: [*] conn: UDP 53