From 6cc724caebafb2ff815ece1e4535523f2aa4ef10 Mon Sep 17 00:00:00 2001 From: Kirk Byers Date: Fri, 17 Nov 2017 17:20:57 -0800 Subject: [PATCH 1/7] Canonical interfaces fix dictionary update behavior. --- napalm/base/helpers.py | 73 ++++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/napalm/base/helpers.py b/napalm/base/helpers.py index d8fe510d5..f00a31c9f 100644 --- a/napalm/base/helpers.py +++ b/napalm/base/helpers.py @@ -258,52 +258,55 @@ def as_number(as_number_val): return int(as_number_str) -def int_split_on_match(split_interface): - ''' - simple fuction to split on first digit, slash, or space match - ''' - head = split_interface.rstrip(r'/\0123456789 ') - tail = split_interface[len(head):].lstrip() - return head, tail - - -def canonical_interface_name(interface, update_os_mapping=None): - ''' - Function to retun interface canonical name +def split_interface(intf_name): + """Split an interface name based on first digit, slash, or space match.""" + head = intf_name.rstrip(r'/\0123456789 ') + tail = intf_name[len(head):].lstrip() + return (head, tail) + + +def canonical_interface_name(interface, addl_name_map=None): + """Function to return an interface's canonical name (fully expanded name). + This puposely does not use regex, or first X characters, to ensure there is no chance for false positives. As an example, Po = PortChannel, and PO = POS. With either character or regex, that would produce a false positive. - ''' - - interface_type, interface_number = int_split_on_match(interface) + """ + name_map = {} + name_map.update(base_interfaces) + interface_type, interface_number = split_interface(interface) - if isinstance(update_os_mapping, dict): - base_interfaces.update(update_os_mapping) + if isinstance(addl_name_map, dict): + name_map.update(addl_name_map) # check in dict for mapping - if base_interfaces.get(interface_type): - long_int = base_interfaces.get(interface_type) + if name_map.get(interface_type): + long_int = name_map.get(interface_type) return long_int + str(interface_number) - # if nothing matched, at least return the original + # if nothing matched, return the original name else: return interface -def abbreviated_interface_name(interface, update_os_mapping=None): - ''' - Function to retun interface canonical name - This puposely does not use regex, or first X characters, to ensure - there is no chance for false positives. As an example, Po = PortChannel, and - PO = POS. With either character or regex, that would produce a false positive. - ''' +def abbreviated_interface_name(interface, addl_name_map=None): + """Function to return an abbreviated representation of the interface name.""" + reverse_name_map = {} + reverse_name_map.update(reverse_mapping) + interface_type, interface_number = split_interface(interface) - interface_type, interface_number = int_split_on_match(interface) + if isinstance(addl_name_map, dict): + reverse_name_map.update(addl_name_map) - if isinstance(update_os_mapping, dict): - base_interfaces.update(update_os_mapping) - # check in dict for mapping + # Try to ensure canonical type. if base_interfaces.get(interface_type): - long_int = base_interfaces.get(interface_type) - return reverse_mapping[long_int] + str(interface_number) - # if nothing matched, at least return the original + canonical_type = base_interfaces.get(interface_type) else: - return interface + canonical_type = interface_type + + try: + abbreviated_name = reverse_mapping[canonical_type] + str(interface_number) + return abbreviated_name + except KeyError: + pass + + # If abbreviated name lookup fails, return original name + return interface From dad0f52974ee900dff3623c0178f94eb8aafcb3f Mon Sep 17 00:00:00 2001 From: Kirk Byers Date: Fri, 17 Nov 2017 17:25:41 -0800 Subject: [PATCH 2/7] Fixing argument reference. --- napalm/base/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/napalm/base/base.py b/napalm/base/base.py index e1f023601..c7c6ee347 100644 --- a/napalm/base/base.py +++ b/napalm/base/base.py @@ -1587,6 +1587,6 @@ def compliance_report(self, validation_file=None, validation_source=None): def _canonical_int(self, interface): """Expose the helper function within this class.""" if self.use_canonical_interface is True: - return napalm.base.helpers.canonical_interface_name(interface, update_os_mapping=None) + return napalm.base.helpers.canonical_interface_name(interface, addl_name_map=None) else: return interface From f26183a00894c1078d5af6d81739b223ca6e8ef1 Mon Sep 17 00:00:00 2001 From: itdependsnetworks Date: Sun, 19 Nov 2017 01:58:25 -0500 Subject: [PATCH 3/7] add tests for and fix canonical name --- napalm/base/canonical_map.py | 1 + napalm/base/helpers.py | 16 ++++++++------- test/base/test_helpers.py | 39 ++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/napalm/base/canonical_map.py b/napalm/base/canonical_map.py index 45326624a..cf8f15e78 100644 --- a/napalm/base/canonical_map.py +++ b/napalm/base/canonical_map.py @@ -42,6 +42,7 @@ "Hu": "HundredGigabitEthernet", "Loopback": "Loopback", "Lo": "Loopback", + "lo": "Loopback", "Management": "Management", "Mgmt": "Management", "Ma": "Management", diff --git a/napalm/base/helpers.py b/napalm/base/helpers.py index f00a31c9f..9b5cc72b9 100644 --- a/napalm/base/helpers.py +++ b/napalm/base/helpers.py @@ -260,7 +260,7 @@ def as_number(as_number_val): def split_interface(intf_name): """Split an interface name based on first digit, slash, or space match.""" - head = intf_name.rstrip(r'/\0123456789 ') + head = intf_name.rstrip(r'/\0123456789. ') tail = intf_name[len(head):].lstrip() return (head, tail) @@ -287,18 +287,20 @@ def canonical_interface_name(interface, addl_name_map=None): return interface -def abbreviated_interface_name(interface, addl_name_map=None): +def abbreviated_interface_name(interface, addl_name_map=None, addl_reverse_map=None): """Function to return an abbreviated representation of the interface name.""" - reverse_name_map = {} - reverse_name_map.update(reverse_mapping) + name_map = {} + name_map.update(base_interfaces) interface_type, interface_number = split_interface(interface) if isinstance(addl_name_map, dict): - reverse_name_map.update(addl_name_map) + name_map.update(addl_name_map) + if isinstance(addl_reverse_map, dict): + reverse_mapping.update(addl_reverse_map) # Try to ensure canonical type. - if base_interfaces.get(interface_type): - canonical_type = base_interfaces.get(interface_type) + if name_map.get(interface_type): + canonical_type = name_map.get(interface_type) else: canonical_type = interface_type diff --git a/test/base/test_helpers.py b/test/base/test_helpers.py index 84b275179..08a455655 100644 --- a/test/base/test_helpers.py +++ b/test/base/test_helpers.py @@ -388,6 +388,45 @@ def test_convert_uptime_string_seconds(self): self.assertEqual(convert_uptime_string_seconds('95w2d10h58m'), 57668280) self.assertEqual(convert_uptime_string_seconds('1h5m'), 3900) + def test_canonical_interface_name(self): + """Test the canonical_interface_name helper function.""" + self.assertEqual(napalm.base.helpers.canonical_interface_name('Fa0/1'), "FastEthernet0/1") + self.assertEqual(napalm.base.helpers.canonical_interface_name('FastEthernet0/1'), "FastEthernet0/1") + self.assertEqual(napalm.base.helpers.canonical_interface_name('TenGig1/1/1.5'), "TenGigabitEthernet1/1/1.5") + self.assertEqual(napalm.base.helpers.canonical_interface_name('Gi1/2'), "GigabitEthernet1/2") + self.assertEqual(napalm.base.helpers.canonical_interface_name('HundredGigE105/1/1'), "HundredGigabitEthernet105/1/1") + self.assertEqual(napalm.base.helpers.canonical_interface_name('Lo0'), "Loopback0") + self.assertEqual(napalm.base.helpers.canonical_interface_name('lo0'), "Loopback0") + self.assertEqual(napalm.base.helpers.canonical_interface_name('no_match0/1'), "no_match0/1") + self.assertEqual(napalm.base.helpers.canonical_interface_name('lo', + addl_name_map = + {"lo":"something_custom"}), + "something_custom") + self.assertEqual(napalm.base.helpers.canonical_interface_name('uniq', + addl_name_map = + {"uniq":"something_custom"}), + "something_custom") + + def test_abbreviated_interface_name(self): + """Test the abbreviated_interface_name helper function.""" + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('Fa0/1'), "Fa0/1") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('FastEthernet0/1'), "Fa0/1") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('TenGig1/1/1.5'), "Te1/1/1.5") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('Gi1/2'), "Gi1/2") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('HundredGigE105/1/1'), "Hu105/1/1") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('Lo0'), "Lo0") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('lo0'), "Lo0") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('no_match0/1'), "no_match0/1") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('loop10', + addl_name_map = + {"loop":"Loopback"}), + "Lo10") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('loop10', + addl_name_map = + {"loop":"Loopback"}, + addl_reverse_map = + {"Loopback":"lo"}), + "lo10") class FakeNetworkDriver(NetworkDriver): From 7fb0e9b91ceda4e7393b40ca1e7b98588e136ae4 Mon Sep 17 00:00:00 2001 From: itdependsnetworks Date: Sun, 19 Nov 2017 11:09:54 -0500 Subject: [PATCH 4/7] update doc strings --- napalm/base/helpers.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/napalm/base/helpers.py b/napalm/base/helpers.py index 9b5cc72b9..864c78f22 100644 --- a/napalm/base/helpers.py +++ b/napalm/base/helpers.py @@ -268,10 +268,17 @@ def split_interface(intf_name): def canonical_interface_name(interface, addl_name_map=None): """Function to return an interface's canonical name (fully expanded name). - This puposely does not use regex, or first X characters, to ensure - there is no chance for false positives. As an example, Po = PortChannel, and - PO = POS. With either character or regex, that would produce a false positive. + Use of explicit matches used to indicate a clear understanding on any potential + match. Regex and other looser matching methods were not implmented to avoid false + positive matches. As an example, it would make sense to do "[P|p][O|o]" which would + incorrectly match PO = POS and Po = Port-channel, leading to a false positive, not + easily troubleshot, found, or known. + + :param interface: The interface you are attempting to expand. + :param addl_name_map: A dict containing key/value pairs that is mean to update the base mapping. + Used if a OS has specific differences. e.g. {"Po": "PortChannel"} vs {"Po": "Port-Channel"} """ + name_map = {} name_map.update(base_interfaces) interface_type, interface_number = split_interface(interface) @@ -288,7 +295,17 @@ def canonical_interface_name(interface, addl_name_map=None): def abbreviated_interface_name(interface, addl_name_map=None, addl_reverse_map=None): - """Function to return an abbreviated representation of the interface name.""" + """Function to return an abbreviated representation of the interface name. + + :param interface: The interface you are attempting to expand. + :param addl_name_map (optional): A dict containing key/value pairs that updates + the base mapping. Used if an OS has specific differences. e.g. {"Po": "PortChannel"} vs + {"Po": "Port-Channel"} + :param addl_reverse_map (optional): A dict containing key/value pairs that updates + the reverse mapping. Used if an OS has specific differences. e.g. {"PortChannel": "Po"} vs + {"PortChannel": "po"} + """ + name_map = {} name_map.update(base_interfaces) interface_type, interface_number = split_interface(interface) From 59808968481a5efae4bc25dc59aa85fac65cee72 Mon Sep 17 00:00:00 2001 From: itdependsnetworks Date: Sun, 19 Nov 2017 11:25:28 -0500 Subject: [PATCH 5/7] fix pylama --- test/base/test_helpers.py | 47 ++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/test/base/test_helpers.py b/test/base/test_helpers.py index 08a455655..a74f0cdb2 100644 --- a/test/base/test_helpers.py +++ b/test/base/test_helpers.py @@ -391,42 +391,43 @@ def test_convert_uptime_string_seconds(self): def test_canonical_interface_name(self): """Test the canonical_interface_name helper function.""" self.assertEqual(napalm.base.helpers.canonical_interface_name('Fa0/1'), "FastEthernet0/1") - self.assertEqual(napalm.base.helpers.canonical_interface_name('FastEthernet0/1'), "FastEthernet0/1") - self.assertEqual(napalm.base.helpers.canonical_interface_name('TenGig1/1/1.5'), "TenGigabitEthernet1/1/1.5") - self.assertEqual(napalm.base.helpers.canonical_interface_name('Gi1/2'), "GigabitEthernet1/2") - self.assertEqual(napalm.base.helpers.canonical_interface_name('HundredGigE105/1/1'), "HundredGigabitEthernet105/1/1") + self.assertEqual(napalm.base.helpers.canonical_interface_name('FastEthernet0/1'), + 'FastEthernet0/1') + self.assertEqual(napalm.base.helpers.canonical_interface_name('TenGig1/1/1.5'), + "TenGigabitEthernet1/1/1.5") + self.assertEqual(napalm.base.helpers.canonical_interface_name('Gi1/2'), + "GigabitEthernet1/2") + self.assertEqual(napalm.base.helpers.canonical_interface_name('HundredGigE105/1/1'), + "HundredGigabitEthernet105/1/1") self.assertEqual(napalm.base.helpers.canonical_interface_name('Lo0'), "Loopback0") self.assertEqual(napalm.base.helpers.canonical_interface_name('lo0'), "Loopback0") - self.assertEqual(napalm.base.helpers.canonical_interface_name('no_match0/1'), "no_match0/1") + self.assertEqual(napalm.base.helpers.canonical_interface_name('no_match0/1'), + "no_match0/1") self.assertEqual(napalm.base.helpers.canonical_interface_name('lo', - addl_name_map = - {"lo":"something_custom"}), - "something_custom") + addl_name_map={"lo": "something_custom"}), "something_custom") self.assertEqual(napalm.base.helpers.canonical_interface_name('uniq', - addl_name_map = - {"uniq":"something_custom"}), - "something_custom") + addl_name_map={"uniq": "something_custom"}), "something_custom") def test_abbreviated_interface_name(self): """Test the abbreviated_interface_name helper function.""" self.assertEqual(napalm.base.helpers.abbreviated_interface_name('Fa0/1'), "Fa0/1") - self.assertEqual(napalm.base.helpers.abbreviated_interface_name('FastEthernet0/1'), "Fa0/1") - self.assertEqual(napalm.base.helpers.abbreviated_interface_name('TenGig1/1/1.5'), "Te1/1/1.5") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('FastEthernet0/1'), + "Fa0/1") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('TenGig1/1/1.5'), + "Te1/1/1.5") self.assertEqual(napalm.base.helpers.abbreviated_interface_name('Gi1/2'), "Gi1/2") - self.assertEqual(napalm.base.helpers.abbreviated_interface_name('HundredGigE105/1/1'), "Hu105/1/1") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('HundredGigE105/1/1'), + "Hu105/1/1") self.assertEqual(napalm.base.helpers.abbreviated_interface_name('Lo0'), "Lo0") self.assertEqual(napalm.base.helpers.abbreviated_interface_name('lo0'), "Lo0") - self.assertEqual(napalm.base.helpers.abbreviated_interface_name('no_match0/1'), "no_match0/1") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('no_match0/1'), + "no_match0/1") self.assertEqual(napalm.base.helpers.abbreviated_interface_name('loop10', - addl_name_map = - {"loop":"Loopback"}), - "Lo10") + addl_name_map={"loop": "Loopback"}), "Lo10") self.assertEqual(napalm.base.helpers.abbreviated_interface_name('loop10', - addl_name_map = - {"loop":"Loopback"}, - addl_reverse_map = - {"Loopback":"lo"}), - "lo10") + addl_name_map={"loop": "Loopback"}, + addl_reverse_map={"Loopback": "lo"}), "lo10") + class FakeNetworkDriver(NetworkDriver): From e74435c818e1db6580a2c55b0c724af06f04806c Mon Sep 17 00:00:00 2001 From: itdependsnetworks Date: Sun, 19 Nov 2017 13:10:27 -0500 Subject: [PATCH 6/7] fix rev update dict --- napalm/base/helpers.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/napalm/base/helpers.py b/napalm/base/helpers.py index 864c78f22..f34acc00a 100644 --- a/napalm/base/helpers.py +++ b/napalm/base/helpers.py @@ -275,8 +275,9 @@ def canonical_interface_name(interface, addl_name_map=None): easily troubleshot, found, or known. :param interface: The interface you are attempting to expand. - :param addl_name_map: A dict containing key/value pairs that is mean to update the base mapping. - Used if a OS has specific differences. e.g. {"Po": "PortChannel"} vs {"Po": "Port-Channel"} + :param addl_name_map (optional): A dict containing key/value pairs that updates + the base mapping. Used if an OS has specific differences. e.g. {"Po": "PortChannel"} vs + {"Po": "Port-Channel"} """ name_map = {} @@ -297,7 +298,7 @@ def canonical_interface_name(interface, addl_name_map=None): def abbreviated_interface_name(interface, addl_name_map=None, addl_reverse_map=None): """Function to return an abbreviated representation of the interface name. - :param interface: The interface you are attempting to expand. + :param interface: The interface you are attempting to abbreviate. :param addl_name_map (optional): A dict containing key/value pairs that updates the base mapping. Used if an OS has specific differences. e.g. {"Po": "PortChannel"} vs {"Po": "Port-Channel"} @@ -312,8 +313,12 @@ def abbreviated_interface_name(interface, addl_name_map=None, addl_reverse_map=N if isinstance(addl_name_map, dict): name_map.update(addl_name_map) + + rev_name_map = {} + rev_name_map.update(reverse_mapping) + if isinstance(addl_reverse_map, dict): - reverse_mapping.update(addl_reverse_map) + rev_name_map.update(addl_reverse_map) # Try to ensure canonical type. if name_map.get(interface_type): @@ -322,7 +327,7 @@ def abbreviated_interface_name(interface, addl_name_map=None, addl_reverse_map=N canonical_type = interface_type try: - abbreviated_name = reverse_mapping[canonical_type] + str(interface_number) + abbreviated_name = rev_name_map[canonical_type] + str(interface_number) return abbreviated_name except KeyError: pass From 6395912db9b612461749f501a98263e462cd1dd3 Mon Sep 17 00:00:00 2001 From: itdependsnetworks Date: Sun, 19 Nov 2017 14:51:44 -0500 Subject: [PATCH 7/7] more consitent test, fix st --- napalm/base/helpers.py | 4 ++-- test/base/test_helpers.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/napalm/base/helpers.py b/napalm/base/helpers.py index f34acc00a..44682d671 100644 --- a/napalm/base/helpers.py +++ b/napalm/base/helpers.py @@ -289,7 +289,7 @@ def canonical_interface_name(interface, addl_name_map=None): # check in dict for mapping if name_map.get(interface_type): long_int = name_map.get(interface_type) - return long_int + str(interface_number) + return long_int + py23_compat.text_type(interface_number) # if nothing matched, return the original name else: return interface @@ -327,7 +327,7 @@ def abbreviated_interface_name(interface, addl_name_map=None, addl_reverse_map=N canonical_type = interface_type try: - abbreviated_name = rev_name_map[canonical_type] + str(interface_number) + abbreviated_name = rev_name_map[canonical_type] + py23_compat.text_type(interface_number) return abbreviated_name except KeyError: pass diff --git a/test/base/test_helpers.py b/test/base/test_helpers.py index a74f0cdb2..7c08e9e25 100644 --- a/test/base/test_helpers.py +++ b/test/base/test_helpers.py @@ -403,10 +403,10 @@ def test_canonical_interface_name(self): self.assertEqual(napalm.base.helpers.canonical_interface_name('lo0'), "Loopback0") self.assertEqual(napalm.base.helpers.canonical_interface_name('no_match0/1'), "no_match0/1") - self.assertEqual(napalm.base.helpers.canonical_interface_name('lo', - addl_name_map={"lo": "something_custom"}), "something_custom") - self.assertEqual(napalm.base.helpers.canonical_interface_name('uniq', - addl_name_map={"uniq": "something_custom"}), "something_custom") + self.assertEqual(napalm.base.helpers.canonical_interface_name('lo10', + addl_name_map={"lo": "something_custom"}), "something_custom10") + self.assertEqual(napalm.base.helpers.canonical_interface_name('uniq0/1/1', + addl_name_map={"uniq": "something_custom"}), "something_custom0/1/1") def test_abbreviated_interface_name(self): """Test the abbreviated_interface_name helper function.""" @@ -420,8 +420,8 @@ def test_abbreviated_interface_name(self): "Hu105/1/1") self.assertEqual(napalm.base.helpers.abbreviated_interface_name('Lo0'), "Lo0") self.assertEqual(napalm.base.helpers.abbreviated_interface_name('lo0'), "Lo0") - self.assertEqual(napalm.base.helpers.abbreviated_interface_name('no_match0/1'), - "no_match0/1") + self.assertEqual(napalm.base.helpers.abbreviated_interface_name('something_custom0/1'), + "something_custom0/1") self.assertEqual(napalm.base.helpers.abbreviated_interface_name('loop10', addl_name_map={"loop": "Loopback"}), "Lo10") self.assertEqual(napalm.base.helpers.abbreviated_interface_name('loop10',