From 87ebc56de619fbe8e0632b1a21eb082777a7c816 Mon Sep 17 00:00:00 2001
From: Sloane Hertel <shertel@redhat.com>
Date: Thu, 14 Mar 2019 13:22:18 -0500
Subject: [PATCH] Allow parent groups to be variables or literal (#53649)

* Allow parent groups to be variables or literal, requires {{ }}
* Check strict before failing on templating errors
* Don't add a group if an invalid parent group was provided
---
 .../53649-allow-parent-group-as-variable.yaml |  3 +
 lib/ansible/plugins/inventory/__init__.py     |  7 ++
 .../plugins/inventory/test_constructed.py     | 65 +++++++++++++++++++
 3 files changed, 75 insertions(+)
 create mode 100644 changelogs/fragments/53649-allow-parent-group-as-variable.yaml

diff --git a/changelogs/fragments/53649-allow-parent-group-as-variable.yaml b/changelogs/fragments/53649-allow-parent-group-as-variable.yaml
new file mode 100644
index 00000000000..7ca2b2f698b
--- /dev/null
+++ b/changelogs/fragments/53649-allow-parent-group-as-variable.yaml
@@ -0,0 +1,3 @@
+minor_changes:
+  - inventory keyed_groups - allow the parent_group to be specified as a variable by using
+    brackets, such as "{{ placement.region }}", or as a string if brackets are not used.
diff --git a/lib/ansible/plugins/inventory/__init__.py b/lib/ansible/plugins/inventory/__init__.py
index 32cba841f3a..5330d42eb91 100644
--- a/lib/ansible/plugins/inventory/__init__.py
+++ b/lib/ansible/plugins/inventory/__init__.py
@@ -404,6 +404,13 @@ class Constructable(object):
                         prefix = keyed.get('prefix', '')
                         sep = keyed.get('separator', '_')
                         raw_parent_name = keyed.get('parent_group', None)
+                        if raw_parent_name:
+                            try:
+                                raw_parent_name = self.templar.template(raw_parent_name)
+                            except AnsibleError as e:
+                                if strict:
+                                    raise AnsibleParserError("Could not generate parent group %s for group %s: %s" % (raw_parent_name, key, to_native(e)))
+                                continue
 
                         new_raw_group_names = []
                         if isinstance(key, string_types):
diff --git a/test/units/plugins/inventory/test_constructed.py b/test/units/plugins/inventory/test_constructed.py
index d298ffe00cb..40f660c82f7 100644
--- a/test/units/plugins/inventory/test_constructed.py
+++ b/test/units/plugins/inventory/test_constructed.py
@@ -19,6 +19,7 @@
 
 import pytest
 
+from ansible.errors import AnsibleParserError
 from ansible.plugins.inventory.constructed import InventoryModule
 from ansible.inventory.data import InventoryData
 from ansible.template import Templar
@@ -116,3 +117,67 @@ def test_keyed_parent_groups(inventory_module):
     all_regions = inventory_module.inventory.groups['region_list']
     assert all_regions.child_groups == [region_group]
     assert region_group.hosts == [host1, host2]
+
+
+def test_parent_group_templating(inventory_module):
+    inventory_module.inventory.add_host('cow')
+    inventory_module.inventory.set_variable('cow', 'sound', 'mmmmmmmmmm')
+    inventory_module.inventory.set_variable('cow', 'nickname', 'betsy')
+    host = inventory_module.inventory.get_host('cow')
+    keyed_groups = [
+        {
+            'key': 'sound',
+            'prefix': 'sound',
+            'parent_group': '{{ nickname }}'
+        },
+        {
+            'key': 'nickname',
+            'prefix': '',
+            'separator': '',
+            'parent_group': 'nickname'  # statically-named parent group, conflicting with hostvar
+        },
+        {
+            'key': 'nickname',
+            'separator': '',
+            'parent_group': '{{ location | default("field") }}'
+        }
+    ]
+    inventory_module._add_host_to_keyed_groups(
+        keyed_groups, host.vars, host.name, strict=True
+    )
+    # first keyed group, "betsy" is a parent group name dynamically generated
+    betsys_group = inventory_module.inventory.groups['betsy']
+    assert [child.name for child in betsys_group.child_groups] == ['sound_mmmmmmmmmm']
+    # second keyed group, "nickname" is a statically-named root group
+    nicknames_group = inventory_module.inventory.groups['nickname']
+    assert [child.name for child in nicknames_group.child_groups] == ['betsy']
+    # second keyed group actually generated the parent group of the first keyed group
+    # assert that these are, in fact, the same object
+    assert nicknames_group.child_groups[0] == betsys_group
+    # second keyed group has two parents
+    locations_group = inventory_module.inventory.groups['field']
+    assert [child.name for child in locations_group.child_groups] == ['betsy']
+
+
+def test_parent_group_templating_error(inventory_module):
+    inventory_module.inventory.add_host('cow')
+    inventory_module.inventory.set_variable('cow', 'nickname', 'betsy')
+    host = inventory_module.inventory.get_host('cow')
+    keyed_groups = [
+        {
+            'key': 'nickname',
+            'separator': '',
+            'parent_group': '{{ location.barn-yard }}'
+        }
+    ]
+    with pytest.raises(AnsibleParserError) as err_message:
+        inventory_module._add_host_to_keyed_groups(
+            keyed_groups, host.vars, host.name, strict=True
+        )
+        assert 'Could not generate parent group' in err_message
+    # invalid parent group did not raise an exception with strict=False
+    inventory_module._add_host_to_keyed_groups(
+        keyed_groups, host.vars, host.name, strict=False
+    )
+    # assert group was never added with invalid parent
+    assert 'betsy' not in inventory_module.inventory.groups