From db1b24fcf627e00ca7a541164538bdfe860ddf2c Mon Sep 17 00:00:00 2001 From: Maysa Macedo Date: Thu, 17 Oct 2019 08:54:47 +0000 Subject: [PATCH] Ensure Network Policy handles egress traffic to a SVC We're not taking into account the case of a Network Policy with an egress rule to a pod that contains a Service sitting in front of it. Right now, only an egress rule to the matched pod is created, when one for the matched SVC is also required. Related-Bug: 1849139 Change-Id: I9830f30ba1fde3e5ec1a98fcbca22af992dd1bec --- .../controller/drivers/network_policy.py | 106 +++++++++++++++++- .../controller/drivers/test_network_policy.py | 7 +- 2 files changed, 106 insertions(+), 7 deletions(-) diff --git a/kuryr_kubernetes/controller/drivers/network_policy.py b/kuryr_kubernetes/controller/drivers/network_policy.py index 07fbc27f5..afa8bed22 100644 --- a/kuryr_kubernetes/controller/drivers/network_policy.py +++ b/kuryr_kubernetes/controller/drivers/network_policy.py @@ -12,7 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +import ipaddress import netaddr + from oslo_log import log as logging from neutronclient.common import exceptions as n_exc @@ -25,6 +27,8 @@ from kuryr_kubernetes.controller.drivers import utils as driver_utils from kuryr_kubernetes import exceptions from kuryr_kubernetes import utils +CONF = config.CONF + LOG = logging.getLogger(__name__) @@ -131,9 +135,10 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): - Ensure traffic is allowed from the host """ default_cidrs = [] - default_cidrs.append(utils.get_subnet_cidr( - config.CONF.neutron_defaults.service_subnet)) - worker_subnet_id = config.CONF.pod_vif_nested.worker_nodes_subnet + if CONF.octavia_defaults.enforce_sg_rules: + default_cidrs.append(utils.get_subnet_cidr( + CONF.neutron_defaults.service_subnet)) + worker_subnet_id = CONF.pod_vif_nested.worker_nodes_subnet if worker_subnet_id: default_cidrs.append(utils.get_subnet_cidr(worker_subnet_id)) for cidr in default_cidrs: @@ -314,6 +319,13 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): cidr=cidr, pods=pods) if sg_rule not in crd_rules: crd_rules.append(sg_rule) + if (direction == 'egress' and + CONF.octavia_defaults.enforce_sg_rules): + rules = self._create_svc_egress_sg_rule( + sg_id, policy_namespace, resource=resource, + port=container_port, + protocol=port.get('protocol')) + crd_rules.extend(rules) def _create_sg_rule_body_on_text_port(self, sg_id, direction, port, resources, crd_rules, pod_selector, @@ -369,9 +381,16 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): protocol=port.get('protocol'), pods=pods) crd_rules.append(sg_rule) + if (direction == 'egress' and + CONF.octavia_defaults.enforce_sg_rules): + rules = self._create_svc_egress_sg_rule( + sg_id, policy_namespace, port=container_port, + protocol=port.get('protocol')) + crd_rules.extend(rules) def _create_sg_rule_on_number_port(self, allowed_resources, sg_id, - direction, port, sg_rule_body_list): + direction, port, sg_rule_body_list, + policy_namespace): for resource in allowed_resources: cidr, ns = self._get_resource_details(resource) # NOTE(maysams): Skipping resource that do not have @@ -386,6 +405,12 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): cidr=cidr, namespace=ns)) sg_rule_body_list.append(sg_rule) + if (direction == 'egress' and + CONF.octavia_defaults.enforce_sg_rules): + rule = self._create_svc_egress_sg_rule( + sg_id, policy_namespace, resource=resource, + port=port.get('port'), protocol=port.get('protocol')) + sg_rule_body_list.extend(rule) def _create_all_pods_sg_rules(self, port, sg_id, direction, sg_rule_body_list, pod_selector, @@ -402,6 +427,12 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): sg_id, direction, port.get('port'), protocol=port.get('protocol'))) sg_rule_body_list.append(sg_rule) + if (direction == 'egress' and + CONF.octavia_defaults.enforce_sg_rules): + rule = self._create_svc_egress_sg_rule( + sg_id, policy_namespace, port=port.get('port'), + protocol=port.get('protocol')) + sg_rule_body_list.extend(rule) def _create_default_sg_rule(self, sg_id, direction, sg_rule_body_list): default_rule = { @@ -499,7 +530,7 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): else: self._create_sg_rule_on_number_port( allowed_resources, sg_id, direction, port, - sg_rule_body_list) + sg_rule_body_list, policy_namespace) if allow_all: self._create_all_pods_sg_rules( port, sg_id, direction, sg_rule_body_list, @@ -523,11 +554,21 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): cidr=cidr, namespace=namespace) sg_rule_body_list.append(rule) + if (direction == 'egress' and + CONF.octavia_defaults.enforce_sg_rules): + rule = self._create_svc_egress_sg_rule( + sg_id, policy_namespace, resource=resource) + sg_rule_body_list.extend(rule) if allow_all: rule = driver_utils.create_security_group_rule_body( sg_id, direction, port_range_min=1, port_range_max=65535) + if (direction == 'egress' and + CONF.octavia_defaults.enforce_sg_rules): + rule = self._create_svc_egress_sg_rule( + sg_id, policy_namespace) + sg_rule_body_list.extend(rule) sg_rule_body_list.append(rule) else: LOG.debug('This network policy specifies no %(direction)s ' @@ -536,6 +577,61 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver): 'rule_direction': rule_direction, 'policy': policy['metadata']['selfLink']}) + def _create_svc_egress_sg_rule(self, sg_id, policy_namespace, + resource=None, port=None, + protocol=None): + sg_rule_body_list = [] + services = driver_utils.get_services() + if not resource: + svc_subnet = utils.get_subnet_cidr( + CONF.neutron_defaults.service_subnet) + rule = driver_utils.create_security_group_rule_body( + sg_id, 'egress', port, protocol=protocol, cidr=svc_subnet) + sg_rule_body_list.append(rule) + return sg_rule_body_list + + for service in services.get('items'): + if self._is_pod(resource): + pod_labels = resource['metadata'].get('labels') + svc_selector = service['spec'].get('selector') + if not svc_selector or not pod_labels: + continue + else: + if not driver_utils.match_labels( + svc_selector, pod_labels): + continue + elif resource.get('cidr'): + # NOTE(maysams) Accounts for traffic to pods under + # a service matching an IPBlock rule. + svc_namespace = service['metadata']['namespace'] + if svc_namespace != policy_namespace: + continue + svc_selector = service['spec'].get('selector') + pods = driver_utils.get_pods({'selector': svc_selector}, + svc_namespace).get('items') + if not self._pods_in_ip_block(pods, resource): + continue + else: + ns_name = service['metadata']['namespace'] + if ns_name != resource['metadata']['name']: + continue + cluster_ip = service['spec'].get('clusterIP') + if not cluster_ip: + continue + rule = driver_utils.create_security_group_rule_body( + sg_id, 'egress', port, protocol=protocol, + cidr=cluster_ip) + sg_rule_body_list.append(rule) + return sg_rule_body_list + + def _pods_in_ip_block(self, pods, resource): + for pod in pods: + pod_ip = driver_utils.get_pod_ip(pod) + if (ipaddress.ip_address(pod_ip) + in ipaddress.ip_network(resource.get('cidr'))): + return True + return False + def parse_network_policy_rules(self, policy, sg_id): """Create security group rule bodies out of network policies. diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy.py index 64fa38fbc..2ac028490 100644 --- a/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy.py +++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy.py @@ -366,6 +366,7 @@ class TestNetworkPolicyDriver(test_base.TestCase): self.assertEqual([], resp) self.kubernetes.get.assert_called_once() + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_services') @mock.patch.object(network_policy.NetworkPolicyDriver, '_get_resource_details') @mock.patch.object(network_policy.NetworkPolicyDriver, @@ -374,7 +375,7 @@ class TestNetworkPolicyDriver(test_base.TestCase): 'create_security_group_rule_body') def test_parse_network_policy_rules_with_rules( self, m_create, m_get_namespaces, - m_get_resource_details): + m_get_resource_details, m_get_svcs): subnet_cidr = '10.10.0.0/24' namespace = 'myproject' m_get_namespaces.return_value = [get_namespace_obj()] @@ -432,6 +433,7 @@ class TestNetworkPolicyDriver(test_base.TestCase): self._driver.parse_network_policy_rules(policy, self._sg_id) m_create_sg_rule.assert_called() + @mock.patch('kuryr_kubernetes.controller.drivers.utils.get_services') @mock.patch.object(network_policy.NetworkPolicyDriver, '_get_resource_details') @mock.patch.object(network_policy.NetworkPolicyDriver, @@ -439,7 +441,8 @@ class TestNetworkPolicyDriver(test_base.TestCase): @mock.patch('kuryr_kubernetes.controller.drivers.utils.' 'create_security_group_rule_body') def test_parse_network_policy_rules_with_no_ports( - self, m_create, m_get_namespaces, m_get_resource_details): + self, m_create, m_get_namespaces, m_get_resource_details, + m_get_svcs): subnet_cidr = '10.10.0.0/24' namespace = 'myproject' m_get_namespaces.return_value = [get_namespace_obj()]