From a7b14ec1becb947cc290deebe309ef06a12d1f7e Mon Sep 17 00:00:00 2001
From: Brian Coca <bcoca@users.noreply.github.com>
Date: Thu, 11 Jul 2019 13:49:49 -0400
Subject: [PATCH] Fix strat inv (#58982)

* Fix strategy functions that update inventory

* added tests
---
 changelogs/fragments/fix_strategy_inv_up.yml    |  2 ++
 lib/ansible/plugins/strategy/__init__.py        |  8 ++++++--
 test/integration/targets/inventory/runme.sh     |  4 ++++
 test/integration/targets/inventory/strategy.yml | 12 ++++++++++++
 4 files changed, 24 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/fragments/fix_strategy_inv_up.yml
 create mode 100644 test/integration/targets/inventory/strategy.yml

diff --git a/changelogs/fragments/fix_strategy_inv_up.yml b/changelogs/fragments/fix_strategy_inv_up.yml
new file mode 100644
index 00000000000..b9314245c72
--- /dev/null
+++ b/changelogs/fragments/fix_strategy_inv_up.yml
@@ -0,0 +1,2 @@
+bugfixes:
+    - Fix strategy functions that update inventory and back 'add_host' and 'group_by' actions.
diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index f8a82d8d330..8106b0adcfc 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -711,7 +711,7 @@ class StrategyBase:
             new_groups = host_info.get('groups', [])
             for group_name in new_groups:
                 if group_name not in self._inventory.groups:
-                    self._inventory.add_group(group_name)
+                    group_name = self._inventory.add_group(group_name)
                 new_group = self._inventory.groups[group_name]
                 new_group.add_host(self._inventory.hosts[host_name])
 
@@ -738,11 +738,15 @@ class StrategyBase:
         group_name = result_item.get('add_group')
         parent_group_names = result_item.get('parent_groups', [])
 
-        for name in [group_name] + parent_group_names:
+        if group_name not in self._inventory.groups:
+            group_name = self._inventory.add_group(group_name)
+
+        for name in parent_group_names:
             if name not in self._inventory.groups:
                 # create the new group and add it to inventory
                 self._inventory.add_group(name)
                 changed = True
+
         group = self._inventory.groups[group_name]
         for parent_group_name in parent_group_names:
             parent_group = self._inventory.groups[parent_group_name]
diff --git a/test/integration/targets/inventory/runme.sh b/test/integration/targets/inventory/runme.sh
index 8fb90ea65b4..31854c8778e 100755
--- a/test/integration/targets/inventory/runme.sh
+++ b/test/integration/targets/inventory/runme.sh
@@ -9,3 +9,7 @@ if [ "$?" != "1" ]; then
     echo "Non-matching limit should cause failure"
     exit 1
 fi
+
+ansible-playbook -i ../../inventory "$@" strategy.yml
+ANSIBLE_TRANSFORM_INVALID_GROUP_CHARS=always ansible-playbook -i ../../inventory "$@" strategy.yml
+ANSIBLE_TRANSFORM_INVALID_GROUP_CHARS=never ansible-playbook -i ../../inventory "$@" strategy.yml
diff --git a/test/integration/targets/inventory/strategy.yml b/test/integration/targets/inventory/strategy.yml
new file mode 100644
index 00000000000..5c1cbd2bd50
--- /dev/null
+++ b/test/integration/targets/inventory/strategy.yml
@@ -0,0 +1,12 @@
+- name: Check that 'invalid' group works, problem exposed in #58980
+  hosts: localhost
+  tasks:
+    - name: add a host to a group, that has - to trigger substitution
+      add_host:
+        name: localhost
+        groups: Not-Working
+
+    - name: group hosts by distribution, with dash to trigger substitution
+      group_by:
+        key: "{{ ansible_distribution }}-{{ ansible_distribution_version }}"
+      changed_when: false