strategy fix invalid variables being registered (#79706) (#79818)

also added tests for normal and bad cases

(cherry picked from commit 281474e809a0a76f6a045224d9051efda6e1f0ec)

7 files changed, 65 insertions, 1 deletions

diff --git a/changelogs/fragments/strategy_badid_fix.yml b/changelogs/fragments/strategy_badid_fix.yml
new file mode 100644
index 0000000000..c32edee1eb
--- /dev/null
+++ b/changelogs/fragments/strategy_badid_fix.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - strategy plugins now correctly identify bad registered variables, even on skip.
diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index e3c4b02da6..5cc05ee3f3 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -54,7 +54,7 @@ from ansible.template import Templar
 from ansible.utils.display import Display
 from ansible.utils.fqcn import add_internal_fqcns
 from ansible.utils.unsafe_proxy import wrap_var
-from ansible.utils.vars import combine_vars
+from ansible.utils.vars import combine_vars, isidentifier
 from ansible.vars.clean import strip_internal_keys, module_response_deepcopy
 
 display = Display()
@@ -750,6 +750,10 @@ class StrategyBase:
 
             # register final results
             if original_task.register:
+
+                if not isidentifier(original_task.register):
+                    raise AnsibleError("Invalid variable name in 'register' specified: '%s'" % original_task.register)
+
                 host_list = self.get_task_hosts(iterator, original_host, original_task)
 
                 clean_copy = strip_internal_keys(module_response_deepcopy(task_result._result))
diff --git a/test/integration/targets/register/aliases b/test/integration/targets/register/aliases
new file mode 100644
index 0000000000..b76d5f677a
--- /dev/null
+++ b/test/integration/targets/register/aliases
@@ -0,0 +1,2 @@
+shippable/posix/group3
+context/controller  # this "module" is actually an action that runs on the controller
diff --git a/test/integration/targets/register/can_register.yml b/test/integration/targets/register/can_register.yml
new file mode 100644
index 0000000000..da610101fd
--- /dev/null
+++ b/test/integration/targets/register/can_register.yml
@@ -0,0 +1,21 @@
+- hosts: testhost
+  gather_facts: false
+  tasks:
+    - name: test registering
+      debug: msg='does nothing really but register this'
+      register: debug_msg
+
+    - name: validate registering
+      assert:
+        that:
+          - debug_msg is defined
+
+    - name: test registering skipped
+      debug: msg='does nothing really but register this'
+      when: false
+      register: debug_skipped
+
+    - name: validate registering
+      assert:
+        that:
+          - debug_skipped is defined
diff --git a/test/integration/targets/register/invalid.yml b/test/integration/targets/register/invalid.yml
new file mode 100644
index 0000000000..bdca9d667f
--- /dev/null
+++ b/test/integration/targets/register/invalid.yml
@@ -0,0 +1,11 @@
+- hosts: testhost
+  gather_facts: false
+  tasks:
+    - name: test registering
+      debug: msg='does nothing really but register this'
+      register: 200
+
+    - name: never gets here
+      assert:
+        that:
+          - 200 is not defined
diff --git a/test/integration/targets/register/invalid_skipped.yml b/test/integration/targets/register/invalid_skipped.yml
new file mode 100644
index 0000000000..0ad31f510a
--- /dev/null
+++ b/test/integration/targets/register/invalid_skipped.yml
@@ -0,0 +1,12 @@
+- hosts: testhost
+  gather_facts: false
+  tasks:
+    - name: test registering bad var when skipped
+      debug: msg='does nothing really but register this'
+      when: false
+      register: 200
+
+    - name: never gets here
+      assert:
+        that:
+          - 200 is not defined
diff --git a/test/integration/targets/register/runme.sh b/test/integration/targets/register/runme.sh
new file mode 100755
index 0000000000..8adc504755
--- /dev/null
+++ b/test/integration/targets/register/runme.sh
@@ -0,0 +1,12 @@
+#!/usr/bin/env bash
+
+set -eux
+
+# does it work?
+ansible-playbook can_register.yml -i ../../inventory -v "$@"
+
+# ensure we do error when it its apprpos
+set +e
+result="$(ansible-playbook invalid.yml -i ../../inventory -v "$@" 2>&1)"
+set -e
+grep -q "Invalid variable name in " <<< "${result}"