ansible-community / ansible.content_builder

A collection to scaffold Ansible plugins.
GNU General Public License v3.0
31 stars 26 forks source link

vmware_rest: doc-default-does-not-match-spec #69

Open mariolenz opened 10 months ago

mariolenz commented 10 months ago
SUMMARY
ERROR: plugins/modules/vcenter_vm_hardware_adapter_sata.py:0:0: doc-default-does-not-match-spec: Argument 'bus' in argument_spec defines default as (0) but documentation defines default as (None)
ERROR: plugins/modules/vcenter_vm_hardware_adapter_scsi.py:0:0: doc-default-does-not-match-spec: Argument 'bus' in argument_spec defines default as (0) but documentation defines default as (None)

The default here is 0, which looks like you're somewhere testing if default:. And since 0 is false, the default is not documented.

I don't know your code at all, but this already looks a little bit suspicious:

https://github.com/ansible-community/ansible.content_builder/blob/1e706b48b7a1d825d464b697c43355f77cb24fb4/plugins/action/generate_cloud_modules.py#L322-L323

Might be the wrong place in the code, but something like this would explain the missing documentation. Maybe better test on is not None or so.

I might be completely wrong, but I definitively see this issue in the sanity tests.

ISSUE TYPE
ADDITIONAL INFORMATION

ansible-collections/vmware.vmware_rest#432 ansible-collections/vmware.vmware_rest#434

mariolenz commented 10 months ago

https://github.com/ansible-community/ansible.content_builder/blob/1e706b48b7a1d825d464b697c43355f77cb24fb4/plugins/action/generate_cloud_modules.py#L322-L323

I don't think this is the problem, or at lease not the only one. I've tried to change it to if parameter.get("default") is not None but this didn't help :-/

One other thing that I've found and looks fishy to me:

https://github.com/ansible-community/ansible.content_builder/blob/1a8071e914aa1f25a84670b320c2295736a00818/plugins/action/generate_cloud_modules.py#L405-L410

Having "special cases" like this is an accident waiting to happen, imho.

I'll try to investigate a little bit more, maybe I'm able to find the root cause.

mariolenz commented 9 months ago

I think this is the problem:

https://github.com/ansible-community/ansible.content_builder/blob/1a8071e914aa1f25a84670b320c2295736a00818/plugins/action/generate_cloud_modules.py#L405-L407

It injects a default that isn't known anywhere else in the code or in the spec, at least as far as I can see. I think generating the documentation is correct, there is no default for bus. It's tampering with the argument_spec here that leads to the problem.

I don't know why there is this default or if it's really needed. But it looks like it's been in vmware_rest_code_generator since the very first commit:

https://github.com/ansible-collections/vmware_rest_code_generator/blob/d58dab46fa97345ce429324280c4d2af03edf6c9/refresh_modules.py#L129-L131