From ae0abbdecb4bf37e47b1b75907f10e3f7a36b269 Mon Sep 17 00:00:00 2001 From: Tanya Date: Tue, 8 Aug 2023 11:53:27 +0300 Subject: [PATCH] Intersects based queries optimized (#556) * Optimized implementation of EquivalenceQuery. Signed-off-by: Tanya * 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 * 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 * Ignoring 'complex function' lint error. Returning 'passed' code for skipped queries. Signed-off-by: Tanya * 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 * Removed redundant method. Signed-off-by: Tanya * 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 * 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 * Fixed lint errors Signed-off-by: Tanya * Enabled strongEquivalence optimized implementation. Signed-off-by: Tanya * 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 * 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 * Fixed small inaccuracy in handling host endpoints in optimized solution. Adding docs Signed-off-by: Tanya * Implemented optimized InterferesQuery Signed-off-by: Tanya * 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 * Optimized implementation of intersects and forbids queries. Signed-off-by: Tanya * Fixed bug in creation of optimized istio policy properties. Signed-off-by: Tanya * Opened for optimized run those queries that do not call allowed_connections. Signed-off-by: Tanya * Fixed typo. Signed-off-by: Tanya * Fixing lint error. Signed-off-by: Tanya * Fixed confusing description of IntersectsQuery Signed-off-by: Tanya --------- Signed-off-by: Tanya --- nca/NetworkConfig/NetworkConfigQuery.py | 25 +++++++++++++++++++++++++ nca/Parsers/IstioPolicyYamlParser.py | 5 +++++ nca/Resources/NetworkPolicy.py | 2 +- nca/SchemeRunner.py | 3 ++- tests/run_all_tests.py | 2 +- 5 files changed, 34 insertions(+), 3 deletions(-) diff --git a/nca/NetworkConfig/NetworkConfigQuery.py b/nca/NetworkConfig/NetworkConfigQuery.py index ad4d9530..2897d9d4 100644 --- a/nca/NetworkConfig/NetworkConfigQuery.py +++ b/nca/NetworkConfig/NetworkConfigQuery.py @@ -1932,6 +1932,10 @@ 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): @@ -1939,6 +1943,12 @@ def exec(self, cmd_line_flag=False, only_captured=True): 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() @@ -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) diff --git a/nca/Parsers/IstioPolicyYamlParser.py b/nca/Parsers/IstioPolicyYamlParser.py index 3dd89b06..4a7db974 100644 --- a/nca/Parsers/IstioPolicyYamlParser.py +++ b/nca/Parsers/IstioPolicyYamlParser.py @@ -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 @@ -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) @@ -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: diff --git a/nca/Resources/NetworkPolicy.py b/nca/Resources/NetworkPolicy.py index 357af5b1..d2d15e86 100644 --- a/nca/Resources/NetworkPolicy.py +++ b/nca/Resources/NetworkPolicy.py @@ -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 \ diff --git a/nca/SchemeRunner.py b/nca/SchemeRunner.py index f74c3f1c..80be11e6 100644 --- a/nca/SchemeRunner.py +++ b/nca/SchemeRunner.py @@ -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) diff --git a/tests/run_all_tests.py b/tests/run_all_tests.py index ddeaecd6..1fbba2dc 100644 --- a/tests/run_all_tests.py +++ b/tests/run_all_tests.py @@ -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')