improve mindfa str expr + reorder dimensions in TCP properties: hosts… (#434)

* improve mindfa str expr + reorder dimensions in TCP properties: hosts before paths

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

Author: adisos

nca/CoreDS/DimensionsManager.py

@@ -24,11 +24,10 @@ class __DimensionsManager:
         def __init__(self):
             # TODO: verify alphabet for regex type dimensions, currently using one default alphabet
             #  currently valid chars are: ['.', '/', '-', 0-9, a-z, A-Z ]
-            self.default_dfa_alphabet_chars = ".\\w/\\-"
-            self.default_dfa_alphabet_str = "[" + self.default_dfa_alphabet_chars + "]*"
             self.default_interval_domain_tuple = (0, 100000)
             self.domain_str_to_dfa_map = dict()
-            dfa_all_words_default = self._get_dfa_from_alphabet_str(self.default_dfa_alphabet_str)
+            dfa_all_words_default = self._get_dfa_from_alphabet_str(MinDFA.default_alphabet_regex)
+            dfa_all_words_path_domain = self._get_dfa_path_domain()
             ports_interval = CanonicalIntervalSet.get_interval_set(1, 65535)
             all_methods_interval = MethodSet(True)
             all_peers_interval = CanonicalIntervalSet.get_interval_set(0, 10000)  # assuming max possible peer number
@@ -37,7 +36,7 @@ def __init__(self):
             self.dim_dict["dst_ports"] = (DimensionsManager.DimensionType.IntervalSet, ports_interval)
             self.dim_dict["methods"] = (DimensionsManager.DimensionType.IntervalSet, all_methods_interval)
             self.dim_dict["peers"] = (DimensionsManager.DimensionType.IntervalSet, all_peers_interval)
-            self.dim_dict["paths"] = (DimensionsManager.DimensionType.DFA, dfa_all_words_default)
+            self.dim_dict["paths"] = (DimensionsManager.DimensionType.DFA, dfa_all_words_path_domain)
             self.dim_dict["hosts"] = (DimensionsManager.DimensionType.DFA, dfa_all_words_default)
 
             icmp_type_interval = CanonicalIntervalSet.get_interval_set(0, 254)
@@ -58,6 +57,17 @@ def _get_dfa_from_alphabet_str(self, alphabet_str):
             self.domain_str_to_dfa_map[alphabet_str] = new_dfa
             return new_dfa
 
+        @staticmethod
+        def _get_dfa_path_domain():
+            """
+            get a dfa that represents all valid words in the paths domain
+            :rtype MinDFA
+            """
+            regex_str = "/" + MinDFA.default_alphabet_regex
+            new_dfa = MinDFA.dfa_from_regex(regex_str)
+            new_dfa.is_all_words = MinDFA.Ternary.TRUE
+            return new_dfa
+
     instance = None
 
     def __init__(self):
@@ -95,7 +105,7 @@ def set_domain(self, dim_name, dim_type, interval_tuple=None, alphabet_str=None)
             interval = interval_tuple if interval_tuple is not None else self.default_interval_domain_tuple
             domain = CanonicalIntervalSet.get_interval_set(interval[0], interval[1])
         else:
-            alphabet = alphabet_str if alphabet_str is not None else self.default_dfa_alphabet_str
+            alphabet = alphabet_str if alphabet_str is not None else MinDFA.default_alphabet_regex
             domain = self._get_dfa_from_alphabet_str(alphabet)
         self.dim_dict[dim_name] = (dim_type, domain)
 

nca/CoreDS/MinDFA.py

@@ -3,12 +3,12 @@
 # SPDX-License-Identifier: Apache2.0
 #
 from greenery import fsm, parse
-from greenery.rxelems import from_fsm
 from functools import lru_cache
 
 
 # TODO: consider adding abstract base class for MinDFA and CanonicalIntervalSet , with common api
 
+
 class MinDFA:
     """
     MinDFA is a wrapper class for greenery.fsm , to support the api required for dimensions in hypercube-set
@@ -42,6 +42,8 @@ class MinDFA:
         (no mix of MinDFA objects from different dimensions context)
 
     """
+    default_dfa_alphabet_chars = ".\\w/\\-"
+    default_alphabet_regex = "[.\\w/\\-]*"
 
     class Ternary:
         FALSE = 0
@@ -59,10 +61,14 @@ def __init__(self, alphabet, states, initial, finals, map):
                                                             necessary)
         complement_dfa: MinDFA of the complement dfa of self, e.g: relevant when doing subtraction from 'all'.
                         for performance improvement (avoid computation of complement if could use this member instead).
