validate-modules for plugins: do some more schema validations so that some issues that are currently reported for modules are also reported for plugins (#77268)

* Add more sanity tests on schema level (so they also work for plugins).

* Fix various issues the sanity test reported.

* Add changelog fragment.

* Fix function name.
pull/77320/head
Felix Fontein 4 years ago committed by GitHub
parent e3c72230cd
commit 4d984613f5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -0,0 +1,2 @@
minor_changes:
- "ansible-test validate-modules sanity test - add more schema checks to improve quality of plugin documentation (https://github.com/ansible/ansible/pull/77268)."

@ -28,6 +28,7 @@ options:
description: description:
- The groups to add the hostname to. - The groups to add the hostname to.
type: list type: list
elements: str
aliases: [ group, groupname ] aliases: [ group, groupname ]
extends_documentation_fragment: extends_documentation_fragment:
- action_common_attributes - action_common_attributes

@ -20,6 +20,7 @@ options:
description: description:
- A list of string expressions of the same form that can be passed to the 'when' statement. - A list of string expressions of the same form that can be passed to the 'when' statement.
type: list type: list
elements: str
required: true required: true
fail_msg: fail_msg:
description: description:

@ -31,6 +31,7 @@ options:
description: description:
- The list of the parent groups. - The list of the parent groups.
type: list type: list
elements: str
default: all default: all
version_added: "2.4" version_added: "2.4"
attributes: attributes:

@ -54,11 +54,13 @@ options:
description: description:
- List of file names to ignore. - List of file names to ignore.
type: list type: list
elements: str
version_added: "2.2" version_added: "2.2"
extensions: extensions:
description: description:
- List of file extensions to read when using C(dir). - List of file extensions to read when using C(dir).
type: list type: list
elements: str
default: [ json, yaml, yml ] default: [ json, yaml, yml ]
version_added: "2.3" version_added: "2.3"
ignore_unknown_extensions: ignore_unknown_extensions:

@ -59,6 +59,7 @@ options:
- Paths to search on the remote machine for the C(shutdown) command. - Paths to search on the remote machine for the C(shutdown) command.
- I(Only) these paths will be searched for the C(shutdown) command. C(PATH) is ignored in the remote node when searching for the C(shutdown) command. - I(Only) these paths will be searched for the C(shutdown) command. C(PATH) is ignored in the remote node when searching for the C(shutdown) command.
type: list type: list
elements: str
default: ['/sbin', '/bin', '/usr/sbin', '/usr/bin', '/usr/local/sbin'] default: ['/sbin', '/bin', '/usr/sbin', '/usr/bin', '/usr/local/sbin']
version_added: '2.8' version_added: '2.8'

@ -146,8 +146,8 @@ options:
ssh_key_bits: ssh_key_bits:
description: description:
- Optionally specify number of bits in SSH key to create. - Optionally specify number of bits in SSH key to create.
- The default value depends on ssh-keygen.
type: int type: int
default: default set by ssh-keygen
version_added: "0.9" version_added: "0.9"
ssh_key_type: ssh_key_type:
description: description:

@ -34,7 +34,7 @@ options:
(bytes/sec) then this option is ignored. Default is C(0) (no bandwidth (bytes/sec) then this option is ignored. Default is C(0) (no bandwidth
throttling). throttling).
type: str type: str
default: 0 default: '0'
baseurl: baseurl:
description: description:
- URL to the directory where the yum repository's 'repodata' directory - URL to the directory where the yum repository's 'repodata' directory
@ -49,7 +49,7 @@ options:
- Relative cost of accessing this repository. Useful for weighing one - Relative cost of accessing this repository. Useful for weighing one
repo's packages as greater/less than any other. repo's packages as greater/less than any other.
type: str type: str
default: 1000 default: '1000'
deltarpm_metadata_percentage: deltarpm_metadata_percentage:
description: description:
- When the relative size of deltarpm metadata vs pkgs is larger than - When the relative size of deltarpm metadata vs pkgs is larger than
@ -58,14 +58,14 @@ options:
required to be half the size of the packages. Use C(0) to turn off required to be half the size of the packages. Use C(0) to turn off
this check, and always download metadata. this check, and always download metadata.
type: str type: str
default: 100 default: '100'
deltarpm_percentage: deltarpm_percentage:
description: description:
- When the relative size of delta vs pkg is larger than this, delta is - When the relative size of delta vs pkg is larger than this, delta is
not used. Use C(0) to turn off delta rpm processing. Local repositories not used. Use C(0) to turn off delta rpm processing. Local repositories
(with file:// I(baseurl)) have delta rpms turned off by default. (with file:// I(baseurl)) have delta rpms turned off by default.
type: str type: str
default: 75 default: '75'
description: description:
description: description:
- A human readable string describing the repository. This option corresponds to the "name" property in the repo file. - A human readable string describing the repository. This option corresponds to the "name" property in the repo file.
@ -160,7 +160,7 @@ options:
- Determines how yum resolves host names. - Determines how yum resolves host names.
- C(4) or C(IPv4) - resolve to IPv4 addresses only. - C(4) or C(IPv4) - resolve to IPv4 addresses only.
- C(6) or C(IPv6) - resolve to IPv6 addresses only. - C(6) or C(IPv6) - resolve to IPv6 addresses only.
choices: [4, 6, IPv4, IPv6, whatever] choices: ['4', '6', IPv4, IPv6, whatever]
type: str type: str
default: whatever default: whatever
keepalive: keepalive:
@ -182,7 +182,7 @@ options:
- Time (in seconds) after which the metadata will expire. - Time (in seconds) after which the metadata will expire.
- Default value is 6 hours. - Default value is 6 hours.
type: str type: str
default: 21600 default: '21600'
metadata_expire_filter: metadata_expire_filter:
description: description:
- Filter the I(metadata_expire) time, allowing a trade of speed for - Filter the I(metadata_expire) time, allowing a trade of speed for
@ -224,7 +224,7 @@ options:
expire. expire.
- Default value is 6 hours. - Default value is 6 hours.
type: str type: str
default: 21600 default: '21600'
name: name:
description: description:
- Unique repository ID. This option builds the section name of the repository in the repo file. - Unique repository ID. This option builds the section name of the repository in the repo file.
@ -242,7 +242,7 @@ options:
from 1 to 99. from 1 to 99.
- This option only works if the YUM Priorities plugin is installed. - This option only works if the YUM Priorities plugin is installed.
type: str type: str
default: 99 default: '99'
protect: protect:
description: description:
- Protect packages from updates from other repositories. - Protect packages from updates from other repositories.
@ -277,7 +277,7 @@ options:
- Set the number of times any attempt to retrieve a file should retry - Set the number of times any attempt to retrieve a file should retry
before returning an error. Setting this to C(0) makes yum try forever. before returning an error. Setting this to C(0) makes yum try forever.
type: str type: str
default: 10 default: '10'
s3_enabled: s3_enabled:
description: description:
- Enables support for S3 repositories. - Enables support for S3 repositories.
@ -341,7 +341,7 @@ options:
description: description:
- Number of seconds to wait for a connection before timing out. - Number of seconds to wait for a connection before timing out.
type: str type: str
default: 30 default: '30'
ui_repoid_vars: ui_repoid_vars:
description: description:
- When a repository id is displayed, append these yum variables to the - When a repository id is displayed, append these yum variables to the

@ -81,6 +81,7 @@ DOCUMENTATION = """
if you add another one in your string, your prompt will fail with a "Timeout" error. if you add another one in your string, your prompt will fail with a "Timeout" error.
default: [] default: []
type: list type: list
elements: string
ini: ini:
- section: su_become_plugin - section: su_become_plugin
key: localized_prompts key: localized_prompts

@ -78,6 +78,7 @@ DOCUMENTATION = """
- If None (the default) the plugin will try to automatically guess the correct list - If None (the default) the plugin will try to automatically guess the correct list
- The choices available depend on your version of pywinrm - The choices available depend on your version of pywinrm
type: list type: list
elements: string
vars: vars:
- name: ansible_winrm_transport - name: ansible_winrm_transport
kerberos_command: kerberos_command:

@ -43,6 +43,7 @@ options:
the first one from the list will be used instead. the first one from the list will be used instead.
default: [ /var/tmp, /tmp ] default: [ /var/tmp, /tmp ]
type: list type: list
elements: string
env: [{name: ANSIBLE_SYSTEM_TMPDIRS}] env: [{name: ANSIBLE_SYSTEM_TMPDIRS}]
ini: ini:
- section: defaults - section: defaults
@ -61,6 +62,7 @@ options:
- name: ansible_async_dir - name: ansible_async_dir
environment: environment:
type: list type: list
elements: dictionary
default: [{}] default: [{}]
description: description:
- List of dictionaries of environment variables and their values to use when executing commands. - List of dictionaries of environment variables and their values to use when executing commands.
@ -68,6 +70,7 @@ options:
- name: environment - name: environment
admin_users: admin_users:
type: list type: list
elements: string
default: ['root', 'toor'] default: ['root', 'toor']
description: description:
- list of users to be expected to have admin privileges. This is used by the controller to - list of users to be expected to have admin privileges. This is used by the controller to

@ -46,5 +46,6 @@ options:
keyword: keyword:
- name: environment - name: environment
type: list type: list
elements: dictionary
default: [{}] default: [{}]
""" """

@ -21,6 +21,7 @@ DOCUMENTATION = '''
yaml_extensions: yaml_extensions:
description: list of 'valid' extensions for files containing YAML description: list of 'valid' extensions for files containing YAML
type: list type: list
elements: string
default: ['.yaml', '.yml', '.json'] default: ['.yaml', '.yml', '.json']
env: env:
- name: ANSIBLE_YAML_FILENAME_EXT - name: ANSIBLE_YAML_FILENAME_EXT

@ -24,10 +24,12 @@ DOCUMENTATION = """
files: files:
description: A list of file names. description: A list of file names.
type: list type: list
elements: string
default: [] default: []
paths: paths:
description: A list of paths in which to look for the files. description: A list of paths in which to look for the files.
type: list type: list
elements: string
default: [] default: []
skip: skip:
type: boolean type: boolean

@ -138,6 +138,7 @@ options:
unredirected_headers: unredirected_headers:
description: A list of headers to not attach on a redirected request description: A list of headers to not attach on a redirected request
type: list type: list
elements: string
version_added: "2.10" version_added: "2.10"
vars: vars:
- name: ansible_lookup_url_unredir_headers - name: ansible_lookup_url_unredir_headers

@ -48,6 +48,7 @@ DOCUMENTATION = '''
- key: yaml_valid_extensions - key: yaml_valid_extensions
section: defaults section: defaults
type: list type: list
elements: string
extends_documentation_fragment: extends_documentation_fragment:
- vars_plugin_staging - vars_plugin_staging
''' '''

@ -1927,16 +1927,6 @@ class ModuleValidator(Validator):
doc_elements = doc_options_arg.get('elements', None) doc_elements = doc_options_arg.get('elements', None)
doc_type = doc_options_arg.get('type', 'str') doc_type = doc_options_arg.get('type', 'str')
data_elements = data.get('elements', None) data_elements = data.get('elements', None)
if (doc_elements and not doc_type == 'list'):
msg = "Argument '%s' " % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " defines parameter elements as %s but it is valid only when value of parameter type is list" % doc_elements
self.reporter.error(
path=self.object_path,
code='doc-elements-invalid',
msg=msg
)
if (doc_elements or data_elements) and not (doc_elements == data_elements): if (doc_elements or data_elements) and not (doc_elements == data_elements):
msg = "Argument '%s' in argument_spec" % arg msg = "Argument '%s' in argument_spec" % arg
if context: if context:

@ -14,6 +14,8 @@ from urllib.parse import urlparse
from voluptuous import ALLOW_EXTRA, PREVENT_EXTRA, All, Any, Invalid, Length, Required, Schema, Self, ValueInvalid, Exclusive from voluptuous import ALLOW_EXTRA, PREVENT_EXTRA, All, Any, Invalid, Length, Required, Schema, Self, ValueInvalid, Exclusive
from ansible.module_utils.six import string_types from ansible.module_utils.six import string_types
from ansible.module_utils.common.collections import is_iterable from ansible.module_utils.common.collections import is_iterable
from ansible.module_utils.parsing.convert_bool import boolean
from ansible.parsing.quoting import unquote
from ansible.utils.version import SemanticVersion from ansible.utils.version import SemanticVersion
from ansible.release import __version__ from ansible.release import __version__
@ -384,6 +386,134 @@ def version_added(v, error_code='version-added-invalid', accept_historical=False
return v return v
def check_option_elements(v):
# Check whether elements is there iff type == 'list'
v_type = v.get('type')
v_elements = v.get('elements')
if v_type == 'list' and v_elements is None:
raise _add_ansible_error_code(
Invalid('Argument defines type as list but elements is not defined'),
error_code='parameter-list-no-elements') # FIXME: adjust error code?
if v_type != 'list' and v_elements is not None:
raise _add_ansible_error_code(
Invalid('Argument defines parameter elements as %s but it is valid only when value of parameter type is list' % (v_elements, )),
error_code='doc-elements-invalid')
return v
def get_type_checker(v):
v_type = v.get('type')
if v_type == 'list':
elt_checker, elt_name = get_type_checker({'type': v.get('elements')})
def list_checker(value):
if isinstance(value, string_types):
value = [unquote(x.strip()) for x in value.split(',')]
if not isinstance(value, list):
raise ValueError('Value must be a list')
if elt_checker:
for elt in value:
try:
elt_checker(elt)
except Exception as exc:
raise ValueError('Entry %r is not of type %s: %s' % (elt, elt_name, exc))
return list_checker, ('list of %s' % elt_name) if elt_checker else 'list'
if v_type in ('boolean', 'bool'):
return partial(boolean, strict=False), v_type
if v_type in ('integer', 'int'):
return int, v_type
if v_type == 'float':
return float, v_type
if v_type == 'none':
def none_checker(value):
if value not in ('None', None):
raise ValueError('Value must be "None" or none')
return none_checker, v_type
if v_type in ('str', 'string', 'path', 'tmp', 'temppath', 'tmppath'):
def str_checker(value):
if not isinstance(value, string_types):
raise ValueError('Value must be string')
return str_checker, v_type
if v_type in ('pathspec', 'pathlist'):
def path_list_checker(value):
if not isinstance(value, string_types) and not is_iterable(value):
raise ValueError('Value must be string or list of strings')
return path_list_checker, v_type
if v_type in ('dict', 'dictionary'):
def dict_checker(value):
if not isinstance(value, dict):
raise ValueError('Value must be dictionary')
return dict_checker, v_type
return None, 'unknown'
def check_option_choices(v):
# Check whether choices have the correct type
v_choices = v.get('choices')
if not is_iterable(v_choices):
return v
if v.get('type') == 'list':
# choices for a list type means that every list element must be one of these choices
type_checker, type_name = get_type_checker({'type': v.get('elements')})
else:
type_checker, type_name = get_type_checker(v)
if type_checker is None:
return v
for value in v_choices:
try:
type_checker(value)
except Exception as exc:
raise _add_ansible_error_code(
Invalid(
'Argument defines choices as (%r) but this is incompatible with argument type %s: %s' % (value, type_name, exc)),
error_code='doc-choices-incompatible-type')
return v
def check_option_default(v):
# Check whether default is only present if required=False, and whether default has correct type
v_default = v.get('default')
if v.get('required') and v_default is not None:
raise _add_ansible_error_code(
Invalid(
'Argument is marked as required but specifies a default.'
' Arguments with a default should not be marked as required'),
error_code='no-default-for-required-parameter') # FIXME: adjust error code?
if v_default is None:
return v
type_checker, type_name = get_type_checker(v)
if type_checker is None:
return v
try:
type_checker(v_default)
except Exception as exc:
raise _add_ansible_error_code(
Invalid(
'Argument defines default as (%r) but this is incompatible with parameter type %s: %s' % (v_default, type_name, exc)),
error_code='incompatible-default-type')
return v
def list_dict_option_schema(for_collection, plugin_type): def list_dict_option_schema(for_collection, plugin_type):
if plugin_type == 'module': if plugin_type == 'module':
option_types = Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'sid', 'str') option_types = Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'sid', 'str')
@ -433,7 +563,7 @@ def list_dict_option_schema(for_collection, plugin_type):
partial(check_removal_version, partial(check_removal_version,
version_field='version', version_field='version',
collection_name_field='collection_name', collection_name_field='collection_name',
error_code='invalid-removal-version') error_code='invalid-removal-version'),
) )
env_schema = All( env_schema = All(
Schema({ Schema({
@ -496,7 +626,12 @@ def list_dict_option_schema(for_collection, plugin_type):
# Recursive suboptions # Recursive suboptions
'suboptions': Any(None, *list({str_type: Self} for str_type in string_types)), 'suboptions': Any(None, *list({str_type: Self} for str_type in string_types)),
}) })
suboption_schema = Schema(suboption_schema, extra=PREVENT_EXTRA) suboption_schema = Schema(All(
suboption_schema,
check_option_elements,
check_option_choices,
check_option_default,
), extra=PREVENT_EXTRA)
# This generates list of dicts with keys from string_types and suboption_schema value # This generates list of dicts with keys from string_types and suboption_schema value
# for example in Python 3: {str: suboption_schema} # for example in Python 3: {str: suboption_schema}
@ -506,7 +641,12 @@ def list_dict_option_schema(for_collection, plugin_type):
option_schema.update({ option_schema.update({
'suboptions': Any(None, *list_dict_suboption_schema), 'suboptions': Any(None, *list_dict_suboption_schema),
}) })
option_schema = Schema(option_schema, extra=PREVENT_EXTRA) option_schema = Schema(All(
option_schema,
check_option_elements,
check_option_choices,
check_option_default,
), extra=PREVENT_EXTRA)
option_version_added = Schema( option_version_added = Schema(
All({ All({

@ -96,7 +96,6 @@ lib/ansible/modules/systemd.py validate-modules:return-syntax-error
lib/ansible/modules/sysvinit.py validate-modules:return-syntax-error lib/ansible/modules/sysvinit.py validate-modules:return-syntax-error
lib/ansible/modules/uri.py validate-modules:doc-required-mismatch lib/ansible/modules/uri.py validate-modules:doc-required-mismatch
lib/ansible/modules/user.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/user.py validate-modules:doc-default-does-not-match-spec
lib/ansible/modules/user.py validate-modules:doc-default-incompatible-type
lib/ansible/modules/user.py validate-modules:use-run-command-not-popen lib/ansible/modules/user.py validate-modules:use-run-command-not-popen
lib/ansible/modules/yum.py pylint:disallowed-name lib/ansible/modules/yum.py pylint:disallowed-name
lib/ansible/modules/yum.py validate-modules:parameter-invalid lib/ansible/modules/yum.py validate-modules:parameter-invalid

Loading…
Cancel
Save