From a077bca5d5866603cdc4a2a13aef8f416860aacb Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Tue, 29 Sep 2020 22:33:45 +0200 Subject: ansible-test: improve version number validation, validate some semantic versioning properties (#71679) * Validate removal versions. * Validate that removal collection versions and version_added collection versions conform to semver spec. * Validate removal version numbers in meta/runtime.yml. * Stricter validation for isodates (f.ex. YYYY-M-D is not allowed). * Improve error reporting. * Validate removal collection versions. Co-authored-by: Andrew Klychkov --- changelogs/fragments/71679-ansible-test.yml | 6 ++ .../rst/dev_guide/testing_validate-modules.rst | 6 +- .../_data/sanity/code-smell/runtime-metadata.py | 78 ++++++++++++++++----- .../_data/sanity/pylint/plugins/deprecated.py | 9 +++ .../validate-modules/validate_modules/schema.py | 81 ++++++++++++++++++---- 5 files changed, 149 insertions(+), 31 deletions(-) create mode 100644 changelogs/fragments/71679-ansible-test.yml diff --git a/changelogs/fragments/71679-ansible-test.yml b/changelogs/fragments/71679-ansible-test.yml new file mode 100644 index 0000000000..0fb9f58983 --- /dev/null +++ b/changelogs/fragments/71679-ansible-test.yml @@ -0,0 +1,6 @@ +minor_changes: +- "ansible-test validate-modules - validate removal version numbers (https://github.com/ansible/ansible/pull/71679)." +- "ansible-test validate-modules - ensure that removal collection version numbers and version_added collection version numbers conform to the semantic versioning specification at https://semver.org/ (https://github.com/ansible/ansible/pull/71679)." +- "ansible-test pylint - ensure that removal collection version numbers conform to the semantic versioning specification at https://semver.org/ (https://github.com/ansible/ansible/pull/71679)." +- "ansible-test runtime-metadata - validate removal version numbers, and check removal dates more strictly (https://github.com/ansible/ansible/pull/71679)." +- "ansible-test runtime-metadata - ensure that removal collection version numbers conform to the semantic versioning specification at https://semver.org/ (https://github.com/ansible/ansible/pull/71679)." diff --git a/docs/docsite/rst/dev_guide/testing_validate-modules.rst b/docs/docsite/rst/dev_guide/testing_validate-modules.rst index 044a2c29a4..99a4eaebf7 100644 --- a/docs/docsite/rst/dev_guide/testing_validate-modules.rst +++ b/docs/docsite/rst/dev_guide/testing_validate-modules.rst @@ -62,10 +62,8 @@ Codes **Error Code** **Type** **Level** **Sample Message** ------------------------------------------------------------ ------------------ -------------------- ----------------------------------------------------------------------------------------- ansible-deprecated-version Documentation Error A feature is deprecated and supposed to be removed in the current or an earlier Ansible version - ansible-invalid-version Documentation Error The Ansible version at which a feature is supposed to be removed cannot be parsed ansible-module-not-initialized Syntax Error Execution of the module did not result in initialization of AnsibleModule collection-deprecated-version Documentation Error A feature is deprecated and supposed to be removed in the current or an earlier collection version - collection-invalid-version Documentation Error The collection version at which a feature is supposed to be removed cannot be parsed (it must be a semantic version, see https://semver.org/) deprecated-date Documentation Error A date before today appears as ``removed_at_date`` or in ``deprecated_aliases`` deprecation-mismatch Documentation Error Module marked as deprecated or removed in at least one of the filename, its metadata, or in DOCUMENTATION (setting DOCUMENTATION.deprecated for deprecation or removing all Documentation for removed) but not in all three places. doc-choices-do-not-match-spec Documentation Error Value for "choices" from the argument_spec does not match the documentation @@ -94,8 +92,8 @@ Codes invalid-examples Documentation Error ``EXAMPLES`` is not valid YAML invalid-extension Naming Error Official Ansible modules must have a ``.py`` extension for python modules or a ``.ps1`` for powershell modules invalid-module-schema Documentation Error ``AnsibleModule`` schema validation error + invalid-removal-version Documentation Error The version at which a feature is supposed to be removed cannot be parsed (for collections, it must be a semantic version, see https://semver.org/) invalid-requires-extension Naming Error Module ``#AnsibleRequires -CSharpUtil`` should not end in .cs, Module ``#Requires`` should not end in .psm1 - invalid-tagged-version Documentation Error All version numbers specified in code have to be explicitly tagged with the collection name, in other words, ``community.general:1.2.3`` or ``ansible.builtin:2.10`` last-line-main-call Syntax Error Call to ``main()`` not the last line (or ``removed_module()`` in the case of deprecated & docs only modules) missing-doc-fragment Documentation Error ``DOCUMENTATION`` fragment missing missing-existing-doc-fragment Documentation Warning Pre-existing ``DOCUMENTATION`` fragment missing @@ -131,6 +129,7 @@ Codes parameter-list-no-elements Parameters Error argument in argument_spec "type" is specified as ``list`` without defining "elements" 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 + removal-version-must-be-major Documentation Error According to the semantic versioning specification (https://semver.org/), the only versions in which features are allowed to be removed are major versions (x.0.0) 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`` @@ -162,4 +161,5 @@ Codes required_if-value-type Documentation Error required_if entry's value is not of the type specified for its key required_by-collision Documentation Error required_by entry has repeated terms required_by-unknown Documentation Error required_by entry contains option which does not appear in argument_spec (potentially an alias of an option?) + version-added-must-be-major-or-minor Documentation Error According to the semantic versioning specification (https://semver.org/), the only versions in which features are allowed to be added are major and minor versions (x.y.0) ============================================================ ================== ==================== ========================================================================================= diff --git a/test/lib/ansible_test/_data/sanity/code-smell/runtime-metadata.py b/test/lib/ansible_test/_data/sanity/code-smell/runtime-metadata.py index b986db2b57..6d5689516a 100755 --- a/test/lib/ansible_test/_data/sanity/code-smell/runtime-metadata.py +++ b/test/lib/ansible_test/_data/sanity/code-smell/runtime-metadata.py @@ -7,13 +7,17 @@ import datetime import os import re import sys +from distutils.version import StrictVersion +from functools import partial + import yaml -from voluptuous import Any, MultipleInvalid, PREVENT_EXTRA +from voluptuous import All, Any, MultipleInvalid, PREVENT_EXTRA from voluptuous import Required, Schema, Invalid from voluptuous.humanize import humanize_error from ansible.module_utils.six import string_types +from ansible.utils.version import SemanticVersion def isodate(value): @@ -25,6 +29,10 @@ def isodate(value): msg = 'Expected ISO 8601 date string (YYYY-MM-DD), or YAML date' if not isinstance(value, string_types): raise Invalid(msg) + # From Python 3.7 in, there is datetime.date.fromisoformat(). For older versions, + # we have to do things manually. + if not re.match('^[0-9]{4}-[0-9]{2}-[0-9]{2}$', value): + raise Invalid(msg) try: datetime.datetime.strptime(value, '%Y-%m-%d').date() except ValueError: @@ -32,7 +40,35 @@ def isodate(value): return value -def validate_metadata_file(path): +def removal_version(value, is_ansible): + """Validate a removal version string.""" + msg = ( + 'Removal version must be a string' if is_ansible else + 'Removal version must be a semantic version (https://semver.org/)' + ) + if not isinstance(value, string_types): + raise Invalid(msg) + try: + if is_ansible: + version = StrictVersion() + version.parse(value) + else: + version = SemanticVersion() + version.parse(value) + if version.major != 0 and (version.minor != 0 or version.patch != 0): + raise Invalid('removal_version (%r) must be a major release, not a minor or patch release ' + '(see specification at https://semver.org/)' % (value, )) + except ValueError: + raise Invalid(msg) + return value + + +def any_value(value): + """Accepts anything.""" + return value + + +def validate_metadata_file(path, is_ansible): """Validate explicit runtime metadata file""" try: with open(path, 'r') as f_path: @@ -51,19 +87,29 @@ def validate_metadata_file(path): # plugin_routing schema - deprecation_tombstoning_schema = Any(Schema( - { - Required('removal_date'): Any(isodate), - 'warning_text': Any(*string_types), - }, - extra=PREVENT_EXTRA - ), Schema( - { - Required('removal_version'): Any(*string_types), - 'warning_text': Any(*string_types), - }, - extra=PREVENT_EXTRA - )) + deprecation_tombstoning_schema = All( + # The first schema validates the input, and the second makes sure no extra keys are specified + Schema( + { + 'removal_version': partial(removal_version, is_ansible=is_ansible), + 'removal_date': Any(isodate), + 'warning_text': Any(*string_types), + } + ), + Schema( + Any( + { + Required('removal_version'): any_value, + 'warning_text': any_value, + }, + { + Required('removal_date'): any_value, + 'warning_text': any_value, + } + ), + extra=PREVENT_EXTRA + ) + ) plugin_routing_schema = Any( Schema({ @@ -143,7 +189,7 @@ def main(): print('%s:%d:%d: %s' % (path, 0, 0, ("Should be called '%s'" % collection_runtime_file))) continue - validate_metadata_file(path) + validate_metadata_file(path, is_ansible=path not in (collection_legacy_file, collection_runtime_file)) if __name__ == '__main__': diff --git a/test/lib/ansible_test/_data/sanity/pylint/plugins/deprecated.py b/test/lib/ansible_test/_data/sanity/pylint/plugins/deprecated.py index c88e5e5ae1..9ded3e3bd7 100644 --- a/test/lib/ansible_test/_data/sanity/pylint/plugins/deprecated.py +++ b/test/lib/ansible_test/_data/sanity/pylint/plugins/deprecated.py @@ -81,6 +81,13 @@ MSGS = { "ansible-deprecated-both-version-and-date", "Only one of version and date must be specified", {'minversion': (2, 6)}), + 'E9511': ("Removal version (%r) must be a major release, not a minor or " + "patch release (see the specification at https://semver.org/)", + "removal-version-must-be-major", + "Used when a call to Display.deprecated or " + "AnsibleModule.deprecate for a collection specifies a version " + "which is not of the form x.0.0", + {'minversion': (2, 6)}), } @@ -189,6 +196,8 @@ class AnsibleDeprecatedChecker(BaseChecker): if collection_name == self.collection_name and self.collection_version is not None: if self.collection_version >= semantic_version: self.add_message('collection-deprecated-version', node=node, args=(version,)) + if semantic_version.major != 0 and (semantic_version.minor != 0 or semantic_version.patch != 0): + self.add_message('removal-version-must-be-major', node=node, args=(version,)) except ValueError: self.add_message('collection-invalid-deprecated-version', node=node, args=(version,)) diff --git a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/schema.py b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/schema.py index 42a2ada4af..9aeb2d7d48 100644 --- a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/schema.py +++ b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/schema.py @@ -143,6 +143,37 @@ def options_with_apply_defaults(v): return v +def check_removal_version(v, version_field, collection_name_field, error_code='invalid-removal-version'): + version = v.get(version_field) + collection_name = v.get(collection_name_field) + if not isinstance(version, string_types) or not isinstance(collection_name, string_types): + # If they are not strings, schema validation will have already complained. + return v + if collection_name == 'ansible.builtin': + try: + parsed_version = StrictVersion() + parsed_version.parse(version) + except ValueError as exc: + raise _add_ansible_error_code( + Invalid('%s (%r) is not a valid ansible-base version: %s' % (version_field, version, exc)), + error_code=error_code) + return v + try: + parsed_version = SemanticVersion() + parsed_version.parse(version) + if parsed_version.major != 0 and (parsed_version.minor != 0 or parsed_version.patch != 0): + raise _add_ansible_error_code( + Invalid('%s (%r) must be a major release, not a minor or patch release (see specification at ' + 'https://semver.org/)' % (version_field, version)), + error_code='removal-version-must-be-major') + except ValueError as exc: + raise _add_ansible_error_code( + Invalid('%s (%r) is not a valid collection version (see specification at https://semver.org/): ' + '%s' % (version_field, version, exc)), + error_code=error_code) + return v + + def option_deprecation(v): if v.get('removed_in_version') or v.get('removed_at_date'): if v.get('removed_in_version') and v.get('removed_at_date'): @@ -154,6 +185,10 @@ def option_deprecation(v): Invalid('If removed_in_version or removed_at_date is specified, ' 'removed_from_collection must be specified as well'), error_code='deprecation-collection-missing') + check_removal_version(v, + version_field='removed_in_version', + collection_name_field='removed_from_collection', + error_code='invalid-removal-version') return if v.get('removed_from_collection'): raise Invalid('removed_from_collection cannot be specified without either ' @@ -180,17 +215,23 @@ def argument_spec_schema(for_collection): 'removed_at_date': date(), 'removed_from_collection': collection_name, 'options': Self, - 'deprecated_aliases': Any([Any( - { - Required('name'): Any(*string_types), - Required('date'): date(), - Required('collection_name'): collection_name, - }, - { - Required('name'): Any(*string_types), - Required('version'): version(for_collection), - Required('collection_name'): collection_name, - }, + 'deprecated_aliases': Any([All( + Any( + { + Required('name'): Any(*string_types), + Required('date'): date(), + Required('collection_name'): collection_name, + }, + { + Required('name'): Any(*string_types), + Required('version'): version(for_collection), + Required('collection_name'): collection_name, + }, + ), + partial(check_removal_version, + version_field='version', + collection_name_field='collection_name', + error_code='invalid-removal-version') )]), } } @@ -249,6 +290,12 @@ def version_added(v, error_code='version-added-invalid', accept_historical=False try: version = SemanticVersion() version.parse(version_added) + if version.major != 0 and version.patch != 0: + raise _add_ansible_error_code( + Invalid('version_added (%r) must be a major or minor release, ' + 'not a patch release (see specification at ' + 'https://semver.org/)' % (version_added, )), + error_code='version-added-must-be-major-or-minor') except ValueError as exc: raise _add_ansible_error_code( Invalid('version_added (%r) is not a valid collection version ' @@ -408,11 +455,21 @@ def deprecation_schema(for_collection): } version_schema.update(main_fields) - return Any( + result = Any( Schema(version_schema, extra=PREVENT_EXTRA), Schema(date_schema, extra=PREVENT_EXTRA), ) + if for_collection: + result = All( + result, + partial(check_removal_version, + version_field='removed_in', + collection_name_field='removed_from_collection', + error_code='invalid-removal-version')) + + return result + def author(value): if value is None: -- cgit v1.2.3