issue_403 - AllCaptured tasks (#407)

* AllCaptured tasks: 1. improve message when no policies are considered.
2. adding information about ingress captured with istio
Signed-off-by: shireenf-ibm <[email protected]>

Signed-off-by: shireenf-ibm <[email protected]>

* Update nca/NetworkConfig/NetworkConfigQuery.py

Co-authored-by: Adi Sosnovich <[email protected]>

Signed-off-by: shireenf-ibm <[email protected]>
Co-authored-by: Adi Sosnovich <[email protected]>

Author: shireenf-ibm

docs/SchemeFileFormat.md

@@ -44,29 +44,29 @@ Possible entries (sources) in the list under `networkPolicyList` or `resourceLis
 ###  <a name="queryobject"></a>Query object
 Each query object instructs the tool to run a specific check on one or more sets of policies.
 
-| Field | Description | Value |
-|-------|-------------|-------|
-|name   |Query name|string|
-|emptiness|Checks all NetworkConfigs for empty selectors/rules|list of [config set](#configsets) names|
-|redundancy|Checks each set of NetworkConfigs for redundant policies and for redundant rules within each policy|list of [config set](#configsets) names|
-|equivalence|Checks semantic equivalence between each pair of NetworkConfigs sets|list of [config set](#configsets) names|
-|strongEquivalence|Like equivalence, but comparisons are policy-wise|list of [config set](#configsets) names|
-|semanticDiff|Checks semantic diff between each pair of NetworkConfigs sets|list of [config set](#configsets) names|
-|forbids|Checks whether the first set denies all connections **explicitly** allowed by the other sets|list of [config set](#configsets) names|
-|permits|Checks whether the first set allows all connections **explicitly** allowed by the other sets|list of [config set](#configsets) names|
-|interferes|Checks whether any set interferes with the first set|list of [config set](#configsets) names|
-|pairwiseInterferes|Checks whether any two sets in the list interfere each other|list of [config set](#configsets) names|
-|containment|Checks whether any set is semantically contained in the first set (does not allow additional connections)|list of [config set](#configsets) names|
-|twoWayContainment|Checks what are the relations - equivalence, contains, contained, disjoint, neither - between the first set and each of the other sets|list of [config set](#configsets) names|
-|disjointness|Reports pairs of policies with overlapping sets of captured pods|list of [config set](#configsets) names|
-|vacuity|Checks whether the set of policies changes cluster default behavior|list of [config set](#configsets) names|
-|sanity|Checks all NetworkConfigs for sanity check - includes emptiness, vacuity and redundancies|list of [config set](#configsets) names|
-|allCaptured|Checks that all pods are captured by at least one NetworkPolicy|list of [config set](#configsets) names|
-|connectivityMap|Reports a summary of the allowed connections in the cluster|list of [config set](#configsets) names| 
-|expected|The expected sum of returned results by all sub-queries in this query (a warning is issued on mismatch)|integer|
-|expectedOutput|The file path of the expected output of this query (for connectivityMap or semanticDiff queries) |string|
-|expectedNotExecuted|The number of input configs/config pairs that the query is not expected to be run on. Reasons for not executing the configs are listed [here](CmdLineQueriesResults.md#a-query-will-not-be-executed-when) |integer|
-|outputConfiguration| A dict object with the required output configuration|[outputConfig](#outputconfig) object|
+| Field | Description                                                                                                                                                                                               | Value |
+|-------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-------|
+|name   | Query name                                                                                                                                                                                                |string|
+|emptiness| Checks all NetworkConfigs for empty selectors/rules                                                                                                                                                       |list of [config set](#configsets) names|
+|redundancy| Checks each set of NetworkConfigs for redundant policies and for redundant rules within each policy                                                                                                       |list of [config set](#configsets) names|
+|equivalence| Checks semantic equivalence between each pair of NetworkConfigs sets                                                                                                                                      |list of [config set](#configsets) names|
+|strongEquivalence| Like equivalence, but comparisons are policy-wise                                                                                                                                                         |list of [config set](#configsets) names|
+|semanticDiff| Checks semantic diff between each pair of NetworkConfigs sets                                                                                                                                             |list of [config set](#configsets) names|
+|forbids| Checks whether the first set denies all connections **explicitly** allowed by the other sets                                                                                                              |list of [config set](#configsets) names|
+|permits| Checks whether the first set allows all connections **explicitly** allowed by the other sets                                                                                                              |list of [config set](#configsets) names|
+|interferes| Checks whether any set interferes with the first set                                                                                                                                                      |list of [config set](#configsets) names|
+|pairwiseInterferes| Checks whether any two sets in the list interfere each other                                                                                                                                              |list of [config set](#configsets) names|
+|containment| Checks whether any set is semantically contained in the first set (does not allow additional connections)                                                                                                 |list of [config set](#configsets) names|
+|twoWayContainment| Checks what are the relations - equivalence, contains, contained, disjoint, neither - between the first set and each of the other sets                                                                    |list of [config set](#configsets) names|
+|disjointness| Reports pairs of policies with overlapping sets of captured pods                                                                                                                                          |list of [config set](#configsets) names|
+|vacuity| Checks whether the set of policies changes cluster default behavior                                                                                                                                       |list of [config set](#configsets) names|
+|sanity| Checks all NetworkConfigs for sanity check - includes emptiness, vacuity and redundancies                                                                                                                 |list of [config set](#configsets) names|
+|allCaptured| Checks that all pods are captured by at least one NetworkPolicy in each existing k8s/calico/istio network layer                                                                                           |list of [config set](#configsets) names|
+|connectivityMap| Reports a summary of the allowed connections in the cluster                                                                                                                                               |list of [config set](#configsets) names| 
+|expected| The expected sum of returned results by all sub-queries in this query (a warning is issued on mismatch)                                                                                                   |integer|
+|expectedOutput| The file path of the expected output of this query (for connectivityMap or semanticDiff queries)                                                                                                          |string|
+|expectedNotExecuted| The number of input configs/config pairs that the query is not expected to be run on. Reasons for not executing the configs are listed [here](CmdLineQueriesResults.md#a-query-will-not-be-executed-when) |integer|
+|outputConfiguration| A dict object with the required output configuration                                                                                                                                                      |[outputConfig](#outputconfig) object|
 
 #### <a name="configsets"></a>Config sets
 Each entry in the list of config sets should be either

nca/NetworkConfig/NetworkConfigQuery.py

@@ -14,7 +14,7 @@
 from nca.Resources.CalicoNetworkPolicy import CalicoNetworkPolicy
 from nca.Resources.IngressPolicy import IngressPolicy
 from nca.Utils.OutputConfiguration import OutputConfiguration
-from .QueryOutputHandler import QueryAnswer, DictOutputHandler,  StringOutputHandler, \
+from .QueryOutputHandler import QueryAnswer, DictOutputHandler, StringOutputHandler, \
     PoliciesAndRulesExplanations, PodsListsExplanations, ConnectionsDiffExplanation, IntersectPodsExplanation, \
     PoliciesWithCommonPods, PeersAndConnections, ComputedExplanation
 from .NetworkLayer import NetworkLayerName
@@ -1453,7 +1453,7 @@ def exec(self, cmd_line_flag):
 class AllCapturedQuery(NetworkConfigQuery):
     """
     Check that all pods are captured
-    Applies only for k8s/calico policies
+    Applies for k8s/calico/istio policies (checks only ingress direction for istio)
     """
 
     def _get_pod_name(self, pod):
@@ -1463,44 +1463,81 @@ def _get_pod_name(self, pod):
         """
         return pod.workload_name if self.output_config.outputEndpoints == 'deployments' else str(pod)
 
-    def _get_uncaptured_resources_explanation(self, uncaptured_pods):
+    def _get_uncaptured_xgress_pods(self, layer_name, is_ingress=True):
         """
-        get numerical result + set of names of ingress/egress uncaptured pods
-        :param PeerSet uncaptured_pods: the set of uncaptured
-        :return: (int,set[str]): (the number of uncaptured resources , uncaptured pods names)
+        returns the uncaptured ingress/egress pods set and its length for the given layer
+        :param NetworkLayerName layer_name: the layer to check uncaptured pods in
+        :param bool is_ingress: indicates if to check pods affected by ingress/egress
+        :return: - number of uncaptured pod
+                 - set of the uncaptured pods
+        :rtype: (int,set[str])
         """
-        if not uncaptured_pods:
-            return 0, ''
-        uncaptured_resources = set(self._get_pod_name(pod) for pod in uncaptured_pods)  # no duplicate resources in set
+        existing_pods = self.config.peer_container.get_all_peers_group()
+        uncaptured_xgress_pods = existing_pods - self.config.get_affected_pods(is_ingress, layer_name)
+        if not uncaptured_xgress_pods:
+            return 0, set()
+        uncaptured_resources = set(self._get_pod_name(pod) for pod in uncaptured_xgress_pods)  # no duplicate resources in set
         return len(uncaptured_resources), uncaptured_resources
 
+    def _compute_uncaptured_pods_by_layer(self, layer_name, ingress_only=False):
+        """
+        computes and returns the result of allcaptured query on the given layer if it includes policies
+        :param NetworkLayerName layer_name: the layer to check uncaptured pods in
+        :param bool ingress_only: a flag to indicate if to check captured pods only for ingress affected policies
+        :return: 1- if there are uncaptured pods then return an explanation containing them, else None
+                 2- the number of uncaptured pods on the layer
+        :rtype: (PodsListsExplanations, int)
+        """
+        if layer_name not in self.config.policies_container.layers:
+            return None, 0  # not relevant to compute for non-existed layer
+        if ingress_only:
+            print(f'Warning: AllCaptured query is not considering uncaptured pods in {layer_name.name} egress direction')
+
+        res_ingress, uncaptured_ingress_pods_set = self._get_uncaptured_xgress_pods(layer_name, is_ingress=True)
+        res_egress = 0
+        uncaptured_egress_pods_set = set()
+        if not ingress_only:
+            res_egress, uncaptured_egress_pods_set = self._get_uncaptured_xgress_pods(layer_name, is_ingress=False)
+
+        layer_res = res_ingress + res_egress
+        if layer_res == 0:  # no uncaptured pods in this layer, no explanation would be written
+            return None, 0
+
+        explanation_str = f'workload resources that are not captured by any {layer_name.name} policy that affects '
+        layer_explanation = PodsListsExplanations(explanation_description=explanation_str,
+                                                  pods_list=list(sorted(uncaptured_ingress_pods_set)),
+                                                  egress_pods_list=list(sorted(uncaptured_egress_pods_set)),
+                                                  add_xgress_suffix=True)
+        return layer_explanation, layer_res
+
     def exec(self):
         self.output_config.fullExplanation = True  # assign true for this query - it is always ok to compare its results
         existing_pods = self.config.peer_container.get_all_peers_group()
         if not self.config:
             return QueryAnswer(bool_result=False,
-                               output_result='Flat network in ' + self.config.name,
+                               output_result=f'There are no network policies in {self.config.name}. '
+                                             f'All workload resources are non captured',
                                numerical_result=len(existing_pods))
 
-        if NetworkLayerName.K8s_Calico not in self.config.policies_container.layers:
+        if self.config.policies_container.layers.does_contain_single_layer(NetworkLayerName.Ingress):
             return QueryAnswer(bool_result=False,
-                               output_result='AllCapturedQuery applies only for k8s/calico network policies',
+                               output_result='AllCapturedQuery cannot be applied using Ingress resources only',
                                query_not_executed=True)
 
-        uncaptured_ingress_pods = existing_pods - self.config.get_affected_pods(True, NetworkLayerName.K8s_Calico)
-        uncaptured_egress_pods = existing_pods - self.config.get_affected_pods(False, NetworkLayerName.K8s_Calico)
-        if not uncaptured_ingress_pods and not uncaptured_egress_pods:
-            output_str = f'All pods are captured by at least one policy of k8s/calico in {self.config.name}'
+        k8s_calico_pods_list_explanation, k8s_calico_res = self._compute_uncaptured_pods_by_layer(NetworkLayerName.K8s_Calico)
+        istio_pods_list_explanation, istio_res = self._compute_uncaptured_pods_by_layer(NetworkLayerName.Istio, True)
+
+        if k8s_calico_res == 0 and istio_res == 0:
+            output_str = f'All pods are captured by at least one policy in {self.config.name}'
             return QueryAnswer(bool_result=True, output_result=output_str, numerical_result=0)
 
-        res_ingress, uncaptured_ingress_pods_set = self._get_uncaptured_resources_explanation(uncaptured_ingress_pods)
-        res_egress, uncaptured_egress_pods_set = self._get_uncaptured_resources_explanation(uncaptured_egress_pods)
-        res = res_ingress + res_egress
-        output_str = f'There are workload resources not captured by any k8s/calico policy in {self.config.name}'
-        explanation_str = 'workload resources that are not captured by any policy that affects '
-        final_explanation = PodsListsExplanations(explanation_description=explanation_str,
-                                                  pods_list=list(sorted(uncaptured_ingress_pods_set)),
-                                                  egress_pods_list=list(sorted(uncaptured_egress_pods_set)),
-                                                  add_xgress_suffix=True)
-        return QueryAnswer(bool_result=False, output_result=output_str, output_explanation=[final_explanation],
+        final_explanation = []
+        if k8s_calico_pods_list_explanation:
+            final_explanation.append(k8s_calico_pods_list_explanation)
+        if istio_pods_list_explanation:
+            final_explanation.append(istio_pods_list_explanation)
+
+        output_str = f'There are workload resources not captured by any policy in {self.config.name}'
+        res = k8s_calico_res + istio_res
+        return QueryAnswer(bool_result=False, output_result=output_str, output_explanation=final_explanation,
                            numerical_result=res)

tests/calico_testcases/example_policies/testcase26-multi-layer-policies/testcase26-multi-layer-allcaptured-scheme.yaml

@@ -56,28 +56,28 @@ queries:
     allCaptured:
       - testcase26-config-1-k8s-istio-ingress
     expectedOutput: ../../expected_output/multi-layer-all-captured-1.txt
-    expected: 15 # result of non-captured pods in layer k8s-calico
+    expected: 22 # result of 15 non-captured pods in layer k8s-calico and 7 non-captured pods in layer istio
 
   - name: allcaptured-1-yaml
     allCaptured:
       - testcase26-config-1-k8s-istio-ingress
     outputConfiguration:
       outputFormat: yaml
     expectedOutput: ../../expected_output/multi-layer-all-captured-1.yaml
-    expected: 15 # result of non-captured pods in layer k8s-calico
+    expected: 22 # result of 15 non-captured pods in layer k8s-calico and 7 non-captured pods in layer istio
 
   - name: allcaptured-1-json
     allCaptured:
       - testcase26-config-1-k8s-istio-ingress
     outputConfiguration:
       outputFormat: json
     expectedOutput: ../../expected_output/multi-layer-all-captured-1.json
-    expected: 15 # result of non-captured pods in layer k8s-calico
+    expected: 22 # result of 15 non-captured pods in layer k8s-calico and 7 non-captured pods in layer istio
 
   - name: allcaptured-2
     allCaptured:
       - testcase26-config-1-k8s-istio-ingress-default-deny
-    expected: 0 # all is captured on k8-calico layer
+    expected: 7 # 7 non-captured pods in layer istio, all is captured on k8-calico layer
 
   - name: allcaptured-3
     allCaptured:
@@ -92,4 +92,4 @@ queries:
   - name: allcaptured-5
     allCaptured:
       - testcase26-config-1-istio-ingress
-    expectedNotExecuted: 1
+    expected: 0