Fix missing ansible.builtin FQCNs in hardcoded action names (ansible#…

…71824)

* Make sure hard-coded action names also check for FQCN.
* Use _add_internal_fqcn() to avoid hardcoded lists and typoes.

(cherry picked from commit da60525)

ci_complete

changelogs/fragments/71824-action-fqcns.yml

@@ -0,0 +1,2 @@
+bugfixes:
+- "Adjust various hard-coded action names to also include their ``ansible.builtin.`` and ``ansible.legacy.`` prefixed version (https://github.com/ansible/ansible/issues/71817, https://github.com/ansible/ansible/issues/71818, https://github.com/ansible/ansible/pull/71824)."

lib/ansible/cli/adhoc.py

@@ -69,7 +69,7 @@ def _play_ds(self, pattern, async_val, poll):
         mytask = {'action': {'module': context.CLIARGS['module_name'], 'args': parse_kv(context.CLIARGS['module_args'], check_raw=check_raw)}}
 
         # avoid adding to tasks that don't support it, unless set, then give user an error
-        if context.CLIARGS['module_name'] not in ('include_role', 'include_tasks') and any(frozenset((async_val, poll))):
+        if context.CLIARGS['module_name'] not in C._ACTION_ALL_INCLUDE_ROLE_TASKS and any(frozenset((async_val, poll))):
             mytask['async_val'] = async_val
             mytask['poll'] = poll
 
@@ -118,7 +118,7 @@ def run(self):
             raise AnsibleOptionsError(err)
 
         # Avoid modules that don't work with ad-hoc
-        if context.CLIARGS['module_name'] in ('import_playbook',):
+        if context.CLIARGS['module_name'] in C._ACTION_IMPORT_PLAYBOOK:
             raise AnsibleOptionsError("'%s' is not a valid action for ad-hoc commands"
                                       % context.CLIARGS['module_name'])
 

lib/ansible/cli/console.py

@@ -182,7 +182,7 @@ def default(self, arg, forceshell=False):
 
         result = None
         try:
