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

ironic_inspector/plugins/local_link_connection.py

@@ -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)
 

ironic_inspector/test/unit/test_plugins_local_link_connection.py

@@ -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)

releasenotes/notes/fix-llc-switch-id-not-mac-e2de3adc0945ee70.yaml

@@ -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.