[PR #8303/136419c5 backport][stable-8] bug(lookup/merge_variables): Fix rendering foreign variables (#8338)

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 <alexander.petrenz@posteo.de> Co-authored-by: Mark <40321020+m-a-r-k-e@users.noreply.github.com> (cherry picked from commit 136419c5c0) Co-authored-by: Alexander Petrenz <petrenz.a@gmail.com>
2024-09-14 20:13:21 +02:00 · 2024-05-11 17:11:18 +02:00 · 2024-05-11 17:11:18 +02:00 · 01a54bba2f
commit 01a54bba2f
parent 4c39295b97
6 changed files with 165 additions and 83 deletions
--- a/changelogs/fragments/8303-fix-rendering-foreign-variables.yaml
+++ 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)."
--- 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,6 +197,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_type = _verify_and_get_type(var_value)

--- 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 "$@"
--- 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
@ -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:
--- 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
@ -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'"
--- 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,9 +152,7 @@ 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': {
+        hostvars = self.HostVarsMock({
            'host1': {
                'group_names': ['dummy1'],
                'inventory_hostname': 'host1',
@ -158,8 +167,13 @@ class TestMergeVariablesLookup(unittest.TestCase):
                    '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,9 +189,7 @@ 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': {
+        hostvars = self.HostVarsMock({
            'host1': {
                'group_names': ['dummy1'],
                'inventory_hostname': 'host1',
@ -199,8 +211,13 @@ class TestMergeVariablesLookup(unittest.TestCase):
                    '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,9 +233,7 @@ 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': {
+        hostvars = self.HostVarsMock({
            'host1': {
                'group_names': ['dummy1'],
                'inventory_hostname': 'host1',
@ -240,8 +255,12 @@ class TestMergeVariablesLookup(unittest.TestCase):
                    '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,10 +276,7 @@ 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': {
+        hostvars = self.HostVarsMock({
            'host1': {
                'group_names': ['dummy1'],
                'inventory_hostname': 'host1',
@ -276,7 +292,11 @@ class TestMergeVariablesLookup(unittest.TestCase):
                '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']])