+
+        regex_expr: str representation of regex expressions (possibly) with operations (subtract/intersect/union),
+                    from which the MinDFA object was constructed
         """
         self.fsm = fsm.Fsm(initial, finals, alphabet, states, map)
         self.is_all_words = MinDFA.Ternary.UNKNOWN
         self.complement_dfa = None
+        self.regex_expr = ''
 
     def __contains__(self, string):
         return string in self.fsm
@@ -109,6 +115,7 @@ def dfa_from_regex(s, alphabet=None):
         # TODO: currently assuming input str as regex only has '*' operator for infinity
         if '*' not in s:
             res.is_all_words = MinDFA.Ternary.FALSE
+        res.regex_expr = s.replace(MinDFA.default_alphabet_regex, "*")
         return res
 
     @staticmethod
@@ -120,6 +127,7 @@ def dfa_all_words(alphabet):
         """
         res = MinDFA.dfa_from_regex(alphabet)
         res.is_all_words = MinDFA.Ternary.TRUE
+        res.regex_expr = '*'
         return res
 
     # TODO: this function may not be necessary, if keeping the current __eq__ override
@@ -175,15 +183,18 @@ def __str__(self):
         """
         str representation of the language accepted by this DFA:
         - option 1: if language has finite number of words -> return string with all accepted words.
-        - option 2 (costly): convert fsm to regex with greenery
+        - option 2 : a string of regex expressions with accumulated operations, from which the object was constructed.
         :rtype: str
         """
+
         if self.has_finite_len():
             return self._get_strings_set_str()
         if self.is_all_words == MinDFA.Ternary.TRUE:
             return "*"
-        # TODO: consider performance implications of this conversion from MinDFA to regex
-        return str(from_fsm(self.fsm))
+        return self.regex_expr
+        # in comment below: alternative based on conversion from MinDFA to regex
+        # not readable regex result + had performance implications of this conversion from MinDFA to regex
+        # return str(from_fsm(self.fsm))
 
     def get_fsm_str(self):
         """
@@ -219,6 +230,11 @@ def __or__(self, other):
         res = MinDFA.dfa_from_fsm(fsm_res)
         if res.has_finite_len():
             res.is_all_words = MinDFA.Ternary.FALSE
+        # update regex_expr of the result object
+        if self.regex_expr == other.regex_expr:
+            res.regex_expr = self.regex_expr
+        else:
+            res.regex_expr = f'({self.regex_expr})|({other.regex_expr})'
         return res
 
     @lru_cache(maxsize=500)
@@ -231,18 +247,30 @@ def __and__(self, other):
         res = MinDFA.dfa_from_fsm(fsm_res)
         if self.is_all_words == MinDFA.Ternary.FALSE or other.is_all_words == MinDFA.Ternary.FALSE:
             res.is_all_words = MinDFA.Ternary.FALSE
+        # update regex_expr of the result object
+        if self.regex_expr == other.regex_expr:
+            res.regex_expr = self.regex_expr
+        else:
+            res.regex_expr = f'({self.regex_expr})&({other.regex_expr})'
         return res
 
     @lru_cache(maxsize=500)
     def __sub__(self, other):
+        if self.is_all_words == MinDFA.Ternary.TRUE and other.complement_dfa is not None:
+            return other.complement_dfa
+
         fsm_res = self.fsm - other.fsm
         res = MinDFA.dfa_from_fsm(fsm_res)
-        if other.is_all_words == MinDFA.Ternary.TRUE:
+        # update regex_expr of the result object
+        res.regex_expr = f'({self.regex_expr})-({other.regex_expr})'
+
+        if other.is_all_words == MinDFA.Ternary.TRUE:  # res becomes empty
             res.is_all_words = MinDFA.Ternary.FALSE
         elif other:
-            res.is_all_words = MinDFA.Ternary.FALSE
+            res.is_all_words = MinDFA.Ternary.FALSE  # res cannot be all words
         if self.is_all_words == MinDFA.Ternary.TRUE and not other:
             res.is_all_words = MinDFA.Ternary.TRUE
+
         if self.is_all_words == MinDFA.Ternary.TRUE:
             res.complement_dfa = other
             other.complement_dfa = res

nca/CoreDS/TcpLikeProperties.py

@@ -36,7 +36,7 @@ class TcpLikeProperties(CanonicalHyperCubeSet):
     (2) calico: +ve and -ve named ports, no src named ports, and no use of operators between these objects.
     """
 
