Skip to content

Commit

Permalink
Containment based queries optimized (#537)
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]>

* Protecting optimized props policy members from direct access; accessing only by 'getter' methods, to ensure sync is called before access.

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

* Added implemented queries to run_all_tests.py

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

---------

Signed-off-by: Tanya <[email protected]>
  • Loading branch information
tanyaveksler committed Jul 16, 2023
1 parent fae6bc0 commit 59f6126
Show file tree
Hide file tree
Showing 13 changed files with 169 additions and 116 deletions.
86 changes: 55 additions & 31 deletions nca/NetworkConfig/NetworkConfigQuery.py
Original file line number Diff line number Diff line change
Expand Up @@ -1015,8 +1015,8 @@ def _txt_no_fw_rules_format_from_connections_dict(self, connections, peers, conn
: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 txt_no_fw_rules format: the connections between peers excluding connections between
workload to itself (without grouping as fw-rules).
:return the connectivity map in txt_no_fw_rules format: the connections between peers excluding connections
between workload to itself (without grouping as fw-rules).
"""
conn_graph = self._get_conn_graph(connections, peers)
return conn_graph.get_connections_without_fw_rules_txt_format(connectivity_restriction)
Expand Down Expand Up @@ -1130,8 +1130,8 @@ def split_to_tcp_and_non_tcp_conns(conns):
@staticmethod
def convert_props_to_split_by_tcp(props):
"""
given the ConnectivityProperties properties set, convert it to two properties sets, one for TCP only, and the other
for non-TCP only.
given the ConnectivityProperties properties set, convert it to two properties sets, one for TCP only,
and the other for non-TCP only.
:param ConnectivityProperties props: properties describing allowed connections
:return: a tuple of the two properties sets: first for TCP, second for non-TCP
:rtype: tuple(ConnectivityProperties, ConnectivityProperties)
Expand Down Expand Up @@ -1222,6 +1222,35 @@ def filter_conns_by_input_or_internal_constraints(self, conns1, conns2):
res_conns2 = self.config2.filter_conns_by_peer_types(conns2, peers_to_compare) & conns_filter
return res_conns1, res_conns2

def _append_different_conns_to_list(self, conn_diff_props, different_conns_list, props_based_on_config1=True):
"""
Adds difference between config1 and config2 connectivities into the list of differences
:param ConnectivityProperties conn_diff_props: connectivity properties representing a difference
between config1 and config2 connections (or between config2 and config1 connections)
:param list different_conns_list: the list to add differences to
:param bool props_based_on_config1: whether conn_diff_props represent connections present in config1 but not in config2
(the value True) or connections present in config2 but not in config1 (the value False)
"""
no_conns = ConnectionSet()
for cube in conn_diff_props:
conn_cube = conn_diff_props.get_connectivity_cube(cube)
conns, src_peers, dst_peers = \
ConnectionSet.get_connection_set_and_peers_from_cube(conn_cube, self.config1.peer_container)
conns1 = conns if props_based_on_config1 else no_conns
conns2 = no_conns if props_based_on_config1 else conns
if self.output_config.fullExplanation:
if self.config1.optimized_run == 'true':
different_conns_list.append(PeersAndConnections(str(src_peers), str(dst_peers), conns1, conns2))
else: # 'debug': produce the same output format as in the original implementation (per peer pairs)
for src_peer in src_peers:
for dst_peer in dst_peers:
if src_peer != dst_peer:
different_conns_list.append(PeersAndConnections(str(src_peer), str(dst_peer),
conns1, conns2))
else:
different_conns_list.append(PeersAndConnections(src_peers.rep(), dst_peers.rep(), conns1, conns2))
return

@staticmethod
def clone_without_ingress(config):
"""
Expand Down Expand Up @@ -1291,33 +1320,6 @@ def check_equivalence_original(self, layer_name=None):
return QueryAnswer(True, self.name1 + ' and ' + self.name2 + ' are semantically equivalent.',
numerical_result=0)

def _append_different_conns_to_list(self, conn_props, different_conns_list, props_based_on_config1):
"""
Adds difference between config1 and config2 connectivities into the list of differences
:param ConnectivityProperties conn_props: connectivity properties representing a difference between config1 and config2
:param list different_conns_list: the list to add differences to
:param bool props_based_on_config1: whether conn_props represent connections present in config1 but not in config2
(the value True) or connections present in config2 but not in config1 (the value False)
"""
no_conns = ConnectionSet()
for cube in conn_props:
conn_cube = conn_props.get_connectivity_cube(cube)
conns, src_peers, dst_peers = \
ConnectionSet.get_connection_set_and_peers_from_cube(conn_cube, self.config1.peer_container)
conns1 = conns if props_based_on_config1 else no_conns
conns2 = no_conns if props_based_on_config1 else conns
if self.output_config.fullExplanation:
if self.config1.optimized_run == 'true':
different_conns_list.append(PeersAndConnections(str(src_peers), str(dst_peers), conns1, conns2))
else: # 'debug': produce the same output format as in the original implementation (per peer pairs)
for src_peer in src_peers:
for dst_peer in dst_peers:
if src_peer != dst_peer:
different_conns_list.append(PeersAndConnections(str(src_peer), str(dst_peer),
conns1, conns2))
else:
different_conns_list.append(PeersAndConnections(src_peers.rep(), dst_peers.rep(), conns1, conns2))

def check_equivalence_optimized(self, layer_name=None):
conn_props1 = self.config1.allowed_connections_optimized(layer_name)
conn_props2 = self.config2.allowed_connections_optimized(layer_name)
Expand Down Expand Up @@ -1722,6 +1724,13 @@ def exec(self, cmd_line_flag=False, only_captured=False):
return QueryAnswer(False, f'{self.name1} is not contained in {self.name2} ',
output_explanation=[final_explanation], numerical_result=0 if not cmd_line_flag else 1)

if self.config1.optimized_run == 'false':
return self.check_containment_original(cmd_line_flag, only_captured)
else:
return self.check_containment_optimized(cmd_line_flag, only_captured)

def check_containment_original(self, cmd_line_flag=False, only_captured=False):
config1_peers = self.config1.peer_container.get_all_peers_group(include_dns_entries=True)
peers_to_compare = config1_peers | self.disjoint_referenced_ip_blocks()
captured_pods = self.config1.get_captured_pods() | self.config2.get_captured_pods()
not_contained_list = []
Expand All @@ -1745,6 +1754,21 @@ def exec(self, cmd_line_flag=False, only_captured=False):
return QueryAnswer(True, self.name1 + ' is contained in ' + self.name2,
numerical_result=1 if not cmd_line_flag else 0)

def check_containment_optimized(self, cmd_line_flag=False, only_captured=False):
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)
if conns1.contained_in(conns2):
return QueryAnswer(True, self.name1 + ' is contained in ' + self.name2,
numerical_result=1 if not cmd_line_flag else 0)

conns1_not_in_conns2 = conns1 - conns2
different_conns_list = []
self._append_different_conns_to_list(conns1_not_in_conns2, different_conns_list)
return self._query_answer_with_relevant_explanation(sorted(different_conns_list), cmd_line_flag)

def _query_answer_with_relevant_explanation(self, explanation_list, cmd_line_flag):
output_result = f'{self.name1} is not contained in {self.name2}'
explanation_description = f'Connections allowed in {self.name1} which are not a subset of those in {self.name2}'
Expand Down
37 changes: 18 additions & 19 deletions nca/Resources/CalicoNetworkPolicy.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,26 +87,26 @@ def _update_opt_props_by_order(self, is_ingress):
for rule in self.ingress_rules if is_ingress else self.egress_rules:
props = rule.optimized_props.copy()
if rule.action == CalicoPolicyRule.ActionType.Allow:
props -= self.optimized_deny_ingress_props if is_ingress else self.optimized_deny_egress_props
props -= self.optimized_pass_ingress_props if is_ingress else self.optimized_pass_egress_props
props -= self._optimized_deny_ingress_props if is_ingress else self._optimized_deny_egress_props
props -= self._optimized_pass_ingress_props if is_ingress else self._optimized_pass_egress_props
if is_ingress:
self.optimized_allow_ingress_props |= props
self._optimized_allow_ingress_props |= props
else:
self.optimized_allow_egress_props |= props
self._optimized_allow_egress_props |= props
elif rule.action == CalicoPolicyRule.ActionType.Deny:
props -= self.optimized_allow_ingress_props if is_ingress else self.optimized_allow_egress_props
props -= self.optimized_pass_ingress_props if is_ingress else self.optimized_pass_egress_props
props -= self._optimized_allow_ingress_props if is_ingress else self._optimized_allow_egress_props
props -= self._optimized_pass_ingress_props if is_ingress else self._optimized_pass_egress_props
if is_ingress:
self.optimized_deny_ingress_props |= props
self._optimized_deny_ingress_props |= props
else:
self.optimized_deny_egress_props |= props
self._optimized_deny_egress_props |= props
elif rule.action == CalicoPolicyRule.ActionType.Pass:
props -= self.optimized_allow_ingress_props if is_ingress else self.optimized_allow_egress_props
props -= self.optimized_deny_ingress_props if is_ingress else self.optimized_deny_egress_props
props -= self._optimized_allow_ingress_props if is_ingress else self._optimized_allow_egress_props
props -= self._optimized_deny_ingress_props if is_ingress else self._optimized_deny_egress_props
if is_ingress:
self.optimized_pass_ingress_props |= props
self._optimized_pass_ingress_props |= props
else:
self.optimized_pass_egress_props |= props
self._optimized_pass_egress_props |= props

def sync_opt_props(self):
"""
Expand Down Expand Up @@ -169,17 +169,16 @@ def allowed_connections_optimized(self, is_ingress):
and the peer set of captured peers by this policy.
:rtype: tuple (ConnectivityProperties, ConnectivityProperties, PeerSet)
"""
self.sync_opt_props()
res_conns = OptimizedPolicyConnections()
if is_ingress:
res_conns.allowed_conns = self.optimized_allow_ingress_props.copy()
res_conns.denied_conns = self.optimized_deny_ingress_props.copy()
res_conns.pass_conns = self.optimized_pass_ingress_props.copy()
res_conns.allowed_conns = self.optimized_allow_ingress_props().copy()
res_conns.denied_conns = self.optimized_deny_ingress_props().copy()
res_conns.pass_conns = self.optimized_pass_ingress_props().copy()
res_conns.captured = self.selected_peers if self.affects_ingress else Peer.PeerSet()
else:
res_conns.allowed_conns = self.optimized_allow_egress_props.copy()
res_conns.denied_conns = self.optimized_deny_egress_props.copy()
res_conns.pass_conns = self.optimized_pass_egress_props.copy()
res_conns.allowed_conns = self.optimized_allow_egress_props().copy()
res_conns.denied_conns = self.optimized_deny_egress_props().copy()
res_conns.pass_conns = self.optimized_pass_egress_props().copy()
res_conns.captured = self.selected_peers if self.affects_egress else Peer.PeerSet()
return res_conns

Expand Down
5 changes: 2 additions & 3 deletions nca/Resources/IngressPolicy.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def sync_opt_props(self):
return
self._init_opt_props()
for rule in self.egress_rules:
self.optimized_allow_egress_props |= rule.optimized_props
self._optimized_allow_egress_props |= rule.optimized_props
self.optimized_props_in_sync = True

def allowed_connections(self, from_peer, to_peer, is_ingress):
Expand Down Expand Up @@ -110,13 +110,12 @@ def allowed_connections_optimized(self, is_ingress):
and the peer set of captured peers by this policy.
:rtype: tuple (ConnectivityProperties, ConnectivityProperties, PeerSet)
"""
self.sync_opt_props()
res_conns = OptimizedPolicyConnections()
if is_ingress:
res_conns.allowed_conns = ConnectivityProperties.make_empty_props()
res_conns.captured = PeerSet()
else:
res_conns.allowed_conns = self.optimized_allow_egress_props.copy()
res_conns.allowed_conns = self.optimized_allow_egress_props().copy()
res_conns.captured = self.selected_peers if self.affects_egress else PeerSet()
return res_conns

Expand Down
15 changes: 7 additions & 8 deletions nca/Resources/IstioNetworkPolicy.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ def sync_opt_props(self):
self._init_opt_props()
for rule in self.ingress_rules:
if self.action == IstioNetworkPolicy.ActionType.Allow:
self.optimized_allow_ingress_props |= rule.optimized_props
self._optimized_allow_ingress_props |= rule.optimized_props
elif self.action == IstioNetworkPolicy.ActionType.Deny:
self.optimized_deny_ingress_props |= rule.optimized_props
self._optimized_deny_ingress_props |= rule.optimized_props

self.optimized_allow_egress_props = ConnectivityProperties.get_all_conns_props_per_domain_peers()
self._optimized_allow_egress_props = ConnectivityProperties.get_all_conns_props_per_domain_peers()
self.optimized_props_in_sync = True

def allowed_connections(self, from_peer, to_peer, is_ingress):
Expand Down Expand Up @@ -122,15 +122,14 @@ def allowed_connections_optimized(self, is_ingress):
and the peer set of captured peers by this policy.
:rtype: tuple (ConnectivityProperties, ConnectivityProperties, PeerSet)
"""
self.sync_opt_props()
res_conns = OptimizedPolicyConnections()
if is_ingress:
res_conns.allowed_conns = self.optimized_allow_ingress_props.copy()
res_conns.denied_conns = self.optimized_deny_ingress_props.copy()
res_conns.allowed_conns = self.optimized_allow_ingress_props().copy()
res_conns.denied_conns = self.optimized_deny_ingress_props().copy()
res_conns.captured = self.selected_peers
else:
res_conns.allowed_conns = self.optimized_allow_egress_props.copy()
res_conns.denied_conns = self.optimized_deny_egress_props.copy()
res_conns.allowed_conns = self.optimized_allow_egress_props().copy()
res_conns.denied_conns = self.optimized_deny_egress_props().copy()
res_conns.captured = PeerSet()
return res_conns

Expand Down
13 changes: 6 additions & 7 deletions nca/Resources/IstioSidecar.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ def sync_opt_props(self):
if self.optimized_props_in_sync:
return
self._init_opt_props()
self.optimized_allow_ingress_props = ConnectivityProperties.get_all_conns_props_per_domain_peers()
self._optimized_allow_ingress_props = ConnectivityProperties.get_all_conns_props_per_domain_peers()
for rule in self.egress_rules:
self.optimized_allow_egress_props |= rule.optimized_props
self._optimized_allow_egress_props |= rule.optimized_props
self.optimized_props_in_sync = True

def allowed_connections(self, from_peer, to_peer, is_ingress):
Expand Down Expand Up @@ -100,15 +100,14 @@ def allowed_connections(self, from_peer, to_peer, is_ingress):
return PolicyConnections(True, allowed_conns=ConnectionSet())

def allowed_connections_optimized(self, is_ingress):
self.sync_opt_props()
res_conns = OptimizedPolicyConnections()
if is_ingress:
res_conns.allowed_conns = self.optimized_allow_ingress_props.copy()
res_conns.denied_conns = self.optimized_deny_ingress_props.copy()
res_conns.allowed_conns = self.optimized_allow_ingress_props().copy()
res_conns.denied_conns = self.optimized_deny_ingress_props().copy()
res_conns.captured = PeerSet()
else:
res_conns.allowed_conns = self.optimized_allow_egress_props.copy()
res_conns.denied_conns = self.optimized_deny_egress_props.copy()
res_conns.allowed_conns = self.optimized_allow_egress_props().copy()
res_conns.denied_conns = self.optimized_deny_egress_props().copy()
res_conns.captured = self.selected_peers if self.affects_egress else PeerSet()
return res_conns

Expand Down
9 changes: 4 additions & 5 deletions nca/Resources/K8sNetworkPolicy.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ def sync_opt_props(self):
return
self._init_opt_props()
for rule in self.ingress_rules:
self.optimized_allow_ingress_props |= rule.optimized_props
self._optimized_allow_ingress_props |= rule.optimized_props
for rule in self.egress_rules:
self.optimized_allow_egress_props |= rule.optimized_props
self._optimized_allow_egress_props |= rule.optimized_props
self.optimized_props_in_sync = True

def allowed_connections(self, from_peer, to_peer, is_ingress):
Expand Down Expand Up @@ -89,13 +89,12 @@ def allowed_connections_optimized(self, is_ingress):
and the peer set of captured peers by this policy.
:rtype: tuple (ConnectivityProperties, ConnectivityProperties, PeerSet)
"""
self.sync_opt_props()
res_conns = OptimizedPolicyConnections()
if is_ingress:
res_conns.allowed_conns = self.optimized_allow_ingress_props.copy()
res_conns.allowed_conns = self.optimized_allow_ingress_props().copy()
res_conns.captured = self.selected_peers if self.affects_ingress else Peer.PeerSet()
else:
res_conns.allowed_conns = self.optimized_allow_egress_props.copy()
res_conns.allowed_conns = self.optimized_allow_egress_props().copy()
res_conns.captured = self.selected_peers if self.affects_egress else Peer.PeerSet()
return res_conns

Expand Down
Loading

0 comments on commit 59f6126

Please sign in to comment.