openstack/neutron
Code Issues Proposed changes

Merge "[ovn] Use normalized remote prefix IPs in OVN driver"

Browse Source
This commit is contained in:
Zuul 2020-07-29 16:38:08 +00:00 committed by Gerrit Code Review
commit 000e54d0da
4 changed files with 140 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
neutron/common/ovn/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 neutron._i18n import _
from neutron.common.ovn import constants as ovn_const
from neutron.common.ovn 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'] == const.INGRESS_DIRECTION 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):
@ -297,7 +310,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(**acl, may_exist=True))
def update_acls_for_security_group(plugin,
@ -322,7 +335,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(**acl, may_exist=True).execute(check_error=True)
else:
ovn.pg_acl_del(acl['port_group'], acl['direction'],
acl['priority'], acl['match']).execute(

31
neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py
View File

@ -21,6 +21,7 @@ import signal
import threading
import types
import netaddr
from neutron_lib.api.definitions import portbindings
from neutron_lib.api.definitions import segment as segment_def
from neutron_lib.callbacks import events
@ -360,10 +361,40 @@ class OVNMechanismDriver(api.MechanismDriver):
elif event == events.BEFORE_DELETE:
sg_rule = self._plugin.get_security_group_rule(
kwargs['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(
kwargs['context'],
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
neutron/tests/unit/common/ovn/test_acl.py
View File

@ -303,6 +303,19 @@ class TestACLs(base.BaseTestCase):
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',

81
neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py
View File

@ -183,12 +183,36 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
@mock.patch.object(ovn_revision_numbers_db, 'bump_revision')
def test__process_sg_rule_notifications_sgr_create(self, mock_bump):
with mock.patch.object(ovn_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.object(
ovn_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, 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)
mock_bump.assert_called_once_with(
mock.ANY, rule, ovn_const.TYPE_SECURITY_GROUP_RULES)
@mock.patch.object(ovn_revision_numbers_db, '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.object(
ovn_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)
@ -212,6 +236,59 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
mock_delrev.assert_called_once_with(
mock.ANY, 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,