-    dimensions_list = ["src_ports", "dst_ports", "methods", "paths", "hosts", "peers"]
+    dimensions_list = ["src_ports", "dst_ports", "methods", "hosts", "paths", "peers"]
 
     # TODO: change constructor defaults? either all arguments in "allow all" by default, or "empty" by default
     def __init__(self, source_ports=PortSet(), dest_ports=PortSet(), methods=MethodSet(True), paths=None, hosts=None,
@@ -59,37 +59,26 @@ def __init__(self, source_ports=PortSet(), dest_ports=PortSet(), methods=MethodS
         self.base_peer_set = base_peer_set if base_peer_set else PeerSet()
 
         # create the cube from input arguments
-        cube = []
-        active_dims = []
-        if not source_ports.is_all():
-            cube.append(source_ports.port_set)
-            active_dims.append("src_ports")
-        if not dest_ports.is_all():
-            cube.append(dest_ports.port_set)
-            active_dims.append("dst_ports")
-        if not methods.is_whole_range():
-            cube.append(methods)
-            active_dims.append("methods")
-        if paths is not None:
-            cube.append(paths)
-            active_dims.append("paths")
-        if hosts is not None:
-            cube.append(hosts)
-            active_dims.append("hosts")
-        if peers is not None:
-            cube.append(peers)
-            active_dims.append("peers")
+        # create a dict object that holds the values required to build the cube
+        dims_to_values = {"src_ports": {"value": source_ports.port_set,
+                                        "is_all": source_ports.is_all()},
+                          "dst_ports": {"value": dest_ports.port_set,
+                                        "is_all": dest_ports.is_all()},
+                          "methods": {"value": methods,
+                                      "is_all": methods.is_whole_range()},
+                          "hosts": {"value": hosts,
+                                    "is_all": hosts is None},
+                          "paths": {"value": paths,
+                                    "is_all": paths is None},
+                          "peers": {"value": peers,
+                                    "is_all": peers is None}}
+
+        cube, active_dims, has_empty_dim_value = self._get_cube_and_active_dims_from_input_values(dims_to_values)
 
         if not active_dims:
             self.set_all()
-        else:
-            has_empty_dim_value = False
-            for dim_val in cube:
-                if not dim_val:
-                    has_empty_dim_value = True
-                    break
-            if not has_empty_dim_value:
-                self.add_cube(cube, active_dims)
+        elif not has_empty_dim_value:
+            self.add_cube(cube, active_dims)
 
         # assuming named ports are only in dest, not src
         all_ports = PortSet.all_ports_interval.copy()
@@ -99,6 +88,27 @@ def __init__(self, source_ports=PortSet(), dest_ports=PortSet(), methods=MethodS
             # self.excluded_named_ports[port_name] = all_ports - source_ports.port_set
             self.excluded_named_ports[port_name] = all_ports
 
+    @staticmethod
+    def _get_cube_and_active_dims_from_input_values(dims_to_values):
+        """
+        Given initial values, get the matching cube and its active dimensions
+        :param dict dims_to_values: map from dimension name to values properties
+        :rtype tuple(list, list, bool)
+        :return: tuple with: (1) cube values (2) active dimensions (3) bool indication if some dimension is empty
+        """
+        cube = []
+        active_dims = []
+        has_empty_dim_value = False
+        # add values to cube by required order of dimensions
+        for dim in TcpLikeProperties.dimensions_list:
+            dim_val = dims_to_values[dim]["value"]
+            add_to_cube = not dims_to_values[dim]["is_all"]
+            if add_to_cube:
+                cube.append(dim_val)
+                active_dims.append(dim)
+                has_empty_dim_value |= not dim_val
+        return cube, active_dims, has_empty_dim_value
+
     def __bool__(self):
         return super().__bool__() or bool(self.named_ports)
 
@@ -167,8 +177,8 @@ def get_properties_obj(self):
     def __eq__(self, other):
         if isinstance(other, TcpLikeProperties):
             assert self.base_peer_set == other.base_peer_set
-            res = super().__eq__(other) and self.named_ports == other.named_ports and \
-                self.excluded_named_ports == other.excluded_named_ports
+            res = super().__eq__(other) and self.named_ports == other.named_ports \
+                and self.excluded_named_ports == other.excluded_named_ports
             return res
         return False
 

nca/Parsers/GenericIngressLikeYamlParser.py

@@ -40,8 +40,11 @@ def parse_regex_host_value(self, regex_value, rule):
         if regex_value is None:
             return None  # to represent that all is allowed, and this dimension can be inactive in the generated cube
 
+        if regex_value == '*':
+            return DimensionsManager().get_dimension_domain_by_name('hosts')
+
         allowed_chars = "[\\w]"
-        allowed_chars_with_star_regex = "[*" + DimensionsManager().default_dfa_alphabet_chars + "]*"
+        allowed_chars_with_star_regex = "[*" + MinDFA.default_dfa_alphabet_chars + "]*"
         if not re.fullmatch(allowed_chars_with_star_regex, regex_value):
             self.syntax_error(f'Illegal characters in host {regex_value}', rule)
 
@@ -148,3 +151,18 @@ def _make_rules_from_conns(self, tcp_conns):
         for peer_set, conns in peers_to_conns.items():
             res.append(IngressPolicyRule(peer_set, conns))
         return res
+
+    @staticmethod
+    def get_path_prefix_dfa(path_string):
+        """
+        Given a prefix path, get its MinDFA that accepts all relevant paths
+        :param str path_string: a path string from policy, specified as Prefix
+        :rtype MinDFA
+        """
+        if path_string == '/':
+            return DimensionsManager().get_dimension_domain_by_name('paths')
+        allowed_chars = "[" + MinDFA.default_dfa_alphabet_chars + "]"
+        if path_string.endswith('/'):
+            path_string = path_string[:-1]
+        path_regex = f'{path_string}(/{allowed_chars}*)?'
+        return MinDFA.dfa_from_regex(path_regex)

nca/Parsers/IngressPolicyYamlParser.py

@@ -35,7 +35,7 @@ def __init__(self, policy, peer_container, ingress_file_name=''):
     def validate_path_value(self, path_value, path):
         if path_value[0] != '/':
             self.syntax_error(f'Illegal path {path_value} in the rule path', path)
-        pattern = "[" + DimensionsManager().default_dfa_alphabet_chars + "]*"
+        pattern = "[" + MinDFA.default_dfa_alphabet_chars + "]*"
         if not re.fullmatch(pattern, path_value):
             self.syntax_error(f'Illegal characters in path {path_value} in {path}')
 
@@ -149,16 +149,13 @@ def segregate_longest_paths_and_make_dfa(parsed_paths):
         """
         # first, convert path strings to dfas
         parsed_paths_with_dfa = []
-        allowed_chars = "[" + DimensionsManager().default_dfa_alphabet_chars + "]"
         for path_string, path_type, peers, ports in parsed_paths:
             if path_type == 'Exact':
-                path_regex = path_string
-            else:
-                if path_string:
-                    path_regex = path_string + '|' + path_string + '/' + allowed_chars + '*'
-                else:
-                    path_regex = '/' + allowed_chars + '*'
-            parsed_paths_with_dfa.append((path_string, MinDFA.dfa_from_regex(path_regex), path_type, peers, ports))
+                path_dfa = MinDFA.dfa_from_regex(path_string)
+            else:  # Prefix type
+                path_string = '/' if not path_string else path_string
+                path_dfa = GenericIngressLikeYamlParser.get_path_prefix_dfa(path_string)
+            parsed_paths_with_dfa.append((path_string, path_dfa, path_type, peers, ports))
 
         # next, avoid shorter sub-paths to extend to longer ones, using dfa operations
         res = []

nca/Parsers/IstioPolicyYamlParser.py

@@ -269,8 +269,8 @@ def _parse_str_value(self, str_val_input, dim_name, operation):
         :param dict operation: the operation object being parsed
         :return: str: the result regex/str after conversion
         """
-        allowed_chars = "[" + DimensionsManager().default_dfa_alphabet_chars + "]"
-        allowed_chars_with_star_regex = "[*" + DimensionsManager().default_dfa_alphabet_chars + "]*"
+        allowed_chars = "[" + MinDFA.default_dfa_alphabet_chars + "]"
+        allowed_chars_with_star_regex = "[*" + MinDFA.default_dfa_alphabet_chars + "]*"
         if not re.fullmatch(allowed_chars_with_star_regex, str_val_input):
             self.syntax_error(f'Illegal characters in {dim_name} {str_val_input} in {operation}')
 

nca/Parsers/IstioTrafficResourcesYamlParser.py

@@ -4,7 +4,6 @@
 #
 
 from functools import reduce
-from nca.CoreDS.DimensionsManager import DimensionsManager
 from nca.CoreDS.MinDFA import MinDFA
 from nca.CoreDS.Peer import PeerSet
 from nca.CoreDS.MethodSet import MethodSet
@@ -223,9 +222,11 @@ def parse_istio_regex_string(self, resource, attr_name, vs_name):
         if items[0][0] == 'exact':
             pass
         elif items[0][0] == 'prefix':
-            regex += DimensionsManager().default_dfa_alphabet_str
+            if attr_name == 'uri':
+                return self.get_path_prefix_dfa(regex)
+            regex += MinDFA.default_alphabet_regex
         elif items[0][0] == 'regex':
-            regex.replace('.', DimensionsManager().default_dfa_alphabet_chars)
+            regex.replace('.', MinDFA.default_dfa_alphabet_chars)
             if attr_name == 'uri' and resource.get('ignoreUriCase') == 'True':
                 # https://github.com/google/re2/wiki/Syntax#:~:text=group%3B%20non%2Dcapturing-,(%3Fflags%3Are),-set%20flags%20during
                 regex = '(?i:' + regex + ')'