T6828: PR190 revive, vyos_conf match "enforced" (#415)HEADmain

* T6828: PR190 revive, vyos_conf match "enforced"

3 files changed, 568 insertions, 24 deletions

diff --git a/tests/unit/cliconf/__init__.py b/tests/unit/cliconf/__init__.py
new file mode 100644
index 00000000..e69de29b
--- /dev/null
+++ b/tests/unit/cliconf/__init__.py
diff --git a/tests/unit/cliconf/test_utils_vyosconf.py b/tests/unit/cliconf/test_utils_vyosconf.py
new file mode 100644
index 00000000..dbc296e6
--- /dev/null
+++ b/tests/unit/cliconf/test_utils_vyosconf.py
@@ -0,0 +1,217 @@
+#
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+#
+from __future__ import absolute_import, division, print_function
+
+
+__metaclass__ = type
+
+import unittest
+
+from ansible_collections.vyos.vyos.plugins.cliconf_utils.vyosconf import (
+    KEEP_EXISTING_VALUES,
+    VyosConf,
+)
+
+
+class TestListElements(unittest.TestCase):
+    def test_add(self):
+        conf = VyosConf()
+        conf.set_entry(["a", "b"], "c")
+        self.assertEqual(conf.config, {"a": {"b": {"c": {}}}})
+        conf.set_entry(["a", "b"], "d")
+        self.assertEqual(conf.config, {"a": {"b": {"c": {}, "d": {}}}})
+        conf.set_entry(["a", "c"], "b")
+        self.assertEqual(
+            conf.config,
+            {"a": {"b": {"c": {}, "d": {}}, "c": {"b": {}}}},
+        )
+        conf.set_entry(["a", "c", "b"], "d")
+        self.assertEqual(
+            conf.config,
+            {"a": {"b": {"c": {}, "d": {}}, "c": {"b": {"d": {}}}}},
+        )
+
+    def test_del(self):
+        conf = VyosConf()
+        conf.set_entry(["a", "b"], "c")
+        conf.set_entry(["a", "c", "b"], "d")
+        conf.set_entry(["a", "b"], "d")
+        self.assertEqual(
+            conf.config,
+            {"a": {"b": {"c": {}, "d": {}}, "c": {"b": {"d": {}}}}},
+        )
+        conf.del_entry(["a", "c", "b"], "d")
+        self.assertEqual(conf.config, {"a": {"b": {"c": {}, "d": {}}}})
+        conf.set_entry(["a", "b", "c"], "d")
+        conf.del_entry(["a", "b", "c"], "d")
+        self.assertEqual(conf.config, {"a": {"b": {"d": {}}}})
+
+    def test_del_missing_leaf_is_noop(self):
+        """
+        Deleting a leaf that was never set must leave the config unchanged.
+        Regression test: del_entry() used to raise KeyError when the leaf's
+        parent had siblings, and could delete an unrelated ancestor subtree
+        (or the entire config) when the parent path had no siblings.
+        """
+        # parent has siblings: previously raised KeyError
+        conf = VyosConf()
+        conf.set_entry(["a", "b"], "c")
+        conf.set_entry(["a", "b"], "d")
+        conf.del_entry(["a", "b"], "nonexistent")
+        self.assertEqual(conf.config, {"a": {"b": {"c": {}, "d": {}}}})
+
+        # parent path is an unbranched chain: previously deleted the
+        # entire config instead of no-op'ing
+        conf = VyosConf()
+        conf.set_entry(["a", "b"], "d")
+        conf.del_entry(["a", "b"], "c")
+        self.assertEqual(conf.config, {"a": {"b": {"d": {}}}})
+
+        # missing intermediate path element already behaved correctly;
+        # confirm it still does
+        conf = VyosConf()
+        conf.set_entry(["a", "b"], "c")
+        conf.del_entry(["a", "x"], "c")
+        self.assertEqual(conf.config, {"a": {"b": {"c": {}}}})
+
+    def test_parse(self):
+        conf = VyosConf()
+        self.assertListEqual(
+            conf.parse_line("set a b c"),
+            ["set", ["a", "b"], "c"],
+        )
+        self.assertListEqual(
+            conf.parse_line('set a b "c"'),
+            ["set", ["a", "b"], "c"],
+        )
+        self.assertListEqual(
+            conf.parse_line("set a b 'c d'"),
+            ["set", ["a", "b"], "c d"],
+        )
+        self.assertListEqual(
+            conf.parse_line("set a b 'c'"),
+            ["set", ["a", "b"], "c"],
+        )
+        self.assertListEqual(
+            conf.parse_line("delete a b 'c'"),
+            ["delete", ["a", "b"], "c"],
+        )
+        self.assertListEqual(
+            conf.parse_line("del a b 'c'"),
+            ["del", ["a", "b"], "c"],
+        )
+        self.assertListEqual(
+            conf.parse_line("set a b '\"c'"),
+            ["set", ["a", "b"], '"c'],
+        )
+        self.assertListEqual(
+            conf.parse_line("set a b 'c' #this is a comment"),
+            ["set", ["a", "b"], "c"],
+        )
+        self.assertListEqual(
+            conf.parse_line("set a b '#c'"),
+            ["set", ["a", "b"], "#c"],
+        )
+
+    def test_run_commands(self):
+        self.assertEqual(
+            VyosConf(["set a b 'c'", "set a c 'b'"]).config,
+            {"a": {"b": {"c": {}}, "c": {"b": {}}}},
+        )
+        self.assertEqual(
+            VyosConf(["set a b c 'd'", "set a c 'b'", "del a b c d"]).config,
+            {"a": {"c": {"b": {}}}},
+        )
+
+    def test_build_commands(self):
+        self.assertEqual(
+            sorted(
+                VyosConf(
+                    [
+                        "set a b 'c a'",
+                        "set a c a",
+                        "set a c b",
+                        "delete a c a",
+                    ],
+                ).build_commands(),
+            ),
+            sorted(["set a b 'c a'", "set a c b"]),
+        )
+        self.assertEqual(
+            sorted(
+                VyosConf(
+                    [
+                        "set a b 10.0.0.1/24",
+                        "set a c ABCabc123+/=",
+                        "set a d $6$ABC.abc.123.+./=..",
+                    ],
+                ).build_commands(),
+            ),
+            sorted(
+                [
+                    "set a b 10.0.0.1/24",
+                    "set a c 'ABCabc123+/='",
+                    "set a d '$6$ABC.abc.123.+./=..'",
+                ],
+            ),
+        )
+
+    def test_check_commands(self):
+        conf = VyosConf(["set a b 'c a'", "set a c b"])
+        self.assertListEqual(
+            conf.check_commands(
+                ["set a b 'c a'", "del a c b", "set a b 'c'", "del a a a"],
+            ),
+            [True, False, False, True],
+        )
+
+    def test_diff_commands_to(self):
+        conf = VyosConf(["set a b 'c a'", "set a c b"])
+
+        self.assertListEqual(
+            conf.diff_commands_to(VyosConf(["set a c b"])),
+            ["delete a b"],
+        )
+        self.assertListEqual(
+            conf.diff_commands_to(VyosConf(["set a b 'c a'", "set a c b"])),
+            [],
+        )
+
+        # KEEP_EXISTING_VALUES is no longer reachable via 'set'/'delete'
+        # command text (see #6): a literal "..." leaf is now an ordinary
+        # value, not a sentinel, so nothing is suppressed here.
+        self.assertListEqual(
+            conf.diff_commands_to(VyosConf(["set a b ..."])),
+            ["delete a b 'c a'", "delete a c", "set a b ..."],
+        )
+
+    def test_diff_commands_to_keep_existing_values_sentinel(self):
+        # KEEP_EXISTING_VALUES is only reachable via the Python API now.
+        # Build the candidate tree directly to prove diff_to() still
+        # honours it when used that way.
+        conf = VyosConf(["set a b 'c a'", "set a c b"])
+        candidate = VyosConf()
+        candidate.config = {"a": {"b": {KEEP_EXISTING_VALUES: {}}}}
+
+        self.assertListEqual(
+            conf.diff_commands_to(candidate),
+            ["delete a c"],
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()
diff --git a/tests/unit/modules/network/vyos/test_vyos_config.py b/tests/unit/modules/network/vyos/test_vyos_config.py
index d6a75f1d..ffafc85d 100644
--- a/tests/unit/modules/network/vyos/test_vyos_config.py
+++ b/tests/unit/modules/network/vyos/test_vyos_config.py
@@ -146,6 +146,37 @@ class TestVyosConfigModule(TestVyosModule):
         )
         self.execute_module(changed=True, commands=lines, sort=False)
 
+    def test_vyos_config_match_enforce(self):
+        lines = [
+            "set interfaces ethernet eth0 address '1.2.3.4/24'",
+            "set interfaces ethernet eth0 description 'test string'",
+        ]
+        set_module_args(dict(lines=lines, match="enforce"))
+        candidate = "\n".join(lines)
+
+        response = self.cliconf_obj.get_diff(
+            candidate,
+            self.running_config,
+            diff_match="enforce",
+        )
+
+        self.conn.get_diff = MagicMock(return_value=response)
+        result = self.execute_module(changed=True, sort=False)
+
+        self.conn.get_diff.assert_called_once_with(
+            candidate=candidate,
+            running=self.running_config,
+            diff_match="enforce",
+        )
+
+        expected_config_diff = [
+            "delete interfaces ethernet eth1",
+        ]
+        self.assertEqual(response["config_diff"], expected_config_diff)
+
+        expected_commands = expected_config_diff
+        self.assertEqual(result["commands"], expected_commands)
+
     def test_vyos_config_confirm_automatic(self):
         src = load_fixture("vyos_config_src.cfg")
         confirm_timeout = 7
@@ -184,6 +215,326 @@ class TestVyosConfigModule(TestVyosModule):
         self.assertEqual(self.load_config.call_args[1]["confirm"], confirm_timeout)
         self.run_commands.assert_not_called()
 
+    def test_vyos_config_match_enforce_blank_lines(self):
+        """enforce diff must not raise IndexError on blank lines in running config."""
+        running_with_blanks = self.running_config + "\n\n"
+        candidate = "set interfaces ethernet eth0 address 1.2.3.4/24"
+        response = self.cliconf_obj.get_diff(candidate, running_with_blanks, diff_match="enforce")
+        self.assertIn("config_diff", response)
+
+    def test_vyos_config_match_enforce_additions(self):
+        lines = [
+            "set interfaces ethernet eth0 address '1.2.3.4/24'",
+            "set interfaces ethernet eth0 description 'test string'",
+            "set interfaces ethernet eth2 address '192.0.2.1/24'",
+        ]
+        set_module_args(dict(lines=lines, match="enforce"))
+        candidate = "\n".join(lines)
+        response = self.cliconf_obj.get_diff(
+            candidate,
+            self.running_config,
+            diff_match="enforce",
+        )
+        self.conn.get_diff = MagicMock(return_value=response)
+        result = self.execute_module(changed=True, sort=False)
+        self.conn.get_diff.assert_called_once_with(
+            candidate=candidate,
+            running=self.running_config,
+            diff_match="enforce",
+        )
+        self.assertIn(
+            "set interfaces ethernet eth2 address 192.0.2.1/24",
+            response["config_diff"],
+        )
+        self.assertEqual(result["commands"], response["config_diff"])
+
+    def test_vyos_config_match_enforce_rejects_delete_lines(self):
+        """
+        match=enforce treats the candidate as the complete desired end-state.
+        A candidate containing 'delete' lines must be rejected rather than
+        silently producing a diff that removes most/all of the running
+        config (regression test for a candidate that is a no-op/delete-only
+        input generating deletes for everything the candidate omits).
+        """
+        lines = ["delete interfaces ethernet eth0 address"]
+        candidate = "\n".join(lines)
+
+        with self.assertRaises(ValueError):
+            self.cliconf_obj.get_diff(
+                candidate,
+                self.running_config,
+                diff_match="enforce",
+            )
+
+    def test_vyos_config_match_enforce_rejects_empty_candidate(self):
+        """
+        A candidate that is empty, whitespace-only, or comment-only must be
+        rejected rather than silently treated as an empty desired end-state
+        (which would generate deletes for the entire running config).
+        Comment-only candidates are also stripped away entirely by upstream
+        NetworkConfig parsing before reaching VyosConf, so this is a second,
+        distinct route to the same mass-deletion failure mode as the
+        'delete' lines case above.
+        """
+        for candidate in ("", "   ", "# just a comment"):
+            with self.assertRaises(ValueError):
+                self.cliconf_obj.get_diff(
+                    candidate,
+                    self.running_config,
+                    diff_match="enforce",
+                )
+
+    def test_vyos_config_match_enforce_requires_running(self):
+        """
+        diff_match=enforce with running=None must raise a clear ValueError
+        instead of falling through to an AttributeError on
+        running.splitlines().
+        """
+        with self.assertRaises(ValueError):
+            self.cliconf_obj.get_diff(
+                "set system host-name foo",
+                None,
+                diff_match="enforce",
+            )
+
+    def test_vyos_config_match_enforce_ignores_comment_lines(self):
+        """
+        Comment lines mixed in with 'set' lines must be stripped out rather
+        than causing the whole candidate to be rejected as not starting
+        with 'set'.
+        """
+        candidate = "\n".join(
+            [
+                "set interfaces ethernet eth0 address '1.2.3.4/24'",
+                "# a note about this interface",
+                "set interfaces ethernet eth0 description 'test string'",
+            ],
+        )
+        running = "set interfaces ethernet eth0 address '1.2.3.4/24'"
+        response = self.cliconf_obj.get_diff(
+            candidate,
+            running,
+            diff_match="enforce",
+        )
+        self.assertIn(
+            "set interfaces ethernet eth0 description 'test string'",
+            response["config_diff"],
+        )
+
+    def test_sanitize_config_filters_password_delete_lines(self):
+        """
+        sanitize_config()/PASSWORD_NEEDLE must filter 'delete ... password'
+        lines the same way it filters 'set ... password' lines, since
+        match=enforce can generate deletes for password config the candidate
+        omits. Without this, allow_password_change=none/plaintext/encrypted
+        would fail to catch a password-affecting delete.
+        """
+        result = {}
+        commands = [
+            "set system host-name foo",
+            "delete system login user admin authentication encrypted-password",
+            "set system login user admin authentication plaintext-password 'secret'",
+        ]
+        vyos_config.sanitize_config(commands, result, allow="none")
+        self.assertIn(
+            "delete system login user admin authentication encrypted-password",
+            result["filtered"],
+        )
+        self.assertIn(
+            "set system login user admin authentication plaintext-password 'secret'",
+            result["filtered"],
+        )
+        self.assertNotIn("set system host-name foo", result["filtered"])
+
+    def test_vyos_config_match_enforce_refuses_ssh_deletion(self):
+        """
+        match=enforce must refuse to generate 'delete service ssh ...'
+        commands, since this could sever the management connection.
+        Regression test for the incident where an enforce candidate that
+        didn't restate 'service ssh' generated a delete for it.
+        """
+        running = "\n".join(
+            [
+                "set service ssh port '22'",
+                "set service lldp",
+            ],
+        )
+        candidate = "set service lldp"
+
+        with self.assertRaises(ValueError):
+            self.cliconf_obj.get_diff(
+                candidate,
+                running,
+                diff_match="enforce",
+            )
+
+    def test_vyos_config_confirm_defaults_to_automatic_for_match_enforce(self):
+        """
+        confirm defaults to 'automatic' when match=enforce and confirm is
+        not explicitly set, since enforce can generate broad deletes and a
+        bad commit should self-revert rather than leave the device
+        unreachable.
+        """
+        lines = [
+            "set interfaces ethernet eth0 address '1.2.3.4/24'",
+            "set interfaces ethernet eth0 description 'test string'",
+        ]
+        set_module_args(dict(lines=lines, match="enforce"))
+        candidate = "\n".join(lines)
+        response = self.cliconf_obj.get_diff(
+            candidate,
+            self.running_config,
+            diff_match="enforce",
+        )
+        self.conn.get_diff = MagicMock(return_value=response)
+
+        self.execute_module(changed=True, sort=False)
+
+        self.assertEqual(self.load_config.call_args[1]["confirm"], 10)
+        self.run_commands.assert_called_once()
+        self.assertEqual(
+            ["configure", "confirm", "exit"],
+            self.run_commands.call_args[0][1],
+        )
+
+    def test_vyos_config_confirm_stays_none_for_other_match_values(self):
+        """
+        confirm stays 'none' (no confirm kwarg passed, no auto-confirm
+        run_commands call) when match is not 'enforce' and confirm is not
+        explicitly set -- the new conditional default must not change
+        existing behaviour for match=line/none.
+        """
+        lines = ["set system host-name foo"]
+        set_module_args(dict(lines=lines))
+        candidate = "\n".join(lines)
+        self.conn.get_diff = MagicMock(
+            return_value=self.cliconf_obj.get_diff(candidate, self.running_config),
+        )
+
+        self.execute_module(changed=True, commands=lines)
+
+        self.assertIsNone(self.load_config.call_args[1]["confirm"])
+        self.run_commands.assert_not_called()
+
+    def test_vyos_config_match_enforce_rejects_comment_disguised_as_command(self):
+        """
+        A line like 'set # comment' has 3 raw tokens (passing a naive
+        token-count check) but parse_line() strips the trailing comment,
+        leaving no actual path/leaf. This must still be rejected rather
+        than silently contributing an empty/degenerate entry to the diff.
+        """
+        for bad_line in ("set # comment", "set foo # comment"):
+            candidate = "\n".join(["set system host-name foo", bad_line])
+            with self.assertRaises(ValueError):
+                self.cliconf_obj.get_diff(
+                    candidate,
+                    self.running_config,
+                    diff_match="enforce",
+                )
+
+    def test_sanitize_config_filters_collapsed_login_subtree_deletes(self):
+        """
+        match=enforce's scoping can collapse an untouched subtree into a
+        single parent delete (e.g. 'delete system login' when a candidate
+        touches system without restating login, rather than an itemized
+        per-field delete). PASSWORD_NEEDLE alone can't see into a collapsed
+        delete to know it removes a password -- it must be treated as
+        password-bearing by default under any restrictive
+        allow_password_change value.
+        """
+        result = {}
+        commands = [
+            "set system host-name foo",
+            "delete system login",
+        ]
+        vyos_config.sanitize_config(commands, result, allow="none")
+        self.assertIn("delete system login", result["filtered"])
+        self.assertNotIn("set system host-name foo", result["filtered"])
+
+    def test_sanitize_config_filters_collapsed_login_user_subtree_delete(self):
+        """
+        Same collapse risk at the per-user level: 'delete system login
+        user admin' (no specific authentication line) must also be
+        treated as password-bearing.
+        """
+        result = {}
+        commands = [
+            "set system host-name foo",
+            "delete system login user admin",
+        ]
+        vyos_config.sanitize_config(commands, result, allow="none")
+        self.assertIn("delete system login user admin", result["filtered"])
+
+    def test_sanitize_config_allows_collapsed_login_subtree_delete_when_all(self):
+        """
+        allow_password_change=all must still let a collapsed login-subtree
+        delete through, same as it already does for explicit password
+        lines.
+        """
+        result = {}
+        commands = [
+            "set system host-name foo",
+            "delete system login",
+        ]
+        vyos_config.sanitize_config(commands, result, allow="all")
+        self.assertEqual(result["filtered"], [])
+
+    def test_vyos_config_match_enforce_accepts_bracket_format_src(self):
+        """
+        match=enforce must accept bracket-format candidates the same way
+        match=line/none already do -- enforce_candidate_lines is now built
+        from the same shared, correctly-normalized candidate_commands
+        rather than parsing raw candidate text independently (which had no
+        concept of bracket format at all).
+        """
+        candidate = "system {\n    host-name foo\n}\n"
+        response = self.cliconf_obj.get_diff(
+            candidate,
+            self.running_config,
+            diff_match="enforce",
+        )
+        self.assertIn("set system host-name foo", response["config_diff"])
+
+    def test_vyos_config_match_line_ignores_comment_and_blank_lines(self):
+        """
+        A src/lines candidate containing comment or blank lines must not
+        raise under match=line -- these are stripped during candidate
+        normalization the same way match=enforce already does, rather than
+        hitting the 'line must start with set or delete' check.
+        """
+        lines = [
+            "# a note",
+            "",
+            "set system host-name foo",
+        ]
+        set_module_args(dict(lines=lines))
+        candidate = "\n".join(lines)
+        self.conn.get_diff = MagicMock(
+            return_value=self.cliconf_obj.get_diff(candidate, self.running_config),
+        )
+        self.execute_module(changed=True, commands=["set system host-name foo"])
+
+    def test_sanitize_config_filters_collapsed_login_user_authentication_subtree_delete(self):
+        """
+        A candidate that keeps other settings for a user but omits that
+        user's entire authentication subtree collapses to 'delete system
+        login user <name> authentication' -- one level deeper than the
+        per-user collapse already covered. This must also be treated as
+        password-bearing under the default allow_password_change=plaintext,
+        not just under allow_password_change=none.
+        """
+        result = {}
+        commands = [
+            "set system host-name foo",
+            "delete system login user admin authentication",
+        ]
+        vyos_config.sanitize_config(commands, result, allow="plaintext")
+        self.assertIn(
+            "delete system login user admin authentication",
+            result["filtered"],
+        )
+        self.assertNotIn("set system host-name foo", result["filtered"])
+
     # -- replace=config (T6837, cisco.iosxr.iosxr_config replace=config analogue) --
 
     def test_vyos_config_replace_config_requires_src(self):
@@ -359,27 +710,3 @@ class TestVyosConfigModule(TestVyosModule):
         )
         self.execute_module(changed=True, commands=commands)
         self.copy_file.assert_not_called()
-
-    def test_sanitize_config_filters_password_delete_lines(self):
-        """
-        sanitize_config()/PASSWORD_NEEDLE must filter 'delete ... password'
-        lines the same way it filters 'set ... password' lines, since
-        replace=config can generate deletes for password config the
-        candidate omits.
-        """
-        result = {}
-        commands = [
-            "set system host-name foo",
-            "delete system login user admin authentication encrypted-password",
-            "set system login user admin authentication plaintext-password 'secret'",
-        ]
-        vyos_config.sanitize_config(commands, result, allow="none")
-        self.assertIn(
-            "delete system login user admin authentication encrypted-password",
-            result["filtered"],
-        )
-        self.assertIn(
-            "set system login user admin authentication plaintext-password 'secret'",
-            result["filtered"],
-        )
-        self.assertNotIn("set system host-name foo", result["filtered"])