-            check_raw = module in ('command', 'shell', 'script', 'raw')
+            check_raw = module in C._ACTION_ALLOWS_RAW_ARGS
             play_ds = dict(
                 name="Ansible Shell",
                 hosts=self.cwd,

lib/ansible/cli/playbook.py

@@ -8,6 +8,7 @@
 import os
 import stat
 
+from ansible import constants as C
 from ansible import context
 from ansible.cli import CLI
 from ansible.cli.arguments import option_helpers as opt_help
@@ -161,7 +162,7 @@ def _process_block(b):
                                 if isinstance(task, Block):
                                     taskmsg += _process_block(task)
                                 else:
-                                    if task.action == 'meta':
+                                    if task.action in C._ACTION_META:
                                         continue
 
                                     all_tags.update(task.tags)

lib/ansible/constants.py

@@ -17,6 +17,7 @@
 from ansible.module_utils.parsing.convert_bool import boolean, BOOLEANS_TRUE
 from ansible.module_utils.six import string_types
 from ansible.config.manager import ConfigManager, ensure_type, get_ini_config_value
+from ansible.utils.fqcn import add_internal_fqcns
 
 
 def _warning(msg):
@@ -108,8 +109,8 @@ def __getitem__(self, y):
 IGNORE_FILES = ("COPYING", "CONTRIBUTING", "LICENSE", "README", "VERSION", "GUIDELINES")  # ignore during module search
 INTERNAL_RESULT_KEYS = ('add_host', 'add_group')
 LOCALHOST = ('127.0.0.1', 'localhost', '::1')
-MODULE_REQUIRE_ARGS = ('command', 'win_command', 'shell', 'win_shell', 'raw', 'script')
-MODULE_NO_JSON = ('command', 'win_command', 'shell', 'win_shell', 'raw')
+MODULE_REQUIRE_ARGS = tuple(add_internal_fqcns(('command', 'win_command', 'shell', 'win_shell', 'raw', 'script')))
+MODULE_NO_JSON = tuple(add_internal_fqcns(('command', 'win_command', 'shell', 'win_shell', 'raw')))
 RESTRICTED_RESULT_KEYS = ('ansible_rsync_path', 'ansible_playbook_python', 'ansible_facts')
 TREE_DIR = None
 VAULT_VERSION_MIN = 1.0
@@ -196,3 +197,27 @@ def __getitem__(self, y):
 
 for warn in config.WARNINGS:
     _warning(warn)
+
+
+# The following are hard-coded action names
+_ACTION_DEBUG = add_internal_fqcns(('debug', ))
+_ACTION_IMPORT_PLAYBOOK = add_internal_fqcns(('import_playbook', ))
+_ACTION_IMPORT_ROLE = add_internal_fqcns(('import_role', ))
+_ACTION_IMPORT_TASKS = add_internal_fqcns(('import_tasks', ))
+_ACTION_INCLUDE = add_internal_fqcns(('include', ))
+_ACTION_INCLUDE_ROLE = add_internal_fqcns(('include_role', ))
+_ACTION_INCLUDE_TASKS = add_internal_fqcns(('include_tasks', ))
+_ACTION_INCLUDE_VARS = add_internal_fqcns(('include_vars', ))
+_ACTION_META = add_internal_fqcns(('meta', ))
+_ACTION_SET_FACT = add_internal_fqcns(('set_fact', ))
+_ACTION_HAS_CMD = add_internal_fqcns(('command', 'shell', 'script'))
+_ACTION_ALLOWS_RAW_ARGS = _ACTION_HAS_CMD + add_internal_fqcns(('raw', ))
+_ACTION_ALL_INCLUDES = _ACTION_INCLUDE + _ACTION_INCLUDE_TASKS + _ACTION_INCLUDE_ROLE
+_ACTION_ALL_IMPORT_PLAYBOOKS = _ACTION_INCLUDE + _ACTION_IMPORT_PLAYBOOK
+_ACTION_ALL_INCLUDE_IMPORT_TASKS = _ACTION_INCLUDE + _ACTION_INCLUDE_TASKS + _ACTION_IMPORT_TASKS
+_ACTION_ALL_PROPER_INCLUDE_IMPORT_ROLES = _ACTION_INCLUDE_ROLE + _ACTION_IMPORT_ROLE
+_ACTION_ALL_PROPER_INCLUDE_IMPORT_TASKS = _ACTION_INCLUDE_TASKS + _ACTION_IMPORT_TASKS
+_ACTION_ALL_INCLUDE_ROLE_TASKS = _ACTION_INCLUDE_ROLE + _ACTION_INCLUDE_TASKS
+_ACTION_ALL_INCLUDE_TASKS = _ACTION_INCLUDE + _ACTION_INCLUDE_TASKS
+_ACTION_FACT_GATHERING = add_internal_fqcns(('setup', 'gather_facts'))
+_ACTION_WITH_CLEAN_FACTS = _ACTION_SET_FACT + _ACTION_INCLUDE_VARS

lib/ansible/executor/task_executor.py

@@ -573,7 +573,7 @@ def _execute(self, variables=None):
 
         # if this task is a TaskInclude, we just return now with a success code so the
         # main thread can expand the task list for the given host
-        if self._task.action in ('include', 'include_tasks'):
+        if self._task.action in C._ACTION_ALL_INCLUDE_TASKS:
             include_args = self._task.args.copy()
             include_file = include_args.pop('_raw_params', None)
             if not include_file:
@@ -583,7 +583,7 @@ def _execute(self, variables=None):
             return dict(include=include_file, include_args=include_args)
 
         # if this task is a IncludeRole, we just return now with a success code so the main thread can expand the task list for the given host
-        elif self._task.action == 'include_role':
+        elif self._task.action in C._ACTION_INCLUDE_ROLE:
             include_args = self._task.args.copy()
             return dict(include_args=include_args)
 
@@ -710,7 +710,7 @@ def _evaluate_failed_when_result(result):
                 return failed_when_result
 
             if 'ansible_facts' in result:
-                if self._task.action in ('set_fact', 'include_vars'):
+                if self._task.action in C._ACTION_WITH_CLEAN_FACTS:
                     vars_copy.update(result['ansible_facts'])
                 else:
                     # TODO: cleaning of facts should eventually become part of taskresults instead of vars
@@ -774,7 +774,7 @@ def _evaluate_failed_when_result(result):
             variables[self._task.register] = wrap_var(result)
 
         if 'ansible_facts' in result:
-            if self._task.action in ('set_fact', 'include_vars'):
+            if self._task.action in C._ACTION_WITH_CLEAN_FACTS:
                 variables.update(result['ansible_facts'])
             else:
                 # TODO: cleaning of facts should eventually become part of taskresults instead of vars

lib/ansible/executor/task_result.py

@@ -113,7 +113,7 @@ def clean_copy(self):
         result = TaskResult(self._host, self._task, {}, self._task_fields)
 
         # statuses are already reflected on the event type
-        if result._task and result._task.action in ['debug']:
+        if result._task and result._task.action in C._ACTION_DEBUG:
             # debug is verbose by default to display vars, no need to add invocation
             ignore = _IGNORE + ('invocation',)
         else:

lib/ansible/parsing/mod_args.py

@@ -25,6 +25,7 @@
 from ansible.parsing.splitter import parse_kv, split_args
 from ansible.plugins.loader import module_loader, action_loader
 from ansible.template import Templar
+from ansible.utils.fqcn import add_internal_fqcns
 from ansible.utils.sentinel import Sentinel
 
 
@@ -38,7 +39,7 @@
     'raw'
 ))
 
