Skip to content

Commit

Permalink
preparation to support istio egress gateway (#604)
Browse files Browse the repository at this point in the history
* Bug fix in livesim: Changed missing pods labels dictionary to set of pairs (key, value) to allow missing pods having the same label key but different label values.
Bug fix in IngressNetworkLayer._allowed_xgress_conns_optimized: all_allowed_conns should always include allowed_conns.

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

* Small fixes and preparations for implementing istio egress gateway.
Bug fix: considering DNS entries when computing allowed connections in the Ingress layer.
Removing duplicates from the list of missing livesim resources.

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

* More fixes of handling DNS entries in optimized solution.

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

* Uppdating expected results of some livesim tests following the addition of istio-egressgateway resource to livesim, and changing the namespace of istio-ingressgateway to istio-system.

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

* Small fixes.

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

* Referencing istio ingress gateway as istio:ingressgateway (to be aligned with livesim, until #610 is implemented).
Extending IngressPolicy to both directions (ingress/egress)
Splitting livesim ingress/egress gateway resources to separated files.

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

* Changing expected results of some istio ingress/egress gateway tests according to the change in livesim implementation of these gateways.
Temporarily commenting out other expected results, until ingress/egress gateway implementation is completes.

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

* Changing expected results of some istio ingress/egress gateway tests according to the change in livesim implementation of these gateways.

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

* Renamed IngressPolicy to IstioGatewayPolicy.

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

* Renamed IstioGatewayPolicy to GatewayPolicy.

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

* Update nca/Resources/GatewayPolicy.py

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

* Update nca/NetworkConfig/NetworkConfigQuery.py

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

* Renamed Ingress layer to Gateway layer.

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

---------

Signed-off-by: Tanya <[email protected]>
Co-authored-by: Adi Sosnovich <[email protected]>
  • Loading branch information
tanyaveksler and adisos authored Nov 12, 2023
1 parent c1e0c25 commit dac0824
Show file tree
Hide file tree
Showing 18 changed files with 178 additions and 129 deletions.
4 changes: 4 additions & 0 deletions nca/CoreDS/ConnectionSet.py
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,10 @@ def split_peer_set_to_fw_rule_elements(peer_set, cluster_info):
res.append(FWRule.IPBlockElement(peer))
peer_set_copy.remove(peer)
continue
elif isinstance(peer, FWRule.DNSEntry):
res.append(FWRule.DNSElement(peer))
peer_set_copy.remove(peer)
continue
ns_peers = PeerSet(cluster_info.ns_dict[peer.namespace])
if ns_peers.issubset(peer_set_copy):
ns_set.add(peer.namespace)
Expand Down
27 changes: 27 additions & 0 deletions nca/NetworkConfig/LiveSim/istio_gateway/istio_egress_gateway.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
apiVersion: v1
kind: Pod
metadata:
name: istio-egressgateway-livesim
namespace: istio-system
labels:
app: istio-egressgateway
istio: egressgateway
spec:
serviceAccountName: istio-egressgateway
containers:
- name: istio-proxy
image: auto
---

apiVersion: v1
kind: Service
metadata:
name: istio-egressgateway
namespace: istio-system
spec:
ports:
- port: 443
selector:
app: istio-egressgateway
istio: egressgateway
---
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: v1
kind: Pod
metadata:
name: istio-ingressgateway-livesim
namespace: istio-ingressgateway-ns
namespace: istio-system
labels:
app: istio-ingressgateway
istio: ingressgateway
Expand Down
52 changes: 26 additions & 26 deletions nca/NetworkConfig/NetworkConfigQuery.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from nca.FWRules.ClusterInfo import ClusterInfo
from nca.Resources.NetworkPolicy import PolicyConnectionsFilter
from nca.Resources.CalicoNetworkPolicy import CalicoNetworkPolicy
from nca.Resources.IngressPolicy import IngressPolicy
from nca.Resources.GatewayPolicy import GatewayPolicy
from nca.Utils.OutputConfiguration import OutputConfiguration
from .QueryOutputHandler import QueryAnswer, DictOutputHandler, StringOutputHandler, \
PoliciesAndRulesExplanations, PodsListsExplanations, ConnectionsDiffExplanation, IntersectPodsExplanation, \
Expand Down Expand Up @@ -217,7 +217,7 @@ def exec(self):
# collecting non-disjoint policies per network layer
non_disjoint_explanation_list = []
for layer_name, layer in self.config.policies_container.layers.items():
if layer_name == NetworkLayerName.Ingress: # skip ingress layer
if layer_name == NetworkLayerName.Gateway: # skip gateway layer
continue
policies_list = layer.policies_list
for policy1 in policies_list:
Expand Down Expand Up @@ -294,12 +294,12 @@ class VacuityQuery(NetworkConfigQuery):
"""

def exec(self):
# TODO: should handle 'ingress' layer or not? (ingress controller pod is not expected to have egress
# TODO: should handle 'gateway' layer or not? (ingress controller pod is not expected to have egress
# traffic without any Ingress resource)
# currently ignoring ingres layer, removing it from configs on this query
# currently ignoring gateway layer, removing it from configs on this query
self.output_config.fullExplanation = True # assign true for this query - it is ok to compare its results
vacuous_config = self.config.clone_without_policies('vacuousConfig')
self_config = TwoNetworkConfigsQuery.clone_without_ingress(self.config)
self_config = TwoNetworkConfigsQuery.clone_without_gateway_layer(self.config)
vacuous_res = EquivalenceQuery(self_config, vacuous_config).exec()
if not vacuous_res.bool_result:
return QueryAnswer(vacuous_res.bool_result,
Expand Down Expand Up @@ -381,7 +381,7 @@ def exec(self):
redundant_egress_rules = {}
self.output_config.fullExplanation = True # assign true for this query - it is ok to compare its results
for layer_name, layer in self.config.policies_container.layers.items():
if layer_name == NetworkLayerName.Ingress:
if layer_name == NetworkLayerName.Gateway:
continue
policies_list = layer.policies_list
redundant_policies = sorted(list(self.redundant_policies(policies_list, layer_name)))
Expand Down Expand Up @@ -637,7 +637,7 @@ def exec(self): # noqa: C901
policies_issue += '\tNote that it contains a single policy.\n'

for layer_name, layer in self.config.policies_container.layers.items():
if layer_name == NetworkLayerName.Ingress:
if layer_name == NetworkLayerName.Gateway:
continue
policies_list = layer.policies_list
# check for redundant policies in this layer
Expand Down Expand Up @@ -1270,21 +1270,21 @@ def _append_different_conns_to_list(self, conn_diff_props, different_conns_list,
return

@staticmethod
def clone_without_ingress(config):
def clone_without_gateway_layer(config):
"""
Clone config without ingress policies
Clone config without gateway policies
:param NetworkConfig config: the config to clone
:return: resulting config without ingress policies
:return: resulting config without gateway policies
:rtype: NetworkConfig
"""
if NetworkLayerName.Ingress not in config.policies_container.layers or not config.policies_container.layers[
NetworkLayerName.Ingress].policies_list:
return config # no ingress policies in this config
config_without_ingress = config.clone_without_policies(config.name)
if NetworkLayerName.Gateway not in config.policies_container.layers or not config.policies_container.layers[
NetworkLayerName.Gateway].policies_list:
return config # no gateway policies in this config
config_without_gateway = config.clone_without_policies(config.name)
for policy in config.policies_container.policies.values():
if not isinstance(policy, IngressPolicy): # ignoring ingress policies
config_without_ingress.append_policy_to_config(policy)
return config_without_ingress
if not isinstance(policy, GatewayPolicy): # ignoring gateway policies
config_without_gateway.append_policy_to_config(policy)
return config_without_gateway

def execute(self, cmd_line_flag):
return self.exec(cmd_line_flag)
Expand Down Expand Up @@ -2094,13 +2094,13 @@ def exec(self, cmd_line_flag):
query_answer.output_result = output_result_on_permit
return query_answer

if self.config1.policies_container.layers.does_contain_single_layer(NetworkLayerName.Ingress):
if self.config1.policies_container.layers.does_contain_single_layer(NetworkLayerName.Gateway):
return QueryAnswer(bool_result=False,
output_result='Permitted traffic cannot be specified using Ingress resources only',
output_result='Permitted traffic cannot be specified using Ingress/Gateway resources only',
query_not_executed=True)

config1_without_ingress = self.clone_without_ingress(self.config1)
query_answer = ContainmentQuery(config1_without_ingress, self.config2,
config1_without_gateway = self.clone_without_gateway_layer(self.config1)
query_answer = ContainmentQuery(config1_without_gateway, self.config2,
self.output_config).exec(cmd_line_flag=cmd_line_flag, only_captured=True)
if not cmd_line_flag:
query_answer.numerical_result = 1 if query_answer.output_explanation else 0
Expand Down Expand Up @@ -2276,15 +2276,15 @@ def exec(self, cmd_line_flag):
if not self.config1:
return QueryAnswer(False, 'There are no NetworkPolicies in the given forbids config. '
'No traffic is specified as forbidden.', query_not_executed=True)
if self.config1.policies_container.layers.does_contain_single_layer(NetworkLayerName.Ingress):
if self.config1.policies_container.layers.does_contain_single_layer(NetworkLayerName.Gateway):
return QueryAnswer(bool_result=False,
output_result='Forbidden traffic cannot be specified using Ingress resources only',
output_result='Forbidden traffic cannot be specified using Ingress/Gateway resources only',
query_not_executed=True)

config1_without_ingress = self.clone_without_ingress(self.config1)
config1_without_gateway = self.clone_without_gateway_layer(self.config1)

query_answer = \
IntersectsQuery(config1_without_ingress, self.config2, self.output_config).exec(only_captured=True)
IntersectsQuery(config1_without_gateway, self.config2, self.output_config).exec(only_captured=True)
if query_answer.numerical_result == 1:
query_answer.output_result += f'\n{self.name2} forbids connections specified in ' \
f'{self.name1}'
Expand Down Expand Up @@ -2362,7 +2362,7 @@ def exec(self):
self.output_config.fullExplanation = True # assign true for this query - it is always ok to compare its results
# get_all_peers_group() does not require getting dnsEntry peers, since they are not ClusterEP (pods)
existing_pods = self.config.peer_container.get_all_peers_group()
if not self.config or self.config.policies_container.layers.does_contain_single_layer(NetworkLayerName.Ingress):
if not self.config or self.config.policies_container.layers.does_contain_single_layer(NetworkLayerName.Gateway):
return QueryAnswer(bool_result=False,
output_result=f'There are no network policies in {self.config.name}. '
f'All workload resources are non captured',
Expand Down
46 changes: 19 additions & 27 deletions nca/NetworkConfig/NetworkLayer.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@
class NetworkLayerName(Enum):
K8s_Calico = 0
Istio = 1
Ingress = 2
Gateway = 2

def create_network_layer(self, policies):
if self == NetworkLayerName.K8s_Calico:
return K8sCalicoNetworkLayer(policies)
if self == NetworkLayerName.Istio:
return IstioNetworkLayer(policies)
if self == NetworkLayerName.Ingress:
if self == NetworkLayerName.Gateway:
return IngressNetworkLayer(policies)
return None

Expand All @@ -40,7 +40,7 @@ def policy_type_to_layer(policy_type):
elif policy_type in {NetworkPolicy.PolicyType.IstioAuthorizationPolicy, NetworkPolicy.PolicyType.IstioSidecar}:
return NetworkLayerName.Istio
elif policy_type == NetworkPolicy.PolicyType.Ingress:
return NetworkLayerName.Ingress
return NetworkLayerName.Gateway
return None


Expand Down Expand Up @@ -369,10 +369,9 @@ def _allowed_xgress_conns_optimized(self, is_ingress, peer_container, res_conns_
res_conns = self.collect_policies_conns_optimized(is_ingress, IstioNetworkLayer.captured_cond_func)
if not res_conns_filter.calc_all_allowed:
return res_conns

# all the calculations below update res_conns.all_allowed_conns
all_peers_and_ips = peer_container.get_all_peers_group(True)
all_peers_no_ips = peer_container.get_all_peers_group()
all_peers_and_ips = peer_container.get_all_peers_group(add_external_ips=True)
all_peers_no_ips = peer_container.get_all_peers_group(add_external_ips=False)
dns_entries = peer_container.get_all_dns_entries()
# for istio initialize non-captured conns with all possible non-TCP connections
# This is a compact way to represent all peers connections, but it is an over-approximation also containing
Expand Down Expand Up @@ -422,36 +421,29 @@ def _allowed_xgress_conns_optimized(self, is_ingress, peer_container, res_conns_
class IngressNetworkLayer(NetworkLayer):

def _allowed_xgress_conns(self, from_peer, to_peer, is_ingress):
allowed_conns = ConnectionSet()
all_allowed_conns = ConnectionSet(True)
captured_res = False
if not is_ingress:
allowed_conns, _, _, captured_res = self.collect_policies_conns(from_peer, to_peer, is_ingress)
if captured_res:
all_allowed_conns = allowed_conns
allowed_conns, _, _, captured_res = self.collect_policies_conns(from_peer, to_peer, is_ingress)
if captured_res:
all_allowed_conns = allowed_conns
return PolicyConnections(captured=captured_res, allowed_conns=allowed_conns, denied_conns=ConnectionSet(),
all_allowed_conns=all_allowed_conns)

def _allowed_xgress_conns_optimized(self, is_ingress, peer_container, res_conns_filter=PolicyConnectionsFilter()):
res_conns = OptimizedPolicyConnections()
all_peers_and_ips = peer_container.get_all_peers_group(True)
all_peers_no_ips = peer_container.get_all_peers_group()
all_peers_and_ips = peer_container.get_all_peers_group(add_external_ips=True, include_dns_entries=True)
all_peers_no_ips = peer_container.get_all_peers_group(add_external_ips=False, include_dns_entries=True)
non_captured_conns = None
if is_ingress:
if res_conns_filter.calc_all_allowed:
# everything is allowed and non captured
non_captured_conns = ConnectivityProperties.make_conn_props_from_dict({"src_peers": all_peers_and_ips,
"dst_peers": all_peers_no_ips})
res_conns.all_allowed_conns = non_captured_conns
else:
res_conns = self.collect_policies_conns_optimized(is_ingress)
res_conns = self.collect_policies_conns_optimized(is_ingress)
if res_conns_filter.calc_all_allowed:
res_conns.all_allowed_conns = res_conns.allowed_conns
non_captured_peers = all_peers_no_ips - res_conns.captured
if res_conns_filter.calc_all_allowed:
res_conns.all_allowed_conns = res_conns.allowed_conns
if non_captured_peers:
if non_captured_peers:
if is_ingress:
non_captured_conns = ConnectivityProperties.make_conn_props_from_dict({"src_peers": all_peers_and_ips,
"dst_peers": non_captured_peers})
else:
non_captured_conns = ConnectivityProperties.make_conn_props_from_dict({"src_peers": non_captured_peers,
"dst_peers": all_peers_and_ips})
res_conns.all_allowed_conns |= non_captured_conns
res_conns.all_allowed_conns |= non_captured_conns
if non_captured_conns and ExplTracker().is_active():
src_peers, dst_peers = ExplTracker().extract_peers(non_captured_conns)
ExplTracker().add_default_policy(src_peers,
Expand Down
4 changes: 2 additions & 2 deletions nca/NetworkConfig/PoliciesFinder.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ def __init__(self, optimized_run='false'):
self.optimized_run = optimized_run
# following missing resources fields are relevant for "livesim" mode,
# where certain resources are added to enable the analysis
self.missing_istio_gw_pods_with_labels = {}
self.missing_istio_gw_pods_with_labels = set()
self.missing_k8s_ingress_peers = False
self.missing_dns_pods_with_labels = {}
self.missing_dns_pods_with_labels = set()

def set_peer_container(self, peer_container):
"""
Expand Down
24 changes: 12 additions & 12 deletions nca/NetworkConfig/ResourcesHandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,24 +69,24 @@ def get_full_livesim_resource_path(livesim_resource_path):
return os.path.join(current_path, livesim_resource_path)

@staticmethod
def get_relevant_livesim_resources_paths_by_labels_matching(livesim_resource_path, missing_resource_labels_dict):
def get_relevant_livesim_resources_paths_by_labels_matching(livesim_resource_path, missing_resource_labels):
"""
check by labels matching if one of the livesim resources has matching labels for a resource referenced by one
of the parsed policies. If yes, return its path to be added to the configuration, to enable the analysis.
:param str livesim_resource_path: a path to the relevant livesim dir to check for resources
:param dict missing_resource_labels_dict: the labels from parsed policy in the config for
:param set((key, value)) missing_resource_labels: the labels from parsed policy in the config for
which a matching peer was missing
:return: list of paths for relevant livesim resources to add
:rtype list[str]
"""
res = []
res = set()
resource_full_path = ResourcesHandler.get_full_livesim_resource_path(livesim_resource_path)
livesim_resource_labels = ResourcesParser.parse_livesim_yamls(resource_full_path)
for key in missing_resource_labels_dict.keys():
for (key, value) in missing_resource_labels:
for yaml_path, labels in livesim_resource_labels.items():
if missing_resource_labels_dict.get(key) == labels.get(key):
res.append(yaml_path)
return res
if (key, value) in labels:
res.add(yaml_path)
return list(res)

@staticmethod
def analyze_livesim(policy_finder):
Expand All @@ -113,12 +113,12 @@ def analyze_livesim(policy_finder):
livesim_configuration_addons.append(resource_full_path)
ResourcesHandler.livesim_information_message('ingress-controller')

# find Istio ingress gateway
# find Istio ingress/egress gateway
istio_gateway_added_resources = ResourcesHandler.get_relevant_livesim_resources_paths_by_labels_matching(
LiveSimPaths.IstioGwCfgPath, policy_finder.missing_istio_gw_pods_with_labels)
if istio_gateway_added_resources:
livesim_configuration_addons += istio_gateway_added_resources
ResourcesHandler.livesim_information_message('Istio-ingress-gateway')
ResourcesHandler.livesim_information_message('Istio-ingress/egress-gateway')

return livesim_configuration_addons

Expand Down Expand Up @@ -367,15 +367,15 @@ def parse_livesim_yamls(path):
for yaml_file in yaml_files:
pods_finder = PodsFinder()
ns_finder = NamespacesFinder()
labels_found = {}
labels_found = set()
for res_code in yaml_file.data:
ns_finder.parse_yaml_code_for_ns(res_code)
pods_finder.namespaces_finder = ns_finder
pods_finder.add_eps_from_yaml(res_code)
for item in ns_finder.namespaces.values():
labels_found.update(item.labels)
labels_found.update(set(item.labels.items()))
for item in pods_finder.peer_set:
labels_found.update(item.labels)
labels_found.update(set(item.labels.items()))
results.update({yaml_file.path: labels_found})
NcaLogger().collect_msgs()

Expand Down
4 changes: 2 additions & 2 deletions nca/Parsers/GenericIngressLikeYamlParser.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from nca.CoreDS.ProtocolSet import ProtocolSet
from nca.CoreDS.ConnectivityProperties import ConnectivityProperties
from nca.CoreDS.ConnectionSet import ConnectionSet
from nca.Resources.IngressPolicy import IngressPolicyRule
from nca.Resources.GatewayPolicy import GatewayPolicyRule
from .GenericYamlParser import GenericYamlParser


Expand Down Expand Up @@ -81,7 +81,7 @@ def _make_allow_rules(conn_props, src_peers):
new_props = ConnectivityProperties.make_conn_props(new_conn_cube)
new_conns = ConnectionSet()
new_conns.add_connections('TCP', new_props)
res.append(IngressPolicyRule(dst_peer_set, new_conns, rule_opt_props))
res.append(GatewayPolicyRule(dst_peer_set, new_conns, rule_opt_props))
return res

@staticmethod
Expand Down
Loading

0 comments on commit dac0824

Please sign in to comment.