diff --git a/plugins/module_utils/pfsense.py b/plugins/module_utils/pfsense.py index 339d7f4f..7dd159e0 100644 --- a/plugins/module_utils/pfsense.py +++ b/plugins/module_utils/pfsense.py @@ -320,11 +320,23 @@ def copy_dict_to_element(self, src, top_elt, sub=0, prev_elt=None): changed = True elif self.copy_dict_to_element(item, all_sub_elts[idx], sub=sub + 1, prev_elt=prev_elt): changed = True - elif this_elt.text is None and value == '': - pass - elif this_elt.text != value: - this_elt.text = value - changed = True + else: + # If this element previously held a dict/list (has children) + # but the new value is a scalar, remove stale children first. + # Without this, nested elements (e.g. inside ) + # persist even when the parent is set to an empty string, + # causing data from one list entry to bleed into another. + # Note: ET.Element.clear() is not used here because it also + # resets .tail, which would corrupt pretty-print whitespace. + if len(this_elt) > 0: + for child in list(this_elt): + this_elt.remove(child) + changed = True + if this_elt.text is None and value == '': + pass + elif this_elt.text != value: + this_elt.text = value + changed = True self.debug.write('changed=%s this_elt.text=%s value=%s\n' % (changed, this_elt.text, value)) prev_elt = this_elt diff --git a/tests/unit/plugins/modules/fixtures/pfsense_dns_resolver_config_hosts_aliases.xml b/tests/unit/plugins/modules/fixtures/pfsense_dns_resolver_config_hosts_aliases.xml new file mode 100644 index 00000000..d8db7ef6 --- /dev/null +++ b/tests/unit/plugins/modules/fixtures/pfsense_dns_resolver_config_hosts_aliases.xml @@ -0,0 +1,251 @@ + + 23.3 + + normal + pfSense + acme.com + on + + all + All Users + system + 1998 + + + admins + System Administrators + system + 1999 + 0 + page-all + + + + system + + test + + + + + system + + groupe1 + + + + + system + + groupe2 + + + + admin + System Administrator + system + admins + $2y$10$AMCpA.Z.RNaferLp1yzFq.BvaGgfqaJKtQug7OErbocyNagsEK6xW + 0 + user-shell-access + + 2 + + + pfSense.css + + 2000 + 2000 + 0.pfsense.pool.ntp.org + + http + + 5c00e5f9029df + 2 + + yes + + + + 400000 + hadp + hadp + hadp + + monthly + + + + Etc/UTC + + + + + em0 + + dhcp + dhcp6 + + + + + + + + + 0 + + + + em1 + 192.168.1.1 + 24 + + + + + wan + 0 + + + + + em2 + opt1 + 10.0.0.1 + 24 + + + em1.100 + VLAN 100 + 172.16.0.1 + 24 + + + + + em1 + 100 + + VLAN 100 on LAN + em1.100 + + + + + + + 192.168.1.100 + 192.168.1.199 + + + 86400 + 172800 + + + + + + + + hmac-md5 + + + + allow + + + + + + + + + + + + + 10.0.0.100 + 10.0.0.199 + + + 86400 + 172800 + + + opt1.example.com + + + + + hmac-md5 + + + + allow + + + + + + + + + enabled + + + + + + all + all + + + + + + transparent + 4 + 10 + 10 + auto + 512 + 200 + 86400 + 0 + 900 + 10000 + disabled + 1 + + server + example.com + 10.0.0.1 + + + + alias1 + example.com + Alias 1 + + + alias2 + example.com + Alias 2 + + + + + other + example.com + 10.0.0.2 + + + + + + + aggregated change + + + diff --git a/tests/unit/plugins/modules/test_pfsense_dns_resolver.py b/tests/unit/plugins/modules/test_pfsense_dns_resolver.py index a9a7e53e..18130354 100644 --- a/tests/unit/plugins/modules/test_pfsense_dns_resolver.py +++ b/tests/unit/plugins/modules/test_pfsense_dns_resolver.py @@ -9,6 +9,7 @@ from ansible_collections.pfsensible.core.plugins.modules.pfsense_dns_resolver import PFSenseDNSResolverModule from .pfsense_module import TestPFSenseModule from ansible_collections.community.internal_test_tools.tests.unit.compat.mock import patch +from ansible_collections.community.internal_test_tools.tests.unit.plugins.modules.utils import set_module_args class TestPFSenseDNSResolverModule(TestPFSenseModule): @@ -102,6 +103,55 @@ def test_dns_resolver_noop(self): obj = dict() self.do_module_test(obj, changed=False) + def test_dns_resolver_hosts_reorder_aliases_stay(self): + """ test that aliases stay with their parent host when hosts are reordered + + Regression test: copy_dict_to_element() pairs list items by position. + When hosts are provided in a different order than config.xml, a host + with no aliases could inherit stale children from a host that + previously occupied the same position. + """ + # Use a fixture that has two hosts: "server" (with aliases) at position 0, + # "other" (no aliases) at position 1. + self.config_file = 'pfsense_dns_resolver_config_hosts_aliases.xml' + self.xml_result = None + + # Provide hosts reversed: other first, server second. + obj = dict( + hosts=[ + dict(host="other", domain="example.com", ip="10.0.0.2", descr="", aliases=[]), + dict(host="server", domain="example.com", ip="10.0.0.1", descr="", + aliases=[dict(host="alias1", domain="example.com", description="Alias 1"), + dict(host="alias2", domain="example.com", description="Alias 2")]), + ] + ) + # Run the module directly and inspect XML — bypass check_target_elt + # which cannot handle complex nested hosts/aliases structures. + with set_module_args(self.args_from_var(obj, state='present')): + result = self.execute_module(changed=True) + self.assertTrue(self.load_xml_result()) + + # Verify aliases stayed with the correct host after reorder. + # "other" must have NO alias children; "server" must keep its 2. + unbound = self.xml_result.find('unbound') + hosts_elts = unbound.findall('hosts') + self.assertEqual(len(hosts_elts), 2) + + for host_elt in hosts_elts: + hostname = host_elt.find('host').text + aliases_elt = host_elt.find('aliases') + if hostname == 'other': + items = aliases_elt.findall('item') if aliases_elt is not None else [] + self.assertEqual(len(items), 0, + 'Host "other" should have no alias items but got %d — ' + 'aliases bled from "server" due to positional matching' % len(items)) + elif hostname == 'server': + items = aliases_elt.findall('item') if aliases_elt is not None else [] + self.assertEqual(len(items), 2, + 'Host "server" should have 2 alias items but got %d' % len(items)) + else: + self.fail('Unexpected host element with hostname %r in result XML' % hostname) + def test_dns_resolver_domainoverrides_forward_tls_upstream(self): """ test initialization of the DNS Resolver """ obj = dict(