vultr / ansible-collection-vultr

GNU General Public License v3.0
28 stars 17 forks source link

[BUG] - Useless shebangs #81

Closed aueam closed 1 year ago

aueam commented 1 year ago

Describe the bug

The listed files shouldn't be runnable with python because they have relative imports. But thanks to the shebangs they are runnable, which is wrong because it always throws an error.

listed files:

ansible-collection-vultr/plugins/inventory/vultr.py
ansible-collection-vultr/plugins/modules/account_info.py
ansible-collection-vultr/plugins/modules/block_storage.py
ansible-collection-vultr/plugins/modules/block_storage_info.py
ansible-collection-vultr/plugins/modules/dns_domain.py
ansible-collection-vultr/plugins/modules/dns_domain_info.py
ansible-collection-vultr/plugins/modules/dns_record.py
ansible-collection-vultr/plugins/modules/firewall_group.py
ansible-collection-vultr/plugins/modules/firewall_group_info.py
ansible-collection-vultr/plugins/modules/firewall_rule.py
ansible-collection-vultr/plugins/modules/firewall_rule_info.py
ansible-collection-vultr/plugins/modules/instance.py
ansible-collection-vultr/plugins/modules/instance_info.py
ansible-collection-vultr/plugins/modules/os_info.py
ansible-collection-vultr/plugins/modules/plan_info.py
ansible-collection-vultr/plugins/modules/plan_metal_info.py
ansible-collection-vultr/plugins/modules/region_info.py
ansible-collection-vultr/plugins/modules/reserved_ip.py
ansible-collection-vultr/plugins/modules/snapshot.py
ansible-collection-vultr/plugins/modules/snapshot_info.py
ansible-collection-vultr/plugins/modules/ssh_key.py
ansible-collection-vultr/plugins/modules/ssh_key_info.py
ansible-collection-vultr/plugins/modules/startup_script.py
ansible-collection-vultr/plugins/modules/startup_script_info.py
ansible-collection-vultr/plugins/modules/user.py
ansible-collection-vultr/plugins/modules/user_info.py
ansible-collection-vultr/plugins/modules/vpc.py
ansible-collection-vultr/plugins/modules/vpc_info.py

To Reproduce Steps to reproduce the behavior:

  1. download repo and make file executable:
    
    git clone https://github.com/aueam/ansible-collection-vultr.git
    cd ansible-collection-vultr/plugins/inventory/

chmod +x vultr.py

2. run file
```bash
./vultr.py
  1. See an error similar to this:
    Traceback (most recent call last):
    File "/tmp/ansible-collection-vultr/plugins/inventory/./vultr.py", line 160, in <module>
    from ..module_utils.vultr_v2 import VULTR_USER_AGENT
    ImportError: attempted relative import with no known parent package
  2. repeat these steps for every listed file

Expected behavior I expected them not to be runnable.

Desktop (please complete the following information where applicable:

resmo commented 1 year ago

Thanks @aueam for your contribution,

The shebang is a requirement of Ansible and how Ansible work, the shebang in modules is not useless, even though they are not executable (and were nevert meant to be executable as is).

Ansible does some magic packaging before they get transferred to the target and executed, including some module utils which you see in the modules. (AFAIR the shebang will be replaced to whatever ansible_python_interpreter points to)

Read more about the requirement https://docs.ansible.com/ansible/latest/dev_guide/developing_modules_documenting.html#python-shebang-utf-8-coding

mtelka commented 1 year ago

Interesting, @resmo. I believe the ansible_python_interpreter logic simply replaces the #!/usr/bin/python shebang by whatever is Python on the target machine (for example #!/usr/bin/python3) and then the particular file is executed (if it is meant to be executed). In this particular case I believe these files are not executed, but instead imported as Python modules.

OTOH, it is easily possible that I'm wrong here, so please could you show us what ansible does (or could do) with those files to make them executable by utilizing their shebang?

Thank you.

mtelka commented 1 year ago

In addition, for the plugins/inventory/vultr.py file the current shebang does not follow the rule you are referring to :-).

resmo commented 1 year ago

indeed, ironically there shouldn't be a shebang in an inventory plugin because it's executed on the controller.

mtelka commented 1 year ago

I found this: https://docs.ansible.com/ansible/latest/dev_guide/testing/sanity/shebang.html

So it looks like ansible really requires shebang in modules even they are not executable.

I think this bug report could be closed.