diff --git a/ansiblelater/rules/CheckFQCNBuiltin.py b/ansiblelater/rules/CheckFQCNBuiltin.py new file mode 100644 index 0000000..e232fbd --- /dev/null +++ b/ansiblelater/rules/CheckFQCNBuiltin.py @@ -0,0 +1,136 @@ +# Original code written by the authors of ansible-lint + +from ansiblelater.rule import RuleBase +from ansiblelater.utils import load_plugin + + +class CheckFQCNBuiltin(RuleBase): + rid = "ANS128" + 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", + } + + def check(self, candidate, settings): + tasks, errors = self.get_normalized_tasks(candidate, settings) + + _builtins = [ + "add_host", + "apt", + "apt_key", + "apt_repository", + "assemble", + "assert", + "async_status", + "blockinfile", + "command", + "copy", + "cron", + "debconf", + "debug", + "dnf", + "dpkg_selections", + "expect", + "fail", + "fetch", + "file", + "find", + "gather_facts", + "get_url", + "getent", + "git", + "group", + "group_by", + "hostname", + "import_playbook", + "import_role", + "import_tasks", + "include", + "include_role", + "include_tasks", + "include_vars", + "iptables", + "known_hosts", + "lineinfile", + "meta", + "package", + "package_facts", + "pause", + "ping", + "pip", + "raw", + "reboot", + "replace", + "rpm_key", + "script", + "service", + "service_facts", + "set_fact", + "set_stats", + "setup", + "shell", + "slurp", + "stat", + "subversion", + "systemd", + "sysvinit", + "tempfile", + "template", + "unarchive", + "uri", + "user", + "wait_for", + "wait_for_connection", + "yum", + "yum_repository", + ] + + if errors: + return self.Result(candidate.path, errors) + + for task in tasks: + module = task["action"]["__ansible_module_original__"] + + if module not in self.module_aliases: + loaded_module = load_plugin(module) + target = loaded_module.resolved_fqcn + self.module_aliases[module] = target + + if target is None: + self.module_aliases[module] = module + continue + + if target not in self.module_aliases: + self.module_aliases[target] = target + + if module != self.module_aliases[module]: + module_alias = self.module_aliases[module] + if module_alias.startswith("ansible.builtin"): + legacy_module = module_alias.replace( + "ansible.builtin.", + "ansible.legacy.", + 1, + ) + if module != legacy_module: + helptext = self.helptext.format(module_alias=module_alias, module=module) + if module == "ansible.builtin.include": + helptext = ( + "`ansible.builtin.include_task` or `ansible.builtin.import_tasks` " + f"should be used instead of deprecated `{module}`", + ) + + errors.append(self.Error(task["__line__"], helptext)) + else: + if module.count(".") < 2: + errors.append( + self.Error( + task["__line__"], + self.helptext.format(module_alias=module_alias, module=module), + ) + ) + + return self.Result(candidate.path, errors) diff --git a/ansiblelater/settings.py b/ansiblelater/settings.py index 23a50bc..f45e087 100644 --- a/ansiblelater/settings.py +++ b/ansiblelater/settings.py @@ -123,8 +123,8 @@ class Settings: "include_filter": [], "exclude_filter": [], "warning_filter": [ + "ANS128", "ANS999", - "ANS998", ], "ignore_dotfiles": True, "exclude_files": [], diff --git a/ansiblelater/utils/__init__.py b/ansiblelater/utils/__init__.py index 9b81e61..e03bde6 100644 --- a/ansiblelater/utils/__init__.py +++ b/ansiblelater/utils/__init__.py @@ -4,8 +4,10 @@ import contextlib import re import sys from contextlib import suppress +from functools import lru_cache import yaml +from ansible.plugins.loader import module_loader from ansiblelater import logger @@ -114,3 +116,21 @@ class Singleton(type): if cls not in cls._instances: cls._instances[cls] = super().__call__(*args, **kwargs) return cls._instances[cls] + + +@lru_cache +def load_plugin(name): + """Return loaded ansible plugin/module.""" + loaded_module = module_loader.find_plugin_with_context( + name, + ignore_deprecated=True, + check_aliases=True, + ) + if not loaded_module.resolved and name.startswith("ansible.builtin."): + # fallback to core behavior of using legacy + loaded_module = module_loader.find_plugin_with_context( + name.replace("ansible.builtin.", "ansible.legacy."), + ignore_deprecated=True, + check_aliases=True, + ) + return loaded_module diff --git a/ansiblelater/utils/yamlhelper.py b/ansiblelater/utils/yamlhelper.py index b4ae92e..10a4b47 100644 --- a/ansiblelater/utils/yamlhelper.py +++ b/ansiblelater/utils/yamlhelper.py @@ -413,7 +413,10 @@ def normalize_task(task, filename, custom_modules=None): # convert builtin fqn calls to short forms because most rules know only # about short calls - normalized["action"] = {"__ansible_module__": action.removeprefix("ansible.builtin.")} + 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() diff --git a/docs/content/configuration/defaults.md b/docs/content/configuration/defaults.md index 180209f..08dad66 100644 --- a/docs/content/configuration/defaults.md +++ b/docs/content/configuration/defaults.md @@ -77,8 +77,8 @@ rules: # List of rule ID's that should be displayed as a warning instead of an error. By default, # no rules are marked as warnings. This list allows to degrade errors to warnings for each rule. warning_filter: + - "ANS128" - "ANS999" - - "ANS998" # All dotfiles (including hidden folders) are excluded by default. # You can disable this setting and handle dotfiles by yourself with `exclude_files`. diff --git a/docs/content/included_rules/_index.md b/docs/content/included_rules/_index.md index 98136ee..7dfe3d0 100644 --- a/docs/content/included_rules/_index.md +++ b/docs/content/included_rules/_index.md @@ -43,4 +43,5 @@ Reviews are useless without some rules to check against. `ansible-later` comes w | CheckRelativeRolePaths | ANS125 | Don't use a relative path in a role. | | | CheckChangedInWhen | ANS126 | Use handlers instead of `when: changed`. | | | CheckChangedInWhen | ANS127 | Deprecated bare variables in loops must not be used. | | +| CheckFQCNBuiltin | ANS128 | Module actions should use full qualified collection names. | | | CheckDeprecated | ANS999 | Deprecated features of `ansible-later` should not be used. | |