validate-modules: fix version_added validation for top-level, fix error codes (#70869)

* Also validate top-level version_added.

* Fix error code.

* Produce same version_added validation error in schema than in code (and stop returning it twice).

* Return correct error codes for invalid version_added for options and return values.

* Add changelog.

* Fix forgotten closing braket.

* Accept 'historical' for some top-level version_added.
This commit is contained in:
Felix Fontein 2020-07-28 17:10:35 +02:00 committed by GitHub
parent 45c2eb6c0a
commit 7e2cc7db12
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 32 additions and 24 deletions

View file

@ -0,0 +1,3 @@
bugfixes:
- "ansible-test validate-modules - return correct error codes ``option-invalid-version-added`` resp. ``return-invalid-version-added`` instead of the wrong error ``deprecation-either-date-or-version`` when an invalid value of ``version_added`` is specified for an option or a return value (https://github.com/ansible/ansible/pull/70869)."
- "ansible-test validate-modules - ``version_added`` on module level was not validated for modules in collections (https://github.com/ansible/ansible/pull/70869)."

View file

@ -120,7 +120,7 @@ Codes
no-default-for-required-parameter Documentation Error Option is marked as required but specifies a default. Arguments with a default should not be marked as required no-default-for-required-parameter Documentation Error Option is marked as required but specifies a default. Arguments with a default should not be marked as required
nonexistent-parameter-documented Documentation Error Argument is listed in DOCUMENTATION.options, but not accepted by the module nonexistent-parameter-documented Documentation Error Argument is listed in DOCUMENTATION.options, but not accepted by the module
option-incorrect-version-added Documentation Error ``version_added`` for new option is incorrect option-incorrect-version-added Documentation Error ``version_added`` for new option is incorrect
option-invalid-version-added Documentation Error ``version_added`` for new option is not a valid version number option-invalid-version-added Documentation Error ``version_added`` for option is not a valid version number
parameter-invalid Documentation Error Argument in argument_spec is not a valid python identifier parameter-invalid Documentation Error Argument in argument_spec is not a valid python identifier
parameter-invalid-elements Documentation Error Value for "elements" is valid only when value of "type" is ``list`` parameter-invalid-elements Documentation Error Value for "elements" is valid only when value of "type" is ``list``
implied-parameter-type-mismatch Documentation Error Argument_spec implies ``type="str"`` but documentation defines it as different data type implied-parameter-type-mismatch Documentation Error Argument_spec implies ``type="str"`` but documentation defines it as different data type
@ -132,6 +132,7 @@ Codes
parameter-state-invalid-choice Parameters Error Argument ``state`` includes ``get``, ``list`` or ``info`` as a choice. Functionality should be in an ``_info`` or (if further conditions apply) ``_facts`` module. parameter-state-invalid-choice Parameters Error Argument ``state`` includes ``get``, ``list`` or ``info`` as a choice. Functionality should be in an ``_info`` or (if further conditions apply) ``_facts`` module.
python-syntax-error Syntax Error Python ``SyntaxError`` while parsing module python-syntax-error Syntax Error Python ``SyntaxError`` while parsing module
return-syntax-error Documentation Error ``RETURN`` is not valid YAML, ``RETURN`` fragments missing or invalid return-syntax-error Documentation Error ``RETURN`` is not valid YAML, ``RETURN`` fragments missing or invalid
return-invalid-version-added Documentation Error ``version_added`` for return value is not a valid version number
subdirectory-missing-init Naming Error Ansible module subdirectories must contain an ``__init__.py`` subdirectory-missing-init Naming Error Ansible module subdirectories must contain an ``__init__.py``
try-except-missing-has Imports Warning Try/Except ``HAS_`` expression missing try-except-missing-has Imports Warning Try/Except ``HAS_`` expression missing
undocumented-parameter Documentation Error Argument is listed in the argument_spec, but not documented in the module undocumented-parameter Documentation Error Argument is listed in the argument_spec, but not documented in the module

View file

@ -1160,11 +1160,13 @@ class ModuleValidator(Validator):
try: try:
collection_name = doc.get('version_added_collection') collection_name = doc.get('version_added_collection')
version_added = self._create_strict_version( version_added = self._create_strict_version(
str(doc.get('version_added', '0.0') or '0.0'), str(version_added_raw or '0.0'),
collection_name=collection_name) collection_name=collection_name)
except ValueError as e: except ValueError as e:
version_added = doc.get('version_added', '0.0') version_added = version_added_raw or '0.0'
if version_added != 'historical' or self._is_new_module(): if self._is_new_module() or version_added != 'historical':
# already reported during schema validation, except:
if version_added == 'historical':
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code='module-invalid-version-added', code='module-invalid-version-added',
@ -2076,13 +2078,7 @@ class ModuleValidator(Validator):
str(details.get('version_added', '0.0')), str(details.get('version_added', '0.0')),
collection_name=collection_name) collection_name=collection_name)
except ValueError as e: except ValueError as e:
self.reporter.error( # already reported during schema validation
path=self.object_path,
code='option-invalid-version-added',
msg=('version_added for option (%s) is not a valid '
'version for %s. Currently %r. Error: %s' %
(option, collection_name, details.get('version_added', '0.0'), e))
)
continue continue
if collection_name != self.collection_name: if collection_name != self.collection_name:

