ansible / ansible-lint

ansible-lint checks playbooks for practices and behavior that could potentially be improved and can fix some of the most common ones for you
https://ansible.readthedocs.io/projects/lint/
GNU General Public License v3.0
3.44k stars 655 forks source link

exception from ArgsRule.matchtasks (playbook): attempted relative import with no known parent package #4270

Open CAAHS opened 1 month ago

CAAHS commented 1 month ago
Summary

Running ansible-lint(1) emits a diagnostic message:

WARNING Ignored exception from ArgsRule.matchtasks while processing plays/fact-cache.yml (playbook): attempted relative import with no known parent package

Issue Type
OS / ENVIRONMENT
$ ansible-lint --version
ansible-lint 24.7.0 using ansible-core:2.17.2 ansible-compat:24.7.0 ruamel-yaml:0.18.6 ruamel-yaml-clib:0.2.8
STEPS TO REPRODUCE

We use the ansible-lint version as documented and the following plays/fact-cache.yml (playbook):

#!/usr/bin/env ansible-playbook
---

- name: 'Refresh Fact Cache'
  hosts: all
  tasks:
    - name: Refresh all machine facts from the cache
      setup:
        gather_subset: all
        gather_timeout: 10
Desired Behavior

The above documented diagnostics are not emitted.

Actual Behavior

The above documented diagnostics are emitted.

Excerpt from extra verbosity:

$ ansible-lint fixtures plays roles -vvvv
...
DEBUG    Running rule args
WARNING  Ignored exception from ArgsRule.matchtasks while processing plays/fact-cache.yml (playbook): attempted relative import with no known parent package
DEBUG    Ignored exception details
Traceback (most recent call last):
  File "/.../python-3.10/lib/python3.10/site-packages/ansiblelint/_internal/rules.py", line 94, in getmatches
    matches.extend(method(file))
  File "/.../python-3.10/lib/python3.10/site-packages/ansiblelint/rules/__init__.py", line 178, in matchtasks
    result = self.matchtask(task, file=file)
  File "/.../python-3.10/lib/python3.10/site-packages/ansiblelint/rules/args.py", line 161, in matchtask
    spec.loader.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 883, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/.../python-3.10/lib/python3.10/site-packages/ansible/modules/setup.py", line 174, in <module>
    from ..module_utils.basic import AnsibleModule
ImportError: attempted relative import with no known parent package
...

As we're getting a stack trace, also the output of the Ansible playbook syntax check command:

$ ansible-playbook --syntax-check plays/fact-cache.yml

playbook: plays/fact-cache.yml

As we've searched your forge, a reference we best guessed for context: https://github.com/ansible/ansible-lint/pull/4216/files/1b424b5122b3158df808fcde4bdd40c40474dc49#diff-ade5ce2a08eeaf42cfeb0027865a0b7cd2c987da9af081d6604ce9d937d13bd0L146-L148

corubba commented 1 month ago

I had a quick look at this, and noticed the following: The error only occurs when the non-full-qualified task name setup is used, it works fine when using the full-qualified ansible.builtin.setup. In both cases the value of (pre #4216 used) resolved_fqcn is ansible.builtin.setup, but the value of (post #4216 used) plugin_resolved_name differs: it's setup in the non-fq case, and ansible_collections.ansible.builtin.plugins.modules.setup in the fq case.

EDIT: A very hacky/bad solution that seems to work

--- a/src/ansiblelint/rules/args.py
+++ b/src/ansiblelint/rules/args.py
@@ -146,4 +146,4 @@ class ArgsRule(AnsibleLintRule):
             spec = importlib.util.spec_from_file_location(
-                name=loaded_module.plugin_resolved_name,
+                name=loaded_module.plugin_resolved_name if '.' in loaded_module.plugin_resolved_name else loaded_module.resolved_fqcn,
                 location=loaded_module.plugin_resolved_path,
             )
CAAHS commented 1 month ago

Thanks for taking a look @corubba

We can confirm the observation that the module name (task name) is triggering it, and that using a fully qualified name is w/o the diagnostics.

This also already strengthened our first impression towards that this might be a regression when introducing plugin_resolved_name.

We can further confirm that your provided patch is w/o the diagnostic message as well for the (not fully-qualified) name.

Putting your own judgement of the patch aside for a brief moment, what do you think about adding this reports reproducer as a test case that tests for the flaw in support/context so it is already captured? This could also help to prepare a better patch in a more controlled manner. Awaiting your thoughts. @corubba

corubba commented 1 month ago

To be perfectly honest, I don't know. Someone smarter than me needs to have a look at this.