Only set switch_id in local_link_connection if it is a mac address

When the processed lldp data is used for setting the local_link_connection
switch_id, it will set it even if the Chassis ID is not a mac.  Need to
only set it when the ChassisId is a mac address, as is done when using
non-processed lldp data.  Ironic validates that switch_id is either a
mac address or OpenFlow datapath ID.

This fixes a regresssion introduced in Pike.

Change-Id: I566acb5b19852b541df7554870ab2666f7df9614
Closes-Bug: 1748022
This commit is contained in:
Bob Fournier 2018-02-08 09:13:21 -05:00 committed by Dmitry Tantsur
parent 76978212ea
commit 97282c64e9
3 changed files with 40 additions and 2 deletions

View File

@ -18,6 +18,7 @@ import binascii
from construct import core
import netaddr
from oslo_config import cfg
from oslo_utils import netutils
from ironic_inspector.common import lldp_parsers
from ironic_inspector.common import lldp_tlvs as tlv
@ -87,7 +88,8 @@ class GenericLocalLinkConnectionHook(base.ProcessingHook):
'path': '/local_link_connection/%s' % item,
'value': value}
def _get_lldp_processed_patch(self, name, item, lldp_proc_data, port):
def _get_lldp_processed_patch(self, name, item, lldp_proc_data, port,
node_info):
if 'lldp_processed' not in lldp_proc_data:
return
@ -95,6 +97,14 @@ class GenericLocalLinkConnectionHook(base.ProcessingHook):
value = lldp_proc_data['lldp_processed'].get(name)
if value:
# Only accept mac address for chassis ID
if (item == SWITCH_ID_ITEM_NAME and
not netutils.is_valid_mac(value)):
LOG.info("Skipping switch_id since it's not a MAC: %s", value,
node_info=node_info)
return
if (not CONF.processing.overwrite_existing and
item in port.local_link_connection):
return
@ -134,7 +144,8 @@ class GenericLocalLinkConnectionHook(base.ProcessingHook):
for name, item in LLDP_PROC_DATA_MAPPING.items():
patch = self._get_lldp_processed_patch(name, item,
proc_data, port)
proc_data, port,
node_info)
if patch is not None:
patches.append(patch)

View File

@ -193,3 +193,21 @@ class TestGenericLocalLinkConnectionHook(test_base.NodeTest):
]
self.hook.before_update(self.data, self.node_info)
self.assertCalledWithPatch(patches, mock_patch)
@mock.patch.object(node_cache.NodeInfo, 'patch_port')
def test_processed_chassis_id_not_mac(self, mock_patch):
self.data['all_interfaces'] = {
'em1': {"ip": self.ips[0], "mac": self.macs[0],
"lldp_processed": {
"switch_chassis_id": "192.0.2.1",
"switch_port_id": "Ethernet2/66"}
}
}
# skip chassis_id since its not a mac
patches = [
{'path': '/local_link_connection/port_id',
'value': 'Ethernet2/66', 'op': 'add'},
]
self.hook.before_update(self.data, self.node_info)
self.assertCalledWithPatch(patches, mock_patch)

View File

@ -0,0 +1,9 @@
---
fixes:
- |
Fixes bug in which the ``switch_id`` field in a port's ``local_link_connection`` can be set to
a non-MAC address if the processed LLDP has a value other than a
MAC address for ``ChassisID``. The bare metal API requires the ``switch_id``
field to be a MAC address, and will return an error otherwise.
See `bug 1748022 <https://bugs.launchpad.net/ironic-inspector/+bug/1748022>`_
for details.