View file

@ -9,6 +9,7 @@ __metaclass__ = type
import re import re
from distutils.version import StrictVersion from distutils.version import StrictVersion
from functools import partial
from voluptuous import ALLOW_EXTRA, PREVENT_EXTRA, All, Any, Invalid, Length, Required, Schema, Self, ValueInvalid from voluptuous import ALLOW_EXTRA, PREVENT_EXTRA, All, Any, Invalid, Length, Required, Schema, Self, ValueInvalid
from ansible.module_utils.six import string_types from ansible.module_utils.six import string_types
@ -226,7 +227,7 @@ json_value = Schema(Any(
)) ))
def version_added(v): def version_added(v, error_code='version-added-invalid', accept_historical=False):
if 'version_added' in v: if 'version_added' in v:
version_added = v.get('version_added') version_added = v.get('version_added')
if isinstance(version_added, string_types): if isinstance(version_added, string_types):
@ -234,6 +235,8 @@ def version_added(v):
# - or we have a float and we are in ansible/ansible, in which case we're # - or we have a float and we are in ansible/ansible, in which case we're
# also happy. # also happy.
if v.get('version_added_collection') == 'ansible.builtin': if v.get('version_added_collection') == 'ansible.builtin':
if version_added == 'historical' and accept_historical:
return v
try: try:
version = StrictVersion() version = StrictVersion()
version.parse(version_added) version.parse(version_added)
@ -241,7 +244,7 @@ def version_added(v):
raise _add_ansible_error_code( raise _add_ansible_error_code(
Invalid('version_added (%r) is not a valid ansible-base version: ' Invalid('version_added (%r) is not a valid ansible-base version: '
'%s' % (version_added, exc)), '%s' % (version_added, exc)),
error_code='deprecation-either-date-or-version') error_code=error_code)
else: else:
try: try:
version = SemanticVersion() version = SemanticVersion()
@ -251,7 +254,7 @@ def version_added(v):
Invalid('version_added (%r) is not a valid collection version ' Invalid('version_added (%r) is not a valid collection version '
'(see specification at https://semver.org/): ' '(see specification at https://semver.org/): '
'%s' % (version_added, exc)), '%s' % (version_added, exc)),
error_code='deprecation-either-date-or-version') error_code=error_code)
elif 'version_added_collection' in v: elif 'version_added_collection' in v:
# Must have been manual intervention, since version_added_collection is only # Must have been manual intervention, since version_added_collection is only
# added automatically when version_added is present # added automatically when version_added is present
@ -304,7 +307,7 @@ def list_dict_option_schema(for_collection):
option_version_added = Schema( option_version_added = Schema(
All({ All({
'suboptions': Any(None, *[{str_type: Self} for str_type in string_types]), 'suboptions': Any(None, *[{str_type: Self} for str_type in string_types]),
}, version_added), }, partial(version_added, error_code='option-invalid-version-added')),
extra=ALLOW_EXTRA extra=ALLOW_EXTRA
) )
@ -343,7 +346,7 @@ def return_schema(for_collection):
} }
), ),
Schema(return_contains), Schema(return_contains),
Schema(version_added), Schema(partial(version_added, error_code='option-invalid-version-added')),
), ),
Schema(type(None)), Schema(type(None)),
) )
@ -371,7 +374,7 @@ def return_schema(for_collection):
} }
), ),
Schema({any_string_types: return_contains}), Schema({any_string_types: return_contains}),
Schema({any_string_types: version_added}), Schema({any_string_types: partial(version_added, error_code='option-invalid-version-added')}),
), ),
Schema(type(None)), Schema(type(None)),
) )
@ -460,8 +463,13 @@ def doc_schema(module_name, for_collection=False, deprecated_module=False):
doc_schema_dict.update(deprecation_required_scheme) doc_schema_dict.update(deprecation_required_scheme)
return Schema( return Schema(
All(
Schema(
doc_schema_dict, doc_schema_dict,
extra=PREVENT_EXTRA extra=PREVENT_EXTRA
),
partial(version_added, error_code='module-invalid-version-added', accept_historical=not for_collection),
)
) )