Fix NTP/SNMP/probe config on NAPALM proxy minions (bare-name templates)

changelog/69793.fixed.md

@@ -0,0 +1,9 @@
+Fixed NTP, SNMP and RPM-probe configuration on NAPALM (proxy) minions.
+``ntp.set_peers`` / ``set_servers`` / ``delete_peers`` / ``delete_servers``,
+``snmp.update_config`` / ``remove_config`` and ``probes.set_probes`` /
+``delete_probes`` / ``schedule_probes`` no longer fail with ``Local file source
+set_ntp_peers does not exist``. Like ``users.set_users`` (see #62170), these
+functions passed bare template names to ``net.load_template``, which stopped
+resolving when native NAPALM template support was removed in the Sodium release.
+They now resolve the NAPALM-shipped per-driver template to an absolute path and
+render it through the Salt pipeline.

salt/modules/napalm_ntp.py

@@ -230,8 +230,11 @@ def set_peers(*peers, **options):
     commit = options.pop("commit", True)
 
     # pylint: disable=undefined-variable
+    resolved = salt.utils.napalm.template_path(napalm_device, "set_ntp_peers")
+    if resolved is None:
+        return salt.utils.napalm.template_not_available("set_ntp_peers", napalm_device)
     return __salt__["net.load_template"](
-        "set_ntp_peers",
+        resolved,
         peers=peers,
         test=test,
         commit=commit,
@@ -268,8 +271,13 @@ def set_servers(*servers, **options):
     commit = options.pop("commit", True)
 
     # pylint: disable=undefined-variable
+    resolved = salt.utils.napalm.template_path(napalm_device, "set_ntp_servers")
+    if resolved is None:
+        return salt.utils.napalm.template_not_available(
+            "set_ntp_servers", napalm_device
+        )
     return __salt__["net.load_template"](
-        "set_ntp_servers",
+        resolved,
         servers=servers,
         test=test,
         commit=commit,
@@ -307,8 +315,13 @@ def delete_peers(*peers, **options):
     commit = options.pop("commit", True)
 
     # pylint: disable=undefined-variable
+    resolved = salt.utils.napalm.template_path(napalm_device, "delete_ntp_peers")
+    if resolved is None:
+        return salt.utils.napalm.template_not_available(
+            "delete_ntp_peers", napalm_device
+        )
     return __salt__["net.load_template"](
-        "delete_ntp_peers",
+        resolved,
         peers=peers,
         test=test,
         commit=commit,
@@ -347,8 +360,13 @@ def delete_servers(*servers, **options):
     commit = options.pop("commit", True)
 
     # pylint: disable=undefined-variable
+    resolved = salt.utils.napalm.template_path(napalm_device, "delete_ntp_servers")
+    if resolved is None:
+        return salt.utils.napalm.template_not_available(
+            "delete_ntp_servers", napalm_device
+        )
     return __salt__["net.load_template"](
-        "delete_ntp_servers",
+        resolved,
         servers=servers,
         test=test,
         commit=commit,

salt/modules/napalm_probes.py

@@ -254,8 +254,11 @@ def set_probes(
     """
 
     # pylint: disable=undefined-variable
+    resolved = salt.utils.napalm.template_path(napalm_device, "set_probes")
+    if resolved is None:
+        return salt.utils.napalm.template_not_available("set_probes", napalm_device)
     return __salt__["net.load_template"](
-        "set_probes",
+        resolved,
         probes=probes,
         test=test,
         commit=commit,
@@ -310,8 +313,11 @@ def delete_probes(
     """
 
     # pylint: disable=undefined-variable
+    resolved = salt.utils.napalm.template_path(napalm_device, "delete_probes")
+    if resolved is None:
+        return salt.utils.napalm.template_not_available("delete_probes", napalm_device)
     return __salt__["net.load_template"](
-        "delete_probes",
+        resolved,
         probes=probes,
         test=test,
         commit=commit,
@@ -367,8 +373,13 @@ def schedule_probes(
     """
 
     # pylint: disable=undefined-variable
+    resolved = salt.utils.napalm.template_path(napalm_device, "schedule_probes")
+    if resolved is None:
+        return salt.utils.napalm.template_not_available(
+            "schedule_probes", napalm_device
+        )
     return __salt__["net.load_template"](
-        "schedule_probes",
+        resolved,
         probes=probes,
         test=test,
         commit=commit,

salt/modules/napalm_snmp.py

@@ -129,7 +129,14 @@ def remove_config(
         salt '*' snmp.remove_config community='abcd'
     """
 
-    dic = {"template_name": "delete_snmp_config", "test": test, "commit": commit}
+    resolved = salt.utils.napalm.template_path(
+        napalm_device, "delete_snmp_config"  # pylint: disable=undefined-variable
+    )
+    if resolved is None:
+        return salt.utils.napalm.template_not_available(
+            "delete_snmp_config", napalm_device  # pylint: disable=undefined-variable
+        )
+    dic = {"template_name": resolved, "test": test, "commit": commit}
 
     if chassis_id:
         dic["chassis_id"] = chassis_id
@@ -206,7 +213,14 @@ def update_config(
                 True
     """
 
-    dic = {"template_name": "snmp_config", "test": test, "commit": commit}
+    resolved = salt.utils.napalm.template_path(
+        napalm_device, "snmp_config"  # pylint: disable=undefined-variable
+    )
+    if resolved is None:
+        return salt.utils.napalm.template_not_available(
+            "snmp_config", napalm_device  # pylint: disable=undefined-variable
+        )
+    dic = {"template_name": resolved, "test": test, "commit": commit}
 
     if chassis_id:
         dic["chassis_id"] = chassis_id

salt/utils/napalm.py

@@ -16,7 +16,9 @@
 
 import copy
 import importlib
+import inspect
 import logging
+import os.path
 import traceback
 from functools import wraps
 
@@ -100,6 +102,67 @@ def virtual(opts, virtualname, filename):
         )
 
 
+def template_path(napalm_device, template_name):
+    """
+    Return the absolute path to a NAPALM-shipped Jinja template (e.g.
+    ``set_ntp_peers``) for the driver backing this proxy, or ``None`` if the
+    driver does not ship one.
+
+    NAPALM keeps these config templates in a ``templates`` directory next to
+    each driver module and resolves them by walking the driver class MRO
+    (concrete driver first, then its bases). ``net.load_template`` used to route
+    bare template names into NAPALM's own renderer, but that path was removed in
+    the Sodium release; resolving the template to an absolute path lets the
+    still-supported Salt rendering pipeline render it instead.
+    """
+    driver = napalm_device.get("DRIVER") if napalm_device else None
+    if driver is None:
+        return None
+    for klass in type(driver).__mro__:
+        try:
+            module_file = inspect.getfile(klass)
+        except (TypeError, OSError):
+            # Built-in types (e.g. ``object``) raise TypeError; classes without
+            # an on-disk source (``__main__``, frozen) raise OSError. Neither
+            # can ship a template dir, so move on.
+            continue
+        candidate = os.path.join(
+            os.path.dirname(module_file), "templates", f"{template_name}.j2"
+        )
+        if os.path.isfile(candidate):
+            return candidate
+    return None
+
+
+def template_not_available(template_name, napalm_device):
+    """
+    Standard failure payload returned by the NAPALM config helpers when the
+    driver backing this proxy does not ship ``template_name`` (e.g. the ``ios``
+    driver has no user templates). Mirrors the shape of ``net.load_template``'s
+    return so callers and states handle it uniformly.
+
+    Returning here short-circuits ``net.load_template``, which would otherwise be
+    responsible for closing the per-call connection a non-always-alive proxy /
+    minion opened (``proxy_napalm_wrap`` only closes on ``force_reconnect``). So
+    close it here in that mode to avoid leaking the session.
+    """
+    driver_name = napalm_device.get("DRIVER_NAME") if napalm_device else None
+    opts = napalm_device.get("__opts__") if napalm_device else None
+    if opts and not_always_alive(opts) and napalm_device.get("CLOSE", True):
+        try:
+            napalm_device["DRIVER"].close()
+        except Exception:  # pylint: disable=broad-except
+            log.debug("Failed to close the connection", exc_info=True)
+    return {
+        "result": False,
+        "out": None,
+        "comment": (
+            f"The '{template_name}' template is not available for the"
+            f" '{driver_name}' driver."
+        ),
+    }
+
+
 def call(napalm_device, method, *args, **kwargs):
     """
     Calls arbitrary methods from the network driver instance.

tests/pytests/unit/modules/napalm/test_ntp.py

@@ -9,89 +9,93 @@
 from tests.support.mock import MagicMock, patch
 
 
-def mock_net_load_template(template, *args, **kwargs):
-    if template == "set_ntp_peers" or template == "delete_ntp_peers":
-        assert "1.2.3.4" in kwargs["peers"]
-    if template == "set_ntp_servers" or template == "delete_ntp_servers":
-        assert "2.2.3.4" in kwargs["servers"]
-
-
 @pytest.fixture
 def configure_loader_modules():
-    module_globals = {
-        "__salt__": {
-            "config.get": MagicMock(
-                return_value={"test": {"driver": "test", "key": "2orgk34kgk34g"}}
-            ),
-            "file.file_exists": napalm_test_support.true,
-            "file.join": napalm_test_support.join,
-            "file.get_managed": napalm_test_support.get_managed_file,
-            "random.hash": napalm_test_support.random_hash,
-            "net.load_template": mock_net_load_template,
-        }
-    }
-
-    return {napalm_ntp: module_globals}
-
-
-def test_peers():
-    with patch(
-        "salt.utils.napalm.get_device",
-        MagicMock(return_value=napalm_test_support.MockNapalmDevice()),
-    ):
-        ret = napalm_ntp.peers()
-        assert "172.17.17.1" in ret["out"]
+    return {napalm_ntp: {"__salt__": {"config.get": MagicMock(return_value={})}}}
 
 
-def test_servers():
-    with patch(
-        "salt.utils.napalm.get_device",
-        MagicMock(return_value=napalm_test_support.MockNapalmDevice()),
-    ):
-        ret = napalm_ntp.servers()
-        assert "172.17.17.1" in ret["out"]
-
-
-def test_stats():
-    with patch(
+def _mock_device():
+    return patch(
         "salt.utils.napalm.get_device",
         MagicMock(return_value=napalm_test_support.MockNapalmDevice()),
-    ):
-        ret = napalm_ntp.stats()
-        assert ret["out"][0]["reachability"] == 377
+    )
 
 
-def test_set_peers():
-    with patch(
-        "salt.utils.napalm.get_device",
-        MagicMock(return_value=napalm_test_support.MockNapalmDevice()),
-    ):
-        ret = napalm_ntp.set_peers("1.2.3.4", "5.6.7.8")
-        assert ret is None
+# --- read pass-throughs (unchanged behaviour) -------------------------------
 
 
-def test_set_servers():
-    with patch(
-        "salt.utils.napalm.get_device",
-        MagicMock(return_value=napalm_test_support.MockNapalmDevice()),
-    ):
-        ret = napalm_ntp.set_servers("2.2.3.4", "6.6.7.8")
-        assert ret is None
+def test_peers():
+    with _mock_device():
+        assert "172.17.17.1" in napalm_ntp.peers()["out"]
 
 
-def test_delete_servers():
-    with patch(
-        "salt.utils.napalm.get_device",
-        MagicMock(return_value=napalm_test_support.MockNapalmDevice()),
-    ):
-        ret = napalm_ntp.delete_servers("2.2.3.4", "6.6.7.8")
-        assert ret is None
+def test_servers():
+    with _mock_device():
+        assert "172.17.17.1" in napalm_ntp.servers()["out"]
 
 
-def test_delete_peers():
-    with patch(
-        "salt.utils.napalm.get_device",
-        MagicMock(return_value=napalm_test_support.MockNapalmDevice()),
+def test_stats():
+    with _mock_device():
+        assert napalm_ntp.stats()["out"][0]["reachability"] == 377
+
+
+# --- config writers: route onto the driver's resolved template (#62170) -----
+
+RESOLVED = "/opt/napalm/junos/templates/tpl.j2"
+
+
+def _route(func, *args):
+    """Run a config writer with template resolution + net.load_template mocked."""
+    device = napalm_test_support.MockNapalmDevice()
+    load_template = MagicMock(return_value={"result": True, "comment": "", "out": None})
+    tpath = MagicMock(return_value=RESOLVED)
+    with patch("salt.utils.napalm.get_device", MagicMock(return_value=device)), patch(
+        "salt.utils.napalm.template_path", tpath
+    ), patch.dict(napalm_ntp.__salt__, {"net.load_template": load_template}):
+        ret = func(*args, test=True, commit=False)
+    return ret, tpath, load_template, device
+
+
+@pytest.mark.parametrize(
+    "func_name, template_name, arg_key, values",
+    [
+        ("set_peers", "set_ntp_peers", "peers", ("1.2.3.4", "5.6.7.8")),
+        ("set_servers", "set_ntp_servers", "servers", ("2.2.3.4", "6.6.7.8")),
+        ("delete_peers", "delete_ntp_peers", "peers", ("1.2.3.4", "5.6.7.8")),
+        ("delete_servers", "delete_ntp_servers", "servers", ("2.2.3.4", "6.6.7.8")),
+    ],
+)
+def test_writer_routes_resolved_template(func_name, template_name, arg_key, values):
+    ret, tpath, load_template, device = _route(getattr(napalm_ntp, func_name), *values)
+    assert ret == {"result": True, "comment": "", "out": None}
+    # Correct template name requested (guards a set/delete or peers/servers mixup).
+    assert tpath.call_args[0][1] == template_name
+    load_template.assert_called_once()
+    args, kwargs = load_template.call_args
+    # Absolute resolved path passed, not the bare name.
+    assert args[0] == RESOLVED
+    assert values[0] in kwargs[arg_key]
+    assert kwargs["test"] is True
+    assert kwargs["commit"] is False
+    # The open proxy device is threaded through by identity (guards =None / wrong).
+    assert kwargs["inherit_napalm_device"] is device
+
+
+@pytest.mark.parametrize(
+    "func_name, template_name",
+    [
+        ("set_peers", "set_ntp_peers"),
+        ("set_servers", "set_ntp_servers"),
+        ("delete_peers", "delete_ntp_peers"),
+        ("delete_servers", "delete_ntp_servers"),
+    ],
+)
+def test_writer_no_template_for_driver(func_name, template_name):
+    with _mock_device(), patch(
+        "salt.utils.napalm.template_path", MagicMock(return_value=None)
     ):
-        ret = napalm_ntp.delete_peers("1.2.3.4", "5.6.7.8")
-        assert ret is None
+        ret = getattr(napalm_ntp, func_name)("1.2.3.4")
+    assert ret["result"] is False
+    # Exact quoted name (guards a set/delete literal swap in the failure branch).
+    assert f"'{template_name}'" in ret["comment"]
+    assert "not available" in ret["comment"]