ansible-collections / community.general

Ansible Community General Collection
https://galaxy.ansible.com/ui/repo/published/community/general/
GNU General Public License v3.0
825 stars 1.53k forks source link

manageiq_provider: shortcomings in docs and arg specs #5366

Open russoz opened 2 years ago

russoz commented 2 years ago

Summary

The module manageiq_provider has a number of sanity check errors related to undocumented options in the argument spec. This ticket should register the work towards solving them.

Issue Type

Bug Report

Component Name

plugins/modules/remote_management/manageiq/manageiq_provider.py

Ansible Version

$ ansible --version
ansible [core 2.13.4]
  config file = None
  configured module search path = ['/home/az/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/az/.local/share/virtualenvs/ansible-collections__community.general-TVemac3D/lib/python3.9/site-packages/ansible
  ansible collection location = /home/az/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/az/.local/share/virtualenvs/ansible-collections__community.general-TVemac3D/bin/ansible
  python version = 3.9.5 (default, Nov 23 2021, 15:27:38) [GCC 9.3.0]
  jinja version = 3.1.2
  libyaml = True

(but not really relevant to this case)

Community.general Version

main branch (development)

Configuration

$ ansible-config dump --only-changed

(empty)

OS / Environment

Linux Mint (irrelevant to this case)

Steps to Reproduce

Assuming andebox is installed:

$ pip install andebox
$ andebox test -ei -- sanity --docker default plugins/modules/remote_management/manageiq/manageiq_provider.py

The -ei flag will exclude the lines in the sanity ignore files matching the name of the files listed, in this case only manageiq_provider.py.

Expected Results

All tests return OK.

Actual Results

Multiple validation errors:

Running sanity test "validate-modules"
ERROR: Found 51 validate-modules issue(s) which need to be resolved:
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-choices-do-not-match-spec: Argument 'security_protocol' in argument_spec found in ssh_keypair defines choices as (['ssl-with-validation', 'ssl-with-validation-custom-ca', 'ssl-without-validation', 'non-ssl']) but documentation defines choices as ([])
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'certificate_authority' in argument_spec found in ssh_keypair uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'password' in argument_spec found in ssh_keypair uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'path' in argument_spec found in alerts uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'path' in argument_spec found in provider uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'path' in argument_spec found in ssh_keypair uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'project' in argument_spec found in alerts uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'project' in argument_spec found in metrics uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'project' in argument_spec found in provider uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'project' in argument_spec found in ssh_keypair uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'role' in argument_spec found in alerts uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'role' in argument_spec found in metrics uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'role' in argument_spec found in provider uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'role' in argument_spec found in ssh_keypair uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'security_protocol' in argument_spec found in ssh_keypair uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'subscription' in argument_spec found in alerts uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'subscription' in argument_spec found in metrics uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'subscription' in argument_spec found in provider uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'subscription' in argument_spec found in ssh_keypair uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'uid_ems' in argument_spec found in alerts uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'uid_ems' in argument_spec found in metrics uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'uid_ems' in argument_spec found in provider uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: doc-missing-type: Argument 'uid_ems' in argument_spec found in ssh_keypair uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: parameter-type-not-in-doc: Argument 'alerts' in argument_spec defines type as 'dict' but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: parameter-type-not-in-doc: Argument 'metrics' in argument_spec defines type as 'dict' but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: parameter-type-not-in-doc: Argument 'port' in argument_spec found in ssh_keypair defines type as 'int' but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: parameter-type-not-in-doc: Argument 'provider' in argument_spec defines type as 'dict' but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: parameter-type-not-in-doc: Argument 'ssh_keypair' in argument_spec defines type as 'dict' but documentation doesn't define type
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'certificate_authority' found in ssh_keypair is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'password' found in ssh_keypair is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'path' found in alerts is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'path' found in provider is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'path' found in ssh_keypair is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'port' found in ssh_keypair is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'project' found in alerts is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'project' found in metrics is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'project' found in provider is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'project' found in ssh_keypair is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'role' found in alerts is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'role' found in metrics is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'role' found in provider is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'role' found in ssh_keypair is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'security_protocol' found in ssh_keypair is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'subscription' found in alerts is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'subscription' found in metrics is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'subscription' found in provider is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'subscription' found in ssh_keypair is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'uid_ems' found in alerts is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'uid_ems' found in metrics is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'uid_ems' found in provider is listed in the argument_spec, but not documented in the module documentation
ERROR: plugins/modules/remote_management/manageiq/manageiq_provider.py:0:0: undocumented-parameter: Argument 'uid_ems' found in ssh_keypair is listed in the argument_spec, but not documented in the module documentation

Code of Conduct

ansibullbot commented 2 years ago

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot commented 2 years ago

cc @abellotti @cben @dkorn @evertmulder @gtanzillo @yaacov @zgalor click here for bot help

russoz commented 2 years ago

Twisting the output of the tests a bit (lousily and lazily so):

$ andebox test -ei -- sanity --docker default --test validate-modules plugins/modules/remote_management/manageiq/manageiq_provider.py \
    | grep -oP '\S+ in argument_spec found in \w+' | sed -e "s/'//g" -e 's/ in argument_spec found in /:/g' \
    | awk -F: '{ print $2":"$1 }' | sort -u
(...)
alerts:path
alerts:project
alerts:role
alerts:subscription
alerts:uid_ems
metrics:project
metrics:role
metrics:subscription
metrics:uid_ems
provider:path
provider:project
provider:role
provider:subscription
provider:uid_ems
ssh_keypair:certificate_authority
ssh_keypair:password
ssh_keypair:path
ssh_keypair:port
ssh_keypair:project
ssh_keypair:role
ssh_keypair:security_protocol
ssh_keypair:subscription
ssh_keypair:uid_ems

Those are some of the offending pairs options:suboptions, potentially not for all the 51 errors, but this should cover most of them to get started.

russoz commented 2 years ago

The argument_spec for these parameters is being built exactly the same in: https://github.com/ansible-collections/community.general/blob/main/plugins/modules/remote_management/manageiq/manageiq_provider.py#L571-L577

def endpoint_list_spec():
    return dict(
        provider=dict(type='dict', options=endpoint_argument_spec()),
        metrics=dict(type='dict', options=endpoint_argument_spec()),
        alerts=dict(type='dict', options=endpoint_argument_spec()),
        ssh_keypair=dict(type='dict', options=endpoint_argument_spec(), no_log=False),
    )
russoz commented 2 years ago

However the docs for each one of the four is completely different: https://github.com/ansible-collections/community.general/blob/main/plugins/modules/remote_management/manageiq/manageiq_provider.py#L69-L186

Condensed version of the docs, showing only the top level option's description and the suboptions:

  provider:
    description: Default endpoint connection information, required if state is true.
    suboptions:
      hostname:
      port:
      userid:
      password:
      auth_key:
      validate_certs:
      security_protocol:
      certificate_authority:
  metrics:
    description: Metrics endpoint connection information.
    suboptions:
      hostname:
      port:
      userid:
      password:
      auth_key:
      validate_certs:
      security_protocol:
      certificate_authority:
      path:
  alerts:
    description: Alerts endpoint connection information.
    suboptions:
      hostname:
      port:
      userid:
      password:
      auth_key:
      validate_certs:
      security_protocol:
      certificate_authority:
  ssh_keypair:
    description: SSH key pair used for SSH connections to all hosts in this provider.
    suboptions:
      hostname:
      userid:
      auth_key:
      validate_certs:
russoz commented 2 years ago

Given that:

def endpoint_argument_spec():
    return dict(
        role=dict(),
        hostname=dict(required=True),
        port=dict(type='int'),
        validate_certs=dict(default=True, type='bool', aliases=['verify_ssl']),
        certificate_authority=dict(),
        security_protocol=dict(
            choices=[
                'ssl-with-validation',
                'ssl-with-validation-custom-ca',
                'ssl-without-validation',
                'non-ssl',
            ],
        ),
        userid=dict(),
        password=dict(no_log=True),
        auth_key=dict(no_log=True),
        subscription=dict(no_log=True),
        project=dict(),
        uid_ems=dict(),
        path=dict(),
    )

I suppose the problem is that each endpoint in the actual service has a different set of suboptions. The documentation blob could be right, in the sense that it matches the service API, but I cannot find an authoritative reference for these endpoint types (other than looking into ManageIQ code, which I am trying to avoid as my Ruby-fu does not go very far). I mean, I suppose SSL-specific options do not apply to the ssh_keypair endpoint, but I would feel more comfortable having the actual list so that we can ensure the suboptions provided in the module match the service API.

If this appraisal is right, then it seems to me that we should make four separate versions of the endpoint_argument_spec() function, one for each endpoint type.

russoz commented 2 years ago

For the record, some docs were found about the API:

But although they thoroughly describe the interface at the providers level, when it comes down to the endpoints parameters, it is vague - it provides "examples", but there is no authoritative description of the parameters.

russoz commented 2 years ago

Another possibility is that the API accepts all the suboptions, but depending on the role (if I got the terminology right), it will use only some of the parameters.

Fryguy commented 2 years ago

cc @agrare . Yes, different providers take a different set of options, so they are all different. I don't believe we have documentation for each one individually. However, new provider plugins are created somewhat regularly, so would we need to add a new spec here every time?

russoz commented 2 years ago

Either that or we move to a "generic" provider model.

Any chance we can do some gitspelunking on the existing ones? I have tried to do it myself but Ruby was never a forte for me.

ansibullbot commented 2 years ago

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help