Compare removal and deprecation dates and versions in runtime metadata against current version and today (#72625)

* Check deprecation and removal versions w.r.t. current version.

* Check removal dates from tombstones w.r.t. today.

* Add changelog.

changelogs/fragments/ansible-test-runtime-dates-versions.yml

@@ -0,0 +1,3 @@
+minor_changes:
+- "ansible-test runtime-metadata - compare deprecation and tombstone versions to the current version to ensure that they are correct (https://github.com/ansible/ansible/pull/72625)."
+- "ansible-test runtime-metadata - ensure that the tombstone removal date is not in the future (https://github.com/ansible/ansible/pull/72625)."

test/lib/ansible_test/_data/sanity/code-smell/runtime-metadata.py

@@ -7,7 +7,7 @@ import datetime
 import os
 import re
 import sys
-from distutils.version import StrictVersion
+from distutils.version import StrictVersion, LooseVersion
 from functools import partial
 
 import yaml
@@ -20,27 +20,41 @@ from ansible.module_utils.six import string_types
 from ansible.utils.version import SemanticVersion
 
 
-def isodate(value):
+def isodate(value, check_deprecation_date=False, is_tombstone=False):
     """Validate a datetime.date or ISO 8601 date string."""
     # datetime.date objects come from YAML dates, these are ok
     if isinstance(value, datetime.date):
-        return value
+        removal_date = value
-    # make sure we have a string
+    else:
-    msg = 'Expected ISO 8601 date string (YYYY-MM-DD), or YAML date'
+        # make sure we have a string
-    if not isinstance(value, string_types):
+        msg = 'Expected ISO 8601 date string (YYYY-MM-DD), or YAML date'
-        raise Invalid(msg)
+        if not isinstance(value, string_types):
-    # From Python 3.7 in, there is datetime.date.fromisoformat(). For older versions,
+            raise Invalid(msg)
-    # we have to do things manually.
+        # From Python 3.7 in, there is datetime.date.fromisoformat(). For older versions,
-    if not re.match('^[0-9]{4}-[0-9]{2}-[0-9]{2}$', value):
+        # we have to do things manually.
-        raise Invalid(msg)
+        if not re.match('^[0-9]{4}-[0-9]{2}-[0-9]{2}$', value):
-    try:
+            raise Invalid(msg)
-        datetime.datetime.strptime(value, '%Y-%m-%d').date()
+        try:
-    except ValueError:
+            removal_date = datetime.datetime.strptime(value, '%Y-%m-%d').date()
-        raise Invalid(msg)
+        except ValueError:
+            raise Invalid(msg)
+    # Make sure date is correct
+    today = datetime.date.today()
+    if is_tombstone:
+        # For a tombstone, the removal date must be in the past
+        if today < removal_date:
+            raise Invalid(
+                'The tombstone removal_date (%s) must not be after today (%s)' % (removal_date, today))
+    else:
+        # For a deprecation, the removal date must be in the future. Only test this if
+        # check_deprecation_date is truish, to avoid checks to suddenly start to fail.
+        if check_deprecation_date and today > removal_date:
+            raise Invalid(
+                'The deprecation removal_date (%s) must be after today (%s)' % (removal_date, today))
     return value
 
 
-def removal_version(value, is_ansible):
+def removal_version(value, is_ansible, current_version=None, is_tombstone=False):
     """Validate a removal version string."""
     msg = (
         'Removal version must be a string' if is_ansible else
@@ -52,12 +66,24 @@ def removal_version(value, is_ansible):
         if is_ansible:
             version = StrictVersion()
             version.parse(value)
+            version = LooseVersion(value)  # We're storing Ansible's version as a LooseVersion
         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, ))
+        if current_version is not None:
+            if is_tombstone:
+                # For a tombstone, the removal version must not be in the future
+                if version > current_version:
+                    raise Invalid('The tombstone removal_version (%r) must not be after the '
+                                  'current version (%s)' % (value, current_version))
+            else:
+                # For a deprecation, the removal version must be in the future
+                if version <= current_version:
+                    raise Invalid('The deprecation removal_version (%r) must be after the '
+                                  'current version (%s)' % (value, current_version))
     except ValueError:
         raise Invalid(msg)
     return value
@@ -68,7 +94,34 @@ def any_value(value):
     return value
 
 