-RAW_PARAM_MODULES = FREEFORM_ACTIONS.union((
+RAW_PARAM_MODULES = FREEFORM_ACTIONS.union(add_internal_fqcns((
     'include',
     'include_vars',
     'include_tasks',
@@ -49,16 +50,16 @@
     'group_by',
     'set_fact',
     'meta',
-))
+)))
 
-BUILTIN_TASKS = frozenset((
+BUILTIN_TASKS = frozenset(add_internal_fqcns((
     'meta',
     'include',
     'include_tasks',
     'include_role',
     'import_tasks',
     'import_role'
-))
+)))
 
 
 class ModuleArgsParser:

lib/ansible/playbook/__init__.py

@@ -90,14 +90,18 @@ def _load_playbook_data(self, file_name, variable_manager, vars=None):
                 self._loader.set_basedir(cur_basedir)
                 raise AnsibleParserError("playbook entries must be either a valid play or an include statement", obj=entry)
 
-            if any(action in entry for action in ('import_playbook', 'include')):
-                if 'include' in entry:
+            if any(action in entry for action in C._ACTION_ALL_IMPORT_PLAYBOOKS):
+                if any(action in entry for action in C._ACTION_INCLUDE):
                     display.deprecated("'include' for playbook includes. You should use 'import_playbook' instead", version="2.12")
                 pb = PlaybookInclude.load(entry, basedir=self._basedir, variable_manager=variable_manager, loader=self._loader)
                 if pb is not None:
                     self._entries.extend(pb._entries)
                 else:
-                    which = entry.get('import_playbook', entry.get('include', entry))
+                    which = entry
+                    for k in C._ACTION_IMPORT_PLAYBOOK + C._ACTION_INCLUDE:
+                        if k in entry:
+                            which = entry[k]
+                            break
                     display.display("skipping playbook '%s' due to conditional test failure" % which, color=C.COLOR_SKIP)
             else:
                 entry_obj = Play.load(entry, variable_manager=variable_manager, loader=self._loader, vars=vars)

lib/ansible/playbook/block.py

@@ -19,6 +19,7 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+import ansible.constants as C
 from ansible.errors import AnsibleParserError
 from ansible.playbook.attribute import FieldAttribute
 from ansible.playbook.base import Base
@@ -374,8 +375,8 @@ def evaluate_and_append_task(target):
                     filtered_block = evaluate_block(task)
                     if filtered_block.has_tasks():
                         tmp_list.append(filtered_block)
