Repository: napalm-automation/napalm
Pull request #2135 (2024-07-30 23:04:15)

[eos] fix banner when transport is ssh
When setting transport="ssh" the heredoc commands cause an issue because they are not a string.
This allows for heredoc commands to not be converted when using "ssh" transport  

ktbyers: @bewing You know a lot more about this area of the code? Any objections?
bewing: Can we add a SSH test double class to https://github.com/napalm-automation/napalm/blob/develop/test/eos/conftest.py and possibly start testing some of the differences in behavior between the SSH and eAPI transports/drivers/devices?
ktbyers: @bewing Do we want to do this in `conftest.py` or have a separate `test/eos` and `test/eos_ssh` folder.

The latter is probably more consistent with what we did for other drivers (though we also have more code separation at the driver level between `nxos` and `nxos_ssh` (and between `iosxr` and `iosxr_netconf`).

Ultimately, I am fine with either/both (though probably have a preference for separate test folders for the different drivers+transport).
bewing: I also think `eos_ssh` folder makes more sense.  I'll look at cribbing what's necessary from the existing _ssh folders and the standard EOS test double to arrange the boilerplate.  @derekdainys if you can actually collect the data for the unit test it would be great - let me know on slack if you need any guidance
derekdainys: @bewing  Would like to pick this up again
Noticed that we didnt have a test for heredoc that has blank lines for example in a banner and I added it
This caused me to remove the blank line skip in `_load_config`
Added a `test/eos_ssh` folder which sets a transport=ssh and does some heredoc testing

Patch:
diff --git a/napalm/eos/eos.py b/napalm/eos/eos.py
index 56a2898f..d4b4b0e8 100644
--- a/napalm/eos/eos.py
+++ b/napalm/eos/eos.py
@@ -421,18 +421,17 @@ class EOSDriver(NetworkDriver):
 
         for line in lines:
             line = line.strip()
-            if line == "":
-                continue
             if line.startswith("!") and not line.startswith("!!"):
                 continue
             commands.append(line)
 
-        for start, depth in [
-            (s, d) for (s, d) in self.HEREDOC_COMMANDS if s in commands
-        ]:
-            commands = self._multiline_convert(commands, start=start, depth=depth)
+        if self.transport != "ssh":
+            for start, depth in [
+                (s, d) for (s, d) in self.HEREDOC_COMMANDS if s in commands
+            ]:
+                commands = self._multiline_convert(commands, start=start, depth=depth)
 
-        commands = self._mode_comment_convert(commands)
+            commands = self._mode_comment_convert(commands)
 
         if self.transport == "ssh":
             try:


Test patch:
diff --git a/test/eos/test_heredoc.py b/test/eos/test_heredoc.py
index 41859277..9a141176 100644
--- a/test/eos/test_heredoc.py
+++ b/test/eos/test_heredoc.py
@@ -147,3 +147,59 @@ class TestConfigMangling(object):
         ]
 
         self.device.device.run_commands.assert_called_with(expected_result)