-def validate_metadata_file(path, is_ansible):
+def get_ansible_version():
+    """Return current ansible-core version"""
+    from ansible.release import __version__
+
+    return LooseVersion('.'.join(__version__.split('.')[:3]))
+
+
+def get_collection_version():
+    """Return current collection version, or None if it is not available"""
+    import importlib.util
+
+    collection_detail_path = os.path.join(os.path.dirname(os.path.dirname(os.path.dirname(__file__))),
+                                          'collection_detail.py')
+    collection_detail_spec = importlib.util.spec_from_file_location('collection_detail', collection_detail_path)
+    collection_detail = importlib.util.module_from_spec(collection_detail_spec)
+    sys.modules['collection_detail'] = collection_detail
+    collection_detail_spec.loader.exec_module(collection_detail)
+
+    try:
+        result = collection_detail.read_manifest_json('.') or collection_detail.read_galaxy_yml('.')
+        return SemanticVersion(result['version'])
+    except Exception:  # pylint: disable=broad-except
+        # We do not care why it fails, in case we cannot get the version
+        # just return None to indicate "we don't know".
+        return None
+
+
+def validate_metadata_file(path, is_ansible, check_deprecation_dates=False):
     """Validate explicit runtime metadata file"""
     try:
         with open(path, 'r') as f_path:
@@ -82,39 +135,60 @@ def validate_metadata_file(path, is_ansible):
               (path, 0, 0, re.sub(r'\s+', ' ', str(ex))))
         return
 
+    if is_ansible:
+        current_version = get_ansible_version()
+    else:
+        current_version = get_collection_version()
+
     # Updates to schema MUST also be reflected in the documentation
     # ~https://docs.ansible.com/ansible/devel/dev_guide/developing_collections.html
 
     # plugin_routing schema
 
-    deprecation_tombstoning_schema = All(
+    avoid_additional_data = Schema(
+        Any(
+            {
+                Required('removal_version'): any_value,
+                'warning_text': any_value,
+            },
+            {
+                Required('removal_date'): any_value,
+                'warning_text': any_value,
+            }
+        ),
+        extra=PREVENT_EXTRA
+    )
+
+    deprecation_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_version': partial(removal_version, is_ansible=is_ansible,
-                'removal_date': Any(isodate),
+                                           current_version=current_version),
+                'removal_date': partial(isodate, check_deprecation_date=check_deprecation_dates),
                 'warning_text': Any(*string_types),
             }
         ),
+        avoid_additional_data
+    )
+
+    tombstoning_schema = All(
+        # The first schema validates the input, and the second makes sure no extra keys are specified
         Schema(
-            Any(
+            {
-                {
+                'removal_version': partial(removal_version, is_ansible=is_ansible,
-                    Required('removal_version'): any_value,
+                                           current_version=current_version, is_tombstone=True),
-                    'warning_text': any_value,
+                'removal_date': partial(isodate, is_tombstone=True),
-                },
+                'warning_text': Any(*string_types),
-                {
+            }
-                    Required('removal_date'): any_value,
+        ),
-                    'warning_text': any_value,
+        avoid_additional_data
-                }
-            ),
-            extra=PREVENT_EXTRA
-        )
     )
 
     plugin_routing_schema = Any(
         Schema({
-            ('deprecation'): Any(deprecation_tombstoning_schema),
+            ('deprecation'): Any(deprecation_schema),
-            ('tombstone'): Any(deprecation_tombstoning_schema),
+            ('tombstone'): Any(tombstoning_schema),
             ('redirect'): Any(*string_types),
         }, extra=PREVENT_EXTRA),
     )
@@ -184,12 +258,20 @@ def main():
     collection_legacy_file = 'meta/routing.yml'
     collection_runtime_file = 'meta/runtime.yml'
 
+    # This is currently disabled, because if it is enabled this test can start failing
+    # at a random date. For this to be properly activated, we (a) need to be able to return
+    # codes for this test, and (b) make this error optional.
+    check_deprecation_dates = False
+
     for path in paths:
         if path == collection_legacy_file:
             print('%s:%d:%d: %s' % (path, 0, 0, ("Should be called '%s'" % collection_runtime_file)))
             continue
 
-        validate_metadata_file(path, is_ansible=path not in (collection_legacy_file, collection_runtime_file))
+        validate_metadata_file(
+            path,
+            is_ansible=path not in (collection_legacy_file, collection_runtime_file),
+            check_deprecation_dates=check_deprecation_dates)
 
 
 if __name__ == '__main__':