-                elif (task.action == 'meta' or
-                        (task.action == 'include' and task.evaluate_tags([], self._play.skip_tags, all_vars=all_vars)) or
+                elif (task.action in C._ACTION_META or
+                        (task.action in C._ACTION_INCLUDE and task.evaluate_tags([], self._play.skip_tags, all_vars=all_vars)) or
                         task.evaluate_tags(self._play.only_tags, self._play.skip_tags, all_vars=all_vars)):
                     tmp_list.append(task)
             return tmp_list

lib/ansible/playbook/helpers.py

@@ -129,7 +129,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
                 # But if it wasn't, we can add the yaml object now to get more detail
                 raise AnsibleParserError(to_native(e), obj=task_ds, orig_exc=e)
 
-            if action in ('include', 'import_tasks', 'include_tasks'):
+            if action in C._ACTION_ALL_INCLUDE_IMPORT_TASKS:
 
                 if use_handlers:
                     include_class = HandlerTaskInclude
@@ -151,9 +151,9 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
                 # check to see if this include is dynamic or static:
                 # 1. the user has set the 'static' option to false or true
                 # 2. one of the appropriate config options was set
-                if action == 'include_tasks':
+                if action in C._ACTION_INCLUDE_TASKS:
                     is_static = False
-                elif action == 'import_tasks':
+                elif action in C._ACTION_IMPORT_TASKS:
                     is_static = True
                 elif t.static is not None:
                     display.deprecated("The use of 'static' has been deprecated. "
@@ -166,7 +166,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
 
                 if is_static:
                     if t.loop is not None:
-                        if action == 'import_tasks':
+                        if action in C._ACTION_IMPORT_TASKS:
                             raise AnsibleParserError("You cannot use loops on 'import_tasks' statements. You should use 'include_tasks' instead.", obj=task_ds)
                         else:
                             raise AnsibleParserError("You cannot use 'static' on an include with a loop", obj=task_ds)
@@ -248,7 +248,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
                         # nested includes, and we want the include order printed correctly
                         display.vv("statically imported: %s" % include_file)
                     except AnsibleFileNotFound:
-                        if action != 'include' or t.static or \
+                        if action not in C._ACTION_INCLUDE or t.static or \
                            C.DEFAULT_TASK_INCLUDES_STATIC or \
                            C.DEFAULT_HANDLER_INCLUDES_STATIC and use_handlers:
                             raise
@@ -285,7 +285,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
                         tags = tags.split(',')
 
                     if len(tags) > 0:
-                        if action in ('include_tasks', 'import_tasks'):
+                        if action in C._ACTION_ALL_PROPER_INCLUDE_IMPORT_TASKS:
                             raise AnsibleParserError('You cannot specify "tags" inline to the task, it is a task keyword')
                         if len(ti_copy.tags) > 0:
                             raise AnsibleParserError(
@@ -315,7 +315,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
                     t.is_static = False
                     task_list.append(t)
 
-            elif action in ('include_role', 'import_role'):
+            elif action in C._ACTION_ALL_PROPER_INCLUDE_IMPORT_ROLES:
                 ir = IncludeRole.load(
                     task_ds,
                     block=block,
@@ -328,7 +328,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
                 #   1. the user has set the 'static' option to false or true
                 #   2. one of the appropriate config options was set
                 is_static = False
-                if action == 'import_role':
+                if action in C._ACTION_IMPORT_ROLE:
                     is_static = True
 
                 elif ir.static is not None:
@@ -338,7 +338,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h
 
                 if is_static:
                     if ir.loop is not None:
-                        if action == 'import_role':
+                        if action in C._ACTION_IMPORT_ROLE:
                             raise AnsibleParserError("You cannot use loops on 'import_role' statements. You should use 'include_role' instead.", obj=task_ds)
                         else:
                             raise AnsibleParserError("You cannot use 'static' on an include_role with a loop", obj=task_ds)

lib/ansible/playbook/included_file.py

@@ -21,6 +21,7 @@
 
 import os
 
+from ansible import constants as C
 from ansible.errors import AnsibleError
 from ansible.module_utils._text import to_text
 from ansible.playbook.task_include import TaskInclude
@@ -67,7 +68,7 @@ def process_include_results(results, iterator, loader, variable_manager):
             original_host = res._host
             original_task = res._task
 
-            if original_task.action in ('include', 'include_tasks', 'include_role'):
+            if original_task.action in C._ACTION_ALL_INCLUDES:
                 if original_task.loop:
                     if 'results' not in res._result:
                         continue
@@ -111,7 +112,7 @@ def process_include_results(results, iterator, loader, variable_manager):
 
                     templar = Templar(loader=loader, variables=task_vars)
 
-                    if original_task.action in ('include', 'include_tasks'):
+                    if original_task.action in C._ACTION_ALL_INCLUDE_TASKS:
                         include_file = None
                         if original_task:
                             if original_task.static:

lib/ansible/playbook/playbook_include.py

@@ -21,6 +21,7 @@
 
 import os
 
+import ansible.constants as C
 from ansible.errors import AnsibleParserError, AnsibleAssertionError
 from ansible.module_utils.six import iteritems, string_types
 from ansible.parsing.splitter import split_args, parse_kv
@@ -114,7 +115,7 @@ def preprocess_data(self, ds):
             new_ds.ansible_pos = ds.ansible_pos
 
         for (k, v) in iteritems(ds):
-            if k in ('include', 'import_playbook'):
+            if k in C._ACTION_ALL_IMPORT_PLAYBOOKS:
                 self._preprocess_import(ds, new_ds, k, v)
             else:
                 # some basic error checking, to make sure vars are properly

lib/ansible/playbook/role_include.py

@@ -20,6 +20,7 @@
 
 from os.path import basename
 
+import ansible.constants as C
 from ansible.errors import AnsibleParserError
 from ansible.playbook.attribute import FieldAttribute
 from ansible.playbook.block import Block
@@ -126,7 +127,7 @@ def load(data, block=None, role=None, task_include=None, variable_manager=None,
         if ir._role_name is None:
             raise AnsibleParserError("'name' is a required field for %s." % ir.action, obj=data)
 
-        if 'public' in ir.args and ir.action != 'include_role':
+        if 'public' in ir.args and ir.action not in C._ACTION_INCLUDE_ROLE:
             raise AnsibleParserError('Invalid options for %s: public' % ir.action, obj=data)
 
         # validate bad args, otherwise we silently ignore
@@ -140,7 +141,7 @@ def load(data, block=None, role=None, task_include=None, variable_manager=None,
             ir._from_files[from_key] = basename(ir.args.get(key))
 
         apply_attrs = ir.args.get('apply', {})
-        if apply_attrs and ir.action != 'include_role':
+        if apply_attrs and ir.action not in C._ACTION_INCLUDE_ROLE:
             raise AnsibleParserError('Invalid options for %s: apply' % ir.action, obj=data)
         elif not isinstance(apply_attrs, dict):
             raise AnsibleParserError('Expected a dict for apply but got %s instead' % type(apply_attrs), obj=data)