improve connectivity output fot Istio: split by TCP and non-TCP conne… (#406)

* improve connectivity output for Istio: split by TCP and non-TCP connection maps + simplify connection-set output in dot format

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

Author: adisos

nca/FWRules/ConnectivityGraph.py

@@ -62,15 +62,20 @@ def _get_peer_name(self, peer):
             return peer.workload_name, False
         return str(peer), False
 
-    def get_connectivity_dot_format_str(self):
+    def get_connectivity_dot_format_str(self, connectivity_restriction=None):
         """
+        :param Union[str,None] connectivity_restriction: specify if connectivity is restricted to
+               TCP / non-TCP , or not
+        :rtype str
         :return: a string with content of dot format for connectivity graph
         """
-        output_result = f'// The Connectivity Graph of {self.output_config.configName}\n'
+        header_suffix = '' if connectivity_restriction is None else f', for {connectivity_restriction} connections'
+        output_result = f'// The Connectivity Graph of {self.output_config.configName}{header_suffix}\n'
         output_result += 'digraph ' + '{\n'
         if self.output_config.queryName and self.output_config.configName:
-            output_result += f'\tHEADER [shape="box" label=< <B>{self.output_config.queryName}/' \
-                             f'{self.output_config.configName}</B> > fontsize=30 color=webmaroon fontcolor=webmaroon];\n'
+            header_label_str = f'{self.output_config.queryName}/{self.output_config.configName}{header_suffix}'
+            output_result += f'\tHEADER [shape="box" label=< <B>{header_label_str}' \
+                             f'</B> > fontsize=30 color=webmaroon fontcolor=webmaroon];\n'
         peer_lines = set()
         for peer in self.cluster_info.all_peers:
             peer_name, is_ip_block = self._get_peer_name(peer)
@@ -87,7 +92,7 @@ def get_connectivity_dot_format_str(self):
                     line += f'\"{src_peer_name}\"'
                     line += ' -> '
                     line += f'\"{dst_peer_name}\"'
-                    conn_str = str(connections).replace("Protocol:", "")
+                    conn_str = connections.get_simplified_connections_representation(True).replace("Protocol:", "")
                     line += f' [label=\"{conn_str}\" color=\"gold2\" fontcolor=\"darkgreen\"]\n'
                     edge_lines.add(line)
         output_result += ''.join(line for line in sorted(list(peer_lines))) + \

nca/FWRules/MinimizeFWRules.py

@@ -654,11 +654,14 @@ def __init__(self, fw_rules_map, cluster_info, output_config, results_map):
         self.output_config = output_config
         self.results_map = results_map
 
-    def get_fw_rules_in_required_format(self, add_txt_header=True, add_csv_header=True):
+    def get_fw_rules_in_required_format(self, add_txt_header=True, add_csv_header=True, connectivity_restriction=None):
         """
         :param add_txt_header: bool flag to indicate if header of fw-rules query should be added in txt format
         :param add_csv_header: bool flag to indicate if header csv should be added in csv format
+        :param Union[str,None] connectivity_restriction: specify if connectivity is restricted to
+               TCP / non-TCP , or not
         :return: a string or dict representing the computed minimized fw-rules (in a supported format txt/yaml/csv)
+        :rtype: Union[str, dict]
         """
         query_name = self.output_config.queryName
         if self.output_config.configName:
@@ -667,33 +670,39 @@ def get_fw_rules_in_required_format(self, add_txt_header=True, add_csv_header=Tr
         if output_format not in FWRule.supported_formats:
             print(f'error: unexpected outputFormat in output configuration value [should be txt/yaml/csv],  '
                   f'value is: {output_format}')
-        return self.get_fw_rules_content(query_name, output_format, add_txt_header, add_csv_header)
+        return self.get_fw_rules_content(query_name, output_format, add_txt_header, add_csv_header, connectivity_restriction)
 
-    def get_fw_rules_content(self, query_name, req_format, add_txt_header, add_csv_header):
+    def get_fw_rules_content(self, query_name, req_format, add_txt_header, add_csv_header, connectivity_restriction):
         """
         :param query_name: a string of the query name
         :param req_format: a string of the required format, should be in FWRule.supported_formats
         :param add_txt_header:  bool flag to indicate if header of fw-rules query should be added in txt format
         :param add_csv_header: bool flag to indicate if header csv should be added in csv format
+        :param Union[str,None] connectivity_restriction: specify if connectivity is restricted to
+               TCP / non-TCP , or not
         :return: a dict of the fw-rules if the required format is json or yaml, else
         a string of the query name + fw-rules in the required format
         :rtype: Union[str, dict]
         """
         rules_list = self._get_all_rules_list_in_req_format(req_format)
+        key_prefix = '' if connectivity_restriction is None else f'{connectivity_restriction}_'
+        header_prefix = ''
+        if connectivity_restriction is not None:
+            header_prefix = f'For connections of type {connectivity_restriction}, '
 
         if req_format == 'txt':
             res = ''.join(line for line in sorted(rules_list))
             if add_txt_header:
-                res = f'final fw rules for query: {query_name}:\n' + res
+                res = f'{header_prefix}final fw rules for query: {query_name}:\n' + res
             return res
 
         elif req_format in ['yaml', 'json']:
-            return {'rules': rules_list}
+            return {f'{key_prefix}rules': rules_list}
 
         elif req_format in ['csv', 'md']:
             is_csv = req_format == 'csv'
             res = ''
-            header_lines = [[query_name] + [''] * (len(FWRule.rule_csv_header) - 1)]
+            header_lines = [[header_prefix + query_name] + [''] * (len(FWRule.rule_csv_header) - 1)]
             if add_csv_header:
                 if is_csv:
                     header_lines = [FWRule.rule_csv_header] + header_lines

nca/NetworkConfig/NetworkConfigQuery.py

@@ -696,51 +696,135 @@ def exec(self):
                 else:
                     conns, _, _, _ = self.config.allowed_connections(peer1, peer2)
                     if conns:
-                        # TODO: consider separate connectivity maps for config that involves istio -
-                        #  one that handles non-TCP connections, and one for TCP
-                        # TODO: consider avoid "hiding" egress allowed connections, even though they are
-                        #  not covered by authorization policies
-                        if self.config.policies_container.layers.does_contain_single_layer(NetworkLayerName.Istio) and \
-                                self.output_config.connectivityFilterIstioEdges:
-                            should_filter, modified_conns = self.filter_istio_edge(peer2, conns)
-                            if not should_filter:
-                                connections[modified_conns].append((peer1, peer2))
-                                # collect both peers, even if one of them is not in the subset
-                                peers.add(peer1)
-                                peers.add(peer2)
-                        else:
-                            connections[conns].append((peer1, peer2))
-                            # collect both peers, even if one of them is not in the subset
-                            peers.add(peer1)
-                            peers.add(peer2)
+                        connections[conns].append((peer1, peer2))
+                        # collect both peers, even if one of them is not in the subset
+                        peers.add(peer1)
+                        peers.add(peer2)
+
+        # if Istio is a layer in the network config - produce 2 maps, for TCP and for non-TCP
+        # because Istio policies can only capture TCP connectivity
+        if self.config.policies_container.layers.does_contain_layer(NetworkLayerName.Istio):
+            output_res = self.get_connectivity_output_split_by_tcp(connections, peers, peers_to_compare)
+        else:
+            output_res = self.get_connectivity_output_full(connections, peers, peers_to_compare)
 
         res = QueryAnswer(True)
-        if self.output_config.outputFormat == 'dot':
-            conn_graph = ConnectivityGraph(peers, self.config.get_allowed_labels(), self.output_config)
-            conn_graph.add_edges(connections)
-            res.output_explanation = [ComputedExplanation(str_explanation=conn_graph.get_connectivity_dot_format_str())]
+        if self.output_config.outputFormat in ['json', 'yaml']:
+            res.output_explanation = [ComputedExplanation(dict_explanation=output_res)]
         else:
-            conn_graph = ConnectivityGraph(peers_to_compare, self.config.get_allowed_labels(), self.output_config)
-            conn_graph.add_edges(connections)
-            fw_rules = conn_graph.get_minimized_firewall_rules()
-            formatted_rules = fw_rules.get_fw_rules_in_required_format()
-            if self.output_config.outputFormat in ['json', 'yaml']:
-                res.output_explanation = [ComputedExplanation(dict_explanation=formatted_rules)]
-            else:
-                res.output_explanation = [ComputedExplanation(str_explanation=formatted_rules)]
+            res.output_explanation = [ComputedExplanation(str_explanation=output_res)]
         return res
 
+    def get_connectivity_output_full(self, connections, peers, peers_to_compare):
+        """
+        get the connectivity map output considering all connections in the output
+        :param dict connections: the connections' dict (map from connection-set to peer pairs)
+        :param PeerSet peers: the peers to consider for dot output
+        :param PeerSet peers_to_compare: the peers to consider for fw-rules output
+        :rtype Union[str,dict]
+        """
+        if self.output_config.outputFormat == 'dot':
+            dot_full = self.dot_format_from_connections_dict(connections, peers)
+            return dot_full
+        # handle formats other than dot
+        formatted_rules = self.fw_rules_from_connections_dict(connections, peers_to_compare)
+        return formatted_rules
+
+    def get_connectivity_output_split_by_tcp(self, connections, peers, peers_to_compare):
+        """
+        get the connectivity map output as two parts: TCP and non-TCP
+        :param dict connections: the connections' dict (map from connection-set to peer pairs)
+        :param PeerSet peers: the peers to consider for dot output
+        :param PeerSet peers_to_compare: the peers to consider for fw-rules output
+        :rtype Union[str,dict]
+        """
+        connectivity_tcp_str = 'TCP'
+        connectivity_non_tcp_str = 'non-TCP'
+        connections_tcp, connections_non_tcp = self.convert_connections_to_split_by_tcp(connections)
+        if self.output_config.outputFormat == 'dot':
+            dot_tcp = self.dot_format_from_connections_dict(connections_tcp, peers, connectivity_tcp_str)
+            dot_non_tcp = self.dot_format_from_connections_dict(connections_non_tcp, peers, connectivity_non_tcp_str)
+            # concatenate the two graphs into one dot file
+            res_str = dot_tcp + dot_non_tcp
+            return res_str
+        # handle formats other than dot
+        formatted_rules_tcp = self.fw_rules_from_connections_dict(connections_tcp, peers_to_compare,
+                                                                  connectivity_tcp_str)
+        formatted_rules_non_tcp = self.fw_rules_from_connections_dict(connections_non_tcp, peers_to_compare,
+                                                                      connectivity_non_tcp_str)
+        if self.output_config.outputFormat in ['json', 'yaml']:
+            # get a dict object containing the two maps on different keys (TCP_rules and non-TCP_rules)
+            rules = formatted_rules_tcp
+            rules.update(formatted_rules_non_tcp)
+            return rules
+        # remaining formats: txt / csv / md : concatenate the two strings of the conn-maps
+        if self.output_config.outputFormat == 'txt':
+            res_str = f'{formatted_rules_tcp}\n{formatted_rules_non_tcp}'
+        else:
+            res_str = formatted_rules_tcp + formatted_rules_non_tcp
+        return res_str
+
+    def dot_format_from_connections_dict(self, connections, peers, connectivity_restriction=None):
+        """
+        :param dict connections: the connections' dict (map from connection-set to peer pairs)
+        :param PeerSet peers: the peers to consider for dot output
+        :param Union[str,None] connectivity_restriction: specify if connectivity is restricted to
+               TCP / non-TCP , or not
+        :rtype str
+        :return the connectivity map in dot-format, considering connectivity_restriction if required
+        """
+        conn_graph = ConnectivityGraph(peers, self.config.get_allowed_labels(), self.output_config)
+        conn_graph.add_edges(connections)
+        return conn_graph.get_connectivity_dot_format_str(connectivity_restriction)
+
+    def fw_rules_from_connections_dict(self, connections, peers_to_compare, connectivity_restriction=None):
+        """
+        :param dict connections: the connections' dict (map from connection-set to peer pairs)
+        :param PeerSet peers_to_compare: the peers to consider for fw-rules output
+        :param Union[str,None] connectivity_restriction: specify if connectivity is restricted to
+               TCP / non-TCP , or not
+        :return the connectivity map in fw-rules, considering connectivity_restriction if required
+        :rtype: Union[str, dict]
+        """
+        conn_graph = ConnectivityGraph(peers_to_compare, self.config.get_allowed_labels(), self.output_config)
+        conn_graph.add_edges(connections)
+        fw_rules = conn_graph.get_minimized_firewall_rules()
+        formatted_rules = fw_rules.get_fw_rules_in_required_format(connectivity_restriction=connectivity_restriction)
+        return formatted_rules
+
+    def convert_connections_to_split_by_tcp(self, connections):
+        """
+        given the connections' dict , convert it to two connection maps, one for TCP only, and the other
+        for non-TCP only.
+        :param dict connections: the connections' dict (map from connection-set to peer pairs)
+        :return: a tuple of the two connection maps : first for TCP, second for non-TCP
+        :rtype: tuple(dict, dict)
+        """
+        connections_tcp = defaultdict(list)
+        connections_non_tcp = defaultdict(list)
+        for conn, peers_list in connections.items():
+            tcp_conns, non_tcp_conns = self.split_to_tcp_and_non_tcp_conns(conn)
+            connections_tcp[tcp_conns] += peers_list
+            connections_non_tcp[non_tcp_conns] += peers_list
+
+        return connections_tcp, connections_non_tcp
+
     @staticmethod
-    def filter_istio_edge(peer2, conns):
-        # currently only supporting authorization policies, that do not capture egress rules
-        if isinstance(peer2, IpBlock):
-            return True, None
-        # remove allowed connections for non TCP protocols
-        # https://istio.io/latest/docs/ops/configuration/traffic-management/protocol-selection/
-        # Non-TCP based protocols, such as UDP, are not proxied. These protocols will continue to function as normal,
-        # without any interception by the Istio proxy
-        conns_new = conns - ConnectionSet.get_non_tcp_connections()
-        return False, conns_new
+    def split_to_tcp_and_non_tcp_conns(conns):
+        """
+        split a ConnectionSet object to two objects: one within TCP only, the other within non-TCP protocols
+        :param ConnectionSet conns: a  ConnectionSet object
+        :return: a tuple of the two ConnectionSet objects: first for TCP, second for non-TCP
+        :rtype: tuple(ConnectionSet, ConnectionSet)
+        """
+        tcp_conns = conns - ConnectionSet.get_non_tcp_connections()
+        non_tcp_conns = conns - tcp_conns
+        if non_tcp_conns == ConnectionSet.get_non_tcp_connections():
+            non_tcp_conns = ConnectionSet(True)  # all connections in terms of non-TCP
+        if tcp_conns == ConnectionSet.get_all_tcp_connections():
+            tcp_conns = ConnectionSet(True)  # all connections in terms of TCP
+
+        return tcp_conns, non_tcp_conns
 
 
 class TwoNetworkConfigsQuery(BaseNetworkQuery):
@@ -853,7 +937,8 @@ def exec(self, cmd_line_flag=False, layer_name=None):
         if different_conns_list:
             return self._query_answer_with_relevant_explanation(sorted(different_conns_list))
 
-        return QueryAnswer(True, self.name1 + ' and ' + self.name2 + ' are semantically equivalent.', numerical_result=0)
+        return QueryAnswer(True, self.name1 + ' and ' + self.name2 + ' are semantically equivalent.',
+                           numerical_result=0)
 
     def _query_answer_with_relevant_explanation(self, explanation_list):
         output_result = self.name1 + ' and ' + self.name2 + ' are not semantically equivalent.'

nca/NetworkConfig/NetworkLayer.py

@@ -81,6 +81,14 @@ def does_contain_single_layer(self, layer_name):
         """
         return len(self) == 1 and list(self.keys())[0] == layer_name
 
+    def does_contain_layer(self, layer_name):
+        """
+        Checks if the given layer is in the map.
+        :param NetworkLayerName layer_name: the layer to check
+        :return: True if the layer is in the map
+        """
+        return layer_name in self
+
     @staticmethod
     def empty_layer_allowed_connections(layer_name, from_peer, to_peer):
         """

tests/calico_testcases/expected_output/disjointness-various-policies-full-explanation.txt

@@ -2,4 +2,4 @@ There are policies capturing the same pods in np_various_policies
 policies with overlapping captured pods:
 NetworkPolicy_1: kube-system/testcase1-equiv-networkpolicy, NetworkPolicy_2: kube-system/testcase1-nonequiv-networkpolicy, pods: kube-system/calico-node-mgdlr, kube-system/file-plugin-7bfb8b69bf-p86gk, kube-system/keepalived-watcher-57ghx, kube-system/keepalived-watcher-gzdfm, kube-system/keepalived-watcher-wczq8, kube-system/kube-fluentd-h6rjg, kube-system/storage-watcher-8494b4b8bb-f8csd, kube-system/tiller-deploy-5c45c9966b-nqwz6, kube-system/vpn-858f6d9777-2bw5m
 NetworkPolicy_1: kube-system/testcase1-equiv-networkpolicy, NetworkPolicy_2: testcase1-global-networkpolicy, pods: kube-system/calico-node-mgdlr, kube-system/file-plugin-7bfb8b69bf-p86gk, kube-system/keepalived-watcher-57ghx, kube-system/keepalived-watcher-gzdfm, kube-system/keepalived-watcher-wczq8, kube-system/kube-fluentd-h6rjg, kube-system/storage-watcher-8494b4b8bb-f8csd, kube-system/tiller-deploy-5c45c9966b-nqwz6, kube-system/vpn-858f6d9777-2bw5m
-NetworkPolicy_1: kube-system/testcase1-nonequiv-networkpolicy, NetworkPolicy_2: testcase1-global-networkpolicy, pods: kube-system/calico-node-mgdlr, kube-system/file-plugin-7bfb8b69bf-p86gk, kube-system/keepalived-watcher-57ghx, kube-system/keepalived-watcher-gzdfm, kube-system/keepalived-watcher-wczq8, kube-system/kube-fluentd-h6rjg, kube-system/storage-watcher-8494b4b8bb-f8csd, kube-system/tiller-deploy-5c45c9966b-nqwz6, kube-system/vpn-858f6d9777-2bw5m
+NetworkPolicy_1: kube-system/testcase1-nonequiv-networkpolicy, NetworkPolicy_2: testcase1-global-networkpolicy, pods: kube-system/calico-node-mgdlr, kube-system/file-plugin-7bfb8b69bf-p86gk, kube-system/keepalived-watcher-57ghx, kube-system/keepalived-watcher-gzdfm, kube-system/keepalived-watcher-wczq8, kube-system/kube-fluentd-h6rjg, kube-system/storage-watcher-8494b4b8bb-f8csd, kube-system/tiller-deploy-5c45c9966b-nqwz6, kube-system/vpn-858f6d9777-2bw5m

tests/calico_testcases/expected_output/disjointness-various-policies-full-explanation.yaml

@@ -44,4 +44,4 @@
       - kube-system/kube-fluentd-h6rjg
       - kube-system/storage-watcher-8494b4b8bb-f8csd
       - kube-system/tiller-deploy-5c45c9966b-nqwz6
-      - kube-system/vpn-858f6d9777-2bw5m
+      - kube-system/vpn-858f6d9777-2bw5m

tests/calico_testcases/expected_output/equiv-all-range1.json

@@ -188,4 +188,4 @@
     "numerical_result": 0,
     "textual_result": "equiv-ranges-writing-games/kube-system/testcase16-nets-all-range-partitioned-4-net-notNets and equiv-ranges-writing-games/kube-system/testcase16-all-range-with-nets-notNets-single-ips are semantically equivalent."
   }
-]
+]