DNS Resolver: Adding hosts should not delete what already exists

[stratus-ss]

Is your feature request related to a problem? Please describe.
Currently when you use this module to add host entries, all previous entries are deleted and replaced with only the entries in the calling playbook. This behaviour is not ideal. You should have the option to retain current entries.

Describe the solution you'd like
There should be a clear delineated option to either overwrite or append to the host list.

Additional context
I believe this can be accomplished by modifying the _params_to_obj() method in pfsense_dns_resolver.py and adding a helper function to determine what currently exists in the target pfsense host. The same can be done for domain overrides.

I have hacked around on this and my working prototype looks like this

     def _params_to_obj(self):
         params = self.params
 
-        obj = dict()
+        # Initialize with existing configuration
+        obj = {}
+        if self.root_elt is not None:
+            # Preserve existing hosts
+            existing_hosts = []
+            for host_elt in self.root_elt.findall("hosts"):
+                host_entry = {}
+                for child in host_elt:
+                    host_entry[child.tag] = child.text
+                existing_hosts.append(host_entry)
+
+            # Preserve existing domain overrides
+            existing_overrides = []
+            for override_elt in self.root_elt.findall("domainoverrides"):
+                override_entry = {}
+                for child in override_elt:
+                    override_entry[child.tag] = child.text
+                existing_overrides.append(override_entry)
+
+            obj["hosts"] = existing_hosts
+            obj["domainoverrides"] = existing_overrides
-            self._get_ansible_param(obj, "hosts")
-            self._get_ansible_param(obj, "domainoverrides")
+
+            # Append new hosts if provided
+            if params.get("hosts"):
+                if "hosts" not in obj:
+                    obj["hosts"] = []
+                for host in params["hosts"]:
+                    if host.get("aliases"):
+                        host["aliases"] = {"item": host["aliases"]}
+                    else:
+                        host["aliases"] = "\n\t\t\t"
+                obj["hosts"].extend(params["hosts"])
+
+            # Append new domain overrides if provided
+            if params.get("domainoverrides"):
+                if "domainoverrides" not in obj:
+                    obj["domainoverrides"] = []
+                obj["domainoverrides"].extend(params["domainoverrides"])
-            # wrap <item> to all hosts.alias
-            for host in obj["hosts"]:
-                if host["aliases"]:
-                    tmp_aliases = host["aliases"]
-                    host["aliases"] = {
-                        "item": tmp_aliases
-                    }
-                else:
-                    # Default is an empty element
-                    host["aliases"] = "\n\t\t\t"
-

+    def _merge_param(self, obj, params, param_name, transform=None):
+        """Helper method to merge parameters only if they are provided"""
+        if params.get(param_name) is not None:
+            value = params[param_name]
+            if transform:
+                value = transform(value)
+            obj[param_name] = value
+
+    def _get_current_config(self):
+        """Get the current configuration from the target"""
+        if self.root_elt is None:
+            return None
+
+        current = {}
+
+        # Extract existing configuration into a dict
+        for child in self.root_elt:
+            if child.tag == "hosts":
+                if "hosts" not in current:
+                    current["hosts"] = []
+                host_entry = {}
+                for host_child in child:
+                    host_entry[host_child.tag] = host_child.text
+                current["hosts"].append(host_entry)
+
+            elif child.tag == "domainoverrides":
+                if "domainoverrides" not in current:
+                    current["domainoverrides"] = []
+                override_entry = {}
+                for override_child in child:
+                    override_entry[override_child.tag] = override_child.text
+                current["domainoverrides"].append(override_entry)
+
+            else:
+                current[child.tag] = child.text
+
+        return current
+

I'm happy to work a PR through, but I am not sure if this meets any requirements the project has.
#worksOnMyMachine

I have a commit in my fork which does the following:

• leaves the current host over rides intact
• Updates a record if the host and the domain exist in the host override
• leaves the custom options intact

[opoplawski]

How do you remove hosts or domainoverrides then? I think we need an extra parameter to determine what the intent is - merge or strict definition.

[opoplawski]

A PR would be great to work through this.

[stratus-ss]

The libvirt community module uses "flags" for some options. That might be reasonable to implement here.

    - name: remove and undefine vm
      community.libvirt.virt:
        command: undefine
        name: "{{ vm }}"
        flags: managed_save,snapshots_metadata,delete_volumes

I'm not sure if this is a reasonable approach or too complex. It might be too complex or out of style for this project however.

Before I start working on a PR, I would love some input as to the way the author(s) think might be a reasonable approach. Should it be a completely different option? Should there be 2 options? What do you think