From 0ba96d2be8b512db85b62dc2c6d4c33b77a2f1f0 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Tue, 15 Dec 2020 15:05:38 +0100 Subject: [PATCH] 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. --- .../ansible-test-runtime-dates-versions.yml | 3 + .../sanity/code-smell/runtime-metadata.py | 152 ++++++++++++++---- 2 files changed, 120 insertions(+), 35 deletions(-) create mode 100644 changelogs/fragments/ansible-test-runtime-dates-versions.yml diff --git a/changelogs/fragments/ansible-test-runtime-dates-versions.yml b/changelogs/fragments/ansible-test-runtime-dates-versions.yml new file mode 100644 index 00000000000..b65c5235d7b --- /dev/null +++ b/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)." 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 5eb5a4f24f3..3434ad938a8 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,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 - # make sure we have a string - 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: - raise Invalid(msg) + removal_date = value + else: + # make sure we have a string + 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: + removal_date = datetime.datetime.strptime(value, '%Y-%m-%d').date() + 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_date': Any(isodate), + 'removal_version': partial(removal_version, is_ansible=is_ansible, + 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( - { - Required('removal_version'): any_value, - 'warning_text': any_value, - }, - { - Required('removal_date'): any_value, - 'warning_text': any_value, - } - ), - extra=PREVENT_EXTRA - ) + { + 'removal_version': partial(removal_version, is_ansible=is_ansible, + current_version=current_version, is_tombstone=True), + 'removal_date': partial(isodate, is_tombstone=True), + 'warning_text': Any(*string_types), + } + ), + avoid_additional_data ) plugin_routing_schema = Any( Schema({ - ('deprecation'): Any(deprecation_tombstoning_schema), - ('tombstone'): Any(deprecation_tombstoning_schema), + ('deprecation'): Any(deprecation_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__':