[WIP] Stop validate-modules from giving spurious E324 with suboptions

[cben]

SUMMARY

validate-modules gives spurious errors like this (example from #38725):

ERROR: lib/ansible/modules/remote_management/manageiq/manageiq_dynamic_resource_definition.py:0:0: E324 Value for "default" from the argument_spec ({'verify_ssl': True}) for "manageiq_connection" does not match the documentation (None)

This is wrong — manageiq_connection has suboptions, both doc and argspec agree tha verify_ssl suboption has default True.

The real fix would be implementing the TODO: needs to recursively traverse suboptions.
I'm not up to that, just trying to prevent the spurious error in this situation...
Suboptions will remain unchecked.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

validate-modules linter, specifically doc vs argspec default comparison implemented in #36247.

ANSIBLE VERSION

on latest devel,

ansible 2.5.0
  config file = /home/bpaskinc/.ansible.cfg
  configured module search path = [u'/home/bpaskinc/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.14 (default, Mar 14 2018, 13:36:31) [GCC 7.3.1 20180303 (Red Hat 7.3.1-5)]

ADDITIONAL INFORMATION

For above example, doc dict is:

{'author': 'Ahmed Bashir (@ahmbas)',
 'description': ['The manageiq_dynamic_resource_definition module supports adding, updating and deleting dynamic resource definitions in ManageIQ.'],
 'module': 'manageiq_dynamic_resource_definition',
 'options': {u'manageiq_connection': {u'description': [u'ManageIQ connection configuration information.'],
                                      u'required': True,
                                      u'suboptions': {u'ca_bundle_path': {u'description': [u'The path to a CA bundle file or directory with certificates. defaults to None.']},
                                                      u'password': {u'description': [u'ManageIQ password. C(MIQ_PASSWORD) env var if set. otherwise, required if no token is passed in.']},
                                                      u'token': {u'description': [u'ManageIQ token. C(MIQ_TOKEN) env var if set. otherwise, required if no username or password is passed in.']},
                                                      u'url': {u'description': [u'ManageIQ environment url. C(MIQ_URL) env var if set. otherwise, it is required to pass it.'],
                                                               u'required': True},
                                                      u'username': {u'description': [u'ManageIQ username. C(MIQ_USERNAME) env var if set. otherwise, required if no token is passed in.']},
                                                      u'verify_ssl': {u'default': True,
                                                                      u'description': [u'Whether SSL certificates should be verified for HTTPS requests. defaults to True.']}}},
             'name': {'description': ["The dynamic resource definition's name."]},
             'properties': {'description': ["The dynamic resource definition's properties."],
                            'suboptions': {'associations': {'description': 'dictionary with service and provider class.'},
                                           'attributes': {'description': 'dictionary of attributes consists of name and type.'},
                                           'methods': {'description': 'list of methods to add to dynamic resource definition.'}}},
             'state': {'choices': ['absent', 'present'],
                       'default': 'present',
                       'description': ['absent - dynamic resource definitions should not exist, present - dynamic resource definitions should be.']}},
 u'requirements': [u'manageiq-client U(https://github.com/ManageIQ/manageiq-api-client-python/)'],
 'short_description': 'Management of dynamic resource definitions in ManageIQ.',
 'version_added': '2.6'}

spec dict is:

{'manageiq_connection': {'default': {'verify_ssl': True},
                         'options': {'ca_bundle_path': {'default': None,
                                                        'required': False},
                                     'password': {'default': None,
                                                  'no_log': True},
                                     'token': {'default': None,
                                               'no_log': True},
                                     'url': {'default': None},
                                     'username': {'default': None},
                                     'verify_ssl': {'default': True,
                                                    'type': 'bool'}},
                         'type': 'dict'},
 'name': {'required': True, 'type': 'str'},
 'properties': {'type': 'dict'},
 'state': {'choices': ['absent', 'present'], 'default': 'present'}}

Note that beside each sub-option having a default key, whole 'manageiq_connection' got 'default': {'verify_ssl': True}.
If you look closely at error, this {'verify_ssl': True} is exactly what confuses it.

• find full set of ignores to delete (hope CI will help me with that).

• invstigate new error: ERROR: lib/ansible/modules/remote_management/manageiq/manageiq_provider.py:0:0: E326 Value for "choices" from the argument_spec ([]) for "api_version" does not match the documentation (['v2', 'v3'])

[ansibot]

cc @mattclay
click here for bot help

[ansibot]

The test ansible-test sanity --test validate-modules [explain] failed with 6 errors:

lib/ansible/modules/cloud/vmware/vmware_dvs_portgroup.py:0:0: E324 Value for "default" from the argument_spec (None) for "network_policy" does not match the documentation ({'promiscuous': False, 'forged_transmits': False, 'mac_changes': False})
lib/ansible/modules/cloud/vmware/vmware_dvs_portgroup.py:0:0: E324 Value for "default" from the argument_spec (None) for "port_policy" does not match the documentation ({'traffic_filter_override': False, 'network_rp_override': False, 'live_port_move': False, 'security_override': False, 'vendor_config_override': False, 'port_config_reset_at_disconnect': True, 'uplink_teaming_override': False, 'block_override': True, 'shaping_override': False, 'vlan_override': False, 'ipfix_override': False})
lib/ansible/modules/cloud/vmware/vmware_dvs_portgroup.py:0:0: E324 Value for "default" from the argument_spec (None) for "teaming_policy" does not match the documentation ({'notify_switches': True, 'load_balance_policy': 'loadbalance_srcid', 'inbound_policy': False, 'rolling_order': False})
lib/ansible/modules/cloud/vmware/vmware_portgroup.py:0:0: E324 Value for "default" from the argument_spec (None) for "network_policy" does not match the documentation ({'mac_changes': False, 'promiscuous_mode': False, 'forged_transmits': False})
lib/ansible/modules/cloud/vmware/vmware_portgroup.py:0:0: E324 Value for "default" from the argument_spec (None) for "teaming_policy" does not match the documentation ({'notify_switches': True, 'load_balance_policy': 'loadbalance_srcid', 'inbound_policy': False, 'rolling_order': False})
lib/ansible/modules/remote_management/manageiq/manageiq_provider.py:0:0: E326 Value for "choices" from the argument_spec ([]) for "api_version" does not match the documentation (['v2', 'v3'])

click here for bot help

[mattclay]

cc @sivel

[sivel]

This is unfortunately the wrong place to handle this. What has been implemented in the manageiq modules is a workaround, due to an unreported bug in subspec options handling.

There should be no need to supply a default on an option with options, where the suboptions already supply a default.

I have submitted a PR to resolve this within the options parser correctly, which would negate the need for the default workaround:

#38967

Once the above PR has been merged, I will be adding a further check to validate modules to disallow supplying default and options together.

If you have further questions please stop by IRC or the mailing list:

• IRC: #ansible on irc.freenode.net
• mailing list: https://groups.google.com/forum/#!forum/ansible-project