diff --git a/ansiblelater/rule.py b/ansiblelater/rule.py index c275e57..af1af50 100644 --- a/ansiblelater/rule.py +++ b/ansiblelater/rule.py @@ -149,11 +149,12 @@ class RuleBase(metaclass=RuleExtendedMeta): # No need to normalize_task if we are skipping it. continue - normalized.append( - normalize_task( - task, candidate.path, settings["ansible"]["custom_modules"] - ) + normalized_task = normalize_task( + task, candidate.path, settings["ansible"]["custom_modules"] ) + normalized_task["__raw_task__"] = task + + normalized.append(normalized_task) except LaterError as ex: e = ex.original diff --git a/ansiblelater/rules/CheckFQCNBuiltin.py b/ansiblelater/rules/CheckFQCNBuiltin.py index e232fbd..77cf833 100644 --- a/ansiblelater/rules/CheckFQCNBuiltin.py +++ b/ansiblelater/rules/CheckFQCNBuiltin.py @@ -9,11 +9,7 @@ class CheckFQCNBuiltin(RuleBase): helptext = "use FQCN `{module_alias}` for module action `{module}`" description = "Module actions should use full qualified collection names" types = ["playbook", "task", "handler", "rolevars", "hostvars", "groupvars"] - module_aliases = { - "block": "block", - "always": "always", - "rescue": "rescue", - } + module_aliases = {"block/always/rescue": "block/always/rescue"} def check(self, candidate, settings): tasks, errors = self.get_normalized_tasks(candidate, settings) diff --git a/ansiblelater/rules/CheckKeyOrder.py b/ansiblelater/rules/CheckKeyOrder.py index 581b6bb..e5ce893 100644 --- a/ansiblelater/rules/CheckKeyOrder.py +++ b/ansiblelater/rules/CheckKeyOrder.py @@ -21,7 +21,7 @@ SORTER_TASKS = ( class CheckKeyOrder(RuleBase): rid = "ANS129" - description = "Check optimized key order" + description = "Check for recommended key order" helptext = "{type} key order can be improved to `{sorted_keys}`" types = ["playbook", "task", "handler"] diff --git a/ansiblelater/settings.py b/ansiblelater/settings.py index 9402853..82cbaa1 100644 --- a/ansiblelater/settings.py +++ b/ansiblelater/settings.py @@ -144,7 +144,7 @@ class Settings: "exclude": [ "meta", "debug", - "block", + "block/always/rescue", "include_role", "import_role", "include_tasks", diff --git a/ansiblelater/utils/yamlhelper.py b/ansiblelater/utils/yamlhelper.py index b1026bc..025c4fc 100644 --- a/ansiblelater/utils/yamlhelper.py +++ b/ansiblelater/utils/yamlhelper.py @@ -21,7 +21,6 @@ # THE SOFTWARE. import codecs -import copy import os from contextlib import suppress @@ -370,12 +369,63 @@ def _kv_to_dict(v): def normalize_task(task, filename, custom_modules=None): """Ensure tasks have an action key and strings are converted to python objects.""" - if custom_modules is None: - custom_modules = [] + def _normalize(task, custom_modules): + if custom_modules is None: + custom_modules = [] - ansible_action_type = task.get("__ansible_action_type__", "task") - if "__ansible_action_type__" in task: - del task["__ansible_action_type__"] + normalized = {} + ansible_parsed_keys = ("action", "local_action", "args", "delegate_to") + + if is_nested_task(task): + _extract_ansible_parsed_keys_from_task(normalized, task, ansible_parsed_keys) + # Add dummy action for block/always/rescue statements + normalized["action"] = { + "__ansible_module__": "block/always/rescue", + "__ansible_module_original__": "block/always/rescue", + "__ansible_arguments__": "block/always/rescue", + } + return normalized + + builtin = list(ansible.parsing.mod_args.BUILTIN_TASKS) + builtin = list(set(builtin + custom_modules)) + ansible.parsing.mod_args.BUILTIN_TASKS = frozenset(builtin) + mod_arg_parser = ModuleArgsParser(task) + + try: + action, arguments, normalized["delegate_to"] = mod_arg_parser.parse() + except AnsibleParserError as e: + raise LaterAnsibleError(e) from e + + # denormalize shell -> command conversion + if "_uses_shell" in arguments: + action = "shell" + del arguments["_uses_shell"] + + for k, v in list(task.items()): + if k in ansible_parsed_keys or k == action: + # we don"t want to re-assign these values, which were + # determined by the ModuleArgsParser() above + continue + + normalized[k] = v + + # convert builtin fqn calls to short forms because most rules know only + # about short calls + normalized["action"] = { + "__ansible_module__": action.removeprefix("ansible.builtin."), + "__ansible_module_original__": action, + } + + if "_raw_params" in arguments: + normalized["action"]["__ansible_arguments__"] = ( + arguments["_raw_params"].strip().split() + ) + del arguments["_raw_params"] + else: + normalized["action"]["__ansible_arguments__"] = [] + normalized["action"].update(arguments) + + return normalized # temp. extract metadata ansible_meta = {} @@ -387,44 +437,11 @@ def normalize_task(task, filename, custom_modules=None): ansible_meta[key] = task.pop(key, default) - normalized = {} + ansible_action_type = task.get("__ansible_action_type__", "task") + if "__ansible_action_type__" in task: + del task["__ansible_action_type__"] - builtin = list(ansible.parsing.mod_args.BUILTIN_TASKS) - builtin = list(set(builtin + custom_modules)) - ansible.parsing.mod_args.BUILTIN_TASKS = frozenset(builtin) - mod_arg_parser = ModuleArgsParser(task) - - try: - action, arguments, normalized["delegate_to"] = mod_arg_parser.parse() - except AnsibleParserError as e: - raise LaterAnsibleError(e) from e - - # denormalize shell -> command conversion - if "_uses_shell" in arguments: - action = "shell" - del arguments["_uses_shell"] - - for k, v in list(task.items()): - if k in ("action", "local_action", "args", "delegate_to") or k == action: - # we don"t want to re-assign these values, which were - # determined by the ModuleArgsParser() above - continue - - normalized[k] = v - - # convert builtin fqn calls to short forms because most rules know only - # about short calls - normalized["action"] = { - "__ansible_module__": action.removeprefix("ansible.builtin."), - "__ansible_module_original__": action, - } - - if "_raw_params" in arguments: - normalized["action"]["__ansible_arguments__"] = arguments["_raw_params"].strip().split() - del arguments["_raw_params"] - else: - normalized["action"]["__ansible_arguments__"] = [] - normalized["action"].update(arguments) + normalized = _normalize(task, custom_modules) normalized[FILENAME_KEY] = filename normalized["__ansible_action_type__"] = ansible_action_type @@ -446,13 +463,8 @@ def action_tasks(yaml, candidate): # Add sub-elements of block/rescue/always to tasks list tasks.extend(extract_from_list(tasks, ["block", "rescue", "always"])) - # Remove block/rescue/always elements from tasks list - block_rescue_always = ("block", "rescue", "always") - tasks[:] = [task for task in tasks if all(k not in task for k in block_rescue_always)] - allowed = ["include", "include_tasks", "import_playbook", "import_tasks"] - - return [task for task in tasks if set(allowed).isdisjoint(task.keys())] + return tasks def task_to_str(task): @@ -483,8 +495,6 @@ def extract_from_list(blocks, candidates): meta_data.pop(key, None) actions = add_action_type(block[candidate], candidate, meta_data) - for action in actions: - action["__raw_task__"] = copy.copy(block) results.extend(actions) elif block[candidate] is not None: @@ -575,6 +585,26 @@ def normalized_yaml(file, options): return lines +def is_nested_task(task): + """Check if task includes block/always/rescue.""" + # Cannot really trust the input + if isinstance(task, str): + return False + + return any(task.get(key) for key in ["block", "rescue", "always"]) + + +def _extract_ansible_parsed_keys_from_task(result, task, keys): + """Return a dict with existing key in task.""" + for k, v in list(task.items()): + if k in keys: + # we don't want to re-assign these values, which were + # determined by the ModuleArgsParser() above + continue + result[k] = v + return result + + class UnsafeTag: """Handle custom yaml unsafe tag.""" diff --git a/docs/content/configuration/defaults.md b/docs/content/configuration/defaults.md index 08dad66..3d9f784 100644 --- a/docs/content/configuration/defaults.md +++ b/docs/content/configuration/defaults.md @@ -34,7 +34,7 @@ ansible: exclude: - "meta" - "debug" - - "block" + - "block/always/rescue" - "include_role" - "include_tasks" - "include_vars"