Browse Source

Merge "[ovn] Use normalized remote prefix IPs in OVN driver" into stable/train

changes/66/787966/1
Zuul 4 weeks ago
committed by Gerrit Code Review
parent
commit
e71c524b0b
4 changed files with 143 additions and 7 deletions
  1. +17
    -4
      networking_ovn/common/acl.py
  2. +31
    -0
      networking_ovn/ml2/mech_driver.py
  3. +13
    -0
      networking_ovn/tests/unit/common/test_acl.py
  4. +82
    -3
      networking_ovn/tests/unit/ml2/test_mech_driver.py

+ 17
- 4
networking_ovn/common/acl.py View File

@ -12,14 +12,19 @@
# under the License.
#
import netaddr
from neutron_lib import constants as const
from neutron_lib import exceptions as n_exceptions
from oslo_config import cfg
from oslo_log import log as logging
from networking_ovn._i18n import _
from networking_ovn.common import constants as ovn_const
from networking_ovn.common import utils
LOG = logging.getLogger(__name__)
# Convert the protocol number from integer to strings because that's
# how Neutron will pass it to us
PROTOCOL_NAME_TO_NUM_MAP = {k: str(v) for k, v in
@ -84,9 +89,17 @@ def acl_ethertype(r):
def acl_remote_ip_prefix(r, ip_version):
if not r['remote_ip_prefix']:
return ''
cidr = netaddr.IPNetwork(r['remote_ip_prefix'])
normalized_ip_prefix = "%s/%s" % (cidr.network, cidr.prefixlen)
if r['remote_ip_prefix'] != normalized_ip_prefix:
LOG.info("remote_ip_prefix %(remote_ip_prefix)s configured in "
"rule %(rule_id)s is not normalized. Normalized CIDR "
"%(normalized_ip_prefix)s will be used instead.",
{'remote_ip_prefix': r['remote_ip_prefix'],
'rule_id': r['id'],
'normalized_ip_prefix': normalized_ip_prefix})
src_or_dst = 'src' if r['direction'] == 'ingress' else 'dst'
return ' && %s.%s == %s' % (ip_version, src_or_dst,
r['remote_ip_prefix'])
return ' && %s.%s == %s' % (ip_version, src_or_dst, normalized_ip_prefix)
def _get_protocol_number(protocol):
@ -318,7 +331,7 @@ def add_acls_for_sg_port_group(ovn, security_group, txn):
for r in security_group['security_group_rules']:
acl = _add_sg_rule_acl_for_port_group(
utils.ovn_port_group_name(security_group['id']), r)
txn.add(ovn.pg_acl_add(**acl))
txn.add(ovn.pg_acl_add(may_exist=True, **acl))
def update_acls_for_security_group(plugin,
@ -343,7 +356,7 @@ def update_acls_for_security_group(plugin,
if not keep_name_severity:
acl.pop('name')
acl.pop('severity')
ovn.pg_acl_add(**acl).execute(check_error=True)
ovn.pg_acl_add(may_exist=True, **acl).execute(check_error=True)
else:
ovn.pg_acl_del(acl['port_group'], acl['direction'],
acl['priority'], acl['match']).execute(


+ 31
- 0
networking_ovn/ml2/mech_driver.py View File

@ -21,6 +21,7 @@ import signal
import threading
import types
import netaddr
from neutron.common import utils as n_utils
from neutron.db import provisioning_blocks
from neutron.plugins.ml2 import db as ml2_db
@ -317,8 +318,38 @@ class OVNMechanismDriver(api.MechanismDriver):
admin_context = n_context.get_admin_context()
sg_rule = self._plugin.get_security_group_rule(
admin_context, kwargs.get('security_group_rule_id'))
if sg_rule.get('remote_ip_prefix') is not None:
if self._sg_has_rules_with_same_normalized_cidr(sg_rule):
return
self._ovn_client.delete_security_group_rule(sg_rule)
def _sg_has_rules_with_same_normalized_cidr(self, sg_rule):
compare_keys = [
'ethertype', 'direction', 'protocol',
'port_range_min', 'port_range_max']
sg_rules = self._plugin.get_security_group_rules(
n_context.get_admin_context(),
{'security_group_id': [sg_rule['security_group_id']]})
cidr_sg_rule = netaddr.IPNetwork(sg_rule['remote_ip_prefix'])
normalized_sg_rule_prefix = "%s/%s" % (cidr_sg_rule.network,
cidr_sg_rule.prefixlen)
def _rules_equal(rule1, rule2):
return not any(
rule1.get(key) != rule2.get(key) for key in compare_keys)
for rule in sg_rules:
if not rule.get('remote_ip_prefix') or rule['id'] == sg_rule['id']:
continue
cidr_rule = netaddr.IPNetwork(rule['remote_ip_prefix'])
normalized_rule_prefix = "%s/%s" % (cidr_rule.network,
cidr_rule.prefixlen)
if normalized_sg_rule_prefix != normalized_rule_prefix:
continue
if _rules_equal(sg_rule, rule):
return True
return False
def _is_network_type_supported(self, network_type):
return (network_type in [const.TYPE_LOCAL,
const.TYPE_FLAT,


+ 13
- 0
networking_ovn/tests/unit/common/test_acl.py View File

@ -540,6 +540,19 @@ class TestACLs(base.TestCase):
expected_match = ' && %s.dst == %s' % (ip_version, remote_ip_prefix)
self.assertEqual(expected_match, match)
def test_acl_remote_ip_prefix_not_normalized(self):
sg_rule = fakes.FakeSecurityGroupRule.create_one_security_group_rule({
'direction': 'ingress',
'remote_ip_prefix': '10.10.10.175/26'
}).info()
normalized_ip_prefix = '10.10.10.128/26'
ip_version = 'ip4'
match = ovn_acl.acl_remote_ip_prefix(sg_rule, ip_version)
expected_match = ' && %s.src == %s' % (ip_version,
normalized_ip_prefix)
self.assertEqual(expected_match, match)
def test_acl_remote_group_id(self):
sg_rule = fakes.FakeSecurityGroupRule.create_one_security_group_rule({
'direction': 'ingress',


+ 82
- 3
networking_ovn/tests/unit/ml2/test_mech_driver.py View File

@ -19,6 +19,7 @@ import uuid
import mock
from neutron.common import utils as n_utils
from neutron.db import provisioning_blocks
from neutron.db import securitygroups_db
from neutron.db import segments_db
from neutron.plugins.ml2.drivers import type_geneve # noqa
from neutron.services.revisions import revision_plugin
@ -136,13 +137,38 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
@mock.patch.object(db_rev, 'bump_revision')
def test__process_sg_rule_notifications_sgr_create(self, mock_bump):
with mock.patch(
'networking_ovn.common.acl.update_acls_for_security_group'
) as ovn_acl_up:
with mock.patch.object(
self.mech_driver,
'_sg_has_rules_with_same_normalized_cidr') as has_same_rules, \
mock.patch(
'networking_ovn.common.acl.update_acls_for_security_group'
) as ovn_acl_up:
rule = {'security_group_id': 'sg_id'}
self.mech_driver._process_sg_rule_notification(
resources.SECURITY_GROUP_RULE, events.AFTER_CREATE, {},
security_group_rule=rule)
has_same_rules.assert_not_called()
ovn_acl_up.assert_called_once_with(
mock.ANY, mock.ANY, mock.ANY,
'sg_id', rule, is_add_acl=True)
mock_bump.assert_called_once_with(
rule, ovn_const.TYPE_SECURITY_GROUP_RULES)
@mock.patch.object(db_rev, 'bump_revision')
def test__process_sg_rule_notifications_sgr_create_with_remote_ip_prefix(
self, mock_bump):
with mock.patch.object(
self.mech_driver,
'_sg_has_rules_with_same_normalized_cidr') as has_same_rules, \
mock.patch(
'networking_ovn.common.acl.update_acls_for_security_group'
) as ovn_acl_up:
rule = {'security_group_id': 'sg_id',
'remote_ip_prefix': '1.0.0.0/24'}
self.mech_driver._process_sg_rule_notification(
resources.SECURITY_GROUP_RULE, events.AFTER_CREATE, {},
security_group_rule=rule, context=self.context)
has_same_rules.assert_not_called()
ovn_acl_up.assert_called_once_with(
mock.ANY, mock.ANY, mock.ANY,
'sg_id', rule, is_add_acl=True)
@ -169,6 +195,59 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
mock_delrev.assert_called_once_with(
rule['id'], ovn_const.TYPE_SECURITY_GROUP_RULES)
def test__sg_has_rules_with_same_normalized_cidr(self):
scenarios = [
({'id': 'rule-id', 'security_group_id': 'sec-group-uuid',
'remote_ip_prefix': '10.10.10.175/26',
'protocol': 'tcp'}, False),
({'id': 'rule-id', 'security_group_id': 'sec-group-uuid',
'remote_ip_prefix': '10.10.10.175/26',
'protocol': 'udp'}, False),
({'id': 'rule-id', 'security_group_id': 'sec-group-uuid',
'remote_ip_prefix': '10.10.10.175/26',
'protocol': 'tcp'}, False),
({'id': 'rule-id', 'security_group_id': 'sec-group-uuid',
'remote_ip_prefix': '10.10.10.175/26',
'protocol': 'tcp',
'port_range_min': '2000', 'port_range_max': '2100'}, False),
({'id': 'rule-id', 'security_group_id': 'sec-group-uuid',
'remote_ip_prefix': '192.168.0.0/24',
'protocol': 'tcp',
'port_range_min': '2000', 'port_range_max': '3000',
'direction': 'ingress'}, False),
({'id': 'rule-id', 'security_group_id': 'sec-group-uuid',
'remote_ip_prefix': '10.10.10.175/26',
'protocol': 'tcp',
'port_range_min': '2000', 'port_range_max': '3000',
'direction': 'egress'}, False),
({'id': 'rule-id', 'security_group_id': 'sec-group-uuid',
'remote_ip_prefix': '10.10.10.175/26',
'protocol': 'tcp',
'port_range_min': '2000', 'port_range_max': '3000',
'direction': 'ingress'}, True)]
rules = [
{
'id': 'rule-1-id',
'protocol': 'udp',
}, {
'id': 'rule-2-id',
'remote_ip_prefix': '10.10.10.128/26',
'protocol': 'tcp',
'port_range_min': '2000',
'port_range_max': '3000',
'direction': 'ingress'
}]
with mock.patch.object(securitygroups_db.SecurityGroupDbMixin,
'get_security_group_rules',
return_value=rules):
for rule, expected_result in scenarios:
self.assertEqual(
expected_result,
self.mech_driver._sg_has_rules_with_same_normalized_cidr(
rule))
def test_port_invalid_binding_profile(self):
invalid_binding_profiles = [
{'tag': 0,


Loading…
Cancel
Save