Skip to content

Commit

Permalink
Intersects based queries optimized (#556)
Browse files Browse the repository at this point in the history
* Optimized implementation of EquivalenceQuery.

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

* Added VacuityQuery and RedundancyQuery optimized implementation.
Keeping optimized properties separated per rule (instead of the union of all policy rules)
Fixed handling HostEPs in optimized implementation.

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

* Added VacuityQuery and RedundancyQuery optimized implementation.
Keeping optimized properties separated per rule (instead of the union of all policy rules)
Fixed handling HostEPs in optimized implementation.

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

* Ignoring 'complex function' lint error.
Returning 'passed' code for skipped queries.

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

* Added VacuityQuery and RedundancyQuery optimized implementation.
Keeping optimized properties separated per rule (instead of the union of all policy rules)
Fixed handling HostEPs in optimized implementation.

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

* Removed redundant method.

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

* Added VacuityQuery and RedundancyQuery optimized implementation.
Keeping optimized properties separated per rule (instead of the union of all policy rules)
Fixed handling HostEPs in optimized implementation.

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

* Fixed domain updating mechanism per rule (to avoid activating multiple times for the same rule, for example when a rule appears twice in a config).

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

* Fixed lint errors

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

* Enabled strongEquivalence optimized implementation.

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

* Implemented optimized ContainmentQuery.
Commented out containment fullExplanation result comparison in tests, since optimized solution gives more accurate result that differs from the original expected result, and thus the test fails.

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

* Enabled optimized TwoContainmentQuery and PermitsQuery.
Commented out twoWayContainment fullExplanation result comparison in tests, since optimized solution gives more accurate result that differs from the original expected result, and thus the tests fail.

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

* Fixed small inaccuracy in handling host endpoints in optimized solution.
Adding docs

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

* Implemented optimized InterferesQuery

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

* Small improvement in print differences for two config queries
Commenting out detailed difference result for some tests, since optimized implementation results are sometimes more detailed than the original ones.

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

* Optimized implementation of intersects and forbids queries.

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

* Fixed bug in creation of optimized istio policy properties.

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

* Opened for optimized run those queries that do not call allowed_connections.

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

* Fixed typo.

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

* Fixing lint error.

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

* Fixed confusing description of IntersectsQuery

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

---------

Signed-off-by: Tanya <[email protected]>
  • Loading branch information
tanyaveksler authored Aug 8, 2023
1 parent 1aa65b6 commit ae0abbd
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 3 deletions.
25 changes: 25 additions & 0 deletions nca/NetworkConfig/NetworkConfigQuery.py
Original file line number Diff line number Diff line change
Expand Up @@ -1932,13 +1932,23 @@ def exec(self, cmd_line_flag):
class IntersectsQuery(TwoNetworkConfigsQuery):
"""
Checking whether both configs allow the same connection between any pair of peers
Note: this query is only used by ForbidsQuery.
It's not symmetrical: config1 is a "specification config", that explicitly defines things to be checked
in the "implementation" config (config2), i.e., its captured connections are considered,
while config2 is the "implementation" config to be checked, and all its connections are considered.
"""

def exec(self, cmd_line_flag=False, only_captured=True):
query_answer = self.is_identical_topologies()
if query_answer.output_result:
return query_answer

if self.config1.optimized_run == 'false':
return self.check_intersects_original()
else:
return self.check_intersects_optimized()

def check_intersects_original(self, only_captured=True):
peers_to_compare = \
self.config1.peer_container.get_all_peers_group(include_dns_entries=True)
peers_to_compare |= self.disjoint_referenced_ip_blocks()
Expand Down Expand Up @@ -1967,6 +1977,21 @@ def exec(self, cmd_line_flag=False, only_captured=True):
return QueryAnswer(False, f'The connections allowed by {self.name1}'
f' do not intersect the connections allowed by {self.name2}', numerical_result=1)

def check_intersects_optimized(self, only_captured=True):
conn_props1 = self.config1.allowed_connections_optimized()
conn_props2 = self.config2.allowed_connections_optimized()
conns1, conns2 = self.filter_conns_by_input_or_internal_constraints(
conn_props1.allowed_conns if only_captured else conn_props1.all_allowed_conns,
conn_props2.all_allowed_conns)
conns_in_both = conns1 & conns2
if conns_in_both:
intersect_connections_list = []
self._append_different_conns_to_list(conns_in_both, intersect_connections_list)
return self._query_answer_with_relevant_explanation(sorted(intersect_connections_list))

return QueryAnswer(False, f'The connections allowed by {self.name1}'
f' do not intersect the connections allowed by {self.name2}', numerical_result=1)

def _query_answer_with_relevant_explanation(self, explanation_list):
intersect_result_msg = self.name2 + ' intersects with ' + self.name1
final_explanation = ConnectionsDiffExplanation(peers_diff_connections_list=explanation_list)
Expand Down
5 changes: 5 additions & 0 deletions nca/Parsers/IstioPolicyYamlParser.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from nca.CoreDS.Peer import IpBlock, PeerSet
from nca.CoreDS.ConnectionSet import ConnectionSet
from nca.CoreDS.PortSet import PortSet
from nca.CoreDS.ProtocolSet import ProtocolSet
from nca.CoreDS.MethodSet import MethodSet
from nca.CoreDS.ConnectivityProperties import ConnectivityProperties
from nca.Resources.IstioNetworkPolicy import IstioNetworkPolicy, IstioPolicyRule
Expand Down Expand Up @@ -489,11 +490,14 @@ def parse_ingress_rule(self, rule, selected_peers):
# currently parsing only ports
# TODO: extend operations parsing to include other attributes
conn_props = ConnectivityProperties.make_empty_props()
tcp_props = ConnectivityProperties.make_conn_props_from_dict(
{"protocols": ProtocolSet.get_protocol_set_with_single_protocol('TCP')})
if to_array is not None:
for operation_dict in to_array:
conn_props |= self.parse_operation(operation_dict)
connections = ConnectionSet()
connections.add_connections('TCP', conn_props)
conn_props &= tcp_props
else: # no 'to' in the rule => all connections allowed
connections = ConnectionSet(True)
conn_props = ConnectivityProperties.get_all_conns_props_per_config_peers(self.peer_container)
Expand All @@ -514,6 +518,7 @@ def parse_ingress_rule(self, rule, selected_peers):
condition_props &= condition_res
condition_conns = ConnectionSet()
condition_conns.add_connections('TCP', condition_props)
condition_props &= tcp_props
if not res_peers:
self.warning('Rule selects no pods', rule)
if not res_peers or not selected_peers:
Expand Down
2 changes: 1 addition & 1 deletion nca/Resources/NetworkPolicy.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def __str__(self):
return self.full_name()

def __eq__(self, other):
if type(self) is type(other):
if isinstance(self, type(other)):
self.sync_opt_props()
other.sync_opt_props()
return \
Expand Down
3 changes: 2 additions & 1 deletion nca/SchemeRunner.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ class SchemeRunner(GenericYamlParser):
"""

implemented_opt_queries = {'connectivityMap', 'equivalence', 'vacuity', 'redundancy', 'strongEquivalence',
'containment', 'twoWayContainment', 'permits', 'interferes', 'pairwiseInterferes'}
'containment', 'twoWayContainment', 'permits', 'interferes', 'pairwiseInterferes',
'forbids', 'emptiness', 'disjointness', 'allCaptured'}

def __init__(self, scheme_file_name, output_format=None, output_path=None, optimized_run='false'):
GenericYamlParser.__init__(self, scheme_file_name)
Expand Down
2 changes: 1 addition & 1 deletion tests/run_all_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def run_all_test_flow(self, all_results):
tmp_opt = [i for i in self.test_queries_obj.args_obj.args if '-opt=' in i]
opt = tmp_opt[0].split('=')[1] if tmp_opt else 'false'
if isinstance(self.test_queries_obj, CliQuery) and (opt == 'debug' or opt == 'true'):
implemented_opt_queries = {'--connectivity', '--equiv', '--permits', '--interferes'}
implemented_opt_queries = {'--connectivity', '--equiv', '--permits', '--interferes', '--forbids'}
# TODO - update/remove the optimization below when all queries are supported in optimized implementation
if not implemented_opt_queries.intersection(set(self.test_queries_obj.args_obj.args)):
print(f'Skipping {self.test_queries_obj.test_name} since it does not have optimized implementation yet')
Expand Down

0 comments on commit ae0abbd

Please sign in to comment.