From 136419c5c0bb7d2822ca9ced26d7a37d0e1861be Mon Sep 17 00:00:00 2001 From: Alexander Petrenz Date: Sat, 11 May 2024 16:51:51 +0200 Subject: [PATCH] bug(lookup/merge_variables): Fix rendering foreign variables (#8303) * manually prepare variables of foreign host including hostvars property * render variables from context of current host * add integration test for cross host merge * lint fixes * adjust cross host merge unit tests to provide a tiny bit of the HostVars Class API * add license information * lint * add changelog fragment * Update tests/integration/targets/lookup_merge_variables/test_cross_host_merge_play.yml Okay Co-authored-by: Mark <40321020+m-a-r-k-e@users.noreply.github.com> * Update tests/integration/targets/lookup_merge_variables/test_cross_host_merge_play.yml Okay Co-authored-by: Mark <40321020+m-a-r-k-e@users.noreply.github.com> * Update tests/integration/targets/lookup_merge_variables/test_cross_host_merge_play.yml Okay Co-authored-by: Mark <40321020+m-a-r-k-e@users.noreply.github.com> * rename _HostVars to HostVarsMock * removing unnecessary task --------- Co-authored-by: Gitlab CI Co-authored-by: Mark <40321020+m-a-r-k-e@users.noreply.github.com> --- .../8303-fix-rendering-foreign-variables.yaml | 2 + 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 ++++++++++-------- 6 files changed, 165 insertions(+), 83 deletions(-) create mode 100644 changelogs/fragments/8303-fix-rendering-foreign-variables.yaml create mode 100644 tests/integration/targets/lookup_merge_variables/test_cross_host_merge_inventory.yml create mode 100644 tests/integration/targets/lookup_merge_variables/test_cross_host_merge_play.yml diff --git a/changelogs/fragments/8303-fix-rendering-foreign-variables.yaml b/changelogs/fragments/8303-fix-rendering-foreign-variables.yaml new file mode 100644 index 0000000000..c2162771f2 --- /dev/null +++ b/changelogs/fragments/8303-fix-rendering-foreign-variables.yaml @@ -0,0 +1,2 @@ +bugfixes: + - "merge_variables lookup plugin - fixing cross host merge: providing access to foreign hosts variables to the perspective of the host that is performing the merge (https://github.com/ansible-collections/community.general/pull/8303)." diff --git a/plugins/lookup/merge_variables.py b/plugins/lookup/merge_variables.py index 4fc33014c0..ce7621ad23 100644 --- a/plugins/lookup/merge_variables.py +++ b/plugins/lookup/merge_variables.py @@ -157,7 +157,9 @@ 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"]): - cross_host_merge_result = self._merge_vars(term, cross_host_merge_result, variables["hostvars"][host]) + 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) ret.append(cross_host_merge_result) return ret @@ -195,7 +197,8 @@ class LookupModule(LookupBase): result = initial_value for var_name in var_merge_names: - var_value = self._templar.template(variables[var_name]) # Render jinja2 templates + 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_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 4e66476be4..ada6908dd7 100755 --- a/tests/integration/targets/lookup_merge_variables/runme.sh +++ b/tests/integration/targets/lookup_merge_variables/runme.sh @@ -14,3 +14,6 @@ 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 new file mode 100644 index 0000000000..938457023e --- /dev/null +++ b/tests/integration/targets/lookup_merge_variables/test_cross_host_merge_inventory.yml @@ -0,0 +1,33 @@ +--- +# 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 new file mode 100644 index 0000000000..51cd6f1ba3 --- /dev/null +++ b/tests/integration/targets/lookup_merge_variables/test_cross_host_merge_play.yml @@ -0,0 +1,21 @@ +--- +# 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 66cb2f08bb..ba8209439a 100644 --- a/tests/unit/plugins/lookup/test_merge_variables.py +++ b/tests/unit/plugins/lookup/test_merge_variables.py @@ -18,6 +18,17 @@ 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={}) @@ -141,25 +152,28 @@ class TestMergeVariablesLookup(unittest.TestCase): {'var': [{'item5': 'value5', 'item6': 'value6'}]}, ]) def test_merge_dict_group_all(self, mock_set_options, mock_get_option, mock_template): - 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'}] - } + 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'}] } } }) + variables = { + 'inventory_hostname': 'host1', + 'hostvars': hostvars + } + + results = self.merge_vars_lookup.run(['__merge_var'], variables) self.assertEqual(results, [ {'var': [ @@ -175,32 +189,35 @@ class TestMergeVariablesLookup(unittest.TestCase): {'var': [{'item5': 'value5', 'item6': 'value6'}]}, ]) def test_merge_dict_group_single(self, mock_set_options, mock_get_option, mock_template): - 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'}] - } + 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'}] } } }) + variables = { + 'inventory_hostname': 'host1', + 'hostvars': hostvars + } + + results = self.merge_vars_lookup.run(['__merge_var'], variables) self.assertEqual(results, [ {'var': [ @@ -216,32 +233,34 @@ class TestMergeVariablesLookup(unittest.TestCase): {'var': [{'item5': 'value5', 'item6': 'value6'}]}, ]) def test_merge_dict_group_multiple(self, mock_set_options, mock_get_option, mock_template): - 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'}] - } + 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'}] } } }) + variables = { + 'inventory_hostname': 'host1', + 'hostvars': hostvars + } + results = self.merge_vars_lookup.run(['__merge_var'], variables) self.assertEqual(results, [ {'var': [ @@ -257,26 +276,27 @@ class TestMergeVariablesLookup(unittest.TestCase): ['item5'], ]) def test_merge_list_group_multiple(self, mock_set_options, mock_get_option, mock_template): - 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'] - } + 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'] } }) + variables = { + 'inventory_hostname': 'host1', + 'hostvars': hostvars + } + results = self.merge_vars_lookup.run(['__merge_var'], variables) self.assertEqual(results, [['item1', 'item5']])