From 8a9ffcb0d4c0c44fc226810fd7f08dab9b79fcc0 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Fri, 27 May 2022 11:58:04 +0200 Subject: [PATCH] [OVN] Make binding profile validation more robust 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 --- neutron/common/ovn/utils.py | 10 ++-- neutron/tests/unit/common/ovn/test_utils.py | 49 ++++++++++++++----- .../ovn/mech_driver/test_mech_driver.py | 2 +- 3 files changed, 43 insertions(+), 18 deletions(-) diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index e118eae67fb..455507fcd5e 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -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: diff --git a/neutron/tests/unit/common/ovn/test_utils.py b/neutron/tests/unit/common/ovn/test_utils.py index 42f41e5ebce..7f36f53b5a8 100644 --- a/neutron/tests/unit/common/ovn/test_utils.py +++ b/neutron/tests/unit/common/ovn/test_utils.py @@ -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, - {portbindings.VNIC_TYPE: self.VNIC_FAKE_NORMAL, - constants.OVN_PORT_BINDING_PROFILE: expect}) + # 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: 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() diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 1ce380609eb..324a14ff5b3 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -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):