T7391 domain search fix for 1.4+ (#418)

* t7391 init

* sanity

* vyos_system unit test

* added vyos_system v14 test cases

* vyos_system integration tests fix

* changelog

* domain search integration test

* Update plugins/modules/vyos_system.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: omnom62

changelogs/fragments/T7391_domain_search.yaml

@@ -0,0 +1,4 @@
+---
+trivial:
+  - vyos_system - Added support for domain_search for 1.4+
+  - test_vyos_system - Added test for domain_search

plugins/module_utils/network/vyos/vyos.py

@@ -68,6 +68,7 @@ def get_capabilities(module):
 def get_config(module, flags=None, format=None):
     flags = [] if flags is None else flags
     global _DEVICE_CONFIGS
+
     # If _DEVICE_CONFIGS is non-empty and module.params["match"] is "none",
     # return the cached device configurations. This avoids redundant calls
     # to the connection when no specific match criteria are provided.

plugins/modules/vyos_system.py

@@ -16,6 +16,7 @@
 # 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
 
 
@@ -94,21 +95,27 @@
       - sub1.example.com
       - sub2.example.com
 """
+from re import M, findall
 
 from ansible.module_utils.basic import AnsibleModule
 
+from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.utils.version import (
+    LooseVersion,
+)
 from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.vyos import (
     get_config,
+    get_os_version,
     load_config,
 )
 
 
-def spec_key_to_device_key(key):
+def spec_key_to_device_key(key, module):
     device_key = key.replace("_", "-")
 
-    # domain-search is longer than just it's key
+    # domain-search differs in 1.3- and 1.4+
     if device_key == "domain-search":
-        device_key += " domain"
+        if LooseVersion(get_os_version(module)) <= LooseVersion("1.3"):
+            device_key += " domain"
 
     return device_key
 
@@ -119,19 +126,20 @@ def config_to_dict(module):
     config = {"domain_search": [], "name_server": []}
 
     for line in data.split("\n"):
-        if line.startswith("set system host-name"):
-            config["host_name"] = line[22:-1]
-        elif line.startswith("set system domain-name"):
-            config["domain_name"] = line[24:-1]
-        elif line.startswith("set system domain-search domain"):
-            config["domain_search"].append(line[33:-1])
-        elif line.startswith("set system name-server"):
-            config["name_server"].append(line[24:-1])
-
+        config_line = findall(r"^set system\s+(\S+)(?:\s+domain)?\s+'([^']+)'", line, M)
+        if config_line:
+            if config_line[0][0] == "host-name":
+                config["host_name"] = config_line[0][1]
+            elif config_line[0][0] == "domain-name":
+                config["domain_name"] = config_line[0][1]
+            elif config_line[0][0] == "domain-search":
+                config["domain_search"].append(config_line[0][1])
+            elif config_line[0][0] == "name-server":
+                config["name_server"].append(config_line[0][1])
     return config
 
 
-def spec_to_commands(want, have):
+def spec_to_commands(want, have, module):
     commands = []
 
     state = want.pop("state")
@@ -140,15 +148,15 @@ def spec_to_commands(want, have):
     if state == "absent" and all(v is None for v in want.values()):
         # Clear everything
         for key in have:
-            commands.append("delete system %s" % spec_key_to_device_key(key))
+            commands.append("delete system %s" % spec_key_to_device_key(key, module))
 
     for key in want:
         if want[key] is None:
             continue
 
         current = have.get(key)
         proposed = want[key]
-        device_key = spec_key_to_device_key(key)
+        device_key = spec_key_to_device_key(key, module)
 
         # These keys are lists which may need to  be reconciled with the device
         if key in ["domain_search", "name_server"]:
@@ -201,7 +209,7 @@ def main():
     want = map_param_to_obj(module)
     have = config_to_dict(module)
 
-    commands = spec_to_commands(want, have)
+    commands = spec_to_commands(want, have, module)
     result["commands"] = commands
 
     if commands:

tests/integration/targets/vyos_system/tests/cli/domain_search.yaml

@@ -0,0 +1,43 @@
+---
+- debug: msg="START cli/domain_search.yaml on connection={{ ansible_connection }}"
+
+- name: ensure facts
+  include_tasks: _get_version.yaml
+
+- name: setup
+  ignore_errors: true
+  vyos.vyos.vyos_system:
+    domain_search:
+      - nbg.bufanda.ke
+    state: absent
+
+- name: configure domain search setting
+  register: result
+  vyos.vyos.vyos_system:
+    domain_search:
+      - nbg.bufanda.ke
+
+- assert:
+    that:
+      - result.changed == true
+      - result.commands|length == 1
+      - "{{ merged['commands'] | symmetric_difference(result['commands']) |length == 0 }}"
+
+- name: configure domain search setting
+  register: result
+  vyos.vyos.vyos_system:
+    domain_search:
+      - nbg.bufanda.ke
+
+- assert:
+    that:
+      - result.changed == false
+
+- name: teardown
+  ignore_errors: true
+  vyos.vyos.vyos_system:
+    domain_search:
+      - nbg.bufanda.ke
+    state: absent
+
+- debug: msg="END cli/basic.yaml on connection={{ ansible_connection }}"

tests/integration/targets/vyos_system/vars/pre-v1_4.yaml

@@ -0,0 +1,4 @@
+---
+merged:
+  commands:
+    - set system domain-search domain 'nbg.bufanda.ke'

tests/integration/targets/vyos_system/vars/v1_4.yaml

@@ -0,0 +1,4 @@
+---
+merged:
+  commands:
+    - set system domain-search 'nbg.bufanda.ke'

tests/unit/modules/network/vyos/test_vyos_system.py

@@ -45,11 +45,26 @@ def setUp(self):
         )
         self.load_config = self.mock_load_config.start()
 
+        self.mock_get_os_version = patch(
+            "ansible_collections.vyos.vyos.plugins.modules.vyos_system.get_os_version",
+        )
+        self.test_version = "1.2"
+        self.get_os_version = self.mock_get_os_version.start()
+        self.get_os_version.return_value = self.test_version
+        self.mock_facts_get_os_version = patch(
+            "ansible_collections.vyos.vyos.plugins.modules.vyos_system.get_os_version",
+        )
+        self.get_facts_os_version = self.mock_facts_get_os_version.start()
+        self.get_facts_os_version.return_value = self.test_version
+        self.maxDiff = None
+
     def tearDown(self):
         super(TestVyosSystemModule, self).tearDown()
 
         self.mock_get_config.stop()
         self.mock_load_config.stop()
+        self.mock_get_os_version.stop()
+        self.mock_facts_get_os_version.stop()
 
     def load_fixtures(self, commands=None, filename=None):
         self.get_config.return_value = load_fixture("vyos_config_config.cfg")
@@ -112,3 +127,67 @@ def test_vyos_system_clear_all(self):
             "delete system name-server",
         ]
         self.execute_module(changed=True, commands=commands)
+
+
+class TestVyosSystemModule14(TestVyosModule):
+    module = vyos_system
+
+    def setUp(self):
+        super(TestVyosSystemModule14, self).setUp()
+
+        self.mock_get_config = patch(
+            "ansible_collections.vyos.vyos.plugins.modules.vyos_system.get_config",
+        )
+        self.get_config = self.mock_get_config.start()
+
+        self.mock_load_config = patch(
+            "ansible_collections.vyos.vyos.plugins.modules.vyos_system.load_config",
+        )
+        self.load_config = self.mock_load_config.start()
+
+        self.mock_get_os_version = patch(
+            "ansible_collections.vyos.vyos.plugins.modules.vyos_system.get_os_version",
+        )
+        self.test_version = "1.4"
+        self.get_os_version = self.mock_get_os_version.start()
+        self.get_os_version.return_value = self.test_version
+        self.mock_facts_get_os_version = patch(
+            "ansible_collections.vyos.vyos.plugins.modules.vyos_system.get_os_version",
+        )
+        self.get_facts_os_version = self.mock_facts_get_os_version.start()
+        self.get_facts_os_version.return_value = self.test_version
+        self.maxDiff = None
+
+    def tearDown(self):
+        super(TestVyosSystemModule14, self).tearDown()
+
+        self.mock_get_config.stop()
+        self.mock_load_config.stop()
+        self.mock_get_os_version.stop()
+        self.mock_facts_get_os_version.stop()
+
+    def load_fixtures(self, commands=None, filename=None):
+        self.get_config.return_value = load_fixture("vyos_config_config.cfg")
+
+    def test_vyos_system_domain_search(self):
+        set_module_args(dict(domain_search=["foo.example.com", "bar.example.com"]))
+        commands = [
+            "set system domain-search 'foo.example.com'",
+            "set system domain-search 'bar.example.com'",
+        ]
+        self.execute_module(changed=True, commands=commands)
+
+    def test_vyos_system_clear_domain_search(self):
+        set_module_args(dict(domain_search=[]))
+        commands = ["delete system domain-search"]
+        self.execute_module(changed=True, commands=commands)
+
+    def test_vyos_system_clear_all(self):
+        set_module_args(dict(state="absent"))
+        commands = [
+            "delete system host-name",
+            "delete system domain-search",
+            "delete system domain-name",
+            "delete system name-server",
+        ]
+        self.execute_module(changed=True, commands=commands)