feat!: fix templates with the caller's values replace run_device_action
The driver filled run_device_action with an application's remediations and their values: community "public", "Managed by netOrk", the caller's address guessed from ss and a /24 ufw rule, the proxy from an optional_args key. It implements napalm-device-types' fix templates instead (4.12), and the caller supplies every value: - configure_snmp_agent(settings): net-snmp installed, snmpd.conf with one rocommunity line per allowed source, udp/161 opened in ufw or iptables for those sources only, snmpd enabled and restarted. Every step reads its exit status. A community, source or text that would break the file is refused. - configure_package_proxy(url): apt's 00proxy, or removed for "". - grant_container_engine_access(user): usermod -aG docker for the user named. - ensure_package(name): from the base package, over get_packages and install_package. run_device_action, its four _action_* methods and the unused apt_update_upgrade are gone. The sudo hint no longer names an application's settings. BREAKING CHANGE: run_device_action is removed; callers use the templates. Requires napalm-device-types 4.12. Closes #27
This commit is contained in:
Christian Manivong committed
1 parent
c98dd928e6
commit
272bf21f8e
4 files changed
+322
-358
No files matched your search
@@ -0,0 +1,182 @@
|
||||
"""The fix templates (napalm-device-types ``fixes``): the caller supplies every value.
|
||||
|
||||
They replace ``run_device_action``. The driver knows how each step is done on a
|
||||
Linux host; the community, the allowed sources, the proxy and the user are the
|
||||
caller's (napalm-device-types docs/STRUCTURE.md).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from napalm_linux import LinuxDriver
|
||||
from napalm_linux.linux import render_snmpd_conf
|
||||
|
||||
|
||||
class TestSnmpdConf:
|
||||
def test_one_read_only_line_per_allowed_source(self):
|
||||
conf = render_snmpd_conf(
|
||||
{
|
||||
"community": "c0mm",
|
||||
"allowed_sources": ["10.0.0.5", "10.1.0.0/24"],
|
||||
"contact": "ops",
|
||||
"location": "rack 1",
|
||||
}
|
||||
)
|
||||
|
||||
assert conf == (
|
||||
"agentAddress udp:161\n"
|
||||
"rocommunity c0mm 10.0.0.5/32\n"
|
||||
"rocommunity c0mm 10.1.0.0/24\n"
|
||||
"sysLocation rack 1\n"
|
||||
"sysContact ops\n"
|
||||
)
|
||||
|
||||
def test_without_sources_every_address_may_ask(self):
|
||||
assert "rocommunity c0mm\n" in render_snmpd_conf(
|
||||
{"community": "c0mm", "allowed_sources": []}
|
||||
)
|
||||
|
||||
def test_a_listen_address_binds_the_agent(self):
|
||||
conf = render_snmpd_conf({"community": "c", "allowed_sources": [], "listen": "10.0.0.1"})
|
||||
assert conf.startswith("agentAddress udp:10.0.0.1:161\n")
|
||||
|
||||
def test_no_application_name_is_written(self):
|
||||
conf = render_snmpd_conf({"community": "c", "allowed_sources": []})
|
||||
assert "netork" not in conf.lower()
|
||||
|
||||
@pytest.mark.parametrize("community", ["two words", 'quo"te', "has#hash", ""])
|
||||
def test_a_community_that_would_break_the_file_is_refused(self, community):
|
||||
with pytest.raises(ValueError):
|
||||
render_snmpd_conf({"community": community, "allowed_sources": []})
|
||||
|
||||
def test_a_source_that_is_no_address_is_refused(self):
|
||||
with pytest.raises(ValueError):
|
||||
render_snmpd_conf({"community": "c", "allowed_sources": ["10.0.0.5; rm -rf /"]})
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def driver():
|
||||
d = LinuxDriver("h", "admin", "p")
|
||||
d._device = MagicMock()
|
||||
d._pkg_manager = "apt"
|
||||
d._root = True
|
||||
return d
|
||||
|
||||
|
||||
class TestSnmpAgentSteps:
|
||||
SETTINGS = {"community": "c0mm", "allowed_sources": ["10.0.0.5"]}
|
||||
|
||||
def _record(self, driver, status=0, firewall="ufw"):
|
||||
ran: list[str] = []
|
||||
|
||||
def sudo_status(command, read_timeout=100):
|
||||
ran.append(command)
|
||||
return ("", status)
|
||||
|
||||
driver._sudo_status = sudo_status
|
||||
driver._send = lambda command, read_timeout=100: (
|
||||
f"/usr/sbin/{firewall}" if command == f"command -v {firewall} 2>/dev/null" else ""
|
||||
)
|
||||
driver._install_file_as_root = MagicMock(
|
||||
return_value=MagicMock(exit_code=0, stdout="", stderr="")
|
||||
)
|
||||
return ran
|
||||
|
||||
def test_installs_writes_opens_and_starts(self, driver):
|
||||
ran = self._record(driver)
|
||||
|
||||
result = driver.configure_snmp_agent(self.SETTINGS)
|
||||
|
||||
assert result["success"] is True
|
||||
assert [s["step"] for s in result["steps"]] == ["install", "configure", "firewall", "start"]
|
||||
assert "apt-get install -y snmpd snmp" in ran[0]
|
||||
path, content = driver._install_file_as_root.call_args.args
|
||||
assert path == "/etc/snmp/snmpd.conf" and "rocommunity c0mm 10.0.0.5/32" in content
|
||||
assert ran[1] == "ufw allow from 10.0.0.5/32 to any port 161 proto udp"
|
||||
assert "systemctl restart snmpd" in ran[2]
|
||||
|
||||
def test_the_firewall_opens_only_for_the_given_sources_with_iptables(self, driver):
|
||||
ran = self._record(driver, firewall="iptables")
|
||||
|
||||
driver.configure_snmp_agent(self.SETTINGS)
|
||||
|
||||
assert "iptables -I INPUT -s 10.0.0.5/32 -p udp --dport 161 -j ACCEPT" in ran[1]
|
||||
|
||||
def test_no_sources_change_no_firewall(self, driver):
|
||||
ran = self._record(driver)
|
||||
|
||||
result = driver.configure_snmp_agent({"community": "c0mm", "allowed_sources": []})
|
||||
|
||||
assert result["success"] is True
|
||||
assert not any("ufw" in c or "iptables" in c for c in ran)
|
||||
|
||||
def test_a_failed_install_stops_the_fix(self, driver):
|
||||
ran = self._record(driver, status=100)
|
||||
|
||||
result = driver.configure_snmp_agent(self.SETTINGS)
|
||||
|
||||
assert result["success"] is False
|
||||
assert len(ran) == 1
|
||||
driver._install_file_as_root.assert_not_called()
|
||||
|
||||
|
||||
class TestPackageProxyAndEngineAccess:
|
||||
def test_the_proxy_is_the_caller_s(self, driver):
|
||||
driver._install_file_as_root = MagicMock(
|
||||
return_value=MagicMock(exit_code=0, stdout="", stderr="")
|
||||
)
|
||||
|
||||
result = driver.configure_package_proxy("http://cache.lan:3142")
|
||||
|
||||
assert result["success"] is True
|
||||
path, content = driver._install_file_as_root.call_args.args
|
||||
assert (path, content) == (
|
||||
"/etc/apt/apt.conf.d/00proxy",
|
||||
'Acquire::http::Proxy "http://cache.lan:3142";\n',
|
||||
)
|
||||
|
||||
def test_a_proxy_url_that_would_break_the_file_is_refused(self, driver):
|
||||
driver._install_file_as_root = MagicMock()
|
||||
|
||||
result = driver.configure_package_proxy('http://x"; Acquire::x "y')
|
||||
|
||||
assert result["success"] is False
|
||||
driver._install_file_as_root.assert_not_called()
|
||||
|
||||
def test_only_apt_takes_a_proxy(self, driver):
|
||||
driver._pkg_manager = "dnf"
|
||||
|
||||
assert driver.configure_package_proxy("http://cache.lan:3142")["success"] is False
|
||||
|
||||
def test_engine_access_for_the_user_the_caller_names(self, driver):
|
||||
ran = []
|
||||
driver._sudo_status = lambda command, read_timeout=100: ran.append(command) or ("", 0)
|
||||
|
||||
result = driver.grant_container_engine_access("deploy")
|
||||
|
||||
assert result["success"] is True
|
||||
assert ran == ["usermod -aG docker deploy"]
|
||||
|
||||
def test_a_user_name_that_is_no_user_name_is_refused(self, driver):
|
||||
driver._sudo_status = MagicMock()
|
||||
|
||||
assert driver.grant_container_engine_access("x; reboot")["success"] is False
|
||||
driver._sudo_status.assert_not_called()
|
||||
|
||||
|
||||
def test_run_device_action_is_gone():
|
||||
assert (
|
||||
not hasattr(LinuxDriver, "run_device_action")
|
||||
or LinuxDriver.run_device_action is None
|
||||
or ("run_device_action" not in vars(LinuxDriver))
|
||||
)
|
||||
for name in (
|
||||
"_action_fix_snmp",
|
||||
"_action_fix_apt_proxy",
|
||||
"_action_fix_docker_permissions",
|
||||
"_action_apt_update_upgrade",
|
||||
):
|
||||
assert not hasattr(LinuxDriver, name), name
|
||||
+2
-143
@@ -807,151 +807,10 @@ def test_get_facts_fallback_vendor_when_dmi_empty(driver):
|
||||
assert facts["vendor"] == "Linux" # fallback to VENDOR class attribute
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _action_fix_snmp / _action_apt_update_upgrade
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _fix_snmp_send_side_effect(command: str, read_timeout: float = 100) -> str:
|
||||
"""Canned responses covering every _send() call _action_fix_snmp makes."""
|
||||
if command.startswith("sudo -n true"):
|
||||
return "" # passwordless sudo works
|
||||
if command.startswith("command -v apt"):
|
||||
return "/usr/bin/apt"
|
||||
if command.startswith("command -v"):
|
||||
return "" # ufw/iptables not found, other pkg managers not found
|
||||
if command.startswith("ss -tnp"):
|
||||
return "" # no netork_ip detected — skips firewall step
|
||||
if command.startswith("cat /etc/snmp/snmpd.conf"):
|
||||
return "agentAddress udp:161\nrocommunity public\n"
|
||||
if command.startswith("snmpget"):
|
||||
return "STRING: Linux test"
|
||||
return ""
|
||||
|
||||
|
||||
class TestActionFixSnmp:
|
||||
def test_runs_apt_get_update_before_install(self, driver):
|
||||
driver._pkg_manager = "apt"
|
||||
driver._sudo_password = None
|
||||
sudo_calls: list[str] = []
|
||||
|
||||
def _sudo_side_effect(command: str, read_timeout: float = 100) -> str:
|
||||
sudo_calls.append(command)
|
||||
return ""
|
||||
|
||||
with (
|
||||
patch.object(driver, "_send", side_effect=_fix_snmp_send_side_effect),
|
||||
patch.object(driver, "_sudo", side_effect=_sudo_side_effect),
|
||||
):
|
||||
driver._action_fix_snmp()
|
||||
|
||||
update_idx = next(i for i, c in enumerate(sudo_calls) if "apt-get update" in c)
|
||||
install_idx = next(i for i, c in enumerate(sudo_calls) if "apt-get install" in c)
|
||||
assert update_idx < install_idx
|
||||
|
||||
def test_install_exception_returns_failure_instead_of_raising(self, driver):
|
||||
driver._pkg_manager = "apt"
|
||||
driver._sudo_password = None
|
||||
|
||||
def _sudo_side_effect(command: str, read_timeout: float = 100) -> str:
|
||||
if "apt-get install" in command:
|
||||
raise TimeoutError("connection timed out")
|
||||
return ""
|
||||
|
||||
with (
|
||||
patch.object(driver, "_send", side_effect=_fix_snmp_send_side_effect),
|
||||
patch.object(driver, "_sudo", side_effect=_sudo_side_effect),
|
||||
):
|
||||
result = driver._action_fix_snmp()
|
||||
|
||||
assert result["success"] is False
|
||||
assert "install failed" in result["output"]
|
||||
|
||||
def test_apt_get_update_failure_is_non_fatal(self, driver):
|
||||
"""apt-get update failing (e.g. transient network issue) must not
|
||||
abort the whole action — install is still attempted."""
|
||||
driver._pkg_manager = "apt"
|
||||
driver._sudo_password = None
|
||||
|
||||
def _sudo_side_effect(command: str, read_timeout: float = 100) -> str:
|
||||
if "apt-get update" in command:
|
||||
raise TimeoutError("network unreachable")
|
||||
return ""
|
||||
|
||||
with (
|
||||
patch.object(driver, "_send", side_effect=_fix_snmp_send_side_effect),
|
||||
patch.object(driver, "_sudo", side_effect=_sudo_side_effect),
|
||||
):
|
||||
result = driver._action_fix_snmp()
|
||||
|
||||
assert "apt-get update failed" in result["output"]
|
||||
|
||||
|
||||
class TestActionAptUpdateUpgrade:
|
||||
def test_skips_when_not_apt(self, driver):
|
||||
driver._pkg_manager = "dnf"
|
||||
result = driver._action_apt_update_upgrade()
|
||||
assert result["success"] is True
|
||||
assert "Skipped" in result["output"]
|
||||
|
||||
def test_fails_when_sudo_needs_password_and_none_configured(self, driver):
|
||||
driver._pkg_manager = "apt"
|
||||
driver._sudo_password = None
|
||||
with patch.object(driver, "_send", return_value="__SUDO_NEEDS_PW__"):
|
||||
result = driver._action_apt_update_upgrade()
|
||||
assert result["success"] is False
|
||||
assert "sudo requires a password" in result["output"]
|
||||
|
||||
def test_runs_update_then_upgrade_successfully(self, driver):
|
||||
driver._pkg_manager = "apt"
|
||||
driver._sudo_password = "secret" # noqa: S105
|
||||
with (
|
||||
patch.object(driver, "_send", return_value=""),
|
||||
patch.object(
|
||||
driver,
|
||||
"_sudo",
|
||||
side_effect=["Reading package lists... Done", "0 upgraded, 0 newly installed"],
|
||||
) as mock_sudo,
|
||||
):
|
||||
result = driver._action_apt_update_upgrade()
|
||||
|
||||
assert result["success"] is True
|
||||
assert "[update]" in result["output"]
|
||||
assert "[upgrade]" in result["output"]
|
||||
update_call, upgrade_call = mock_sudo.call_args_list
|
||||
assert "apt-get update" in update_call.args[0]
|
||||
# full-upgrade (not plain upgrade) — plain upgrade silently holds back
|
||||
# packages whose newer version needs to install/remove dependencies.
|
||||
assert "apt-get full-upgrade" in upgrade_call.args[0]
|
||||
|
||||
def test_exception_during_upgrade_returns_failure(self, driver):
|
||||
driver._pkg_manager = "apt"
|
||||
driver._sudo_password = "secret" # noqa: S105
|
||||
with (
|
||||
patch.object(driver, "_send", return_value=""),
|
||||
patch.object(
|
||||
driver, "_sudo", side_effect=["update ok", TimeoutError("connection lost")]
|
||||
),
|
||||
):
|
||||
result = driver._action_apt_update_upgrade()
|
||||
|
||||
assert result["success"] is False
|
||||
assert "[error]" in result["output"]
|
||||
|
||||
|
||||
class TestRunDeviceActionDispatch:
|
||||
def test_apt_update_upgrade_action_dispatches(self, driver):
|
||||
with patch.object(
|
||||
driver, "_action_apt_update_upgrade", return_value={"success": True, "output": ""}
|
||||
) as mock_action:
|
||||
driver.run_device_action("apt_update_upgrade")
|
||||
mock_action.assert_called_once()
|
||||
|
||||
|
||||
class TestNoDockerOfItsOwn:
|
||||
"""Containers are read and handled by netOrk over the Engine API, through
|
||||
``open_container_engine`` (NetOrk/netork#765). The old CLI methods are gone;
|
||||
only the OS action that lets the login user use Docker stays."""
|
||||
only the fix that lets the login user use Docker stays."""
|
||||
|
||||
def test_the_cli_methods_are_gone(self):
|
||||
for name in ("get_docker_info", "get_docker_outdated", "reconstruct_docker_run", "_docker_bin"):
|
||||
@@ -959,7 +818,7 @@ class TestNoDockerOfItsOwn:
|
||||
|
||||
def test_the_engine_access_and_the_permission_fix_stay(self):
|
||||
assert hasattr(LinuxDriver, "open_container_engine")
|
||||
assert hasattr(LinuxDriver, "_action_fix_docker_permissions")
|
||||
assert callable(LinuxDriver.grant_container_engine_access)
|
||||
|
||||
|
||||
class TestUninstallPackage:
|
||||
|
||||
@@ -138,10 +138,16 @@ def _every_line(driver: LinuxDriver, host: Host) -> list[str]:
|
||||
return typed + [run.line for run in host.runs]
|
||||
|
||||
|
||||
_SETTINGS = {"community": "c0mm", "allowed_sources": []}
|
||||
|
||||
ACTIONS = [
|
||||
("_action_fix_snmp", "/etc/snmp/snmpd.conf", b"agentAddress udp:161"),
|
||||
(
|
||||
"_action_fix_apt_proxy",
|
||||
lambda driver: driver.configure_snmp_agent(_SETTINGS),
|
||||
"/etc/snmp/snmpd.conf",
|
||||
b"agentAddress udp:161",
|
||||
),
|
||||
(
|
||||
lambda driver: driver.configure_package_proxy("http://proxy:3142"),
|
||||
"/etc/apt/apt.conf.d/00proxy",
|
||||
b'Acquire::http::Proxy "http://proxy:3142";',
|
||||
),
|
||||
@@ -160,7 +166,7 @@ class TestStagedWrite:
|
||||
host = Host()
|
||||
driver = _driver(host)
|
||||
|
||||
getattr(driver, action)()
|
||||
action(driver)
|
||||
|
||||
for planted in _PLANTED:
|
||||
assert not any(planted in line for line in _every_line(driver, host))
|
||||
@@ -171,7 +177,7 @@ class TestStagedWrite:
|
||||
host = Host()
|
||||
driver = _driver(host)
|
||||
|
||||
getattr(driver, action)()
|
||||
action(driver)
|
||||
|
||||
stage = next(run for run in host.unprivileged if "mktemp" in run.line)
|
||||
assert "umask 077" in stage.line
|
||||
@@ -182,7 +188,7 @@ class TestStagedWrite:
|
||||
host = Host()
|
||||
driver = _driver(host)
|
||||
|
||||
getattr(driver, action)()
|
||||
action(driver)
|
||||
|
||||
install = next(script for script in host.root_scripts if "install -m" in script)
|
||||
part = f"{target}.netork-tmp"
|
||||
@@ -194,7 +200,7 @@ class TestStagedWrite:
|
||||
host = Host()
|
||||
driver = _driver(host)
|
||||
|
||||
getattr(driver, action)()
|
||||
action(driver)
|
||||
|
||||
assert not any(re.search(r"\bch(own|mod)\b", script) for script in host.root_scripts)
|
||||
|
||||
@@ -202,7 +208,7 @@ class TestStagedWrite:
|
||||
host = Host(stage_output="mktemp: No space left on device\n", stage_status=1)
|
||||
driver = _driver(host)
|
||||
|
||||
getattr(driver, action)()
|
||||
action(driver)
|
||||
|
||||
assert not any("install -m" in script for script in host.root_scripts)
|
||||
|
||||
@@ -210,7 +216,7 @@ class TestStagedWrite:
|
||||
host = Host(stage_output="/etc/shadow")
|
||||
driver = _driver(host)
|
||||
|
||||
getattr(driver, action)()
|
||||
action(driver)
|
||||
|
||||
assert not any("/etc/shadow" in script for script in host.root_scripts)
|
||||
|
||||
@@ -223,7 +229,7 @@ def test_a_failed_install_is_reported():
|
||||
driver = _driver(host)
|
||||
driver._device.send_command.side_effect = lambda command, **_k: ""
|
||||
|
||||
result = driver._action_fix_apt_proxy()
|
||||
result = driver.configure_package_proxy("http://proxy:3142")
|
||||
|
||||
assert result["success"] is False
|
||||
assert "install: cannot create" in result["output"]
|
||||
|
||||
Reference in new issue
Block a user