+
+    def test_heredoc_with_blank_lines(self):
+        raw_config = dedent(
+            """\
+        hostname vEOS
+        ip name-server 192.0.2.1
+        !
+        banner login
+
+        This is a banner that spans
+        multiple lines in order to test
+        HEREDOC conversion
+
+        EOF
+        !
+        management api http-commands
+          protocol https certificate
+          ---BEGIN CERTIFICATE---
+          FAKE-CERTIFICATE-DATA
+          ---END CERTIFICATE---
+          EOF
+          ---BEGIN PRIVATE KEY---
+          FAKE-KEY-DATA
+          ---END PRIVATE KEY---
+          EOF
+        !
+        management ssh
+          idle-timeout 15
+        !
+        """
+        )
+
+        self.device.device.run_commands = mock.MagicMock()
+
+        self.device._load_config(config=raw_config)
+
+        expected_result = [
+            "configure session {}".format(self.device.config_session),
+            "rollback clean-config",
+            "hostname vEOS",
+            "ip name-server 192.0.2.1",
+            {
+                "cmd": "banner login",
+                "input": "\nThis is a banner that spans\nmultiple lines in order to test\nHEREDOC conversion\n",  # noqa
+            },
+            "management api http-commands",
+            {
+                "cmd": "protocol https certificate",
+                "input": "---BEGIN CERTIFICATE---\nFAKE-CERTIFICATE-DATA\n---END CERTIFICATE---\nEOF\n---BEGIN PRIVATE KEY---\nFAKE-KEY-DATA\n---END PRIVATE KEY---",  # noqa
+            },
+            "management ssh",
+            "idle-timeout 15",
+            "end",
+        ]
+
+        self.device.device.run_commands.assert_called_with(expected_result)
diff --git a/test/eos_ssh/__init__.py b/test/eos_ssh/__init__.py
new file mode 100644
index 00000000..e69de29b
diff --git a/test/eos_ssh/conftest.py b/test/eos_ssh/conftest.py
new file mode 100644
index 00000000..774d1bf3
--- /dev/null
+++ b/test/eos_ssh/conftest.py
@@ -0,0 +1,69 @@
+"""Test fixtures."""
+
+from builtins import super
+
+import pytest
+
+from napalm.base.test import conftest as parent_conftest
+
+from napalm.base.test.double import BaseTestDouble
+
+from napalm.eos import eos
+
+
+@pytest.fixture(scope="class")
+def set_device_parameters(request):
+    """Set up the class."""
+
+    def fin():
+        request.cls.device.close()
+
+    request.addfinalizer(fin)
+
+    request.cls.driver = eos.EOSDriver
+    request.cls.patched_driver = PatchedEOSDriver
+    request.cls.vendor = "eos"
+    parent_conftest.set_device_parameters(request)
+
+
+def pytest_generate_tests(metafunc):
+    """Generate test cases dynamically."""
+    parent_conftest.pytest_generate_tests(metafunc, __file__)
+
+
+class PatchedEOSDriver(eos.EOSDriver):
+    """Patched EOS Driver."""
+
+    def __init__(self, hostname, username, password, timeout=60, optional_args=None):
+
+        optional_args = {"transport": "ssh"}
+
+        super().__init__(
+            hostname, username, password, timeout, optional_args=optional_args
+        )
+
+        self.patched_attrs = ["device"]
+        self.device = FakeEOSDevice()
+
+    def _obtain_lock(self, wait_time=None):
+        pass
+
+    def close(self):
+        pass
+
+    def open(self):
+        pass
+
+
+class FakeEOSDevice(BaseTestDouble):
+    """EOS device test double."""
+
+    def __init__(self):
+        super(FakeEOSDevice, self).__init__()
+        self.connection = object()
+
+    def send_command_expect(self, command, **kwargs):
+        return self.send_config_set([command])
+
+    def send_config_set(self, commands, **kwargs):
+        return self.run_commands(commands)
diff --git a/test/eos_ssh/test_heredoc.py b/test/eos_ssh/test_heredoc.py
new file mode 100644
index 00000000..c148ffd9
--- /dev/null
+++ b/test/eos_ssh/test_heredoc.py
@@ -0,0 +1,190 @@
+from unittest import mock
+import pytest
+from textwrap import dedent
+
+
+@pytest.mark.usefixtures("set_device_parameters")
+class TestConfigMangling(object):
+    def test_heredoc(self):
+        raw_config = dedent(
+            """\
+        hostname vEOS
+        ip name-server 192.0.2.1
+        !
+        banner login
+        This is a banner that spans
+        multiple lines in order to test
+        HEREDOC conversion
+        EOF
+        !
+        management ssh
+          idle-timeout 15
+        !
+        """
+        )
+
+        self.device.device.run_commands = mock.MagicMock()
+
+        self.device._load_config(config=raw_config)
+
+        expected_result = [
+            "configure session {}".format(self.device.config_session),
+            "rollback clean-config",
+            "hostname vEOS",
+            "ip name-server 192.0.2.1",
+            "banner login",
+            "This is a banner that spans",
+            "multiple lines in order to test",
+            "HEREDOC conversion",
+            "EOF",
+            "management ssh",
+            "idle-timeout 15",
+        ]
+
+        self.device.device.run_commands.assert_called_with(expected_result)
+
+    def test_mode_comment(self):
+        raw_config = dedent(
+            """\
+        ip access-list standard test1
+            !! This is a
+            !! multiline mode comment
+            !! for standard ACL test1
+            permit host 192.0.2.1
+        !
+        ip access-list standard test2
+            !! This is a single-line mode comment for standard ACL test2
+            permit host 192.0.2.2
+        !
+        ip access-list standard test3
+            comment
+            This is a multi-line HEREDOC
+            comment for standard ACL test3
+            EOF
+            permit host 192.0.2.3
+        !
+        """
+        )
+
+        self.device.device.run_commands = mock.MagicMock()
+
+        self.device._load_config(config=raw_config, replace=False)
+
+        expected_result = [
+            "configure session {}".format(self.device.config_session),
+            "ip access-list standard test1",
+            "!! This is a",
+            "!! multiline mode comment",
+            "!! for standard ACL test1",
+            "permit host 192.0.2.1",
+            "ip access-list standard test2",
+            "!! This is a single-line mode comment for standard ACL test2",
+            "permit host 192.0.2.2",
+            "ip access-list standard test3",
+            "comment",
+            "This is a multi-line HEREDOC",
+            "comment for standard ACL test3",
+            "EOF",
+            "permit host 192.0.2.3",
+        ]
+
+        self.device.device.run_commands.assert_called_with(expected_result)
+
+    def test_heredoc_with_bangs(self):
+        raw_config = dedent(
+            """\
+        hostname vEOS
+        ip name-server 192.0.2.1
+        !
+        banner login
+        !! This is a banner that contains
+        !!!bangs!
+        EOF
+        !
+        management ssh
+          idle-timeout 15
+        !
+        """
+        )
+
+        self.device.device.run_commands = mock.MagicMock()
+
+        self.device._load_config(config=raw_config)
+
+        expected_result = [
+            "configure session {}".format(self.device.config_session),
+            "rollback clean-config",
+            "hostname vEOS",
+            "ip name-server 192.0.2.1",
+            "banner login",
+            "!! This is a banner that contains",
+            "!!!bangs!",
+            "EOF",
+            "management ssh",
+            "idle-timeout 15",
+        ]
+
+        self.device.device.run_commands.assert_called_with(expected_result)
+
+    def test_heredoc_with_blank_lines(self):
+        raw_config = dedent(
+            """\
+        hostname vEOS
+        ip name-server 192.0.2.1
+        !
+        banner login
+
+        This is a banner that spans
+        multiple lines in order to test
+        HEREDOC conversion
+
+        EOF
+        !
+        management api http-commands
+          protocol https certificate
+          ---BEGIN CERTIFICATE---
+          FAKE-CERTIFICATE-DATA
+          ---END CERTIFICATE---
+          EOF
+          ---BEGIN PRIVATE KEY---
+          FAKE-KEY-DATA
+          ---END PRIVATE KEY---
+          EOF
+        !
+        management ssh
+          idle-timeout 15
+        !
+        """
+        )
+
+        self.device.device.run_commands = mock.MagicMock()
+
+        self.device._load_config(config=raw_config)
+
+        expected_result = [
+            "configure session {}".format(self.device.config_session),
+            "rollback clean-config",
+            "hostname vEOS",
+            "ip name-server 192.0.2.1",
+            "banner login",
+            "",
+            "This is a banner that spans",
+            "multiple lines in order to test",
+            "HEREDOC conversion",
+            "",
+            "EOF",
+            "management api http-commands",
+            "protocol https certificate",
+            "---BEGIN CERTIFICATE---",
+            "FAKE-CERTIFICATE-DATA",
+            "---END CERTIFICATE---",
+            "EOF",
+            "---BEGIN PRIVATE KEY---",
+            "FAKE-KEY-DATA",
+            "---END PRIVATE KEY---",
+            "EOF",
+            "management ssh",
+            "idle-timeout 15",
+        ]
+
+        self.device.device.run_commands.assert_called_with(expected_result)