search

CiscoDevNet / ansible-mso

Cisco MSO Ansible Collection
https://galaxy.ansible.com/cisco/mso
GNU General Public License v3.0
26 stars 32 forks source link

mso_dhcp_option_policy_option module has distinct options/aliases which are equal #489

Closed felixfontein closed 4 months ago

felixfontein commented 4 months ago

Description

The mso_dhcp_option_policy_option module's dhcp_option_policy option has the alias name, which collides with the option name. This looks like an unintentional bug to me.

Ref: https://github.com/ansible/ansible/pull/83530

Affected Module Name(s):

mso_dhcp_option_policy_option

akinross commented 4 months ago

Hi @felixfontein,

Thank you for making us aware, I have moved it to the top of the todo.

lhercot commented 4 months ago

Thanks for the report.

The alias was only defined in the documentation section and not actually defined in the argument_spec. I thought there was a check that both were aligned but it must be only checking the other way around (alias in spec and not in documentation).

@felixfontein would it be possible to also add the reverse check in sanity?

lhercot commented 4 months ago

We have merged the fix and released v2.8.0 which include this fix.

felixfontein commented 4 months ago

Thanks for fixing this!

The alias was only defined in the documentation section and not actually defined in the argument_spec. I thought there was a check that both were aligned but it must be only checking the other way around (alias in spec and not in documentation).

@felixfontein would it be possible to also add the reverse check in sanity?

That's interesting, I thought that the sanity tests already ensure this.

On the other hand, while thinking a bit longer about this, the reason why the tests didn't flag it so far is that they allow the documentation to use one of the aliases instead of the option name (and describe the real option name as an alias). Or at least they did in the past.

In any case, I'll take a closer look. They definitely should make sure that the set of option name + aliases is the same for documentation and argument spec.

felixfontein commented 4 months ago

While looking into this I found another bug in validate-modules, it won't notice if you add aliases to the wrong option in documentation: https://github.com/ansible/ansible/issues/83598

This together with the matching functionality is basically what happened in your case: the test checked that name was there somehow in documentation, but it didn't notice it was the documentation of another option.