openstack
/
kuryr-kubernetes
Code Issues Proposed changes

Fix duplicated sg rules on NP crd 

While handling the creation of a Network
Policy it's possible that the CRD is patched
with repeated sg rules, which is not allowed
resulting in validation error as the repeated
sg rules will not have the sg rule id.

Closes-Bug: #1887167
Change-Id: Ia7814ddcea0d6948ff280a3e03a896bbc442891c
This commit is contained in:
Maysa Macedo 2020-07-09 22:21:57 +00:00
parent 58617a9b54
commit abc679c9f2
2 changed files with 34 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
kuryr_kubernetes/controller/drivers/network_policy.py
View File

@ -314,11 +314,10 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver):
if sg_rule not in crd_rules:
crd_rules.append(sg_rule)
if direction == 'egress':
rules = self._create_svc_egress_sg_rule(
sg_id, policy_namespace, resource=resource,
port=container_port,
self._create_svc_egress_sg_rule(
sg_id, policy_namespace, crd_rules,
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,
@ -378,10 +377,9 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver):
pods=pods)
crd_rules.append(sg_rule)
if direction == 'egress':
rules = self._create_svc_egress_sg_rule(
sg_id, policy_namespace, port=container_port,
protocol=port.get('protocol'))
crd_rules.extend(rules)
self._create_svc_egress_sg_rule(
sg_id, policy_namespace, crd_rules,
port=container_port, protocol=port.get('protocol'))
def _create_sg_rule_on_number_port(self, allowed_resources, sg_id,
direction, port, sg_rule_body_list,
@ -401,10 +399,10 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver):
namespace=ns))
sg_rule_body_list.append(sg_rule)
if direction == 'egress':
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)
self._create_svc_egress_sg_rule(
sg_id, policy_namespace, sg_rule_body_list,
resource=resource, port=port.get('port'),
protocol=port.get('protocol'))
def _create_all_pods_sg_rules(self, port, sg_id, direction,
sg_rule_body_list, pod_selector,
@ -424,10 +422,10 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver):
protocol=port.get('protocol')))
sg_rule_body_list.append(sg_rule)
if direction == 'egress':
rule = self._create_svc_egress_sg_rule(
sg_id, policy_namespace, port=port.get('port'),
self._create_svc_egress_sg_rule(
sg_id, policy_namespace, sg_rule_body_list,
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):
for ethertype in (constants.IPv4, constants.IPv6):
@ -553,8 +551,8 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver):
sg_rule_body_list.append(rule)
if direction == 'egress':
rule = self._create_svc_egress_sg_rule(
sg_id, policy_namespace, resource=resource)
sg_rule_body_list.extend(rule)
sg_id, policy_namespace, sg_rule_body_list,
resource=resource)
if allow_all:
for ethertype in (constants.IPv4, constants.IPv6):
rule = driver_utils.create_security_group_rule_body(
@ -564,9 +562,8 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver):
ethertype=ethertype)
sg_rule_body_list.append(rule)
if direction == 'egress':
rule = self._create_svc_egress_sg_rule(
sg_id, policy_namespace)
sg_rule_body_list.extend(rule)
self._create_svc_egress_sg_rule(
sg_id, policy_namespace, sg_rule_body_list)
else:
LOG.debug('This network policy specifies no %(direction)s '
'%(rule_direction)s and no ports: %(policy)s',
@ -575,17 +572,17 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver):
'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 = []
sg_rule_body_list, resource=None,
port=None, protocol=None):
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
if rule not in sg_rule_body_list:
sg_rule_body_list.append(rule)
return
for service in services.get('items'):
if self._is_pod(resource):
@ -618,8 +615,8 @@ class NetworkPolicyDriver(base.NetworkPolicyDriver):
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
if rule not in sg_rule_body_list:
sg_rule_body_list.append(rule)
def _pods_in_ip_block(self, pods, resource):
for pod in pods:

13
kuryr_kubernetes/tests/unit/controller/drivers/test_network_policy.py
View File

@ -699,6 +699,7 @@ class TestNetworkPolicyDriver(test_base.TestCase):
cidr=mock.ANY, protocol='TCP')
self.assertEqual(len(crd_rules), 1)
@mock.patch('kuryr_kubernetes.utils.get_subnet_cidr')
@mock.patch('kuryr_kubernetes.controller.drivers.utils.'
'create_security_group_rule_body')
@mock.patch('kuryr_kubernetes.controller.drivers.utils.get_ports')
@ -706,7 +707,8 @@ class TestNetworkPolicyDriver(test_base.TestCase):
def test__create_sg_rule_body_on_text_port_egress_match(self,
m_get_pods,
m_get_ports,
m_create_sgr):
m_create_sgr,
m_get_subnet_cidr):
def _create_sgr_cont(container_ports, allow_all, resource,
matched_pods, crd_rules, sg_id, direction, port,
@ -722,6 +724,9 @@ class TestNetworkPolicyDriver(test_base.TestCase):
namespace = mock.sentinel.namespace
direction = 'egress'
self._driver._create_sg_rules_with_container_ports = _create_sgr_cont
m_get_subnet_cidr.return_value = '10.0.0.128/26'
m_create_sgr.side_effect = [mock.sentinel.sgr1, mock.sentinel.sgr2,
mock.sentinel.sgr3]
m_get_pods.return_value = {'items': [pod]}
m_get_ports.return_value = container_ports
@ -736,12 +741,14 @@ class TestNetworkPolicyDriver(test_base.TestCase):
allow_all=True)
m_get_ports.assert_called_with(resources[0], port)
calls = [mock.call(self._sg_id, direction, container_ports[0][1],
protocol=port['protocol'], ethertype=e,
pods='foo') for e in ('IPv4', 'IPv6')]
calls.append(mock.call(self._sg_id, direction, container_ports[0][1],
protocol=port['protocol'],
cidr='10.0.0.128/26'))
m_create_sgr.assert_has_calls(calls)
# NOTE(gryf): there are 3 rules created in case of egress direction,
# since additional one is created for specific cidr in service subnet.
self.assertEqual(len(crd_rules), 3)