From 721bd814a61cd4dec433465fff55a5235853c9a2 Mon Sep 17 00:00:00 2001 From: Riccardo Pittau Date: Fri, 1 May 2020 13:31:25 +0200 Subject: [PATCH] Use latest version of python construct Also increasing minimum version to 2.9.39 for String class name change to PaddedString. Change-Id: Ie7b80f4698a77208c89797b82f7e41fcd8dbf168 --- ironic_inspector/common/lldp_parsers.py | 13 +++++++++---- ironic_inspector/common/lldp_tlvs.py | 16 +++++++--------- ironic_inspector/plugins/lldp_basic.py | 1 - .../plugins/local_link_connection.py | 5 ++--- .../test/unit/test_plugins_lldp_basic.py | 2 +- lower-constraints.txt | 2 +- .../construct-fly-free-fab62c0a5cb71fa5.yaml | 6 ++++++ requirements.txt | 2 +- 8 files changed, 27 insertions(+), 20 deletions(-) create mode 100644 releasenotes/notes/construct-fly-free-fab62c0a5cb71fa5.yaml diff --git a/ironic_inspector/common/lldp_parsers.py b/ironic_inspector/common/lldp_parsers.py index 1f7c21a78..8eaa87274 100644 --- a/ironic_inspector/common/lldp_parsers.py +++ b/ironic_inspector/common/lldp_parsers.py @@ -102,6 +102,10 @@ class LLDPParser(object): """Add a single name/value pair to the nv dict""" self.set_value(name, struct.value) + def add_nested_value(self, struct, name, data): + """Add a single nested name/value pair to the dict""" + self.set_value(name, struct.value.value) + def parse_tlv(self, tlv_type, data): """Parse TLVs from mapping table @@ -193,10 +197,10 @@ class LLDPBasicMgmtParser(LLDPParser): self.parser_map = { tlv.LLDP_TLV_CHASSIS_ID: - (self.add_single_value, tlv.ChassisId, - LLDP_CHASSIS_ID_NM, False), + (self.add_nested_value, tlv.ChassisId, LLDP_CHASSIS_ID_NM, + False), tlv.LLDP_TLV_PORT_ID: - (self.add_single_value, tlv.PortId, LLDP_PORT_ID_NM, False), + (self.add_nested_value, tlv.PortId, LLDP_PORT_ID_NM, False), tlv.LLDP_TLV_TTL: (None, None, None, False), tlv.LLDP_TLV_PORT_DESCRIPTION: (self.add_single_value, tlv.PortDesc, LLDP_PORT_DESC_NM, @@ -221,7 +225,8 @@ class LLDPBasicMgmtParser(LLDPParser): There can be multiple Mgmt Address TLVs, store in list. """ - self.append_value(name, struct.address) + if struct.address: + self.append_value(name, struct.address) def _get_capabilities_list(self, caps): """Get capabilities from bit map""" diff --git a/ironic_inspector/common/lldp_tlvs.py b/ironic_inspector/common/lldp_tlvs.py index 4c2d441ed..39ac09086 100644 --- a/ironic_inspector/common/lldp_tlvs.py +++ b/ironic_inspector/common/lldp_tlvs.py @@ -107,11 +107,11 @@ IANA_ADDRESS_FAMILY_ID_MAPPING = { ('mac', 6): MACAddress, } -IANAAddress = core.Embedded(core.Struct( +IANAAddress = core.Struct( 'family' / core.Enum(core.Int8ub, **mapping_for_enum( IANA_ADDRESS_FAMILY_ID_MAPPING)), 'value' / core.Switch(construct.this.family, mapping_for_switch( - IANA_ADDRESS_FAMILY_ID_MAPPING)))) + IANA_ADDRESS_FAMILY_ID_MAPPING))) # Note that 'GreedyString()' is used in cases where string len is not defined CHASSIS_ID_MAPPING = { @@ -132,9 +132,8 @@ CHASSIS_ID_MAPPING = { ChassisId = core.Struct( 'subtype' / core.Enum(core.Byte, **mapping_for_enum( CHASSIS_ID_MAPPING)), - 'value' / - core.Embedded(core.Switch(construct.this.subtype, - mapping_for_switch(CHASSIS_ID_MAPPING))) + 'value' / core.Switch(construct.this.subtype, + mapping_for_switch(CHASSIS_ID_MAPPING)) ) PORT_ID_MAPPING = { @@ -150,9 +149,8 @@ PORT_ID_MAPPING = { PortId = core.Struct( 'subtype' / core.Enum(core.Byte, **mapping_for_enum( PORT_ID_MAPPING)), - 'value' / - core.Embedded(core.Switch(construct.this.subtype, - mapping_for_switch(PORT_ID_MAPPING))) + 'value' / core.Switch(construct.this.subtype, + mapping_for_switch(PORT_ID_MAPPING)) ) PortDesc = core.Struct('value' / core.GreedyString("utf8")) @@ -215,7 +213,7 @@ Dot1_VlanName = core.Struct( 'vlanid' / core.Int16ub, 'name_len' / core.Rebuild(core.Int8ub, construct.len_(construct.this.value)), - 'vlan_name' / core.String(construct.this.name_len, "utf8") + 'vlan_name' / core.PaddedString(construct.this.name_len, "utf8") ) Dot1_ProtocolIdentity = core.Struct( diff --git a/ironic_inspector/plugins/lldp_basic.py b/ironic_inspector/plugins/lldp_basic.py index 2b697a3ae..a770b292a 100644 --- a/ironic_inspector/plugins/lldp_basic.py +++ b/ironic_inspector/plugins/lldp_basic.py @@ -69,7 +69,6 @@ class LLDPBasicProcessingHook(base.ProcessingHook): for iface in inventory['interfaces']: if_name = iface['name'] - tlvs = iface.get('lldp') if tlvs is None: LOG.warning("No LLDP Data found for interface %s", diff --git a/ironic_inspector/plugins/local_link_connection.py b/ironic_inspector/plugins/local_link_connection.py index 956f1d305..673c8f016 100644 --- a/ironic_inspector/plugins/local_link_connection.py +++ b/ironic_inspector/plugins/local_link_connection.py @@ -67,7 +67,7 @@ class GenericLocalLinkConnectionHook(base.ProcessingHook): return item = PORT_ID_ITEM_NAME - value = port_id.value + value = port_id.value.value if port_id.value else None elif tlv_type == tlv.LLDP_TLV_CHASSIS_ID: try: chassis_id = tlv.ChassisId.parse(data) @@ -79,7 +79,7 @@ class GenericLocalLinkConnectionHook(base.ProcessingHook): # Only accept mac address for chassis ID if 'mac_address' in chassis_id.subtype: item = SWITCH_ID_ITEM_NAME - value = chassis_id.value + value = chassis_id.value.value if item and value: if (not CONF.processing.overwrite_existing and @@ -98,7 +98,6 @@ 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)): diff --git a/ironic_inspector/test/unit/test_plugins_lldp_basic.py b/ironic_inspector/test/unit/test_plugins_lldp_basic.py index c83af751c..4ecaf2bea 100644 --- a/ironic_inspector/test/unit/test_plugins_lldp_basic.py +++ b/ironic_inspector/test/unit/test_plugins_lldp_basic.py @@ -286,7 +286,7 @@ class TestLLDPBasicProcessingHook(test_base.NodeTest): }] self.hook.before_update(self.data, self.node_info) self.assertEqual(self.expected, self.data['all_interfaces']) - self.assertEqual(2, mock_log.call_count) + self.assertEqual(1, mock_log.call_count) @mock.patch.object(nv.LOG, 'warning', autospec=True) def test_truncated_mac(self, mock_log): diff --git a/lower-constraints.txt b/lower-constraints.txt index 8d7d282a9..1478b6358 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -2,7 +2,7 @@ alembic==0.8.10 automaton==1.9.0 Babel==2.3.4 bandit==1.1.0 -construct==2.8.10 +construct==2.9.39 coverage==4.0 doc8==0.6.0 eventlet==0.18.2 diff --git a/releasenotes/notes/construct-fly-free-fab62c0a5cb71fa5.yaml b/releasenotes/notes/construct-fly-free-fab62c0a5cb71fa5.yaml new file mode 100644 index 000000000..26f338bfd --- /dev/null +++ b/releasenotes/notes/construct-fly-free-fab62c0a5cb71fa5.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - | + Remove upper constraint for python construct library and use the latest + version available. + The minimum compatible version for python construct is now 2.9.39 diff --git a/requirements.txt b/requirements.txt index 95c0dd40b..34c0a2f51 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,7 @@ # process, which may cause wedges in the gate later. automaton>=1.9.0 # Apache-2.0 alembic>=0.8.10 # MIT -construct<2.9,>=2.8.10 # MIT +construct>=2.9.39 # MIT eventlet!=0.18.3,!=0.20.1,>=0.18.2 # MIT Flask>=1.0 # BSD futurist>=1.2.0 # Apache-2.0