From 99192df02a16272478deac9fe401c3c6933aef8c Mon Sep 17 00:00:00 2001 From: Brandon Ewing Date: Wed, 22 Jan 2025 13:10:04 -0600 Subject: [PATCH 1/6] Support non-total model testing in helper In support of adding details to NTP server dictionaries, support detection of non-total TypedDicts and change the logic to only error if there are EXTRA keys instead of missing keys. --- napalm/base/test/helpers.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/napalm/base/test/helpers.py b/napalm/base/test/helpers.py index de81e48ed..1b7cd57cd 100644 --- a/napalm/base/test/helpers.py +++ b/napalm/base/test/helpers.py @@ -4,23 +4,26 @@ def test_model(model, data): """Return if the dictionary `data` complies with the `model`.""" # Access the underlying schema for a TypedDict directly - model = model.__annotations__ - same_keys = set(model.keys()) == set(data.keys()) + annotations = model.__annotations__ + if model.__total__: + same_keys = set(annotations.keys()) == set(data.keys()) + else: + same_keys = set(data.keys()) <= set(annotations.keys()) if not same_keys: print( "model_keys: {}\ndata_keys: {}".format( - sorted(model.keys()), sorted(data.keys()) + sorted(annotations.keys()), sorted(data.keys()) ) ) correct_class = True - for key, instance_class in model.items(): - correct_class = isinstance(data[key], instance_class) and correct_class + for key in data.keys(): + correct_class = isinstance(data[key], annotations[key]) and correct_class if not correct_class: print( "key: {}\nmodel_class: {}\ndata_class: {}".format( - key, instance_class, data[key].__class__ + key, annotations[key], data[key].__class__ ) ) From 145c7b73f790c135996a30277e637ad026d24100 Mon Sep 17 00:00:00 2001 From: Brandon Ewing Date: Wed, 22 Jan 2025 13:11:27 -0600 Subject: [PATCH 2/6] Update NTP data return definition Support returning details regarding individual NTP servers based on openconfig models (/system/ntp/servers/server) --- napalm/base/base.py | 15 +++++++++++++-- napalm/base/models.py | 20 ++++++++++++++++++-- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/napalm/base/base.py b/napalm/base/base.py index e7226cba6..7c69c61b1 100644 --- a/napalm/base/base.py +++ b/napalm/base/base.py @@ -921,12 +921,23 @@ def get_ntp_servers(self) -> Dict[str, models.NTPServerDict]: """ Returns the NTP servers configuration as dictionary. The keys of the dictionary represent the IP Addresses of the servers. - Inner dictionaries do not have yet any available keys. + Inner dictionaries MAY contain information regarding per-server configuration. Example:: { - '192.168.0.1': {}, + '192.168.0.1': + { + 'address': '192.168.0.1', + 'port': 123, + 'version': 4, + 'association_type': 'SERVER', + 'iburst': False, + 'prefer': False, + 'network_instance': 'default', + 'source_address': '192.0.2.1', + 'key_id': -1, + }, '17.72.148.53': {}, '37.187.56.220': {}, '162.158.20.18': {} diff --git a/napalm/base/models.py b/napalm/base/models.py index 395c3c8dc..ad104bc56 100644 --- a/napalm/base/models.py +++ b/napalm/base/models.py @@ -224,7 +224,15 @@ NTPPeerDict = TypedDict( "NTPPeerDict", { - # will populate it in the future wit potential keys + "address": str, + "port": int, + "version": int, + "association_type": str, + "iburst": bool, + "prefer": bool, + "network_instance": str, + "source_address": str, + "key_id": int, }, total=False, ) @@ -232,7 +240,15 @@ NTPServerDict = TypedDict( "NTPServerDict", { - # will populate it in the future wit potential keys + "address": str, + "port": int, + "version": int, + "association_type": str, + "iburst": bool, + "prefer": bool, + "network_instance": str, + "source_address": str, + "key_id": int, }, total=False, ) From c02a74981239ef4e085fe5a0caddc5b2e6ef8a2a Mon Sep 17 00:00:00 2001 From: Brandon Ewing Date: Wed, 22 Jan 2025 13:25:13 -0600 Subject: [PATCH 3/6] Add support for NTP server details to EOS --- napalm/eos/eos.py | 79 ++++++++++++++++--- .../details/expected_result.json | 24 ++++++ .../details/show_ip_interface.json | 48 +++++++++++ .../details/show_ipv6_interface.json | 46 +++++++++++ .../show_running_config___section_ntp.text | 2 + .../normal/expected_result.json | 36 ++++++++- 6 files changed, 223 insertions(+), 12 deletions(-) create mode 100644 test/eos/mocked_data/test_get_ntp_servers/details/expected_result.json create mode 100644 test/eos/mocked_data/test_get_ntp_servers/details/show_ip_interface.json create mode 100644 test/eos/mocked_data/test_get_ntp_servers/details/show_ipv6_interface.json create mode 100644 test/eos/mocked_data/test_get_ntp_servers/details/show_running_config___section_ntp.text diff --git a/napalm/eos/eos.py b/napalm/eos/eos.py index 76f95f551..827bee3f1 100644 --- a/napalm/eos/eos.py +++ b/napalm/eos/eos.py @@ -1202,21 +1202,78 @@ def get_arp_table(self, vrf=""): return arp_table def get_ntp_servers(self): - commands = ["show running-config | section ntp"] + result = {} - raw_ntp_config = self._run_commands(commands, encoding="text")[0].get( - "output", "" - ) + commands = ["show running-config | section ntp"] - ntp_config = napalm.base.helpers.textfsm_extractor( - self, "ntp_peers", raw_ntp_config + raw_ntp_config = ( + self._run_commands(commands, encoding="text")[0] + .get("output", "") + .splitlines() ) - return { - str(ntp_peer.get("ntppeer")): {} - for ntp_peer in ntp_config - if ntp_peer.get("ntppeer", "") - } + for server in raw_ntp_config: + details = { + "port": 123, + "version": 4, + "association_type": "SERVER", + "iburst": False, + "prefer": False, + "network_instance": "default", + "source_address": "", + "key_id": -1, + } + tokens = server.split() + if tokens[2] == "vrf": + details["network_instance"] = tokens[3] + server_ip = details["address"] = tokens[4] + idx = 5 + else: + server_ip = details["address"] = tokens[2] + idx = 3 + try: + parsed_address = napalm.base.helpers.ipaddress.ip_address(server_ip) + family = parsed_address.version + except ValueError: + # Assume family of 4, unless local-interface has no IPv4 addresses + family = 4 + while idx < len(tokens): + if tokens[idx] == "iburst": + details["iburst"] = True + idx += 1 + + elif tokens[idx] == "key": + details["key_id"] = int(tokens[idx + 1]) + idx += 2 + + elif tokens[idx] == "local-interface": + interfaces = self.get_interfaces_ip() + intf = tokens[idx + 1] + if family == 6 and interfaces[intf]["ipv6"]: + details["source_address"] = list( + interfaces[intf]["ipv6"].keys() + )[0] + elif interfaces[intf]["ipv4"]: + details["source_address"] = list( + interfaces[intf]["ipv4"].keys() + )[0] + elif interfaces[intf]["ipv6"]: + details["source_address"] = list( + interfaces[intf]["ipv6"].keys() + )[0] + idx += 2 + + elif tokens[idx] == "version": + details["version"] = int(tokens[idx + 1]) + idx += 2 + + elif tokens[idx] == "prefer": + details["prefer"] = True + idx += 1 + + result[server_ip] = details + + return result def get_ntp_stats(self): ntp_stats = [] diff --git a/test/eos/mocked_data/test_get_ntp_servers/details/expected_result.json b/test/eos/mocked_data/test_get_ntp_servers/details/expected_result.json new file mode 100644 index 000000000..cbdf65d1b --- /dev/null +++ b/test/eos/mocked_data/test_get_ntp_servers/details/expected_result.json @@ -0,0 +1,24 @@ +{ + "1.2.3.4": { + "port": 123, + "version": 4, + "association_type": "SERVER", + "iburst": true, + "prefer": true, + "network_instance": "FOO", + "source_address": "", + "key_id": -1, + "address": "1.2.3.4" + }, + "4.3.2.1": { + "port": 123, + "version": 4, + "association_type": "SERVER", + "iburst": false, + "prefer": false, + "network_instance": "FOO", + "source_address": "172.20.20.2", + "key_id": -1, + "address": "4.3.2.1" + } +} diff --git a/test/eos/mocked_data/test_get_ntp_servers/details/show_ip_interface.json b/test/eos/mocked_data/test_get_ntp_servers/details/show_ip_interface.json new file mode 100644 index 000000000..955643613 --- /dev/null +++ b/test/eos/mocked_data/test_get_ntp_servers/details/show_ip_interface.json @@ -0,0 +1,48 @@ +{ + "interfaces": { + "Management0": { + "name": "Management0", + "lineProtocolStatus": "up", + "interfaceStatus": "connected", + "mtu": 1500, + "interfaceAddressBrief": { + "ipAddr": { + "address": "172.20.20.2", + "maskLen": 24 + } + }, + "ipv4Routable240": false, + "ipv4Routable0": false, + "enabled": true, + "description": "", + "interfaceAddress": { + "primaryIp": { + "address": "172.20.20.2", + "maskLen": 24 + }, + "secondaryIps": {}, + "secondaryIpsOrderedList": [], + "virtualIp": { + "address": "0.0.0.0", + "maskLen": 0 + }, + "virtualSecondaryIps": {}, + "virtualSecondaryIpsOrderedList": [], + "broadcastAddress": "255.255.255.255", + "dhcp": false + }, + "proxyArp": false, + "proxyArpAllowDefault": false, + "localProxyArp": false, + "gratuitousArp": false, + "routedAddr": "00:1c:73:7b:8c:1d", + "isVrrpBackup": false, + "vrf": "default", + "urpf": "disable", + "addresslessForwarding": "isInvalid", + "directedBroadcastEnabled": false, + "maxMssIngress": 0, + "maxMssEgress": 0 + } + } +} diff --git a/test/eos/mocked_data/test_get_ntp_servers/details/show_ipv6_interface.json b/test/eos/mocked_data/test_get_ntp_servers/details/show_ipv6_interface.json new file mode 100644 index 000000000..a4b8142b0 --- /dev/null +++ b/test/eos/mocked_data/test_get_ntp_servers/details/show_ipv6_interface.json @@ -0,0 +1,46 @@ +{ + "interfaces": { + "Management0": { + "name": "Management0", + "lineProtocolStatus": "up", + "interfaceStatus": "connected", + "mtu": 1500, + "linkLocal": { + "address": "fe80::21c:73ff:fe7b:8c1d", + "subnet": "fe80::/64", + "active": true, + "leastpref": false, + "dadfailed": false + }, + "state": "enabled", + "addresses": [ + { + "address": "2001:172:20:20::2", + "subnet": "2001:172:20:20::/64", + "active": true, + "leastpref": false, + "dadfailed": false + } + ], + "globalAddressesAreVirtual": false, + "multicastGroupAddresses": [ + "ff02::1", + "ff02::1:ff00:2", + "ff02::1:ff7b:8c1d" + ], + "dadStatus": "unavailable", + "dadAttempts": -1, + "ndReachableTime": 30000, + "ndRetransmitInterval": 1000, + "enhancedDad": false, + "autoConfigStatus": "stateless", + "urpf": "disable", + "urpfV4V6Mismatch": false, + "vrf": "default", + "addrSource": "manual", + "maxMssIngress": 0, + "maxMssEgress": 0, + "acceptUnsolicitedNa": false + } + } +} diff --git a/test/eos/mocked_data/test_get_ntp_servers/details/show_running_config___section_ntp.text b/test/eos/mocked_data/test_get_ntp_servers/details/show_running_config___section_ntp.text new file mode 100644 index 000000000..98a32b9ba --- /dev/null +++ b/test/eos/mocked_data/test_get_ntp_servers/details/show_running_config___section_ntp.text @@ -0,0 +1,2 @@ +ntp server vrf FOO 1.2.3.4 prefer iburst +ntp server vrf FOO 4.3.2.1 local-interface Management0 diff --git a/test/eos/mocked_data/test_get_ntp_servers/normal/expected_result.json b/test/eos/mocked_data/test_get_ntp_servers/normal/expected_result.json index b3a9d7d23..d59b3d99e 100644 --- a/test/eos/mocked_data/test_get_ntp_servers/normal/expected_result.json +++ b/test/eos/mocked_data/test_get_ntp_servers/normal/expected_result.json @@ -1 +1,35 @@ -{"1.2.3.4": {}, "2001:0db8:0a0b:12f0:0000:0000:0000:0001": {}, "5.6.7.8": {}} +{ + "1.2.3.4": { + "port": 123, + "version": 4, + "association_type": "SERVER", + "iburst": false, + "prefer": false, + "network_instance": "default", + "source_address": "", + "key_id": -1, + "address": "1.2.3.4" + }, + "5.6.7.8": { + "port": 123, + "version": 4, + "association_type": "SERVER", + "iburst": false, + "prefer": false, + "network_instance": "default", + "source_address": "", + "key_id": -1, + "address": "5.6.7.8" + }, + "2001:0db8:0a0b:12f0:0000:0000:0000:0001": { + "port": 123, + "version": 4, + "association_type": "SERVER", + "iburst": false, + "prefer": false, + "network_instance": "default", + "source_address": "", + "key_id": -1, + "address": "2001:0db8:0a0b:12f0:0000:0000:0000:0001" + } +} From 053e89989096832c03566c10030f5a0a4bef6282 Mon Sep 17 00:00:00 2001 From: Brandon Ewing Date: Thu, 23 Jan 2025 09:09:54 -0600 Subject: [PATCH 4/6] Add kwarg for model subset testing Since we only have a limited set of models that we are testing subsets of keys for, we should explicitly only allow subsets when testing those getters. --- napalm/base/test/getters.py | 8 ++++++-- napalm/base/test/helpers.py | 12 +++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/napalm/base/test/getters.py b/napalm/base/test/getters.py index dadd7699b..0056aada6 100644 --- a/napalm/base/test/getters.py +++ b/napalm/base/test/getters.py @@ -311,7 +311,9 @@ def test_get_ntp_peers(self, test_case): for peer, peer_details in get_ntp_peers.items(): assert isinstance(peer, str) - assert helpers.test_model(models.NTPPeerDict, peer_details) + assert helpers.test_model( + models.NTPPeerDict, peer_details, allow_subset=True + ) return get_ntp_peers @@ -323,7 +325,9 @@ def test_get_ntp_servers(self, test_case): for server, server_details in get_ntp_servers.items(): assert isinstance(server, str) - assert helpers.test_model(models.NTPServerDict, server_details) + assert helpers.test_model( + models.NTPServerDict, server_details, allow_subset=True + ) return get_ntp_servers diff --git a/napalm/base/test/helpers.py b/napalm/base/test/helpers.py index 1b7cd57cd..0ce734c34 100644 --- a/napalm/base/test/helpers.py +++ b/napalm/base/test/helpers.py @@ -1,14 +1,16 @@ """Several methods to help with the tests.""" -def test_model(model, data): +def test_model(model, data, allow_subset=False): """Return if the dictionary `data` complies with the `model`.""" # Access the underlying schema for a TypedDict directly annotations = model.__annotations__ - if model.__total__: - same_keys = set(annotations.keys()) == set(data.keys()) - else: + if allow_subset: same_keys = set(data.keys()) <= set(annotations.keys()) + source = data + else: + same_keys = set(annotations.keys()) == set(data.keys()) + source = annotations if not same_keys: print( @@ -18,7 +20,7 @@ def test_model(model, data): ) correct_class = True - for key in data.keys(): + for key in source.keys(): correct_class = isinstance(data[key], annotations[key]) and correct_class if not correct_class: print( From 95b0abaa5637908aa49f430297fb5b62ca79c621 Mon Sep 17 00:00:00 2001 From: Brandon Ewing Date: Thu, 23 Jan 2025 12:38:30 -0600 Subject: [PATCH 5/6] Refactor nxos ntp for mypy typing Making NTPPeerDict and NTPServerDict non-identical caused issues with typing since we wre unable to assert the type of the return dictionary easily. This refactor allows us to explicitly state the return type for each function. --- napalm/base/models.py | 12 +----------- napalm/nxos/nxos.py | 27 ++++++++++++++++----------- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/napalm/base/models.py b/napalm/base/models.py index ad104bc56..159c0ac75 100644 --- a/napalm/base/models.py +++ b/napalm/base/models.py @@ -223,17 +223,7 @@ NTPPeerDict = TypedDict( "NTPPeerDict", - { - "address": str, - "port": int, - "version": int, - "association_type": str, - "iburst": bool, - "prefer": bool, - "network_instance": str, - "source_address": str, - "key_id": int, - }, + {}, total=False, ) diff --git a/napalm/nxos/nxos.py b/napalm/nxos/nxos.py index 48a67f483..782cf7c1b 100644 --- a/napalm/nxos/nxos.py +++ b/napalm/nxos/nxos.py @@ -1204,27 +1204,32 @@ def get_arp_table(self, vrf: str = "") -> List[models.ARPTableDict]: ) return arp_table - def _get_ntp_entity( - self, peer_type: str - ) -> Dict[str, Union[models.NTPPeerDict, models.NTPServerDict]]: - ntp_entities: Dict[str, Union[models.NTPPeerDict, models.NTPServerDict]] = {} + def _filter_ntp_table(self, peer_type: str) -> List[str]: + ret = [] command = "show ntp peers" ntp_peers_table = self._get_command_table(command, "TABLE_peers", "ROW_peers") - for ntp_peer in ntp_peers_table: if ntp_peer.get("serv_peer", "").strip() != peer_type: continue peer_addr = napalm.base.helpers.ip(ntp_peer.get("PeerIPAddress").strip()) - # Ignore the type of the following line until NTP data is modelled - ntp_entities[peer_addr] = {} # type: ignore - - return ntp_entities + ret.append(peer_addr) + return ret def get_ntp_peers(self) -> Dict[str, models.NTPPeerDict]: - return self._get_ntp_entity("Peer") + ntp_entities: Dict[str, models.NTPPeerDict] = {} + peers = self._filter_ntp_table("Peer") + for peer_addr in peers: + ntp_entities[peer_addr] = {} + + return ntp_entities def get_ntp_servers(self) -> Dict[str, models.NTPServerDict]: - return self._get_ntp_entity("Server") + ntp_entities: Dict[str, models.NTPServerDict] = {} + peers = self._filter_ntp_table("Server") + for peer_addr in peers: + ntp_entities[peer_addr] = {} + + return ntp_entities def get_ntp_stats(self) -> List[models.NTPStats]: ntp_stats: List[models.NTPStats] = [] From 8e9f538299bd62293db33cf8bc636a6278e2fd60 Mon Sep 17 00:00:00 2001 From: Brandon Ewing Date: Thu, 23 Jan 2025 15:06:12 -0600 Subject: [PATCH 6/6] remove unused EOS NTP peer textfsm --- napalm/eos/utils/textfsm_templates/ntp_peers.tpl | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 napalm/eos/utils/textfsm_templates/ntp_peers.tpl diff --git a/napalm/eos/utils/textfsm_templates/ntp_peers.tpl b/napalm/eos/utils/textfsm_templates/ntp_peers.tpl deleted file mode 100644 index 58042e0ba..000000000 --- a/napalm/eos/utils/textfsm_templates/ntp_peers.tpl +++ /dev/null @@ -1,6 +0,0 @@ -Value NTPPeer (\w+.*) - -Start - ^ntp\s+server\s+${NTPPeer} -> Record - -EOF