Removed no longer used named_ports and excluded_named_ports in Connec…

…tivityProperties.

Removed outdated unit tests.

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

nca/CoreDS/ConnectivityProperties.py

@@ -24,8 +24,6 @@ class ConnectivityProperties(CanonicalHyperCubeSet):
 
     ConnectivityProperties potentially hold all the dimensions, including sets of source peers and destination peers.
     The connectivity properties are built at the parse time for every policy.
-    The named ports are resolved during the construction, therefore in the optimized solution named_ports and
-    excluded_named_ports fields are not used.
 
     The src_peers and dst_peers dimensions are special dimensions,  they do not have constant domain. Their domain
     depends on the current set of peers in the system (as appears in BasePeerSet singleton). This set grows when
@@ -36,23 +34,9 @@ class ConnectivityProperties(CanonicalHyperCubeSet):
     and to set it to relevant values per query, and to make a special treatment of these dimensions
     in the above operators.
 
-    Also, including support for (included and excluded) named ports (relevant for dest ports only).
-
-    The representation with named ports is considered a mid-representation, and is required due to the late binding
-    of the named ports to real ports numbers.
-    The method convert_named_ports is responsible for applying this late binding, and is called by a policy's method
-    allowed_connections() to get policy's allowed connections, given <src, dest> peers and direction ingress/egress
-    Given a specific dest-peer context, the pod's named ports mapping is known, and used for the named ports conversion.
-    Some of the operators for ConnectivityProperties are not supported for objects with (included and excluded) named ports.
-    For example, in the general case, the result for (all but port "x") | (all but port 10) has 2 options:
-        (1) if the dest pod has named port "x" mapped to 10 -> the result would be: (all but port 10)
-        (2) otherwise, the result would be: (all ports)
-    Thus, for the 'or' operator, the assumption is that excluded named ports is empty for both input objects.
-    Some methods, such as bool(), str(), may not return accurate results on objects with named ports (included/excluded)
-    since they depend on the late binding with actual dest pod context.
-    The current actual flow for using named ports is limited for the following:
-    (1) k8s: only +ve named ports, no src named ports, and only use of 'or' operators between these objects.
-    (2) calico: +ve and -ve named ports, no src named ports, and no use of operators between these objects.
+    Also, including support for (included and excluded) named ports (relevant for dest ports only), which are
+    resolved during the construction of ConnectivityProperties.
+
     """
 
     def __init__(self, dimensions_list=None, create_all=False):
@@ -61,8 +45,6 @@ def __init__(self, dimensions_list=None, create_all=False):
         :param create_all: whether to create full connectivity properties.
         """
         super().__init__(dimensions_list if dimensions_list else ConnectivityCube.all_dimensions_list)
-        self.named_ports = {}  # a mapping from dst named port (String) to src ports interval set
-        self.excluded_named_ports = {}  # a mapping from dst named port (String) to src ports interval set
         if create_all:
             self.set_all()
 
@@ -88,16 +70,9 @@ def _make_conn_props_no_named_ports_resolution(conn_cube):
         src_ports = conn_cube["src_ports"]
         dst_ports = conn_cube["dst_ports"]
         assert not src_ports.named_ports and not src_ports.excluded_named_ports
-        all_ports = PortSet.all_ports_interval.copy()
-        for port_name in dst_ports.named_ports:
-            res.named_ports[port_name] = src_ports.port_set
-        for port_name in dst_ports.excluded_named_ports:
-            res.excluded_named_ports[port_name] = all_ports
+        assert not dst_ports.named_ports and not dst_ports.excluded_named_ports
         return res
 
-    def __bool__(self):
-        return super().__bool__() or bool(self.named_ports)
-
     def __str__(self):
         if self.is_all():
             return 'All connections'
@@ -183,9 +158,7 @@ def get_properties_obj(self):
 
     def __eq__(self, other):
         if isinstance(other, ConnectivityProperties):
-            res = super().__eq__(other) and self.named_ports == other.named_ports and \
-                self.excluded_named_ports == other.excluded_named_ports
-            return res
+            return super().__eq__(other)
         return False
 
     def __and__(self, other):
@@ -203,85 +176,6 @@ def __sub__(self, other):
         res -= other
         return res
 
