openstack/neutron
Code Issues Proposed changes

[OVN] Make binding profile validation more robust

Browse Source 
The main purpose of Neutron's validation of the binding profile
is to make sure expected keys are present and that their values
are of the expected type.

The Nova compute component updates the binding profile as part of
instance creation.  Depending on the version of the Nova compute
component and which hardware it interfaces with, the information
provided by Nova in the binding profile may differ.

Nova also has limited information at its disposal at the point in
time it updates the port binding profile, so it would be
non-trivial for it to provide information conditionally based on
things like VNIC_TYPE and existing binding profile data.

Make the Neutron binding profile validation more robust for both
upgrade and heterogeneous hardware scenarios by accepting the
presence of surplus keys in the binding profile.  The data that
Neutron expects will still be validated and any surplus keys will
be pruned before further processing internally in Neutron.

Closes-Bug: #1975743
Change-Id: I3a91f442a1fd72f9027f10f2b1b6572cee3f8360
This commit is contained in:
Frode Nordahl
2022-05-27 11:58:04 +02:00
parent 2b77f12d2a
commit 8a9ffcb0d4
3 changed files with 43 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
neutron/common/ovn/utils.py
View File

@@ -255,6 +255,11 @@ def validate_and_get_data_from_binding_profile(port):
) % constants.PORT_CAP_PARAM
raise n_exc.InvalidInput(error_message=msg)
# Note that only the keys mentioned in each parameter set, as defined in
# constants.OVN_PORT_BINDING_PROFILE_PARAMS, will be evaluated.
#
# Any surplus keys provided by Nova will be ignored and pruned from the
# Dict returned by this function.
for pbp_param_set in constants.OVN_PORT_BINDING_PROFILE_PARAMS:
if pbp_param_set.vnic_type:
if pbp_param_set.vnic_type != vnic_type:
@@ -270,13 +275,10 @@ def validate_and_get_data_from_binding_profile(port):
pass
if len(param_dict) == 0:
continue
if len(param_dict) != len(param_keys):
if param_keys - binding_profile.keys():
msg = _('Invalid binding:profile. %s are all '
'required.') % param_keys
raise n_exc.InvalidInput(error_message=msg)
if (len(binding_profile) != len(param_keys)):
msg = _('Invalid binding:profile. too many parameters')
raise n_exc.InvalidInput(error_message=msg)
break
if not param_dict:

47
neutron/tests/unit/common/ovn/test_utils.py
View File

@@ -489,7 +489,7 @@ class TestValidateAndGetDataFromBindingProfile(base.BaseTestCase):
constants.OVNPortBindingProfileParamSet(
{
'key': [str],
'other_key': [str],
'other_key': [int],
'third_key': [str]
},
self.VNIC_FAKE_OTHER, constants.PORT_CAP_SWITCHDEV),
@@ -605,6 +605,24 @@ class TestValidateAndGetDataFromBindingProfile(base.BaseTestCase):
constants.OVN_PORT_BINDING_PROFILE: expect}),
expect)
def test_valid_input_surplus_keys(self):
# Confirm that extra keys are allowed
binding_profile = {
constants.PORT_CAP_PARAM: [constants.PORT_CAP_SWITCHDEV],
'pci_vendor_info': 'dead:beef',
'pci_slot': '0000:ca:fe.42',
'physical_network': 'physnet1',
'optional_information_provided_by_nova': 'not_consumed_by_neutron',
}
expect = binding_profile.copy()
del(expect[constants.PORT_CAP_PARAM])
del(expect['optional_information_provided_by_nova'])
self.assertDictEqual(
expect,
utils.validate_and_get_data_from_binding_profile(
{portbindings.VNIC_TYPE: portbindings.VNIC_DIRECT,
constants.OVN_PORT_BINDING_PROFILE: binding_profile}))
def test_unknown_profile_items_pruned(self):
# Confirm that unknown profile items are pruned
self.assertEqual(
@@ -642,36 +660,41 @@ class TestValidateAndGetDataFromBindingProfile(base.BaseTestCase):
def test_overlapping_param_set_different_vnic_type(self):
# Confirm overlapping param sets discerned by vnic_type
expect = {
binding_profile = {
'key': 'value',
'other_key': 'value',
}
# This param set is not valid for VNIC_FAKE_NORMAL
self.assertRaises(
neutron_lib.exceptions.InvalidInput,
utils.validate_and_get_data_from_binding_profile,
# This param set is valid for VNIC_FAKE_NORMAL with 'other_key' pruned.
expect = binding_profile.copy()
del(expect['other_key'])
self.assertDictEqual(
expect,
utils.validate_and_get_data_from_binding_profile(
{portbindings.VNIC_TYPE: self.VNIC_FAKE_NORMAL,
constants.OVN_PORT_BINDING_PROFILE: expect})
constants.OVN_PORT_BINDING_PROFILE: binding_profile}))
# It is valid for VNIC_FAKE_OTHER
expect = binding_profile.copy()
self.assertDictEqual(
expect,
utils.validate_and_get_data_from_binding_profile(
{portbindings.VNIC_TYPE: self.VNIC_FAKE_OTHER,
constants.OVN_PORT_BINDING_PROFILE: expect}))
constants.OVN_PORT_BINDING_PROFILE: binding_profile}))
def test_overlapping_param_set_different_vnic_type_and_capability(self):
# Confirm overlapping param sets discerned by vnic_type and capability
expect = {
binding_profile = {
'key': 'value',
'other_key': 'value',
'other_key': 42,
'third_key': 'value',
}
# This param set is not valid for VNIC_FAKE_OTHER without capability
expect = binding_profile.copy()
del(expect['third_key'])
self.assertRaises(
neutron_lib.exceptions.InvalidInput,
utils.validate_and_get_data_from_binding_profile,
{portbindings.VNIC_TYPE: self.VNIC_FAKE_OTHER,
constants.OVN_PORT_BINDING_PROFILE: expect})
constants.OVN_PORT_BINDING_PROFILE: binding_profile})
# This param set is also not valid as the capabilities do not match
binding_profile = {
constants.PORT_CAP_PARAM: ['fake-capability'],
@@ -688,7 +711,7 @@ class TestValidateAndGetDataFromBindingProfile(base.BaseTestCase):
binding_profile = {
constants.PORT_CAP_PARAM: [constants.PORT_CAP_SWITCHDEV],
'key': 'value',
'other_key': 'value',
'other_key': 42,
'third_key': 'value',
}
expect = binding_profile.copy()

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

@@ -4002,7 +4002,7 @@ class TestOVNVtepPortBinding(OVNMechanismDriverTestCase):
with self.subnet(n):
self._create_port(self.fmt, n['network']['id'],
arg_list=(OVN_PROFILE,),
expected_res_status=400,
expected_res_status=404,
**binding)
def test_create_port_with_vtep_options_and_check_vtep_keys(self):