From f0df94e0f873335e85507cf195bd994cfefc344c Mon Sep 17 00:00:00 2001 From: Roy Lenferink Date: Sun, 23 Mar 2025 11:32:50 +0100 Subject: [PATCH] Revert "bug(lookup/merge_variables): Fix rendering foreign variables (#8303)" This reverts commit 136419c5c0bb7d2822ca9ced26d7a37d0e1861be. This because - with the introduction of data tagging to Ansible - the internal `HostVars.raw_get()` function is no longer available and only methods defined by `Mapping` should be used. See https://github.com/ansible/ansible/issues/84799#issuecomment-2745254573 for more context. --- plugins/lookup/merge_variables.py | 7 +- .../targets/lookup_merge_variables/runme.sh | 3 - .../test_cross_host_merge_inventory.yml | 33 ---- .../test_cross_host_merge_play.yml | 21 -- .../plugins/lookup/test_merge_variables.py | 182 ++++++++---------- 5 files changed, 83 insertions(+), 163 deletions(-) delete mode 100644 tests/integration/targets/lookup_merge_variables/test_cross_host_merge_inventory.yml delete mode 100644 tests/integration/targets/lookup_merge_variables/test_cross_host_merge_play.yml diff --git a/plugins/lookup/merge_variables.py b/plugins/lookup/merge_variables.py index e352524292..747b596eff 100644 --- a/plugins/lookup/merge_variables.py +++ b/plugins/lookup/merge_variables.py @@ -157,9 +157,7 @@ class LookupModule(LookupBase): cross_host_merge_result = initial_value for host in variables["hostvars"]: if self._is_host_in_allowed_groups(variables["hostvars"][host]["group_names"]): - host_variables = dict(variables["hostvars"].raw_get(host)) - host_variables["hostvars"] = variables["hostvars"] # re-add hostvars - cross_host_merge_result = self._merge_vars(term, cross_host_merge_result, host_variables) + cross_host_merge_result = self._merge_vars(term, cross_host_merge_result, variables["hostvars"][host]) ret.append(cross_host_merge_result) return ret @@ -197,8 +195,7 @@ class LookupModule(LookupBase): result = initial_value for var_name in var_merge_names: - with self._templar.set_temporary_context(available_variables=variables): # tmp. switch renderer to context of current variables - var_value = self._templar.template(variables[var_name]) # Render jinja2 templates + var_value = self._templar.template(variables[var_name]) # Render jinja2 templates var_type = _verify_and_get_type(var_value) if prev_var_type is None: diff --git a/tests/integration/targets/lookup_merge_variables/runme.sh b/tests/integration/targets/lookup_merge_variables/runme.sh index ada6908dd7..4e66476be4 100755 --- a/tests/integration/targets/lookup_merge_variables/runme.sh +++ b/tests/integration/targets/lookup_merge_variables/runme.sh @@ -14,6 +14,3 @@ ANSIBLE_MERGE_VARIABLES_PATTERN_TYPE=suffix \ ANSIBLE_LOG_PATH=/tmp/ansible-test-merge-variables \ ansible-playbook -i test_inventory_all_hosts.yml test_all_hosts.yml "$@" - -ANSIBLE_LOG_PATH=/tmp/ansible-test-merge-variables \ - ansible-playbook -i test_cross_host_merge_inventory.yml test_cross_host_merge_play.yml "$@" diff --git a/tests/integration/targets/lookup_merge_variables/test_cross_host_merge_inventory.yml b/tests/integration/targets/lookup_merge_variables/test_cross_host_merge_inventory.yml deleted file mode 100644 index 938457023e..0000000000 --- a/tests/integration/targets/lookup_merge_variables/test_cross_host_merge_inventory.yml +++ /dev/null @@ -1,33 +0,0 @@ ---- -# Copyright (c) 2020, Thales Netherlands -# Copyright (c) 2021, Ansible Project -# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) -# SPDX-License-Identifier: GPL-3.0-or-later - -common: - vars: - provider_instances: - servicedata1: - host: "{{ hostvars[groups['provider'] | first].inventory_hostname }}" - user: usr - pass: pwd - servicedata2: - host: down - user: usr2 - pass: pwd2 - hosts: - host1: - host2: - -consumer: - vars: - service_data: "{{ provider_instances.servicedata1 }}" - merge2__1: "{{ service_data }}" # service_data is a variable only known to host2, so normally it´s not available for host1 that is performing the merge - hosts: - host2: - -provider: - vars: - merge_result: "{{ lookup('community.general.merge_variables', 'merge2__', pattern_type='prefix', groups=['consumer']) }}" - hosts: - host1: diff --git a/tests/integration/targets/lookup_merge_variables/test_cross_host_merge_play.yml b/tests/integration/targets/lookup_merge_variables/test_cross_host_merge_play.yml deleted file mode 100644 index 51cd6f1ba3..0000000000 --- a/tests/integration/targets/lookup_merge_variables/test_cross_host_merge_play.yml +++ /dev/null @@ -1,21 +0,0 @@ ---- -# Copyright (c) 2020, Thales Netherlands -# Copyright (c) 2021, Ansible Project -# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) -# SPDX-License-Identifier: GPL-3.0-or-later - -- name: Test merge_variables lookup plugin (merging host reference variables) - hosts: host1 - connection: local - gather_facts: false - tasks: - - name: Print merge result - ansible.builtin.debug: - msg: "{{ merge_result }}" - - name: Validate merge result - ansible.builtin.assert: - that: - - "merge_result | length == 3" - - "merge_result.host == 'host1'" - - "merge_result.user == 'usr'" - - "merge_result.pass == 'pwd'" diff --git a/tests/unit/plugins/lookup/test_merge_variables.py b/tests/unit/plugins/lookup/test_merge_variables.py index c8b0ca6fab..ab3b22f354 100644 --- a/tests/unit/plugins/lookup/test_merge_variables.py +++ b/tests/unit/plugins/lookup/test_merge_variables.py @@ -18,17 +18,6 @@ from ansible_collections.community.general.plugins.lookup import merge_variables class TestMergeVariablesLookup(unittest.TestCase): - class HostVarsMock(dict): - - def __getattr__(self, item): - return super().__getitem__(item) - - def __setattr__(self, item, value): - return super().__setitem__(item, value) - - def raw_get(self, host): - return super().__getitem__(host) - def setUp(self): self.loader = DictDataLoader({}) self.templar = Templar(loader=self.loader, variables={}) @@ -152,28 +141,25 @@ class TestMergeVariablesLookup(unittest.TestCase): {'var': [{'item5': 'value5', 'item6': 'value6'}]}, ]) def test_merge_dict_group_all(self, mock_set_options, mock_get_option, mock_template): - hostvars = self.HostVarsMock({ - 'host1': { - 'group_names': ['dummy1'], - 'inventory_hostname': 'host1', - '1testlist__merge_var': { - 'var': [{'item1': 'value1', 'item2': 'value2'}] - } - }, - 'host2': { - 'group_names': ['dummy1'], - 'inventory_hostname': 'host2', - '2otherlist__merge_var': { - 'var': [{'item5': 'value5', 'item6': 'value6'}] + results = self.merge_vars_lookup.run(['__merge_var'], { + 'inventory_hostname': 'host1', + 'hostvars': { + 'host1': { + 'group_names': ['dummy1'], + 'inventory_hostname': 'host1', + '1testlist__merge_var': { + 'var': [{'item1': 'value1', 'item2': 'value2'}] + } + }, + 'host2': { + 'group_names': ['dummy1'], + 'inventory_hostname': 'host2', + '2otherlist__merge_var': { + 'var': [{'item5': 'value5', 'item6': 'value6'}] + } } } }) - variables = { - 'inventory_hostname': 'host1', - 'hostvars': hostvars - } - - results = self.merge_vars_lookup.run(['__merge_var'], variables) self.assertEqual(results, [ {'var': [ @@ -189,35 +175,32 @@ class TestMergeVariablesLookup(unittest.TestCase): {'var': [{'item5': 'value5', 'item6': 'value6'}]}, ]) def test_merge_dict_group_single(self, mock_set_options, mock_get_option, mock_template): - hostvars = self.HostVarsMock({ - 'host1': { - 'group_names': ['dummy1'], - 'inventory_hostname': 'host1', - '1testlist__merge_var': { - 'var': [{'item1': 'value1', 'item2': 'value2'}] - } - }, - 'host2': { - 'group_names': ['dummy1'], - 'inventory_hostname': 'host2', - '2otherlist__merge_var': { - 'var': [{'item5': 'value5', 'item6': 'value6'}] - } - }, - 'host3': { - 'group_names': ['dummy2'], - 'inventory_hostname': 'host3', - '3otherlist__merge_var': { - 'var': [{'item3': 'value3', 'item4': 'value4'}] + results = self.merge_vars_lookup.run(['__merge_var'], { + 'inventory_hostname': 'host1', + 'hostvars': { + 'host1': { + 'group_names': ['dummy1'], + 'inventory_hostname': 'host1', + '1testlist__merge_var': { + 'var': [{'item1': 'value1', 'item2': 'value2'}] + } + }, + 'host2': { + 'group_names': ['dummy1'], + 'inventory_hostname': 'host2', + '2otherlist__merge_var': { + 'var': [{'item5': 'value5', 'item6': 'value6'}] + } + }, + 'host3': { + 'group_names': ['dummy2'], + 'inventory_hostname': 'host3', + '3otherlist__merge_var': { + 'var': [{'item3': 'value3', 'item4': 'value4'}] + } } } }) - variables = { - 'inventory_hostname': 'host1', - 'hostvars': hostvars - } - - results = self.merge_vars_lookup.run(['__merge_var'], variables) self.assertEqual(results, [ {'var': [ @@ -233,34 +216,32 @@ class TestMergeVariablesLookup(unittest.TestCase): {'var': [{'item5': 'value5', 'item6': 'value6'}]}, ]) def test_merge_dict_group_multiple(self, mock_set_options, mock_get_option, mock_template): - hostvars = self.HostVarsMock({ - 'host1': { - 'group_names': ['dummy1'], - 'inventory_hostname': 'host1', - '1testlist__merge_var': { - 'var': [{'item1': 'value1', 'item2': 'value2'}] - } - }, - 'host2': { - 'group_names': ['dummy2'], - 'inventory_hostname': 'host2', - '2otherlist__merge_var': { - 'var': [{'item5': 'value5', 'item6': 'value6'}] - } - }, - 'host3': { - 'group_names': ['dummy3'], - 'inventory_hostname': 'host3', - '3otherlist__merge_var': { - 'var': [{'item3': 'value3', 'item4': 'value4'}] + results = self.merge_vars_lookup.run(['__merge_var'], { + 'inventory_hostname': 'host1', + 'hostvars': { + 'host1': { + 'group_names': ['dummy1'], + 'inventory_hostname': 'host1', + '1testlist__merge_var': { + 'var': [{'item1': 'value1', 'item2': 'value2'}] + } + }, + 'host2': { + 'group_names': ['dummy2'], + 'inventory_hostname': 'host2', + '2otherlist__merge_var': { + 'var': [{'item5': 'value5', 'item6': 'value6'}] + } + }, + 'host3': { + 'group_names': ['dummy3'], + 'inventory_hostname': 'host3', + '3otherlist__merge_var': { + 'var': [{'item3': 'value3', 'item4': 'value4'}] + } } } }) - variables = { - 'inventory_hostname': 'host1', - 'hostvars': hostvars - } - results = self.merge_vars_lookup.run(['__merge_var'], variables) self.assertEqual(results, [ {'var': [ @@ -276,27 +257,26 @@ class TestMergeVariablesLookup(unittest.TestCase): ['item5'], ]) def test_merge_list_group_multiple(self, mock_set_options, mock_get_option, mock_template): - hostvars = self.HostVarsMock({ - 'host1': { - 'group_names': ['dummy1'], - 'inventory_hostname': 'host1', - '1testlist__merge_var': ['item1'] - }, - 'host2': { - 'group_names': ['dummy2'], - 'inventory_hostname': 'host2', - '2otherlist__merge_var': ['item5'] - }, - 'host3': { - 'group_names': ['dummy3'], - 'inventory_hostname': 'host3', - '3otherlist__merge_var': ['item3'] + print() + results = self.merge_vars_lookup.run(['__merge_var'], { + 'inventory_hostname': 'host1', + 'hostvars': { + 'host1': { + 'group_names': ['dummy1'], + 'inventory_hostname': 'host1', + '1testlist__merge_var': ['item1'] + }, + 'host2': { + 'group_names': ['dummy2'], + 'inventory_hostname': 'host2', + '2otherlist__merge_var': ['item5'] + }, + 'host3': { + 'group_names': ['dummy3'], + 'inventory_hostname': 'host3', + '3otherlist__merge_var': ['item3'] + } } }) - variables = { - 'inventory_hostname': 'host1', - 'hostvars': hostvars - } - results = self.merge_vars_lookup.run(['__merge_var'], variables) self.assertEqual(results, [['item1', 'item5']])