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 <aaklychkov@mail.ru>

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)."

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)
 ============================================================   ==================   ====================   =========================================================================================

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__':

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,))
 

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: