From bf848c5b2230d0edef5fd758a81767b620f5f07c Mon Sep 17 00:00:00 2001 From: Maysa Macedo Date: Wed, 16 Jan 2019 14:14:59 +0000 Subject: [PATCH] Fix CRD update when NP has namespaceSelectors To enforce the network policy isolation, the network policy security group driver must be used. Thus the code needs to be there instead of in the namespace security group driver (which is used for namespace isolation) Currently, when using the correct Network Policy drivers and handlers the CRD is not updated on events applied over namespaces that matches a NP. This commit fixes the issue by moving the support of this functionality from 'NamespacePodSecurityGroupsDriver' to NetworkPolicySecurityGroupsDriver. Closes-bug: 1811995 Partially Implements: blueprint k8s-network-policies Change-Id: Idaf70ea8cb7677296d6bea59b4d551bbb87e0422 --- .../drivers/namespace_security_groups.py | 130 +------- .../drivers/network_policy_security_groups.py | 214 ++++++++++--- .../controller/handlers/namespace.py | 3 +- .../drivers/test_namespace_security_groups.py | 140 -------- .../test_network_policy_security_groups.py | 299 +++++++++++++++--- 5 files changed, 432 insertions(+), 354 deletions(-) diff --git a/kuryr_kubernetes/controller/drivers/namespace_security_groups.py b/kuryr_kubernetes/controller/drivers/namespace_security_groups.py index 2684b7541..334c86095 100644 --- a/kuryr_kubernetes/controller/drivers/namespace_security_groups.py +++ b/kuryr_kubernetes/controller/drivers/namespace_security_groups.py @@ -21,7 +21,6 @@ from kuryr_kubernetes import clients from kuryr_kubernetes import config from kuryr_kubernetes import constants from kuryr_kubernetes.controller.drivers import base -from kuryr_kubernetes.controller.drivers import utils from kuryr_kubernetes import exceptions from neutronclient.common import exceptions as n_exc @@ -68,73 +67,6 @@ def _get_net_crd(namespace): return net_crd -def _create_sg_rule(sg_id, direction, cidr, port=None, namespace=None): - if port: - sg_rule = utils.create_security_group_rule_body( - sg_id, direction, port.get('port'), - protocol=port.get('protocol'), cidr=cidr, namespace=namespace) - else: - sg_rule = utils.create_security_group_rule_body( - sg_id, direction, port_range_min=1, - port_range_max=65535, cidr=cidr, namespace=namespace) - - sgr_id = utils.create_security_group_rule(sg_rule) - - sg_rule['security_group_rule']['id'] = sgr_id - return sg_rule - - -def _parse_rules(direction, crd, namespace): - policy = crd['spec']['networkpolicy_spec'] - sg_id = crd['spec']['securityGroupId'] - - ns_labels = namespace['metadata'].get('labels') - ns_name = namespace['metadata'].get('name') - ns_cidr = utils.get_namespace_subnet_cidr(namespace) - - rule_direction = 'from' - crd_rules = crd['spec'].get('ingressSgRules') - if direction == 'egress': - rule_direction = 'to' - crd_rules = crd['spec'].get('egressSgRules') - - matched = False - rule_list = policy.get(direction, []) - for rule_block in rule_list: - for rule in rule_block.get(rule_direction, []): - pod_selector = rule.get('podSelector') - ns_selector = rule.get('namespaceSelector') - if (ns_selector and ns_labels and - utils.match_selector(ns_selector, ns_labels)): - if pod_selector: - pods = utils.get_pods(pod_selector, ns_name) - for pod in pods.get('items'): - pod_ip = utils.get_pod_ip(pod) - if 'ports' in rule_block: - for port in rule_block['ports']: - matched = True - crd_rules.append(_create_sg_rule( - sg_id, direction, pod_ip, port=port, - namespace=ns_name)) - else: - matched = True - crd_rules.append(_create_sg_rule( - sg_id, direction, pod_ip, - namespace=ns_name)) - else: - if 'ports' in rule_block: - for port in rule_block['ports']: - matched = True - crd_rules.append(_create_sg_rule( - sg_id, direction, ns_cidr, - port=port, namespace=ns_name)) - else: - matched = True - crd_rules.append(_create_sg_rule( - sg_id, direction, ns_cidr, namespace=ns_name)) - return matched, crd_rules - - class NamespacePodSecurityGroupsDriver(base.PodSecurityGroupsDriver): """Provides security groups for Pod based on a configuration option.""" @@ -200,66 +132,16 @@ class NamespacePodSecurityGroupsDriver(base.PodSecurityGroupsDriver): raise def delete_namespace_sg_rules(self, namespace): - ns_name = namespace['metadata']['name'] - LOG.debug("Deleting sg rule for namespace: %s", - ns_name) - - knp_crds = utils.get_kuryrnetpolicy_crds() - for crd in knp_crds.get('items'): - crd_selector = crd['spec'].get('podSelector') - ingress_rule_list = crd['spec'].get('ingressSgRules') - egress_rule_list = crd['spec'].get('egressSgRules') - i_rules = [] - e_rules = [] - - matched = False - for i_rule in ingress_rule_list: - LOG.debug("Parsing ingress rule: %r", i_rule) - rule_namespace = i_rule.get('namespace', None) - - if rule_namespace and rule_namespace == ns_name: - matched = True - utils.delete_security_group_rule( - i_rule['security_group_rule']['id']) - else: - i_rules.append(i_rule) - - for e_rule in egress_rule_list: - LOG.debug("Parsing egress rule: %r", e_rule) - rule_namespace = e_rule.get('namespace', None) - - if rule_namespace and rule_namespace == ns_name: - matched = True - utils.delete_security_group_rule( - e_rule['security_group_rule']['id']) - else: - e_rules.append(e_rule) - - if matched: - utils.patch_kuryr_crd(crd, i_rules, e_rules, crd_selector) + LOG.debug("Security group driver does not create SG rules for " + "namespace.") def create_namespace_sg_rules(self, namespace): - kubernetes = clients.get_kubernetes_client() - ns_name = namespace['metadata']['name'] - LOG.debug("Creating sg rule for namespace: %s", ns_name) - namespace = kubernetes.get( - '{}/namespaces/{}'.format(constants.K8S_API_BASE, ns_name)) - knp_crds = utils.get_kuryrnetpolicy_crds() - for crd in knp_crds.get('items'): - crd_selector = crd['spec'].get('podSelector') - - i_matched, i_rules = _parse_rules('ingress', crd, namespace) - e_matched, e_rules = _parse_rules('egress', crd, namespace) - - if i_matched or e_matched: - utils.patch_kuryr_crd(crd, i_rules, - e_rules, crd_selector) + LOG.debug("Security group driver does not create SG rules for " + "namespace.") def update_namespace_sg_rules(self, namespace): - LOG.debug("Updating sg rule for namespace: %s", - namespace['metadata']['name']) - self.delete_namespace_sg_rules(namespace) - self.create_namespace_sg_rules(namespace) + LOG.debug("Security group driver does not create SG rules for " + "namespace.") def create_sg_rules(self, pod): LOG.debug("Security group driver does not create SG rules for " diff --git a/kuryr_kubernetes/controller/drivers/network_policy_security_groups.py b/kuryr_kubernetes/controller/drivers/network_policy_security_groups.py index a6e11e79c..58f6d89cf 100644 --- a/kuryr_kubernetes/controller/drivers/network_policy_security_groups.py +++ b/kuryr_kubernetes/controller/drivers/network_policy_security_groups.py @@ -44,47 +44,115 @@ def _get_namespace_labels(namespace): return namespaces['metadata'].get('labels') -def _create_sg_rules(crd, pod, pod_selector, rule_block, crd_rules, - direction, matched, namespace=None): +def _create_sg_rule(sg_id, direction, cidr, port=None, namespace=None): + if port: + sg_rule = driver_utils.create_security_group_rule_body( + sg_id, direction, port.get('port'), + protocol=port.get('protocol'), cidr=cidr, namespace=namespace) + else: + sg_rule = driver_utils.create_security_group_rule_body( + sg_id, direction, port_range_min=1, + port_range_max=65535, cidr=cidr, namespace=namespace) + + sgr_id = driver_utils.create_security_group_rule(sg_rule) + + sg_rule['security_group_rule']['id'] = sgr_id + return sg_rule + + +def _create_sg_rules(crd, pod, pod_selector, rule_block, + crd_rules, direction, matched, namespace=None): pod_labels = pod['metadata'].get('labels') # NOTE (maysams) No need to differentiate between podSelector # with empty value or with '{}', as they have same result in here. if (pod_selector and driver_utils.match_selector(pod_selector, pod_labels)): - matched = True pod_ip = driver_utils.get_pod_ip(pod) sg_id = crd['spec']['securityGroupId'] if 'ports' in rule_block: for port in rule_block['ports']: - sg_rule = driver_utils.create_security_group_rule_body( - sg_id, direction, port.get('port'), - protocol=port.get('protocol'), cidr=pod_ip, + sg_rule = _create_sg_rule( + sg_id, direction, cidr=pod_ip, port=port, namespace=namespace) - sgr_id = driver_utils.create_security_group_rule(sg_rule) - sg_rule['security_group_rule']['id'] = sgr_id crd_rules.append(sg_rule) else: - sg_rule = driver_utils.create_security_group_rule_body( - sg_id, direction, - port_range_min=1, - port_range_max=65535, - cidr=pod_ip, - namespace=namespace) - sgr_id = driver_utils.create_security_group_rule(sg_rule) - sg_rule['security_group_rule']['id'] = sgr_id + sg_rule = _create_sg_rule( + sg_id, direction, cidr=pod_ip, namespace=namespace) crd_rules.append(sg_rule) return matched -def _parse_rules(direction, crd, pod): - policy = crd['spec']['networkpolicy_spec'] - +def _parse_selectors_on_pod(crd, pod, pod_selector, namespace_selector, + rule_block, crd_rules, direction): + matched = False pod_namespace = pod['metadata']['namespace'] pod_namespace_labels = _get_namespace_labels(pod_namespace) policy_namespace = crd['metadata']['namespace'] + if namespace_selector == {}: + matched = _create_sg_rules(crd, pod, pod_selector, rule_block, + crd_rules, direction, matched) + elif namespace_selector: + if (pod_namespace_labels and + driver_utils.match_selector(namespace_selector, + pod_namespace_labels)): + matched = _create_sg_rules(crd, pod, pod_selector, + rule_block, crd_rules, + direction, matched, pod_namespace) + else: + if pod_namespace == policy_namespace: + matched = _create_sg_rules(crd, pod, pod_selector, rule_block, + crd_rules, direction, matched, + pod_namespace) + return matched, crd_rules + + +def _parse_selectors_on_namespace(crd, direction, pod_selector, + ns_selector, rule_block, crd_rules, + namespace): + ns_name = namespace['metadata'].get('name') + ns_labels = namespace['metadata'].get('labels') + sg_id = crd['spec']['securityGroupId'] + matched = False + + if (ns_selector and ns_labels and + driver_utils.match_selector(ns_selector, ns_labels)): + if pod_selector: + pods = driver_utils.get_pods(pod_selector, ns_name) + for pod in pods.get('items'): + pod_ip = driver_utils.get_pod_ip(pod) + if 'ports' in rule_block: + for port in rule_block['ports']: + matched = True + crd_rules.append(_create_sg_rule( + sg_id, direction, pod_ip, port=port, + namespace=ns_name)) + else: + matched = True + crd_rules.append(_create_sg_rule( + sg_id, direction, pod_ip, + namespace=ns_name)) + else: + ns_cidr = driver_utils.get_namespace_subnet_cidr(namespace) + if 'ports' in rule_block: + for port in rule_block['ports']: + matched = True + crd_rules.append(_create_sg_rule( + sg_id, direction, ns_cidr, + port=port, namespace=ns_name)) + else: + matched = True + crd_rules.append(_create_sg_rule( + sg_id, direction, ns_cidr, + namespace=ns_name)) + return matched, crd_rules + + +def _parse_rules(direction, crd, pod=None, namespace=None): + policy = crd['spec']['networkpolicy_spec'] + rule_direction = 'from' crd_rules = crd['spec'].get('ingressSgRules') if direction == 'egress': @@ -97,24 +165,15 @@ def _parse_rules(direction, crd, pod): for rule in rule_block.get(rule_direction, []): namespace_selector = rule.get('namespaceSelector') pod_selector = rule.get('podSelector') - if namespace_selector == {}: - if _create_sg_rules(crd, pod, pod_selector, rule_block, - crd_rules, direction, matched): - matched = True - elif namespace_selector: - if (pod_namespace_labels and - driver_utils.match_selector(namespace_selector, - pod_namespace_labels)): - if _create_sg_rules(crd, pod, pod_selector, rule_block, - crd_rules, direction, matched, - pod_namespace): - matched = True - else: - if pod_namespace == policy_namespace: - if _create_sg_rules(crd, pod, pod_selector, rule_block, - crd_rules, direction, matched, - pod_namespace): - matched = True + if pod: + matched, crd_rules = _parse_selectors_on_pod( + crd, pod, pod_selector, namespace_selector, + rule_block, crd_rules, direction) + elif namespace: + matched, crd_rules = _parse_selectors_on_namespace( + crd, direction, pod_selector, namespace_selector, + rule_block, crd_rules, namespace) + return matched, crd_rules @@ -160,8 +219,8 @@ class NetworkPolicySecurityGroupsDriver(base.PodSecurityGroupsDriver): for crd in knp_crds.get('items'): crd_selector = crd['spec'].get('podSelector') - i_matched, i_rules = _parse_rules('ingress', crd, pod) - e_matched, e_rules = _parse_rules('egress', crd, pod) + i_matched, i_rules = _parse_rules('ingress', crd, pod=pod) + e_matched, e_rules = _parse_rules('egress', crd, pod=pod) if i_matched or e_matched: driver_utils.patch_kuryr_crd(crd, i_rules, @@ -211,6 +270,71 @@ class NetworkPolicySecurityGroupsDriver(base.PodSecurityGroupsDriver): self.delete_sg_rules(pod) self.create_sg_rules(pod) + def delete_namespace_sg_rules(self, namespace): + ns_name = namespace['metadata']['name'] + LOG.debug("Deleting sg rule for namespace: %s", + ns_name) + + knp_crds = driver_utils.get_kuryrnetpolicy_crds() + for crd in knp_crds.get('items'): + crd_selector = crd['spec'].get('podSelector') + ingress_rule_list = crd['spec'].get('ingressSgRules') + egress_rule_list = crd['spec'].get('egressSgRules') + i_rules = [] + e_rules = [] + + matched = False + for i_rule in ingress_rule_list: + LOG.debug("Parsing ingress rule: %r", i_rule) + rule_namespace = i_rule.get('namespace', None) + + if rule_namespace and rule_namespace == ns_name: + matched = True + driver_utils.delete_security_group_rule( + i_rule['security_group_rule']['id']) + else: + i_rules.append(i_rule) + + for e_rule in egress_rule_list: + LOG.debug("Parsing egress rule: %r", e_rule) + rule_namespace = e_rule.get('namespace', None) + + if rule_namespace and rule_namespace == ns_name: + matched = True + driver_utils.delete_security_group_rule( + e_rule['security_group_rule']['id']) + else: + e_rules.append(e_rule) + + if matched: + driver_utils.patch_kuryr_crd( + crd, i_rules, e_rules, crd_selector) + + def create_namespace_sg_rules(self, namespace): + kubernetes = clients.get_kubernetes_client() + ns_name = namespace['metadata']['name'] + LOG.debug("Creating sg rule for namespace: %s", ns_name) + namespace = kubernetes.get( + '{}/namespaces/{}'.format(constants.K8S_API_BASE, ns_name)) + knp_crds = driver_utils.get_kuryrnetpolicy_crds() + for crd in knp_crds.get('items'): + crd_selector = crd['spec'].get('podSelector') + + i_matched, i_rules = _parse_rules( + 'ingress', crd, namespace=namespace) + e_matched, e_rules = _parse_rules( + 'egress', crd, namespace=namespace) + + if i_matched or e_matched: + driver_utils.patch_kuryr_crd(crd, i_rules, + e_rules, crd_selector) + + def update_namespace_sg_rules(self, namespace): + LOG.debug("Updating sg rule for namespace: %s", + namespace['metadata']['name']) + self.delete_namespace_sg_rules(namespace) + self.create_namespace_sg_rules(namespace) + def create_namespace_sg(self, namespace, project_id, crd_spec): LOG.debug("Security group driver does not create SGs for the " "namespaces.") @@ -220,18 +344,6 @@ class NetworkPolicySecurityGroupsDriver(base.PodSecurityGroupsDriver): LOG.debug("Security group driver does not implement deleting " "SGs.") - def delete_namespace_sg_rules(self, namespace): - LOG.debug("Security group driver does not delete SG rules for " - "namespace.") - - def create_namespace_sg_rules(self, namespace): - LOG.debug("Security group driver does not create SG rules for " - "namespace.") - - def update_namespace_sg_rules(self, namespace): - LOG.debug("Security group driver does not update SG rules for " - "namespace.") - class NetworkPolicyServiceSecurityGroupsDriver( base.ServiceSecurityGroupsDriver): diff --git a/kuryr_kubernetes/controller/handlers/namespace.py b/kuryr_kubernetes/controller/handlers/namespace.py index d5d82bfb3..f78de2307 100644 --- a/kuryr_kubernetes/controller/handlers/namespace.py +++ b/kuryr_kubernetes/controller/handlers/namespace.py @@ -65,8 +65,7 @@ class NamespaceHandler(k8s_base.ResourceEventHandler): LOG.debug("Got previous namespace labels from annotation: %r", previous_namespace_labels) - if (previous_namespace_labels and - current_namespace_labels != previous_namespace_labels): + if current_namespace_labels != previous_namespace_labels: self._drv_sg.update_namespace_sg_rules(namespace) self._set_namespace_labels(namespace, current_namespace_labels) diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_namespace_security_groups.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_namespace_security_groups.py index 3ccf8611c..400438db3 100644 --- a/kuryr_kubernetes/tests/unit/controller/drivers/test_namespace_security_groups.py +++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_namespace_security_groups.py @@ -290,143 +290,3 @@ class TestNamespacePodSecurityGroupsDriver(test_base.TestCase): cls.delete_sg(m_driver, sg_id) neutron.delete_security_group.assert_called_once_with(sg_id) - - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'patch_kuryr_crd') - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'delete_security_group_rule') - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'get_kuryrnetpolicy_crds') - def test_delete_namespace_sg_rule(self, m_get_knp_crd, m_delete_sg_rule, - m_patch_kuryr_crd): - cls = namespace_security_groups.NamespacePodSecurityGroupsDriver - m_driver = mock.MagicMock(spec=cls) - i_rule = get_matched_crd_obj()['spec']['ingressSgRules'][0] - sg_rule_id = i_rule.get('security_group_rule')['id'] - - m_get_knp_crd.return_value = {"items": [get_matched_crd_obj()]} - - cls.delete_namespace_sg_rules(m_driver, get_match_crd_namespace_obj()) - - m_get_knp_crd.assert_called_once() - m_delete_sg_rule.assert_called_once_with(sg_rule_id) - m_patch_kuryr_crd.assert_called_once() - - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'patch_kuryr_crd') - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'delete_security_group_rule') - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'get_kuryrnetpolicy_crds') - def test_delete_namespace_sg_rule_no_match(self, m_get_knp_crd, - m_delete_sg_rule, - m_patch_kuryr_crd): - cls = namespace_security_groups.NamespacePodSecurityGroupsDriver - m_driver = mock.MagicMock(spec=cls) - - m_get_knp_crd.return_value = {"items": [get_matched_crd_obj()]} - - cls.delete_namespace_sg_rules(m_driver, - get_no_match_crd_namespace_obj()) - - m_get_knp_crd.assert_called_once() - m_delete_sg_rule.assert_not_called() - m_patch_kuryr_crd.assert_not_called() - - @mock.patch('kuryr_kubernetes.controller.drivers.' - 'namespace_security_groups._create_sg_rule') - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'match_selector') - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'get_namespace_subnet_cidr') - def test__parse_rules(self, m_get_ns_subnet_cidr, m_match_selector, - m_create_sg_rule): - crd = get_crd_obj_no_match() - policy = crd['spec']['networkpolicy_spec'] - i_rule = policy.get('ingress')[0] - ns_selector = i_rule['from'][0].get('namespaceSelector') - ns = get_match_crd_namespace_obj() - - m_get_ns_subnet_cidr.return_value = '10.0.2.0/26' - m_match_selector.return_value = True - m_create_sg_rule.return_value = get_sg_rule() - - matched, rules = namespace_security_groups._parse_rules('ingress', - crd, ns) - - m_get_ns_subnet_cidr.assert_called_once_with(ns) - m_match_selector.assert_called_once_with(ns_selector, - ns['metadata']['labels']) - m_create_sg_rule.assert_called_once() - - self.assertEqual(matched, True) - self.assertEqual(rules, [get_sg_rule()]) - - @mock.patch('kuryr_kubernetes.controller.drivers.' - 'namespace_security_groups._create_sg_rule') - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'match_selector') - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'get_namespace_subnet_cidr') - def test__parse_rules_no_match(self, m_get_ns_subnet_cidr, - m_match_selector, m_create_sg_rule): - crd = get_crd_obj_no_match() - policy = crd['spec']['networkpolicy_spec'] - i_rule = policy.get('ingress')[0] - ns_selector = i_rule['from'][0].get('namespaceSelector') - ns = get_no_match_crd_namespace_obj() - - m_get_ns_subnet_cidr.return_value = '10.0.2.0/26' - m_match_selector.return_value = False - - matched, rules = namespace_security_groups._parse_rules('ingress', - crd, ns) - - m_get_ns_subnet_cidr.assert_called_once_with(ns) - m_match_selector.assert_called_once_with(ns_selector, - ns['metadata']['labels']) - m_create_sg_rule.assert_not_called() - - self.assertEqual(matched, False) - self.assertEqual(rules, []) - - @mock.patch('kuryr_kubernetes.controller.drivers.' - 'namespace_security_groups._create_sg_rule') - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'get_pod_ip') - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'get_pods') - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'match_selector') - @mock.patch('kuryr_kubernetes.controller.drivers.utils.' - 'get_namespace_subnet_cidr') - def test__parse_rules_all_selectors(self, m_get_ns_subnet_cidr, - m_match_selector, m_get_pods, - m_get_pod_ip, m_create_sg_rule): - crd = get_crd_obj_with_all_selectors() - policy = crd['spec']['networkpolicy_spec'] - i_rule = policy.get('ingress')[0] - ns_selector = i_rule['from'][0].get('namespaceSelector') - pod_selector = i_rule['from'][0].get('podSelector') - ns = get_match_crd_namespace_obj() - pod = get_match_crd_pod_obj() - - m_get_ns_subnet_cidr.return_value = '10.0.2.0/26' - m_match_selector.return_value = True - m_get_pods.return_value = {"items": [pod]} - m_get_pod_ip.return_value = pod['status']['podIP'] - m_create_sg_rule.return_value = get_sg_rule() - - matched, rules = namespace_security_groups._parse_rules('ingress', - crd, ns) - - m_get_ns_subnet_cidr.assert_called_once_with(ns) - m_match_selector.assert_called_once_with(ns_selector, - ns['metadata']['labels']) - m_get_pods.assert_called_once_with(pod_selector, - ns['metadata']['name']) - m_get_pod_ip.assert_called_once_with(pod) - m_create_sg_rule.assert_called_once() - - self.assertEqual(matched, True) - self.assertEqual(rules, [get_sg_rule()]) diff --git a/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy_security_groups.py b/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy_security_groups.py index 69de89e3e..e4cea1873 100644 --- a/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy_security_groups.py +++ b/kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy_security_groups.py @@ -21,6 +21,131 @@ from kuryr_kubernetes.tests.unit import kuryr_fixtures as k_fix from oslo_config import cfg +def get_no_match_crd_namespace_obj(): + return { + "kind": "Namespace", + "metadata": { + "annotations": { + "openstack.org/kuryr-namespace-label": '{"name": "dev"}', + "openstack.org/kuryr-net-crd": "ns-dev" + }, + "labels": {"name": "prod"}, + "name": "prod", + "selfLink": "/api/v1/namespaces/dev"}} + + +def get_match_crd_namespace_obj(): + return { + "kind": "Namespace", + "metadata": { + "annotations": { + "openstack.org/kuryr-namespace-label": '{"name": "dev"}', + "openstack.org/kuryr-net-crd": "ns-dev" + }, + "labels": { + "name": "dev" + }, + "name": "dev", + "selfLink": "/api/v1/namespaces/dev"}} + + +def get_match_crd_pod_obj(): + return { + 'kind': 'Pod', + 'metadata': { + 'name': mock.sentinel.pod_name, + 'namespace': 'dev', + 'labels': { + 'tier': 'backend'}, + 'annotations': { + 'openstack.org/kuryr-pod-label': '{"tier": "backend"}'}}, + 'status': {'podIP': mock.sentinel.podIP}} + + +def get_sg_rule(): + pod_ip = get_match_crd_pod_obj()['status'].get('podIP') + return { + "namespace": 'dev', + "security_group_rule": { + "description": "Kuryr-Kubernetes NetPolicy SG rule", + "direction": "ingress", + "ethertype": "IPv4", + "id": 'f15ff50a-e8a4-4872-81bf-a04cbb8cb388', + "port_range_max": 6379, + "port_range_min": 6379, + "protocol": "tcp", + "remote_ip_prefix": pod_ip, + "security_group_id": '36923e76-026c-422b-8dfd-7292e7c88228'}} + + +def get_matched_crd_obj(): + return { + "kind": "KuryrNetPolicy", + "metadata": {"name": "np-test-network-policy", + "namespace": "default"}, + "spec": { + "egressSgRules": [], + "ingressSgRules": [get_sg_rule()], + "networkpolicy_spec": { + "ingress": [ + {"from": [ + {"namespaceSelector": { + "matchLabels": {"name": "dev"}}}], + "ports": [ + {"port": 6379, + "protocol": "TCP"}]}], + "podSelector": {"matchLabels": {"app": "demo"}}, + "policyTypes": ["Ingress"]}, + "podSelector": {"matchLabels": {"app": "demo"}}, + "securityGroupId": '36923e76-026c-422b-8dfd-7292e7c88228'}} + + +def get_crd_obj_no_match(): + return { + "kind": "KuryrNetPolicy", + "metadata": {"name": "np-test-network-policy", + "namespace": "default"}, + "spec": { + "egressSgRules": [], + "ingressSgRules": [], + "networkpolicy_spec": { + "ingress": [ + {"from": [ + {"namespaceSelector": { + "matchLabels": {"name": "dev"}}}], + "ports": [ + {"port": 6379, + "protocol": "TCP"}]}], + "podSelector": {"matchLabels": {"app": "demo"}}, + "policyTypes": ["Ingress"]}, + "podSelector": {"matchLabels": {"app": "demo"}}, + "securityGroupId": '36923e76-026c-422b-8dfd-7292e7c88228'}} + + +def get_crd_obj_with_all_selectors(): + return { + "kind": "KuryrNetPolicy", + "metadata": {"name": "np-test-network-policy", + "namespace": "default"}, + "spec": { + "egressSgRules": [], + "ingressSgRules": [], + "networkpolicy_spec": { + "ingress": [ + {"from": [ + {"namespaceSelector": { + "matchLabels": {"name": "dev"}}, + "podSelector": { + "matchLabels": {"tier": "backend"}}}], + "ports": [ + {"port": 6379, + "protocol": "TCP"}]}], + "podSelector": {"matchLabels": {"app": "demo"}}, + "policyTypes": ["Ingress"]}, + "podSelector": {"matchLabels": {"app": "demo"}}, + "securityGroupId": '36923e76-026c-422b-8dfd-7292e7c88228'}} + + class TestNetworkPolicySecurityGroupsDriver(test_base.TestCase): def setUp(self): @@ -103,14 +228,6 @@ class TestNetworkPolicySecurityGroupsDriver(test_base.TestCase): "resourceVersion": "", "selfLink": mock.sentinel.selfLink}} - self._empty_crds = { - "apiVersion": "v1", - "items": [], - "kind": "List", - "metadata": { - "resourceVersion": "", - "selfLink": mock.sentinel.selfLink}} - self._pod = { 'apiVersion': 'v1', 'kind': 'Pod', @@ -166,30 +283,6 @@ class TestNetworkPolicySecurityGroupsDriver(test_base.TestCase): self._driver = ( network_policy_security_groups.NetworkPolicySecurityGroupsDriver()) - self._crd_sg_id = mock.sentinel.crd_sg_id - self._crd_without_rules = { - "apiVersion": "openstack.org/v1", - "kind": "KuryrNetPolicy", - "metadata": {"name": "np-test-network-policy", - "namespace": "default"}, - "spec": { - "egressSgRules": [], - "ingressSgRules": [], - "networkpolicy_spec": { - "ingress": [ - {"from": [ - {"namespaceSelector": { - "matchLabels": {"name": "dev"}}, - "podSelector": { - "matchLabels": {"tier": "backend"}}}], - "ports": [ - {"port": 6379, - "protocol": "TCP"}]}], - "podSelector": {"matchLabels": {"app": "demo"}}, - "policyTypes": ["Ingress"]}, - "podSelector": {"matchLabels": {"app": "demo"}}, - "securityGroupId": self._crd_sg_id}} - self._pod_ip = mock.sentinel.pod_ip self._pod_dev_namespace = { 'apiVersion': 'v1', @@ -209,6 +302,7 @@ class TestNetworkPolicySecurityGroupsDriver(test_base.TestCase): }]}, 'status': {'podIP': self._pod_ip}} + self._crd_sg_id = mock.sentinel.crd_sg_id self._sg_rule_body = { u'security_group_rule': { u'direction': 'ingress', @@ -265,12 +359,12 @@ class TestNetworkPolicySecurityGroupsDriver(test_base.TestCase): m_match_selector, m_create_sg_rule_body, m_create_sg_rule): - m_get_pod_ip.return_value = self._pod_ip m_create_sg_rule_body.return_value = self._sg_rule_body sgr_id = mock.sentinel.sgr_id m_create_sg_rule.return_value = sgr_id - crd = self._crd_without_rules - pod = self._pod_dev_namespace + crd = get_crd_obj_with_all_selectors() + pod = get_match_crd_pod_obj() + m_get_pod_ip.return_value = pod['status'].get('podIP') matched = False new_sg_rule = self._sg_rule_body @@ -298,7 +392,7 @@ class TestNetworkPolicySecurityGroupsDriver(test_base.TestCase): @mock.patch('kuryr_kubernetes.controller.drivers.utils.' 'match_selector', return_value=False) def test__create_sg_rules_no_match(self, m_match_selector): - crd = self._crd_without_rules + crd = get_crd_obj_with_all_selectors() pod = self._pod2 policy = crd['spec']['networkpolicy_spec'] @@ -408,7 +502,7 @@ class TestNetworkPolicySecurityGroupsDriver(test_base.TestCase): @mock.patch('kuryr_kubernetes.controller.drivers.utils.' 'get_kuryrnetpolicy_crds') def test_get_sgs_no_crds(self, m_get_crds): - m_get_crds.return_value = self._empty_crds + m_get_crds.return_value = {"items": []} cfg.CONF.set_override('pod_security_groups', [], group='neutron_defaults') @@ -433,3 +527,134 @@ class TestNetworkPolicySecurityGroupsDriver(test_base.TestCase): m_get_crds.assert_called_once_with(namespace=self._namespace) self.assertEqual([str(self._sg_id), str(self._sg_id2)], resp) + + @mock.patch('kuryr_kubernetes.controller.drivers.utils.' + 'patch_kuryr_crd') + @mock.patch('kuryr_kubernetes.controller.drivers.utils.' + 'delete_security_group_rule') + @mock.patch('kuryr_kubernetes.controller.drivers.utils.' + 'get_kuryrnetpolicy_crds') + def test_delete_namespace_sg_rule(self, m_get_knp_crd, m_delete_sg_rule, + m_patch_kuryr_crd): + cls = network_policy_security_groups.NetworkPolicySecurityGroupsDriver + m_driver = mock.MagicMock(spec=cls) + i_rule = get_matched_crd_obj()['spec']['ingressSgRules'][0] + sg_rule_id = i_rule.get('security_group_rule')['id'] + + m_get_knp_crd.return_value = {"items": [get_matched_crd_obj()]} + + cls.delete_namespace_sg_rules(m_driver, get_match_crd_namespace_obj()) + + m_get_knp_crd.assert_called_once() + m_delete_sg_rule.assert_called_once_with(sg_rule_id) + m_patch_kuryr_crd.assert_called_once() + + @mock.patch('kuryr_kubernetes.controller.drivers.utils.' + 'patch_kuryr_crd') + @mock.patch('kuryr_kubernetes.controller.drivers.utils.' + 'delete_security_group_rule') + @mock.patch('kuryr_kubernetes.controller.drivers.utils.' + 'get_kuryrnetpolicy_crds') + def test_delete_namespace_sg_rule_no_match(self, m_get_knp_crd, + m_delete_sg_rule, + m_patch_kuryr_crd): + cls = network_policy_security_groups.NetworkPolicySecurityGroupsDriver + m_driver = mock.MagicMock(spec=cls) + + m_get_knp_crd.return_value = {"items": [get_matched_crd_obj()]} + + cls.delete_namespace_sg_rules(m_driver, + get_no_match_crd_namespace_obj()) + + m_get_knp_crd.assert_called_once() + m_delete_sg_rule.assert_not_called() + m_patch_kuryr_crd.assert_not_called() + + @mock.patch('kuryr_kubernetes.controller.drivers.' + 'network_policy_security_groups._create_sg_rule') + @mock.patch('kuryr_kubernetes.controller.drivers.utils.' + 'match_selector') + @mock.patch('kuryr_kubernetes.controller.drivers.utils.' + 'get_namespace_subnet_cidr') + def test__parse_rules(self, m_get_ns_subnet_cidr, m_match_selector, + m_create_sg_rule): + crd = get_crd_obj_no_match() + policy = crd['spec']['networkpolicy_spec'] + i_rule = policy.get('ingress')[0] + ns_selector = i_rule['from'][0].get('namespaceSelector') + ns = get_match_crd_namespace_obj() + + m_get_ns_subnet_cidr.return_value = '10.0.2.0/26' + m_match_selector.return_value = True + m_create_sg_rule.return_value = get_sg_rule() + + matched, rules = network_policy_security_groups._parse_rules( + 'ingress', crd, namespace=ns) + + m_get_ns_subnet_cidr.assert_called_once_with(ns) + m_match_selector.assert_called_once_with(ns_selector, + ns['metadata']['labels']) + m_create_sg_rule.assert_called_once() + + self.assertEqual(matched, True) + self.assertEqual(rules, [get_sg_rule()]) + + @mock.patch('kuryr_kubernetes.controller.drivers.' + 'network_policy_security_groups._create_sg_rule') + @mock.patch('kuryr_kubernetes.controller.drivers.utils.' + 'match_selector') + def test__parse_rules_no_match(self, m_match_selector, + m_create_sg_rule): + crd = get_crd_obj_no_match() + policy = crd['spec']['networkpolicy_spec'] + i_rule = policy.get('ingress')[0] + ns_selector = i_rule['from'][0].get('namespaceSelector') + ns = get_no_match_crd_namespace_obj() + + m_match_selector.return_value = False + + matched, rules = network_policy_security_groups._parse_rules( + 'ingress', crd, namespace=ns) + + m_match_selector.assert_called_once_with(ns_selector, + ns['metadata']['labels']) + m_create_sg_rule.assert_not_called() + + self.assertEqual(matched, False) + self.assertEqual(rules, []) + + @mock.patch('kuryr_kubernetes.controller.drivers.' + 'network_policy_security_groups._create_sg_rule') + @mock.patch('kuryr_kubernetes.controller.drivers.utils.' + 'get_pod_ip') + @mock.patch('kuryr_kubernetes.controller.drivers.utils.' + 'get_pods') + @mock.patch('kuryr_kubernetes.controller.drivers.utils.' + 'match_selector') + def test__parse_rules_all_selectors(self, m_match_selector, m_get_pods, + m_get_pod_ip, m_create_sg_rule): + crd = get_crd_obj_with_all_selectors() + policy = crd['spec']['networkpolicy_spec'] + i_rule = policy.get('ingress')[0] + ns_selector = i_rule['from'][0].get('namespaceSelector') + pod_selector = i_rule['from'][0].get('podSelector') + ns = get_match_crd_namespace_obj() + pod = get_match_crd_pod_obj() + + m_match_selector.return_value = True + m_get_pods.return_value = {"items": [pod]} + m_get_pod_ip.return_value = pod['status']['podIP'] + m_create_sg_rule.return_value = get_sg_rule() + + matched, rules = network_policy_security_groups._parse_rules( + 'ingress', crd, namespace=ns) + + m_match_selector.assert_called_once_with(ns_selector, + ns['metadata']['labels']) + m_get_pods.assert_called_once_with(pod_selector, + ns['metadata']['name']) + m_get_pod_ip.assert_called_once_with(pod) + m_create_sg_rule.assert_called_once() + + self.assertEqual(matched, True) + self.assertEqual(rules, [get_sg_rule()])