-    def __iand__(self, other):
-        assert not self.has_named_ports()
-        assert not isinstance(other, ConnectivityProperties) or not other.has_named_ports()
-        super().__iand__(other)
-        return self
-
-    def __ior__(self, other):
-        assert not self.excluded_named_ports
-        assert not isinstance(other, ConnectivityProperties) or not other.excluded_named_ports
-        super().__ior__(other)
-        if isinstance(other, ConnectivityProperties):
-            res_named_ports = dict({})
-            for port_name in self.named_ports:
-                res_named_ports[port_name] = self.named_ports[port_name]
-            for port_name in other.named_ports:
-                if port_name in res_named_ports:
-                    res_named_ports[port_name] |= other.named_ports[port_name]
-                else:
-                    res_named_ports[port_name] = other.named_ports[port_name]
-            self.named_ports = res_named_ports
-        return self
-
-    def __isub__(self, other):
-        assert not self.has_named_ports()
-        assert not isinstance(other, ConnectivityProperties) or not other.has_named_ports()
-        super().__isub__(other)
-        return self
-
-    def contained_in(self, other):
-        """
-        :param ConnectivityProperties other: another connectivity properties
-        :return: Whether all (source port, target port) pairs in self also appear in other
-        :rtype: bool
-        """
-        assert not self.has_named_ports()
-        assert not other.has_named_ports()
-        return super().contained_in(other)
-
-    def has_named_ports(self):
-        return self.named_ports or self.excluded_named_ports
-
-    def get_named_ports(self):
-        res = set()
-        res |= set(self.named_ports.keys())
-        res |= set(self.excluded_named_ports.keys())
-        return res
-
-    def convert_named_ports(self, named_ports, protocol):
-        """
-        Replaces all references to named ports with actual ports, given a mapping
-        NOTE: that this function modifies self
-        :param dict[str, (int, int)] named_ports: The mapping from a named to port (str) to the actual port number
-        :param int protocol: The relevant protocol
-        :return: None
-        """
-        if not named_ports:
-            named_ports = {}
-
-        my_named_ports = self.named_ports
-        self.named_ports = {}
-        my_excluded_named_ports = self.excluded_named_ports
-        self.excluded_named_ports = {}
-
-        active_dims = ["src_ports", "dst_ports"]
-        for port in my_named_ports:
-            real_port = named_ports.get(port)
-            if real_port and real_port[1] == protocol:
-                real_port_number = real_port[0]
-                rectangle = [my_named_ports[port],
-                             CanonicalIntervalSet.get_interval_set(real_port_number, real_port_number)]
-                self.add_cube(rectangle, active_dims)
-        for port in my_excluded_named_ports:
-            real_port = named_ports.get(port)
-            if real_port and real_port[1] == protocol:
-                real_port_number = real_port[0]
-                rectangle = [my_excluded_named_ports[port],
-                             CanonicalIntervalSet.get_interval_set(real_port_number, real_port_number)]
-                self.add_hole(rectangle, active_dims)
-
     def copy(self):
         """
         :rtype: ConnectivityProperties
@@ -290,9 +184,6 @@ def copy(self):
         for layer in self.layers:
             res.layers[self._copy_layer_elem(layer)] = self.layers[layer].copy()
         res.active_dimensions = self.active_dimensions.copy()
-
-        res.named_ports = self.named_ports.copy()
-        res.excluded_named_ports = self.excluded_named_ports.copy()
         return res
 
     def print_diff(self, other, self_name, other_name):
@@ -403,7 +294,7 @@ def make_conn_props(conn_cube):
             return ConnectivityProperties._make_conn_props_no_named_ports_resolution(conn_cube)
 
         # Should resolve named ports
-        assert conn_cube.is_active_dim("dst_peers")
+#        assert conn_cube.is_active_dim("dst_peers")
         # Initialize conn_properties
         if dst_ports.port_set:
             dst_ports_no_named_ports = PortSet()

tests/classes_unit_tests/testConnectivityPropertiesNamedPorts.py

@@ -1,5 +1,4 @@
 import unittest
-from nca.CoreDS.CanonicalIntervalSet import CanonicalIntervalSet
 from nca.CoreDS.PortSet import PortSet
 from nca.CoreDS.ConnectivityCube import ConnectivityCube
 from nca.CoreDS.ConnectivityProperties import ConnectivityProperties
@@ -9,93 +8,6 @@
 
 
 class TestNamedPorts(unittest.TestCase):
-    def test_k8s_flow(self):
-        """
-        dest ports with named ports, and 'or' between Tcp properties with named ports
-        """
-        src_res_ports = PortSet(True)
-        dst_res_ports = PortSet()
-        dst_res_ports.add_port("x")
-        conn_cube = ConnectivityCube.make_from_dict({"src_ports": src_res_ports, "dst_ports": dst_res_ports})
-        tcp_properties1 = ConnectivityProperties.make_conn_props(conn_cube)
-        dst_res_ports2 = PortSet()
-        dst_res_ports2.add_port("y")
-        conn_cube["dst_ports"] = dst_res_ports2
-        tcp_properties2 = ConnectivityProperties.make_conn_props(conn_cube)
-        tcp_properties_res = tcp_properties1 | tcp_properties2
-        named_ports_dict = {"x": (15, 6), "z": (20, 6), "y": (16, 6)}
-        tcp_properties_res.convert_named_ports(named_ports_dict, 6)
-        # print(tcp_properties_res)
-        cubes_list = tcp_properties_res._get_cubes_list_from_layers()
-        expected_res_cubes = [[CanonicalIntervalSet.get_interval_set(15, 16)]]
-        self.assertEqual(expected_res_cubes, cubes_list)
-
-    def test_calico_flow_1(self):
-        """
-        dest ports containing only positive named ports
-        """
-        src_res_ports = PortSet()
-        dst_res_ports = PortSet()
-        src_res_ports.add_port_range(1, 100)
-        dst_res_ports.add_port("x")
-        dst_res_ports.add_port("y")
-        dst_res_ports.add_port("z")
-        dst_res_ports.add_port("w")
-        conn_cube = ConnectivityCube.make_from_dict({"src_ports": src_res_ports, "dst_ports": dst_res_ports})
-        tcp_properties = ConnectivityProperties.make_conn_props(conn_cube)
-        tcp_properties_2 = tcp_properties.copy()
-
-        self.assertTrue(tcp_properties.has_named_ports())
-        self.assertEqual(tcp_properties.get_named_ports(), {"x","y","z", "w"})
-        named_ports_dict = {"x": (15, 6), "z": (20, 6), "y": (200, 17)}
-        tcp_properties.convert_named_ports(named_ports_dict, 6)
-        #print(tcp_properties)
-        expected_res_cubes = {(CanonicalIntervalSet.get_interval_set(1,100), CanonicalIntervalSet.get_interval_set(15,15) | CanonicalIntervalSet.get_interval_set(20,20))}
-        self.assertEqual(expected_res_cubes, tcp_properties._get_cubes_set())
-
-        self.assertTrue(tcp_properties_2.has_named_ports())
-        self.assertEqual(tcp_properties_2.get_named_ports(), {"x","y","z", "w"})
-        tcp_properties_2.convert_named_ports(named_ports_dict, 17)
-        #print(tcp_properties_2)
-        expected_res_cubes = {(CanonicalIntervalSet.get_interval_set(1,100), CanonicalIntervalSet.get_interval_set(200,200))}
-        self.assertEqual(expected_res_cubes, tcp_properties_2._get_cubes_set())
-
-    def test_calico_flow_2(self):
-        """
-        dest ports containing only negative named ports
-        """
-        src_res_ports = PortSet()
-        not_ports = PortSet()
-        not_ports.add_port("x")
-        not_ports.add_port("y")
-        not_ports.add_port("z")
-        not_ports.add_port("w")
-        dst_res_ports = PortSet(True)
-        dst_res_ports -= not_ports
-        src_res_ports.add_port_range(1, 100)
-        conn_cube = ConnectivityCube.make_from_dict({"src_ports": src_res_ports, "dst_ports": dst_res_ports})
-        tcp_properties = ConnectivityProperties.make_conn_props(conn_cube)
-        tcp_properties_2 = tcp_properties.copy()
-
-        self.assertTrue(tcp_properties.has_named_ports())
-        self.assertEqual(tcp_properties.get_named_ports(), {"x","y","z", "w"})
-        named_ports_dict = {"x": (15, 6), "z": (20, 6), "y": (200, 17)}
-        tcp_properties.convert_named_ports(named_ports_dict, 6)
-        #print(tcp_properties)
-        expected_res_cubes = {(CanonicalIntervalSet.get_interval_set(1,100),
-                               CanonicalIntervalSet.get_interval_set(1,14) |
-                               CanonicalIntervalSet.get_interval_set(16,19) |
-                               CanonicalIntervalSet.get_interval_set(21,65535))}
-        self.assertEqual(expected_res_cubes, tcp_properties._get_cubes_set())
-
-        self.assertTrue(tcp_properties_2.has_named_ports())
-        self.assertEqual(tcp_properties_2.get_named_ports(), {"x","y","z", "w"})
-        tcp_properties_2.convert_named_ports(named_ports_dict, 17)
-        #print(tcp_properties_2)
-        expected_res_cubes = {(CanonicalIntervalSet.get_interval_set(1,100),
-                               CanonicalIntervalSet.get_interval_set(1,199) |
-                               CanonicalIntervalSet.get_interval_set(201,65535))}
-        self.assertEqual(expected_res_cubes, tcp_properties_2._get_cubes_set())
 
     def test_optimized_flow(self):
         default_namespace = K8sNamespace("default")
@@ -127,7 +39,6 @@ def test_optimized_flow(self):
                                                      "src_ports": src_ports, "dst_ports": dst_ports,
                                                      "protocols": ProtocolSet.get_protocol_set_with_single_protocol("TCP")})
         props_with_tcp = ConnectivityProperties.make_conn_props(conn_cube)
-        self.assertFalse(props_with_tcp.has_named_ports())
         tcp_ports_for_pod_a = PortSet.make_port_set_with_range(200, 300)
         tcp_ports_for_pod_a.add_port_range(600, 600)
         tcp_ports_for_pod_b = PortSet.make_port_set_with_range(200, 300)
@@ -156,7 +67,6 @@ def test_optimized_flow(self):
                                                      "src_ports": src_ports, "dst_ports": dst_ports,
                                                      "protocols": ProtocolSet.get_protocol_set_with_single_protocol("UDP")})
         props_with_udp = ConnectivityProperties.make_conn_props(conn_cube)
-        self.assertFalse(props_with_udp.has_named_ports())
         udp_ports_for_pod_a = PortSet.make_port_set_with_range(200, 300)
         udp_ports_for_pod_a.add_port_range(400, 400)
         udp_ports_for_pod_b = PortSet.make_port_set_with_range(200, 300)