diff --git a/changelogs/fragments/deprecation-collection-name.yml b/changelogs/fragments/deprecation-collection-name.yml new file mode 100644 index 00000000000..1e796a65d83 --- /dev/null +++ b/changelogs/fragments/deprecation-collection-name.yml @@ -0,0 +1,2 @@ +major_changes: +- "When deprecations are done in code, they to specify a ``collection_name`` so that deprecation warnings can mention which collection - or ansible-base - is deprecating a feature. This affects all ``Display.deprecated()`` or ``AnsibleModule.deprecate()`` or ``Ansible.Basic.Deprecate()`` calls, and ``removed_in_version``/``removed_at_date`` or ``deprecated_aliases`` in module argument specs." diff --git a/docs/docsite/rst/dev_guide/developing_modules_general_windows.rst b/docs/docsite/rst/dev_guide/developing_modules_general_windows.rst index c2219bfaa90..18dfb786ae2 100644 --- a/docs/docsite/rst/dev_guide/developing_modules_general_windows.rst +++ b/docs/docsite/rst/dev_guide/developing_modules_general_windows.rst @@ -209,11 +209,12 @@ options set: - ``aliases``: A list of aliases for the module option - ``choices``: A list of valid values for the module option, if ``type=list`` then each list value is validated against the choices and not the list itself - ``default``: The default value for the module option if not set -- ``deprecated_aliases``: A list of hashtables that define aliases that are deprecated and the versions they will be removed in. Each entry must contain the key ``name`` with either ``version`` or ``date`` +- ``deprecated_aliases``: A list of hashtables that define aliases that are deprecated and the versions they will be removed in. Each entry must contain the keys ``name`` and ``collection_name`` with either ``version`` or ``date`` - ``elements``: When ``type=list``, this sets the type of each list value, the values are the same as ``type`` - ``no_log``: Will sanitise the input value before being returned in the ``module_invocation`` return value - ``removed_in_version``: States when a deprecated module option is to be removed, a warning is displayed to the end user if set - ``removed_at_date``: States the date when a deprecated module option will be removed, a warning is displayed to the end user if set +- ``removed_from_collection``: States from which collection the deprecated module option will be removed; must be specified if one of ``removed_in_version`` and ``removed_at_date`` is specified - ``required``: Will fail when the module option is not set - ``type``: The type of the module option, if not set then it defaults to ``str``. The valid types are; * ``bool``: A boolean value diff --git a/lib/ansible/cli/__init__.py b/lib/ansible/cli/__init__.py index 3f5dacc0dd5..7e9cb44b986 100644 --- a/lib/ansible/cli/__init__.py +++ b/lib/ansible/cli/__init__.py @@ -104,7 +104,9 @@ class CLI(with_metaclass(ABCMeta, object)): alt = '' ver = deprecated[1].get('version') date = deprecated[1].get('date') - display.deprecated("%s option, %s %s" % (name, why, alt), version=ver, date=date) + collection_name = deprecated[1].get('collection_name') + display.deprecated("%s option, %s %s" % (name, why, alt), + version=ver, date=date, collection_name=collection_name) @staticmethod def split_vault_id(vault_id): @@ -353,7 +355,7 @@ class CLI(with_metaclass(ABCMeta, object)): verbosity_arg = next(iter([arg for arg in self.args if arg.startswith('-v')]), None) if verbosity_arg: display.deprecated("Setting verbosity before the arg sub command is deprecated, set the verbosity " - "after the sub command", "ansible.builtin:2.13") + "after the sub command", "2.13", collection_name='ansible.builtin') options.verbosity = verbosity_arg.count('v') return options diff --git a/lib/ansible/cli/doc.py b/lib/ansible/cli/doc.py index fbf0223baca..78e0b17db2c 100644 --- a/lib/ansible/cli/doc.py +++ b/lib/ansible/cli/doc.py @@ -30,7 +30,12 @@ from ansible.plugins.loader import action_loader, fragment_loader from ansible.utils.collection_loader import AnsibleCollectionConfig from ansible.utils.collection_loader._collection_finder import _get_collection_name_from_path from ansible.utils.display import Display -from ansible.utils.plugin_docs import BLACKLIST, untag_versions_and_dates, get_docstring, get_versioned_doclink +from ansible.utils.plugin_docs import ( + BLACKLIST, + remove_current_collection_from_versions_and_dates, + get_docstring, + get_versioned_doclink, +) display = Display() @@ -330,11 +335,18 @@ class DocCLI(CLI): raise ValueError('%s did not contain a DOCUMENTATION attribute' % plugin) doc['filename'] = filename - untag_versions_and_dates(doc, '%s:' % (collection_name, ), is_module=(plugin_type == 'module')) return doc, plainexamples, returndocs, metadata @staticmethod def format_plugin_doc(plugin, plugin_type, doc, plainexamples, returndocs, metadata): + collection_name = 'ansible.builtin' + if plugin.startswith('ansible_collections.'): + collection_name = '.'.join(plugin.split('.')[1:3]) + + # TODO: do we really want this? + # add_collection_to_versions_and_dates(doc, '(unknown)', is_module=(plugin_type == 'module')) + # remove_current_collection_from_versions_and_dates(doc, collection_name, is_module=(plugin_type == 'module')) + # assign from other sections doc['plainexamples'] = plainexamples doc['returndocs'] = returndocs diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index a2d4e217f94..22f758e9cef 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -487,7 +487,7 @@ class TaskExecutor: 'Invoking "%s" only once while using a loop via squash_actions is deprecated. ' 'Instead of using a loop to supply multiple items and specifying `%s: "%s"`, ' 'please use `%s: %s` and remove the loop' % (self._task.action, found, name, found, value_text), - version='ansible.builtin:2.11' + version='2.11', collection_name='ansible.builtin' ) for item in items: variables[loop_var] = item diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index e1569da4c97..33818482383 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -725,10 +725,10 @@ class AnsibleModule(object): warn(warning) self.log('[WARNING] %s' % warning) - def deprecate(self, msg, version=None, date=None): + def deprecate(self, msg, version=None, date=None, collection_name=None): if version is not None and date is not None: raise AssertionError("implementation error -- version and date must not both be set") - deprecate(msg, version=version, date=date) + deprecate(msg, version=version, date=date, collection_name=collection_name) # For compatibility, we accept that neither version nor date is set, # and treat that the same as if version would haven been set if date is not None: @@ -1414,7 +1414,8 @@ class AnsibleModule(object): for deprecation in deprecated_aliases: if deprecation['name'] in param.keys(): deprecate("Alias '%s' is deprecated. See the module docs for more information" % deprecation['name'], - version=deprecation.get('version'), date=deprecation.get('date')) + version=deprecation.get('version'), date=deprecation.get('date'), + collection_name=deprecation.get('collection_name')) return alias_results def _handle_no_log_values(self, spec=None, param=None): @@ -1430,7 +1431,8 @@ class AnsibleModule(object): "%s" % to_native(te), invocation={'module_args': 'HIDDEN DUE TO FAILURE'}) for message in list_deprecations(spec, param): - deprecate(message['msg'], version=message.get('version'), date=message.get('date')) + deprecate(message['msg'], version=message.get('version'), date=message.get('date'), + collection_name=message.get('collection_name')) def _check_arguments(self, spec=None, param=None, legal_inputs=None): self._syslog_facility = 'LOG_USER' @@ -2060,7 +2062,8 @@ class AnsibleModule(object): if isinstance(d, SEQUENCETYPE) and len(d) == 2: self.deprecate(d[0], version=d[1]) elif isinstance(d, Mapping): - self.deprecate(d['msg'], version=d.get('version', None), date=d.get('date', None)) + self.deprecate(d['msg'], version=d.get('version'), date=d.get('date'), + collection_name=d.get('date')) else: self.deprecate(d) # pylint: disable=ansible-deprecated-no-version else: diff --git a/lib/ansible/module_utils/common/parameters.py b/lib/ansible/module_utils/common/parameters.py index b66d956f521..4cf631e1e9d 100644 --- a/lib/ansible/module_utils/common/parameters.py +++ b/lib/ansible/module_utils/common/parameters.py @@ -140,12 +140,14 @@ def list_deprecations(argument_spec, params, prefix=''): if arg_opts.get('removed_at_date') is not None: deprecations.append({ 'msg': "Param '%s' is deprecated. See the module docs for more information" % sub_prefix, - 'date': arg_opts.get('removed_at_date') + 'date': arg_opts.get('removed_at_date'), + 'collection_name': arg_opts.get('removed_from_collection'), }) elif arg_opts.get('removed_in_version') is not None: deprecations.append({ 'msg': "Param '%s' is deprecated. See the module docs for more information" % sub_prefix, - 'version': arg_opts.get('removed_in_version') + 'version': arg_opts.get('removed_in_version'), + 'collection_name': arg_opts.get('removed_from_collection'), }) # Check sub-argument spec sub_argument_spec = arg_opts.get('options') diff --git a/lib/ansible/module_utils/common/warnings.py b/lib/ansible/module_utils/common/warnings.py index 5c9971c93e4..9423e6a429b 100644 --- a/lib/ansible/module_utils/common/warnings.py +++ b/lib/ansible/module_utils/common/warnings.py @@ -18,14 +18,14 @@ def warn(warning): raise TypeError("warn requires a string not a %s" % type(warning)) -def deprecate(msg, version=None, date=None): +def deprecate(msg, version=None, date=None, collection_name=None): if isinstance(msg, string_types): # For compatibility, we accept that neither version nor date is set, # and treat that the same as if version would haven been set if date is not None: - _global_deprecations.append({'msg': msg, 'date': date}) + _global_deprecations.append({'msg': msg, 'date': date, 'collection_name': collection_name}) else: - _global_deprecations.append({'msg': msg, 'version': version}) + _global_deprecations.append({'msg': msg, 'version': version, 'collection_name': collection_name}) else: raise TypeError("deprecate requires a string not a %s" % type(msg)) diff --git a/lib/ansible/module_utils/csharp/Ansible.Basic.cs b/lib/ansible/module_utils/csharp/Ansible.Basic.cs index fb90c847a06..51a543bc135 100644 --- a/lib/ansible/module_utils/csharp/Ansible.Basic.cs +++ b/lib/ansible/module_utils/csharp/Ansible.Basic.cs @@ -86,6 +86,7 @@ namespace Ansible.Basic { "options", new List() { typeof(Hashtable), typeof(Hashtable) } }, { "removed_in_version", new List() { null, typeof(string) } }, { "removed_at_date", new List() { null, typeof(DateTime) } }, + { "removed_from_collection", new List() { null, typeof(string) } }, { "required", new List() { false, typeof(bool) } }, { "required_by", new List() { typeof(Hashtable), typeof(Hashtable) } }, { "required_if", new List() { typeof(List>), typeof(List) } }, @@ -275,14 +276,26 @@ namespace Ansible.Basic public void Deprecate(string message, string version) { - deprecations.Add(new Dictionary() { { "msg", message }, { "version", version } }); + Deprecate(message, version, null); + } + + public void Deprecate(string message, string version, string collectionName) + { + deprecations.Add(new Dictionary() { + { "msg", message }, { "version", version }, { "collection_name", collectionName } }); LogEvent(String.Format("[DEPRECATION WARNING] {0} {1}", message, version)); } public void Deprecate(string message, DateTime date) + { + Deprecate(message, date, null); + } + + public void Deprecate(string message, DateTime date, string collectionName) { string isoDate = date.ToString("yyyy-MM-dd"); - deprecations.Add(new Dictionary() { { "msg", message }, { "date", isoDate } }); + deprecations.Add(new Dictionary() { + { "msg", message }, { "date", isoDate }, { "collection_name", collectionName } }); LogEvent(String.Format("[DEPRECATION WARNING] {0} {1}", message, isoDate)); } @@ -800,6 +813,11 @@ namespace Ansible.Basic string msg = "A deprecated_aliases date must be a DateTime object"; throw new ArgumentException(FormatOptionsContext(msg, " - ")); } + string collectionName = null; + if (depInfo.ContainsKey("collection_name")) + { + collectionName = (string)depInfo["collection_name"]; + } string aliasName = (string)depInfo["name"]; if (parameters.Contains(aliasName)) @@ -808,12 +826,12 @@ namespace Ansible.Basic if (depInfo.ContainsKey("version")) { string depVersion = (string)depInfo["version"]; - Deprecate(FormatOptionsContext(msg, " - "), depVersion); + Deprecate(FormatOptionsContext(msg, " - "), depVersion, collectionName); } if (depInfo.ContainsKey("date")) { DateTime depDate = (DateTime)depInfo["date"]; - Deprecate(FormatOptionsContext(msg, " - "), depDate); + Deprecate(FormatOptionsContext(msg, " - "), depDate, collectionName); } } } @@ -836,14 +854,21 @@ namespace Ansible.Basic if (!String.IsNullOrEmpty(noLogString)) noLogValues.Add(noLogString); } + string collectionName = null; + if (v.ContainsKey("removed_from_collection")) + { + collectionName = (string)v["removed_from_collection"]; + } object removedInVersion = v["removed_in_version"]; if (removedInVersion != null && parameters.Contains(k)) - Deprecate(String.Format("Param '{0}' is deprecated. See the module docs for more information", k), removedInVersion.ToString()); + Deprecate(String.Format("Param '{0}' is deprecated. See the module docs for more information", k), + removedInVersion.ToString(), collectionName); object removedAtDate = v["removed_at_date"]; if (removedAtDate != null && parameters.Contains(k)) - Deprecate(String.Format("Param '{0}' is deprecated. See the module docs for more information", k), (DateTime)removedAtDate); + Deprecate(String.Format("Param '{0}' is deprecated. See the module docs for more information", k), + (DateTime)removedAtDate, collectionName); } } diff --git a/lib/ansible/module_utils/urls.py b/lib/ansible/module_utils/urls.py index 16fd82d847a..0fa6b7180cb 100644 --- a/lib/ansible/module_utils/urls.py +++ b/lib/ansible/module_utils/urls.py @@ -1529,7 +1529,7 @@ def url_argument_spec(): return dict( url=dict(type='str'), force=dict(type='bool', default=False, aliases=['thirsty'], - deprecated_aliases=[dict(name='thirsty', version='ansible.builtin:2.13')]), + deprecated_aliases=[dict(name='thirsty', version='2.13', collection='ansible.builtin')]), http_agent=dict(type='str', default='ansible-httpget'), use_proxy=dict(type='bool', default=True), validate_certs=dict(type='bool', default=True), diff --git a/lib/ansible/modules/copy.py b/lib/ansible/modules/copy.py index b6cd8c681f3..add01b056d1 100644 --- a/lib/ansible/modules/copy.py +++ b/lib/ansible/modules/copy.py @@ -514,7 +514,8 @@ def main(): ) if module.params.get('thirsty'): - module.deprecate('The alias "thirsty" has been deprecated and will be removed, use "force" instead', version='ansible.builtin:2.13') + module.deprecate('The alias "thirsty" has been deprecated and will be removed, use "force" instead', + version='2.13', collection_name='ansible.builtin') src = module.params['src'] b_src = to_bytes(src, errors='surrogate_or_strict') diff --git a/lib/ansible/modules/cron.py b/lib/ansible/modules/cron.py index 6a59acb6f79..781a75facfb 100644 --- a/lib/ansible/modules/cron.py +++ b/lib/ansible/modules/cron.py @@ -618,12 +618,12 @@ def main(): if not name: module.deprecate( msg="The 'name' parameter will be required in future releases.", - version='ansible.builtin:2.12' + version='2.12', collection_name='ansible.builtin' ) if reboot: module.deprecate( msg="The 'reboot' parameter will be removed in future releases. Use 'special_time' option instead.", - version='ansible.builtin:2.12' + version='2.12', collection_name='ansible.builtin' ) if module._diff: diff --git a/lib/ansible/modules/get_url.py b/lib/ansible/modules/get_url.py index 7a48067c3c6..f9a0bb3212f 100644 --- a/lib/ansible/modules/get_url.py +++ b/lib/ansible/modules/get_url.py @@ -446,10 +446,12 @@ def main(): ) if module.params.get('thirsty'): - module.deprecate('The alias "thirsty" has been deprecated and will be removed, use "force" instead', version='ansible.builtin:2.13') + module.deprecate('The alias "thirsty" has been deprecated and will be removed, use "force" instead', + version='2.13', collection_name='ansible.builtin') if module.params.get('sha256sum'): - module.deprecate('The parameter "sha256sum" has been deprecated and will be removed, use "checksum" instead', version='ansible.builtin:2.14') + module.deprecate('The parameter "sha256sum" has been deprecated and will be removed, use "checksum" instead', + version='2.14', collection_name='ansible.builtin') url = module.params['url'] dest = module.params['dest'] diff --git a/lib/ansible/modules/systemd.py b/lib/ansible/modules/systemd.py index 0f0263e1998..b1eefae1037 100644 --- a/lib/ansible/modules/systemd.py +++ b/lib/ansible/modules/systemd.py @@ -357,7 +357,7 @@ def main(): ''' Set CLI options depending on params ''' if module.params['user'] is not None: # handle user deprecation, mutually exclusive with scope - module.deprecate("The 'user' option is being replaced by 'scope'", version='ansible.builtin:2.11') + module.deprecate("The 'user' option is being replaced by 'scope'", version='2.11', collection_name='ansible.builtin') if module.params['user']: module.params['scope'] = 'user' else: diff --git a/lib/ansible/modules/uri.py b/lib/ansible/modules/uri.py index 3cc4273230d..1db9b931f31 100644 --- a/lib/ansible/modules/uri.py +++ b/lib/ansible/modules/uri.py @@ -611,7 +611,8 @@ def main(): ) if module.params.get('thirsty'): - module.deprecate('The alias "thirsty" has been deprecated and will be removed, use "force" instead', version='ansible.builtin:2.13') + module.deprecate('The alias "thirsty" has been deprecated and will be removed, use "force" instead', + version='2.13', collection_name='ansible.builtin') url = module.params['url'] body = module.params['body'] diff --git a/lib/ansible/playbook/__init__.py b/lib/ansible/playbook/__init__.py index bf02c5c6b49..c7dab6b4d1b 100644 --- a/lib/ansible/playbook/__init__.py +++ b/lib/ansible/playbook/__init__.py @@ -79,7 +79,8 @@ class Playbook: self._loader.set_basedir(cur_basedir) raise AnsibleParserError("A playbook must be a list of plays, got a %s instead" % type(ds), obj=ds) elif not ds: - display.deprecated("Empty plays will currently be skipped, in the future they will cause a syntax error", version='ansible.builtin:2.12') + display.deprecated("Empty plays will currently be skipped, in the future they will cause a syntax error", + version='2.12', collection_name='ansible.builtin') # Parse the playbook entries. For plays, we simply parse them # using the Play() object, and includes are parsed using the @@ -92,7 +93,8 @@ class Playbook: if any(action in entry for action in ('import_playbook', 'include')): if 'include' in entry: - display.deprecated("'include' for playbook includes. You should use 'import_playbook' instead", version="ansible.builtin:2.12") + display.deprecated("'include' for playbook includes. You should use 'import_playbook' instead", + version="2.12", collection_name='ansible.builtin') pb = PlaybookInclude.load(entry, basedir=self._basedir, variable_manager=variable_manager, loader=self._loader) if pb is not None: self._entries.extend(pb._entries) diff --git a/lib/ansible/playbook/conditional.py b/lib/ansible/playbook/conditional.py index f94f475de12..a969d1a7529 100644 --- a/lib/ansible/playbook/conditional.py +++ b/lib/ansible/playbook/conditional.py @@ -134,7 +134,8 @@ class Conditional: conditional = templar.template(conditional, disable_lookups=disable_lookups) if bare_vars_warning and not isinstance(conditional, bool): display.deprecated('evaluating %r as a bare variable, this behaviour will go away and you might need to add |bool' - ' to the expression in the future. Also see CONDITIONAL_BARE_VARS configuration toggle' % original, "ansible.builtin:2.12") + ' to the expression in the future. Also see CONDITIONAL_BARE_VARS configuration toggle' % original, + version="2.12", collection_name='ansible.builtin') if not isinstance(conditional, text_type) or conditional == "": return conditional diff --git a/lib/ansible/playbook/helpers.py b/lib/ansible/playbook/helpers.py index 6c4ccf9f9be..45ff837f83d 100644 --- a/lib/ansible/playbook/helpers.py +++ b/lib/ansible/playbook/helpers.py @@ -156,7 +156,8 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h is_static = True elif t.static is not None: display.deprecated("The use of 'static' has been deprecated. " - "Use 'import_tasks' for static inclusion, or 'include_tasks' for dynamic inclusion", version='ansible.builtin:2.12') + "Use 'import_tasks' for static inclusion, or 'include_tasks' for dynamic inclusion", + version='2.12', collection_name='ansible.builtin') is_static = t.static else: is_static = C.DEFAULT_TASK_INCLUDES_STATIC or \ @@ -257,7 +258,8 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h "later. In the future, this will be an error unless 'static: no' is used " "on the include task. If you do not want missing includes to be considered " "dynamic, use 'static: yes' on the include or set the global ansible.cfg " - "options to make all includes static for tasks and/or handlers" % include_file, version="ansible.builtin:2.12" + "options to make all includes static for tasks and/or handlers" % include_file, + version="2.12", collection_name='ansible.builtin' ) task_list.append(t) continue @@ -294,7 +296,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h suppress_extended_error=True, ) display.deprecated("You should not specify tags in the include parameters. All tags should be specified using the task-level option", - version="ansible.builtin:2.12") + version="2.12", collection_name='ansible.builtin') else: tags = ti_copy.tags[:] @@ -332,7 +334,8 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h elif ir.static is not None: display.deprecated("The use of 'static' for 'include_role' has been deprecated. " - "Use 'import_role' for static inclusion, or 'include_role' for dynamic inclusion", version='ansible.builtin:2.12') + "Use 'import_role' for static inclusion, or 'include_role' for dynamic inclusion", + version='2.12', collection_name='ansible.builtin') is_static = ir.static if is_static: diff --git a/lib/ansible/playbook/play_context.py b/lib/ansible/playbook/play_context.py index 48beb746007..b7efe9206b7 100644 --- a/lib/ansible/playbook/play_context.py +++ b/lib/ansible/playbook/play_context.py @@ -342,7 +342,7 @@ class PlayContext(Base): """ helper function to create privilege escalation commands """ display.deprecated( "PlayContext.make_become_cmd should not be used, the calling code should be using become plugins instead", - version="ansible.builtin:2.12" + version="2.12", collection_name='ansible.builtin' ) if not cmd or not self.become: diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 2475b61c7c8..405316d3f8a 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -163,7 +163,8 @@ class Task(Base, Conditional, Taggable, CollectionSearch): raise AnsibleError("you must specify a value when using %s" % k, obj=ds) new_ds['loop_with'] = loop_name new_ds['loop'] = v - # display.deprecated("with_ type loops are being phased out, use the 'loop' keyword instead", version="ansible.builtin:2.10") + # display.deprecated("with_ type loops are being phased out, use the 'loop' keyword instead", + # version="2.10", collection_name='ansible.builtin') def preprocess_data(self, ds): ''' @@ -258,7 +259,8 @@ class Task(Base, Conditional, Taggable, CollectionSearch): if action in ('include',) and k not in self._valid_attrs and k not in self.DEPRECATED_ATTRIBUTES: display.deprecated("Specifying include variables at the top-level of the task is deprecated." " Please see:\nhttps://docs.ansible.com/ansible/playbooks_roles.html#task-include-files-and-encouraging-reuse\n\n" - " for currently supported syntax regarding included files and variables", version="ansible.builtin:2.12") + " for currently supported syntax regarding included files and variables", + version="2.12", collection_name='ansible.builtin') new_ds['vars'][k] = v elif C.INVALID_TASK_ATTRIBUTE_FAILED or k in self._valid_attrs: new_ds[k] = v diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 614c61494db..fc5cf9021d1 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -831,7 +831,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): msg = "Setting the async dir from the environment keyword " \ "ANSIBLE_ASYNC_DIR is deprecated. Set the async_dir " \ "shell option instead" - self._display.deprecated(msg, "ansible.builtin:2.12") + self._display.deprecated(msg, "2.12", collection_name='ansible.builtin') else: # ANSIBLE_ASYNC_DIR is not set on the task, we get the value # from the shell option and temporarily add to the environment diff --git a/lib/ansible/plugins/action/async_status.py b/lib/ansible/plugins/action/async_status.py index 40ce563e41a..e7273c305d4 100644 --- a/lib/ansible/plugins/action/async_status.py +++ b/lib/ansible/plugins/action/async_status.py @@ -33,7 +33,7 @@ class ActionModule(ActionBase): msg = "Setting the async dir from the environment keyword " \ "ANSIBLE_ASYNC_DIR is deprecated. Set the async_dir " \ "shell option instead" - self._display.deprecated(msg, "ansible.builtin:2.12") + self._display.deprecated(msg, "2.12", collection_name='ansible.builtin') else: # inject the async directory based on the shell option into the # module args diff --git a/lib/ansible/plugins/cache/__init__.py b/lib/ansible/plugins/cache/__init__.py index 3b337835d0a..a91b66afe77 100644 --- a/lib/ansible/plugins/cache/__init__.py +++ b/lib/ansible/plugins/cache/__init__.py @@ -50,7 +50,7 @@ class FactCache(RealFactCache): ' ansible.vars.fact_cache.FactCache. If you are looking for the class' ' to subclass for a cache plugin, you want' ' ansible.plugins.cache.BaseCacheModule or one of its subclasses.', - version='ansible.builtin:2.12') + version='2.12', collection_name='ansible.builtin') super(FactCache, self).__init__(*args, **kwargs) @@ -62,7 +62,8 @@ class BaseCacheModule(AnsiblePlugin): def __init__(self, *args, **kwargs): # Third party code is not using cache_loader to load plugin - fall back to previous behavior if not hasattr(self, '_load_name'): - display.deprecated('Rather than importing custom CacheModules directly, use ansible.plugins.loader.cache_loader', version='ansible.builtin:2.14') + display.deprecated('Rather than importing custom CacheModules directly, use ansible.plugins.loader.cache_loader', + version='2.14', collection_name='ansible.builtin') self._load_name = self.__module__.split('.')[-1] self._load_name = resource_from_fqcr(self.__module__) super(BaseCacheModule, self).__init__() diff --git a/lib/ansible/plugins/callback/__init__.py b/lib/ansible/plugins/callback/__init__.py index 2ad7511ccc4..442b1c87e81 100644 --- a/lib/ansible/plugins/callback/__init__.py +++ b/lib/ansible/plugins/callback/__init__.py @@ -242,7 +242,8 @@ class CallbackBase(AnsiblePlugin): def _get_item(self, result): ''' here for backwards compat, really should have always been named: _get_item_label''' cback = getattr(self, 'NAME', os.path.basename(__file__)) - self._display.deprecated("The %s callback plugin should be updated to use the _get_item_label method instead" % cback, version="ansible.builtin:2.11") + self._display.deprecated("The %s callback plugin should be updated to use the _get_item_label method instead" % cback, + version="2.11", collection_name='ansible.builtin') return self._get_item_label(result) def _process_items(self, result): diff --git a/lib/ansible/plugins/connection/__init__.py b/lib/ansible/plugins/connection/__init__.py index 230a6d777ee..0b44295fc27 100644 --- a/lib/ansible/plugins/connection/__init__.py +++ b/lib/ansible/plugins/connection/__init__.py @@ -237,28 +237,28 @@ class ConnectionBase(AnsiblePlugin): def check_become_success(self, b_output): display.deprecated( "Connection.check_become_success is deprecated, calling code should be using become plugins instead", - version="ansible.builtin:2.12" + version="2.12", collection_name='ansible.builtin' ) return self.become.check_success(b_output) def check_password_prompt(self, b_output): display.deprecated( "Connection.check_password_prompt is deprecated, calling code should be using become plugins instead", - version="ansible.builtin:2.12" + version="2.12", collection_name='ansible.builtin' ) return self.become.check_password_prompt(b_output) def check_incorrect_password(self, b_output): display.deprecated( "Connection.check_incorrect_password is deprecated, calling code should be using become plugins instead", - version="ansible.builtin:2.12" + version="2.12", collection_name='ansible.builtin' ) return self.become.check_incorrect_password(b_output) def check_missing_password(self, b_output): display.deprecated( "Connection.check_missing_password is deprecated, calling code should be using become plugins instead", - version="ansible.builtin:2.12" + version="2.12", collection_name='ansible.builtin' ) return self.become.check_missing_password(b_output) diff --git a/lib/ansible/plugins/inventory/__init__.py b/lib/ansible/plugins/inventory/__init__.py index b82dcdea95a..d34c1e0a0f8 100644 --- a/lib/ansible/plugins/inventory/__init__.py +++ b/lib/ansible/plugins/inventory/__init__.py @@ -291,19 +291,21 @@ class DeprecatedCache(object): display.deprecated('InventoryModule should utilize self._cache as a dict instead of self.cache. ' 'When expecting a KeyError, use self._cache[key] instead of using self.cache.get(key). ' 'self._cache is a dictionary and will return a default value instead of raising a KeyError ' - 'when the key does not exist', version='ansible.builtin:2.12') + 'when the key does not exist', version='2.12', collection_name='ansible.builtin') return self.real_cacheable._cache[key] def set(self, key, value): display.deprecated('InventoryModule should utilize self._cache as a dict instead of self.cache. ' 'To set the self._cache dictionary, use self._cache[key] = value instead of self.cache.set(key, value). ' 'To force update the underlying cache plugin with the contents of self._cache before parse() is complete, ' - 'call self.set_cache_plugin and it will use the self._cache dictionary to update the cache plugin', version='ansible.builtin:2.12') + 'call self.set_cache_plugin and it will use the self._cache dictionary to update the cache plugin', + version='2.12', collection_name='ansible.builtin') self.real_cacheable._cache[key] = value self.real_cacheable.set_cache_plugin() def __getattr__(self, name): - display.deprecated('InventoryModule should utilize self._cache instead of self.cache', version='ansible.builtin:2.12') + display.deprecated('InventoryModule should utilize self._cache instead of self.cache', + version='2.12', collection_name='ansible.builtin') return self.real_cacheable._cache.__getattribute__(name) diff --git a/lib/ansible/plugins/inventory/script.py b/lib/ansible/plugins/inventory/script.py index 05bfb0cfd8c..b4094a569f2 100644 --- a/lib/ansible/plugins/inventory/script.py +++ b/lib/ansible/plugins/inventory/script.py @@ -97,7 +97,7 @@ class InventoryModule(BaseInventoryPlugin, Cacheable): display.deprecated( msg="The 'cache' option is deprecated for the script inventory plugin. " "External scripts implement their own caching and this option has never been used", - version="ansible.builtin:2.12" + version="2.12", collection_name='ansible.builtin' ) # Support inventory scripts that are not prefixed with some diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index 4b4e6c2a3e1..c6e03ab35ca 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -71,7 +71,7 @@ def SharedPluginLoaderObj(): '''This only exists for backwards compat, do not use. ''' display.deprecated('SharedPluginLoaderObj is deprecated, please directly use ansible.plugins.loader', - version='ansible.builtin:2.11') + version='2.11', collection_name='ansible.builtin') return plugin_loader @@ -918,7 +918,7 @@ class StrategyBase: "Mixing tag specify styles is prohibited for whole import hierarchy, not only for single import statement", obj=included_file._task._ds) display.deprecated("You should not specify tags in the include parameters. All tags should be specified using the task-level option", - version='ansible.builtin:2.12') + version='2.12', collection_name='ansible.builtin') included_file._task.tags = tags block_list = load_list_of_blocks( diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index 33b31e598ed..0881f8623be 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -661,7 +661,7 @@ class Templar: def set_available_variables(self, variables): display.deprecated( 'set_available_variables is being deprecated. Use "@available_variables.setter" instead.', - version='ansible.builtin:2.13' + version='2.13', collection_name='ansible.builtin' ) self.available_variables = variables diff --git a/lib/ansible/utils/display.py b/lib/ansible/utils/display.py index 1f5f2ee448c..3943522af8a 100644 --- a/lib/ansible/utils/display.py +++ b/lib/ansible/utils/display.py @@ -255,19 +255,6 @@ class Display(with_metaclass(Singleton, object)): if msg and msg[-1] not in ['!', '?', '.']: msg += '.' - # split composite collection info (if any) off date/version and use it if not otherwise spec'd - if date and isinstance(date, string_types): - parts = to_native(date.strip()).split(':', 1) - date = parts[-1] - if len(parts) == 2 and not collection_name: - collection_name = parts[0] - - if version and isinstance(version, string_types): - parts = to_native(version.strip()).split(':', 1) - version = parts[-1] - if len(parts) == 2 and not collection_name: - collection_name = parts[0] - if collection_name == 'ansible.builtin': collection_name = 'ansible-base' diff --git a/lib/ansible/utils/plugin_docs.py b/lib/ansible/utils/plugin_docs.py index b46e50de2c6..26388476316 100644 --- a/lib/ansible/utils/plugin_docs.py +++ b/lib/ansible/utils/plugin_docs.py @@ -40,26 +40,32 @@ def merge_fragment(target, source): target[key] = value -def _process_versions_and_dates(fragment, is_module, callback): +def _process_versions_and_dates(fragment, is_module, return_docs, callback): def process_deprecation(deprecation): + if not isinstance(deprecation, MutableMapping): + return if is_module and 'removed_in' in deprecation: # used in module deprecations - callback(deprecation, 'removed_in') + callback(deprecation, 'removed_in', 'removed_from_collection') if 'removed_at_date' in deprecation: - callback(deprecation, 'removed_at_date') + callback(deprecation, 'removed_at_date', 'removed_from_collection') if not is_module and 'version' in deprecation: # used in plugin option deprecations - callback(deprecation, 'version') + callback(deprecation, 'version', 'removed_from_collection') def process_option_specifiers(specifiers): for specifier in specifiers: + if not isinstance(specifier, MutableMapping): + continue if 'version_added' in specifier: - callback(specifier, 'version_added') - if isinstance(specifier.get('deprecated'), dict): + callback(specifier, 'version_added', 'version_added_collection') + if isinstance(specifier.get('deprecated'), MutableMapping): process_deprecation(specifier['deprecated']) def process_options(options): for option in options.values(): + if not isinstance(option, MutableMapping): + continue if 'version_added' in option: - callback(option, 'version_added') + callback(option, 'version_added', 'version_added_collection') if not is_module: if isinstance(option.get('env'), list): process_option_specifiers(option['env']) @@ -67,40 +73,44 @@ def _process_versions_and_dates(fragment, is_module, callback): process_option_specifiers(option['ini']) if isinstance(option.get('vars'), list): process_option_specifiers(option['vars']) - if isinstance(option.get('suboptions'), dict): + if isinstance(option.get('suboptions'), MutableMapping): process_options(option['suboptions']) def process_return_values(return_values): for return_value in return_values.values(): + if not isinstance(return_value, MutableMapping): + continue if 'version_added' in return_value: - callback(return_value, 'version_added') - if isinstance(return_value.get('contains'), dict): + callback(return_value, 'version_added', 'version_added_collection') + if isinstance(return_value.get('contains'), MutableMapping): process_return_values(return_value['contains']) + if return_docs: + process_return_values(fragment) + return + if 'version_added' in fragment: - callback(fragment, 'version_added') - if isinstance(fragment.get('deprecated'), dict): + callback(fragment, 'version_added', 'version_added_collection') + if isinstance(fragment.get('deprecated'), MutableMapping): process_deprecation(fragment['deprecated']) - if isinstance(fragment.get('options'), dict): + if isinstance(fragment.get('options'), MutableMapping): process_options(fragment['options']) -def tag_versions_and_dates(fragment, prefix, is_module): - def tag(options, option): - options[option] = '%s%s' % (prefix, options[option]) +def add_collection_to_versions_and_dates(fragment, collection_name, is_module, return_docs=False): + def add(options, option, collection_name_field): + if collection_name_field not in options: + options[collection_name_field] = collection_name - _process_versions_and_dates(fragment, is_module, tag) + _process_versions_and_dates(fragment, is_module, return_docs, add) -def untag_versions_and_dates(fragment, prefix, is_module): - def untag(options, option): - v = options[option] - if isinstance(v, string_types): - v = to_native(v) - if v.startswith(prefix): - options[option] = v[len(prefix):] +def remove_current_collection_from_versions_and_dates(fragment, collection_name, is_module, return_docs=False): + def remove(options, option, collection_name_field): + if options.get(collection_name_field) == collection_name: + del options[collection_name_field] - _process_versions_and_dates(fragment, is_module, untag) + _process_versions_and_dates(fragment, is_module, return_docs, remove) def add_fragments(doc, filename, fragment_loader, is_module=False): @@ -147,7 +157,7 @@ def add_fragments(doc, filename, fragment_loader, is_module=False): real_fragment_name = getattr(fragment_class, '_load_name') if real_fragment_name.startswith('ansible_collections.'): real_collection_name = '.'.join(real_fragment_name.split('.')[1:3]) - tag_versions_and_dates(fragment, '%s:' % (real_collection_name, ), is_module=is_module) + add_collection_to_versions_and_dates(fragment, real_collection_name, is_module=is_module) if 'notes' in fragment: notes = fragment.pop('notes') @@ -195,7 +205,7 @@ def get_docstring(filename, fragment_loader, verbose=False, ignore_errors=False, if data.get('doc', False): # tag version_added if collection_name is not None: - tag_versions_and_dates(data['doc'], '%s:' % (collection_name, ), is_module=is_module) + add_collection_to_versions_and_dates(data['doc'], collection_name, is_module=is_module) # add fragments to documentation add_fragments(data['doc'], filename, fragment_loader=fragment_loader, is_module=is_module) diff --git a/lib/ansible/utils/unsafe_proxy.py b/lib/ansible/utils/unsafe_proxy.py index 936d881b10c..8c5d22615e4 100644 --- a/lib/ansible/utils/unsafe_proxy.py +++ b/lib/ansible/utils/unsafe_proxy.py @@ -83,7 +83,7 @@ class UnsafeProxy(object): from ansible.utils.display import Display Display().deprecated( 'UnsafeProxy is being deprecated. Use wrap_var or AnsibleUnsafeBytes/AnsibleUnsafeText directly instead', - version='ansible.builtin:2.13' + version='2.13', collection_name='ansible.builtin' ) # In our usage we should only receive unicode strings. # This conditional and conversion exists to sanity check the values diff --git a/lib/ansible/vars/fact_cache.py b/lib/ansible/vars/fact_cache.py index cab97d3840b..a8e63bc5869 100644 --- a/lib/ansible/vars/fact_cache.py +++ b/lib/ansible/vars/fact_cache.py @@ -99,7 +99,7 @@ class FactCache(MutableMapping): display.deprecated('Calling FactCache().update(key, value) is deprecated. Use' ' FactCache().first_order_merge(key, value) if you want the old' ' behaviour or use FactCache().update({key: value}) if you want' - ' dict-like behaviour.', version='ansible.builtin:2.12') + ' dict-like behaviour.', version='2.12', collection_name='ansible.builtin') return self.first_order_merge(*args) elif len(args) == 1: diff --git a/test/integration/targets/incidental_xml/tasks/test-get-element-content.yml b/test/integration/targets/incidental_xml/tasks/test-get-element-content.yml index e1f29945c7c..58ca7767e76 100644 --- a/test/integration/targets/incidental_xml/tasks/test-get-element-content.yml +++ b/test/integration/targets/incidental_xml/tasks/test-get-element-content.yml @@ -36,7 +36,7 @@ - get_element_attribute_wrong.matches[0]['rating']['subjective'] == 'true' - get_element_attribute_wrong.deprecations is defined - get_element_attribute_wrong.deprecations[0].msg == "Parameter 'attribute=subjective' is ignored when using 'content=attribute' only 'xpath' is used. Please remove entry." - - get_element_attribute_wrong.deprecations[0].version == 'ansible.builtin:2.12' + - get_element_attribute_wrong.deprecations[0].version == '2.12' - name: Get element text xml: diff --git a/test/integration/targets/module_utils_Ansible.Basic/library/ansible_basic_tests.ps1 b/test/integration/targets/module_utils_Ansible.Basic/library/ansible_basic_tests.ps1 index 6eaafda9ea7..9278e386828 100644 --- a/test/integration/targets/module_utils_Ansible.Basic/library/ansible_basic_tests.ps1 +++ b/test/integration/targets/module_utils_Ansible.Basic/library/ansible_basic_tests.ps1 @@ -757,10 +757,12 @@ test_no_log - Invoked with: options = @{ removed1 = @{removed_in_version = "2.1"} removed2 = @{removed_in_version = "2.2"} + removed3 = @{removed_in_version = "2.3"; removed_from_collection = "ansible.builtin"} } } Set-Variable -Name complex_args -Scope Global -Value @{ removed1 = "value" + removed3 = "value" } $m = [Ansible.Basic.AnsibleModule]::Create(@(), $spec) @@ -781,12 +783,19 @@ test_no_log - Invoked with: module_args = @{ removed1 = "value" removed2 = $null + removed3 = "value" } } deprecations = @( + @{ + msg = "Param 'removed3' is deprecated. See the module docs for more information" + version = "2.3" + collection_name = "ansible.builtin" + }, @{ msg = "Param 'removed1' is deprecated. See the module docs for more information" version = "2.1" + collection_name = $null } ) } @@ -798,10 +807,12 @@ test_no_log - Invoked with: options = @{ removed1 = @{removed_at_date = [DateTime]"2020-03-10"} removed2 = @{removed_at_date = [DateTime]"2020-03-11"} + removed3 = @{removed_at_date = [DateTime]"2020-06-07"; removed_from_collection = "ansible.builtin"} } } Set-Variable -Name complex_args -Scope Global -Value @{ removed1 = "value" + removed3 = "value" } $m = [Ansible.Basic.AnsibleModule]::Create(@(), $spec) @@ -822,12 +833,19 @@ test_no_log - Invoked with: module_args = @{ removed1 = "value" removed2 = $null + removed3 = "value" } } deprecations = @( + @{ + msg = "Param 'removed3' is deprecated. See the module docs for more information" + date = "2020-06-07" + collection_name = "ansible.builtin" + }, @{ msg = "Param 'removed1' is deprecated. See the module docs for more information" date = "2020-03-10" + collection_name = $null } ) } @@ -844,12 +862,16 @@ test_no_log - Invoked with: options = @{ option1 = @{ type = "str"; aliases = "alias1"; deprecated_aliases = @(@{name = "alias1"; version = "2.10"}) } option2 = @{ type = "str"; aliases = "alias2"; deprecated_aliases = @(@{name = "alias2"; version = "2.11"}) } - option3 = @{ type = "str"; aliases = "alias3"; deprecated_aliases = @(@{name = "alias3"; date = [DateTime]"2020-03-11"}) } - option4 = @{ type = "str"; aliases = "alias4"; deprecated_aliases = @(@{name = "alias4"; date = [DateTime]"2020-03-09"}) } + option3 = @{ type = "str"; aliases = "alias3"; deprecated_aliases = @(@{name = "alias3"; version = "2.12"; collection_name = "ansible.builtin"}) } + option4 = @{ type = "str"; aliases = "alias4"; deprecated_aliases = @(@{name = "alias4"; date = [DateTime]"2020-03-11"}) } + option5 = @{ type = "str"; aliases = "alias5"; deprecated_aliases = @(@{name = "alias5"; date = [DateTime]"2020-03-09"}) } + option6 = @{ type = "str"; aliases = "alias6"; deprecated_aliases = @(@{name = "alias6"; date = [DateTime]"2020-06-01"; collection_name = "ansible.builtin"}) } } } option4 = @{ type = "str"; aliases = "alias4"; deprecated_aliases = @(@{name = "alias4"; date = [DateTime]"2020-03-10"}) } option5 = @{ type = "str"; aliases = "alias5"; deprecated_aliases = @(@{name = "alias5"; date = [DateTime]"2020-03-12"}) } + option6 = @{ type = "str"; aliases = "alias6"; deprecated_aliases = @(@{name = "alias6"; version = "2.12"; collection_name = "ansible.builtin"}) } + option7 = @{ type = "str"; aliases = "alias7"; deprecated_aliases = @(@{name = "alias7"; date = [DateTime]"2020-06-07"; collection_name = "ansible.builtin"}) } } } @@ -859,11 +881,15 @@ test_no_log - Invoked with: option3 = @{ option1 = "option1" alias2 = "alias2" - option3 = "option3" - alias4 = "alias4" + alias3 = "alias3" + option4 = "option4" + alias5 = "alias5" + alias6 = "alias6" } option4 = "option4" alias5 = "alias5" + alias6 = "alias6" + alias7 = "alias7" } $m = [Ansible.Basic.AnsibleModule]::Create(@(), $spec) @@ -888,31 +914,63 @@ test_no_log - Invoked with: option1 = "option1" option2 = "alias2" alias2 = "alias2" - option3 = "option3" - option4 = "alias4" - alias4 = "alias4" + option3 = "alias3" + alias3 = "alias3" + option4 = "option4" + option5 = "alias5" + alias5 = "alias5" + option6 = "alias6" + alias6 = "alias6" } option4 = "option4" option5 = "alias5" alias5 = "alias5" + option6 = "alias6" + alias6 = "alias6" + option7 = "alias7" + alias7 = "alias7" } } deprecations = @( + @{ + msg = "Alias 'alias7' is deprecated. See the module docs for more information" + date = "2020-06-07" + collection_name = "ansible.builtin" + }, @{ msg = "Alias 'alias1' is deprecated. See the module docs for more information" version = "2.10" + collection_name = $null }, @{ msg = "Alias 'alias5' is deprecated. See the module docs for more information" date = "2020-03-12" - } + collection_name = $null + }, + @{ + msg = "Alias 'alias6' is deprecated. See the module docs for more information" + version = "2.12" + collection_name = "ansible.builtin" + }, @{ msg = "Alias 'alias2' is deprecated. See the module docs for more information - found in option3" version = "2.11" - } + collection_name = $null + }, @{ - msg = "Alias 'alias4' is deprecated. See the module docs for more information - found in option3" + msg = "Alias 'alias5' is deprecated. See the module docs for more information - found in option3" date = "2020-03-09" + collection_name = $null + }, + @{ + msg = "Alias 'alias3' is deprecated. See the module docs for more information - found in option3" + version = "2.12" + collection_name = "ansible.builtin" + }, + @{ + msg = "Alias 'alias6' is deprecated. See the module docs for more information - found in option3" + date = "2020-06-01" + collection_name = "ansible.builtin" } ) } @@ -1141,12 +1199,15 @@ test_no_log - Invoked with: "Deprecate and warn with version" = { $m = [Ansible.Basic.AnsibleModule]::Create(@(), @{}) - $m.Deprecate("message", "2.8") - $actual_deprecate_event = Get-EventLog -LogName Application -Source Ansible -Newest 1 + $m.Deprecate("message", "2.7") + $actual_deprecate_event_1 = Get-EventLog -LogName Application -Source Ansible -Newest 1 + $m.Deprecate("message w collection", "2.8", "ansible.builtin") + $actual_deprecate_event_2 = Get-EventLog -LogName Application -Source Ansible -Newest 1 $m.Warn("warning") $actual_warn_event = Get-EventLog -LogName Application -Source Ansible -Newest 1 - $actual_deprecate_event.Message | Assert-Equals -Expected "undefined win module - [DEPRECATION WARNING] message 2.8" + $actual_deprecate_event_1.Message | Assert-Equals -Expected "undefined win module - [DEPRECATION WARNING] message 2.7" + $actual_deprecate_event_2.Message | Assert-Equals -Expected "undefined win module - [DEPRECATION WARNING] message w collection 2.8" $actual_warn_event.EntryType | Assert-Equals -Expected "Warning" $actual_warn_event.Message | Assert-Equals -Expected "undefined win module - [WARNING] warning" @@ -1166,19 +1227,25 @@ test_no_log - Invoked with: module_args = @{} } warnings = @("warning") - deprecations = @(@{msg = "message"; version = "2.8"}) + deprecations = @( + @{msg = "message"; version = "2.7"; collection_name = $null}, + @{msg = "message w collection"; version = "2.8"; collection_name = "ansible.builtin"} + ) } $actual | Assert-DictionaryEquals -Expected $expected } "Deprecate and warn with date" = { $m = [Ansible.Basic.AnsibleModule]::Create(@(), @{}) - $m.Deprecate("message", [DateTime]"2020-01-02") - $actual_deprecate_event = Get-EventLog -LogName Application -Source Ansible -Newest 1 + $m.Deprecate("message", [DateTime]"2020-01-01") + $actual_deprecate_event_1 = Get-EventLog -LogName Application -Source Ansible -Newest 1 + $m.Deprecate("message w collection", [DateTime]"2020-01-02", "ansible.builtin") + $actual_deprecate_event_2 = Get-EventLog -LogName Application -Source Ansible -Newest 1 $m.Warn("warning") $actual_warn_event = Get-EventLog -LogName Application -Source Ansible -Newest 1 - $actual_deprecate_event.Message | Assert-Equals -Expected "undefined win module - [DEPRECATION WARNING] message 2020-01-02" + $actual_deprecate_event_1.Message | Assert-Equals -Expected "undefined win module - [DEPRECATION WARNING] message 2020-01-01" + $actual_deprecate_event_2.Message | Assert-Equals -Expected "undefined win module - [DEPRECATION WARNING] message w collection 2020-01-02" $actual_warn_event.EntryType | Assert-Equals -Expected "Warning" $actual_warn_event.Message | Assert-Equals -Expected "undefined win module - [WARNING] warning" @@ -1198,7 +1265,10 @@ test_no_log - Invoked with: module_args = @{} } warnings = @("warning") - deprecations = @(@{msg = "message"; date = "2020-01-02"}) + deprecations = @( + @{msg = "message"; date = "2020-01-01"; collection_name = $null}, + @{msg = "message w collection"; date = "2020-01-02"; collection_name = "ansible.builtin"} + ) } $actual | Assert-DictionaryEquals -Expected $expected } @@ -1627,8 +1697,8 @@ test_no_log - Invoked with: $expected_msg = "internal error: argument spec entry contains an invalid key 'invalid', valid keys: apply_defaults, " $expected_msg += "aliases, choices, default, deprecated_aliases, elements, mutually_exclusive, no_log, options, " - $expected_msg += "removed_in_version, removed_at_date, required, required_by, required_if, required_one_of, " - $expected_msg += "required_together, supports_check_mode, type" + $expected_msg += "removed_in_version, removed_at_date, removed_from_collection, required, required_by, required_if, " + $expected_msg += "required_one_of, required_together, supports_check_mode, type" $actual.Keys.Count | Assert-Equals -Expected 3 $actual.failed | Assert-Equals -Expected $true @@ -1660,8 +1730,8 @@ test_no_log - Invoked with: $expected_msg = "internal error: argument spec entry contains an invalid key 'invalid', valid keys: apply_defaults, " $expected_msg += "aliases, choices, default, deprecated_aliases, elements, mutually_exclusive, no_log, options, " - $expected_msg += "removed_in_version, removed_at_date, required, required_by, required_if, required_one_of, " - $expected_msg += "required_together, supports_check_mode, type - found in option_key -> sub_option_key" + $expected_msg += "removed_in_version, removed_at_date, removed_from_collection, required, required_by, required_if, " + $expected_msg += "required_one_of, required_together, supports_check_mode, type - found in option_key -> sub_option_key" $actual.Keys.Count | Assert-Equals -Expected 3 $actual.failed | Assert-Equals -Expected $true @@ -2747,7 +2817,7 @@ test_no_log - Invoked with: option2 = @{ aliases = @("alias2_spec") deprecated_aliases = @( - @{name = "alias2_spec"; version = "2.0"} + @{name = "alias2_spec"; version = "2.0"; collection_name = "ansible.builtin"} ) } } @@ -2761,7 +2831,7 @@ test_no_log - Invoked with: option2 = @{ aliases = @("alias2") deprecated_aliases = @( - @{name = "alias2"; version = "2.0"} + @{name = "alias2"; version = "2.0"; collection_name = "foo.bar"} ) type = "str" } @@ -2786,10 +2856,10 @@ test_no_log - Invoked with: $actual.deprecations.Count | Assert-Equals -Expected 2 $actual.deprecations[0] | Assert-DictionaryEquals -Expected @{ - msg = "Alias 'alias1_spec' is deprecated. See the module docs for more information"; version = "2.0" + msg = "Alias 'alias1_spec' is deprecated. See the module docs for more information"; version = "2.0"; collection_name = $null } $actual.deprecations[1] | Assert-DictionaryEquals -Expected @{ - msg = "Alias 'alias2' is deprecated. See the module docs for more information"; version = "2.0" + msg = "Alias 'alias2' is deprecated. See the module docs for more information"; version = "2.0"; collection_name = "foo.bar" } $actual.changed | Assert-Equals -Expected $false $actual.invocation | Assert-DictionaryEquals -Expected @{ @@ -2926,25 +2996,25 @@ test_no_log - Invoked with: # Single element of the same list type not in a list option1 = @{ aliases = "alias1" - deprecated_aliases = @{name="alias1";version="2.0"} + deprecated_aliases = @{name="alias1";version="2.0";collection_name="foo.bar"} } # Arrays option2 = @{ aliases = ,"alias2" - deprecated_aliases = ,@{name="alias2";version="2.0"} + deprecated_aliases = ,@{name="alias2";version="2.0";collection_name="foo.bar"} } # ArrayList option3 = @{ aliases = [System.Collections.ArrayList]@("alias3") - deprecated_aliases = [System.Collections.ArrayList]@(@{name="alias3";version="2.0"}) + deprecated_aliases = [System.Collections.ArrayList]@(@{name="alias3";version="2.0";collection_name="foo.bar"}) } # Generic.List[Object] option4 = @{ aliases = [System.Collections.Generic.List[Object]]@("alias4") - deprecated_aliases = [System.Collections.Generic.List[Object]]@(@{name="alias4";version="2.0"}) + deprecated_aliases = [System.Collections.Generic.List[Object]]@(@{name="alias4";version="2.0";collection_name="foo.bar"}) } # Generic.List[T] @@ -2954,7 +3024,7 @@ test_no_log - Invoked with: } } } - $spec.options.option5.deprecated_aliases.Add(@{name="alias5";version="2.0"}) + $spec.options.option5.deprecated_aliases.Add(@{name="alias5";version="2.0";collection_name="foo.bar"}) Set-Variable -Name complex_args -Scope Global -Value @{ alias1 = "option1" @@ -2980,6 +3050,7 @@ test_no_log - Invoked with: foreach ($dep in $actual.deprecations) { $dep.msg -like "Alias 'alias?' is deprecated. See the module docs for more information" | Assert-Equals -Expected $true $dep.version | Assert-Equals -Expected '2.0' + $dep.collection_name | Assert-Equals -Expected 'foo.bar' } $actual.invocation | Assert-DictionaryEquals -Expected @{ module_args = @{ 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 76f0d50b0d3..c88e5e5ae17 100644 --- a/test/lib/ansible_test/_data/sanity/pylint/plugins/deprecated.py +++ b/test/lib/ansible_test/_data/sanity/pylint/plugins/deprecated.py @@ -35,7 +35,7 @@ MSGS = { "Display.deprecated or AnsibleModule.deprecate", "ansible-invalid-deprecated-version", "Used when a call to Display.deprecated specifies an invalid " - "tagged Ansible version number", + "Ansible version number", {'minversion': (2, 6)}), 'E9504': ("Deprecated version (%r) found in call to Display.deprecated " "or AnsibleModule.deprecate", @@ -48,20 +48,21 @@ MSGS = { "Display.deprecated or AnsibleModule.deprecate", "collection-invalid-deprecated-version", "Used when a call to Display.deprecated specifies an invalid " - "tagged collection version number", + "collection version number", {'minversion': (2, 6)}), - 'E9506': ("Invalid tagged version (%r) found in call to " - "Display.deprecated or AnsibleModule.deprecate", - "invalid-tagged-version", - "Used when a call to Display.deprecated specifies a version " - "number which has no collection name tag, for example " - "`community.general:1.2.3` or `ansible.builtin:2.10`", + 'E9506': ("No collection name found in call to Display.deprecated or " + "AnsibleModule.deprecate", + "ansible-deprecated-no-collection-name", + "The current collection name in format `namespace.name` must " + "be provided as collection_name when calling Display.deprecated " + "or AnsibleModule.deprecate (`ansible.builtin` for ansible-base)", {'minversion': (2, 6)}), - 'E9507': ("Version tag for wrong collection (%r) found in call to " + 'E9507': ("Wrong collection name (%r) found in call to " "Display.deprecated or AnsibleModule.deprecate", - "wrong-collection-deprecated-version-tag", - "Deprecation versions must be prefixed with the name of this " - "collection (`ansible.builtin:` for Ansible-base)", + "wrong-collection-deprecated", + "The name of the current collection must be passed to the " + "Display.deprecated resp. AnsibleModule.deprecate calls " + "(`ansible.builtin` for ansible-base)", {'minversion': (2, 6)}), 'E9508': ("Expired date (%r) found in call to Display.deprecated " "or AnsibleModule.deprecate", @@ -73,34 +74,18 @@ MSGS = { "Display.deprecated or AnsibleModule.deprecate", "ansible-invalid-deprecated-date", "Used when a call to Display.deprecated specifies an invalid " - "date. It must be a string in format `namespace.name:YYYY-MM-DD` " - "(collection identifier followed by ISO 8601)", + "date. It must be a string in format `YYYY-MM-DD` (ISO 8601)", {'minversion': (2, 6)}), 'E9510': ("Both version and date found in call to " "Display.deprecated or AnsibleModule.deprecate", "ansible-deprecated-both-version-and-date", "Only one of version and date must be specified", {'minversion': (2, 6)}), - 'E9511': ("Invalid tagged date (%r) found in call to " - "Display.deprecated or AnsibleModule.deprecate", - "invalid-tagged-date", - "Used when a call to Display.deprecated specifies a date " - "which has no collection name tag, for example " - "`community.general:2020-01-01` or `ansible.builtin:2020-12-31`", - {'minversion': (2, 6)}), - 'E9512': ("Date tag for wrong collection (%r) found in call to " - "Display.deprecated or AnsibleModule.deprecate", - "wrong-collection-deprecated-date-tag", - "Deprecation dates must be prefixed with the name of this " - "collection (`ansible.builtin:` for Ansible-base)", - {'minversion': (2, 6)}), } ANSIBLE_VERSION = LooseVersion('.'.join(ansible_version_raw.split('.')[:3])) -TAGGED_VERSION_RE = re.compile('^([^.]+.[^.]+):(.*)$') - def _get_expr_name(node): """Funciton to get either ``attrname`` or ``name`` from ``node.func.expr`` @@ -142,7 +127,7 @@ class AnsibleDeprecatedChecker(BaseChecker): 'default': None, 'type': 'string', 'metavar': '', - 'help': 'The collection\'s name used to check tagged version numbers in deprecations.', + 'help': 'The collection\'s name used to check collection names in deprecations.', }), ('collection-version', { 'default': None, @@ -166,43 +151,26 @@ class AnsibleDeprecatedChecker(BaseChecker): def _check_date(self, node, date): if not isinstance(date, str): - self.add_message('invalid-tagged-date', node=node, args=(date,)) - return - - matcher = TAGGED_VERSION_RE.match(date) - if not matcher: - self.add_message('invalid-tagged-date', node=node, args=(date,)) + self.add_message('invalid-date', node=node, args=(date,)) return - collection = matcher.group(1) - date_str = matcher.group(2) - - if collection != (self.collection_name or 'ansible.builtin'): - self.add_message('wrong-collection-deprecated-date-tag', node=node, args=(date,)) - try: - if parse_isodate(date_str) < datetime.date.today(): - self.add_message('ansible-deprecated-date', node=node, args=(date,)) + date_parsed = parse_isodate(date) except ValueError: self.add_message('ansible-invalid-deprecated-date', node=node, args=(date,)) - - def _check_version(self, node, version): - if not isinstance(version, str): - self.add_message('invalid-tagged-version', node=node, args=(version,)) return - matcher = TAGGED_VERSION_RE.match(version) - if not matcher: - self.add_message('invalid-tagged-version', node=node, args=(version,)) - return + if date_parsed < datetime.date.today(): + self.add_message('ansible-deprecated-date', node=node, args=(date,)) - collection = matcher.group(1) - version_no = matcher.group(2) + def _check_version(self, node, version, collection_name): + if not isinstance(version, (str, float)): + self.add_message('invalid-version', node=node, args=(version,)) + return - if collection != (self.collection_name or 'ansible.builtin'): - self.add_message('wrong-collection-deprecated-version-tag', node=node, args=(version,)) + version_no = str(version) - if collection == 'ansible.builtin': + if collection_name == 'ansible.builtin': # Ansible-base try: if not version_no: @@ -212,13 +180,13 @@ class AnsibleDeprecatedChecker(BaseChecker): self.add_message('ansible-deprecated-version', node=node, args=(version,)) except ValueError: self.add_message('ansible-invalid-deprecated-version', node=node, args=(version,)) - else: + elif collection_name: # Collections try: if not version_no: raise ValueError('Version string should not be empty') semantic_version = SemanticVersion(version_no) - if collection == self.collection_name and self.collection_version is not None: + 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,)) except ValueError: @@ -228,6 +196,7 @@ class AnsibleDeprecatedChecker(BaseChecker): def visit_call(self, node): version = None date = None + collection_name = None try: if (node.func.attrname == 'deprecated' and 'display' in _get_expr_name(node) or node.func.attrname == 'deprecate' and _get_expr_name(node)): @@ -246,6 +215,11 @@ class AnsibleDeprecatedChecker(BaseChecker): # This is likely a variable return date = keyword.value.value + if keyword.arg == 'collection_name': + if isinstance(keyword.value.value, astroid.Name): + # This is likely a variable + return + collection_name = keyword.value.value if not version and not date: try: version = node.args[1].value @@ -254,13 +228,18 @@ class AnsibleDeprecatedChecker(BaseChecker): return if version and date: self.add_message('ansible-deprecated-both-version-and-date', node=node) - return + + if collection_name: + this_collection = collection_name == (self.collection_name or 'ansible.builtin') + if not this_collection: + self.add_message('wrong-collection-deprecated', node=node, args=(collection_name,)) + else: + self.add_message('ansible-deprecated-no-collection-name', node=node) if date: self._check_date(node, date) - - if version: - self._check_version(node, version) + elif version: + self._check_version(node, version, collection_name) except AttributeError: # Not the type of node we are interested in pass diff --git a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py index cd851dbc843..24781f32077 100644 --- a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py +++ b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py @@ -44,7 +44,7 @@ from ansible.module_utils.common._collections_compat import Mapping from ansible.module_utils._text import to_native from ansible.plugins.loader import fragment_loader from ansible.utils.collection_loader._collection_finder import _AnsibleCollectionFinder -from ansible.utils.plugin_docs import BLACKLIST, tag_versions_and_dates, add_fragments, get_docstring +from ansible.utils.plugin_docs import BLACKLIST, add_collection_to_versions_and_dates, add_fragments, get_docstring from ansible.utils.version import SemanticVersion from .module_args import AnsibleModuleImportError, AnsibleModuleNotInitialized, get_argument_spec @@ -288,14 +288,22 @@ class ModuleValidator(Validator): else: self.base_module = None - def _create_version(self, v): + def _create_version(self, v, collection_name=None): if not v: raise ValueError('Empty string is not a valid version') + if collection_name == 'ansible.builtin': + return LooseVersion(v) + if collection_name is not None: + return SemanticVersion(v) return self._Version(v) - def _create_strict_version(self, v): + def _create_strict_version(self, v, collection_name=None): if not v: raise ValueError('Empty string is not a valid version') + if collection_name == 'ansible.builtin': + return StrictVersion(v) + if collection_name is not None: + return SemanticVersion(v) return self._StrictVersion(v) def __enter__(self): @@ -887,24 +895,6 @@ class ModuleValidator(Validator): msg='%s: %s' % (combined_path, error_message) ) - @staticmethod - def _split_tagged_version(version_str): - if not isinstance(version_str, string_types): - raise ValueError('Tagged version must be string') - version_str = to_native(version_str) - if ':' not in version_str: - raise ValueError('Tagged version must have ":"') - return version_str.split(':', 1) - - @staticmethod - def _extract_version_from_tag_for_msg(version_str): - if not isinstance(version_str, string_types): - return version_str - version_str = to_native(version_str) - if ':' not in version_str: - return version_str - return version_str.split(':', 1)[1] - def _validate_docs(self): doc_info = self._get_docs() doc = None @@ -945,7 +935,7 @@ class ModuleValidator(Validator): self.name, 'DOCUMENTATION' ) if doc: - tag_versions_and_dates(doc, '%s:' % (self.collection_name, ), is_module=True) + add_collection_to_versions_and_dates(doc, self.collection_name, is_module=True) for error in errors: self.reporter.error( path=self.object_path, @@ -995,6 +985,14 @@ class ModuleValidator(Validator): if 'deprecated' in doc and doc.get('deprecated'): doc_deprecated = True doc_deprecation = doc['deprecated'] + documentation_collection = doc_deprecation.get('removed_from_collection') + if documentation_collection != self.collection_name: + self.reporter.error( + path=self.object_path, + code='deprecation-wrong-collection', + msg='"DOCUMENTATION.deprecation.removed_from_collection must be the current collection name: %r vs. %r' % ( + documentation_collection, self.collection_name) + ) else: doc_deprecated = False @@ -1067,6 +1065,8 @@ class ModuleValidator(Validator): data, errors, traces = parse_yaml(doc_info['RETURN']['value'], doc_info['RETURN']['lineno'], self.name, 'RETURN') + if data: + add_collection_to_versions_and_dates(data, self.collection_name, is_module=True, return_docs=True) self._validate_docs_schema(data, return_schema(for_collection=bool(self.collection)), 'RETURN', 'return-syntax-error') @@ -1124,8 +1124,8 @@ class ModuleValidator(Validator): routing_version = routing_deprecation.get('removal_version') # The versions and dates in the module documentation are auto-tagged, so remove the tag # to make comparison possible and to avoid confusing the user. - documentation_date = self._extract_version_from_tag_for_msg(doc_deprecation.get('removed_at_date')) - documentation_version = self._extract_version_from_tag_for_msg(doc_deprecation.get('removed_in')) + documentation_date = doc_deprecation.get('removed_at_date') + documentation_version = doc_deprecation.get('removed_in') if routing_date != documentation_date: self.reporter.error( path=self.object_path, @@ -1148,45 +1148,42 @@ class ModuleValidator(Validator): def _check_version_added(self, doc, existing_doc): version_added_raw = doc.get('version_added') try: + collection_name = doc.get('version_added_collection') version_added = self._create_strict_version( - self._extract_version_from_tag_for_msg(str(doc.get('version_added', '0.0') or '0.0'))) - except ValueError: + str(doc.get('version_added', '0.0') or '0.0'), + collection_name=collection_name) + except ValueError as e: version_added = doc.get('version_added', '0.0') - if self._is_new_module() or version_added != 'ansible.builtin:historical': - # already reported during schema validation, except: - if version_added == 'ansible.builtin:historical': - self.reporter.error( - path=self.object_path, - code='module-invalid-version-added', - msg='version_added is not a valid version number: %r' % 'historical' - ) + if version_added != 'historical' or self._is_new_module(): + self.reporter.error( + path=self.object_path, + code='module-invalid-version-added', + msg='version_added is not a valid version number: %r. Error: %s' % (version_added, e) + ) return if existing_doc and str(version_added_raw) != str(existing_doc.get('version_added')): self.reporter.error( path=self.object_path, code='module-incorrect-version-added', - msg='version_added should be %r. Currently %r' % ( - self._extract_version_from_tag_for_msg(existing_doc.get('version_added')), - self._extract_version_from_tag_for_msg(version_added_raw)) + msg='version_added should be %r. Currently %r' % (existing_doc.get('version_added'), version_added_raw) ) if not self._is_new_module(): return should_be = '.'.join(ansible_version.split('.')[:2]) - strict_ansible_version = self._create_strict_version(should_be) + strict_ansible_version = self._create_strict_version(should_be, collection_name='ansible.builtin') if (version_added < strict_ansible_version or strict_ansible_version < version_added): self.reporter.error( path=self.object_path, code='module-incorrect-version-added', - msg='version_added should be %r. Currently %r' % ( - should_be, self._extract_version_from_tag_for_msg(version_added_raw)) + msg='version_added should be %r. Currently %r' % (should_be, version_added_raw) ) - def _validate_ansible_module_call(self, docs, dates_tagged=True): + def _validate_ansible_module_call(self, docs): try: spec, args, kwargs = get_argument_spec(self.path, self.collection) except AnsibleModuleNotInitialized: @@ -1208,8 +1205,7 @@ class ModuleValidator(Validator): ) return - self._validate_docs_schema(kwargs, ansible_module_kwargs_schema(for_collection=bool(self.collection), - dates_tagged=dates_tagged), + self._validate_docs_schema(kwargs, ansible_module_kwargs_schema(for_collection=bool(self.collection)), 'AnsibleModule', 'invalid-ansiblemodule-schema') self._validate_argument_spec(docs, spec, kwargs) @@ -1486,8 +1482,7 @@ class ModuleValidator(Validator): removed_at_date = data.get('removed_at_date', None) if removed_at_date is not None: try: - date = self._extract_version_from_tag_for_msg(removed_at_date) - if parse_isodate(date) < datetime.date.today(): + if parse_isodate(removed_at_date) < datetime.date.today(): msg = "Argument '%s' in argument_spec" % arg if context: msg += " found in %s" % " -> ".join(context) @@ -1507,7 +1502,7 @@ class ModuleValidator(Validator): for deprecated_alias in deprecated_aliases: if 'name' in deprecated_alias and 'date' in deprecated_alias: try: - date = self._extract_version_from_tag_for_msg(deprecated_alias['date']) + date = deprecated_alias['date'] if parse_isodate(date) < datetime.date.today(): msg = "Argument '%s' in argument_spec" % arg if context: @@ -1525,57 +1520,97 @@ class ModuleValidator(Validator): # time. pass - if not self.collection or self.collection_version is not None: - if self.collection: - compare_version = self.collection_version - version_of_what = "this collection (%s)" % self.collection_version_str - version_parser_error = "the version number is not a valid semantic version (https://semver.org/)" - code_prefix = 'collection' - else: - compare_version = LOOSE_ANSIBLE_VERSION - version_of_what = "Ansible (%s)" % ansible_version - version_parser_error = "the version number cannot be parsed" - code_prefix = 'ansible' + has_version = False + if self.collection and self.collection_version is not None: + compare_version = self.collection_version + version_of_what = "this collection (%s)" % self.collection_version_str + code_prefix = 'collection' + has_version = True + elif not self.collection: + compare_version = LOOSE_ANSIBLE_VERSION + version_of_what = "Ansible (%s)" % ansible_version + code_prefix = 'ansible' + has_version = True + + removed_in_version = data.get('removed_in_version', None) + if removed_in_version is not None: + try: + collection_name = data.get('removed_from_collection') + removed_in = self._create_version(str(removed_in_version), collection_name=collection_name) + if has_version and collection_name == self.collection_name and compare_version >= removed_in: + msg = "Argument '%s' in argument_spec" % arg + if context: + msg += " found in %s" % " -> ".join(context) + msg += " has a deprecated removed_in_version %r," % removed_in_version + msg += " i.e. the version is less than or equal to the current version of %s" % version_of_what + self.reporter.error( + path=self.object_path, + code=code_prefix + '-deprecated-version', + msg=msg, + ) + except ValueError as e: + msg = "Argument '%s' in argument_spec" % arg + if context: + msg += " found in %s" % " -> ".join(context) + msg += " has an invalid removed_in_version number %r: %s" % (removed_in_version, e) + self.reporter.error( + path=self.object_path, + code='invalid-deprecated-version', + msg=msg, + ) + except TypeError: + msg = "Argument '%s' in argument_spec" % arg + if context: + msg += " found in %s" % " -> ".join(context) + msg += " has an invalid removed_in_version number %r: " % (removed_in_version, ) + msg += " error while comparing to version of %s" % version_of_what + self.reporter.error( + path=self.object_path, + code='invalid-deprecated-version', + msg=msg, + ) - removed_in_version = data.get('removed_in_version', None) - if removed_in_version is not None: - try: - collection_name, removed_in_version = self._split_tagged_version(removed_in_version) - if collection_name == self.collection_name and compare_version >= self._create_version(str(removed_in_version)): + if deprecated_aliases is not None: + for deprecated_alias in deprecated_aliases: + if 'name' in deprecated_alias and 'version' in deprecated_alias: + try: + collection_name = deprecated_alias.get('collection') + version = self._create_version(str(deprecated_alias['version']), collection_name=collection_name) + if has_version and collection_name == self.collection_name and compare_version >= version: + msg = "Argument '%s' in argument_spec" % arg + if context: + msg += " found in %s" % " -> ".join(context) + msg += " has deprecated aliases '%s' with removal in version %r," % ( + deprecated_alias['name'], deprecated_alias['version']) + msg += " i.e. the version is less than or equal to the current version of %s" % version_of_what + self.reporter.error( + path=self.object_path, + code=code_prefix + '-deprecated-version', + msg=msg, + ) + except ValueError as e: msg = "Argument '%s' in argument_spec" % arg if context: msg += " found in %s" % " -> ".join(context) - msg += " has a deprecated removed_in_version '%s'," % removed_in_version - msg += " i.e. the version is less than or equal to the current version of %s" % version_of_what + msg += " has deprecated aliases '%s' with invalid removal version %r: %s" % ( + deprecated_alias['name'], deprecated_alias['version'], e) self.reporter.error( path=self.object_path, - code=code_prefix + '-deprecated-version', + code='invalid-deprecated-version', + msg=msg, + ) + except TypeError: + msg = "Argument '%s' in argument_spec" % arg + if context: + msg += " found in %s" % " -> ".join(context) + msg += " has deprecated aliases '%s' with invalid removal version %r:" % ( + deprecated_alias['name'], deprecated_alias['version']) + msg += " error while comparing to version of %s" % version_of_what + self.reporter.error( + path=self.object_path, + code='invalid-deprecated-version', msg=msg, ) - except ValueError: - # Has been caught in schema validation - pass - - if deprecated_aliases is not None: - for deprecated_alias in deprecated_aliases: - if 'name' in deprecated_alias and 'version' in deprecated_alias: - try: - collection_name, version = self._split_tagged_version(deprecated_alias['version']) - if collection_name == self.collection_name and compare_version >= self._create_version(str(version)): - msg = "Argument '%s' in argument_spec" % arg - if context: - msg += " found in %s" % " -> ".join(context) - msg += " has deprecated aliases '%s' with removal in version '%s'," % ( - deprecated_alias['name'], deprecated_alias['version']) - msg += " i.e. the version is less than or equal to the current version of %s" % version_of_what - self.reporter.error( - path=self.object_path, - code=code_prefix + '-deprecated-version', - msg=msg, - ) - except ValueError: - # Has been caught in schema validation - pass aliases = data.get('aliases', []) if arg in aliases: @@ -1978,15 +2013,18 @@ class ModuleValidator(Validator): return try: + mod_collection_name = existing_doc.get('version_added_collection') mod_version_added = self._create_strict_version( - self._extract_version_from_tag_for_msg(str(existing_doc.get('version_added', '0.0')))) + str(existing_doc.get('version_added', '0.0')), + collection_name=mod_collection_name) except ValueError: + mod_collection_name = self.collection_name mod_version_added = self._create_strict_version('0.0') options = doc.get('options', {}) or {} should_be = '.'.join(ansible_version.split('.')[:2]) - strict_ansible_version = self._create_strict_version(should_be) + strict_ansible_version = self._create_strict_version(should_be, collection_name='ansible.builtin') for option, details in options.items(): try: @@ -1998,11 +2036,21 @@ class ModuleValidator(Validator): if any(name in existing_options for name in names): # The option already existed. Make sure version_added didn't change. for name in names: + existing_collection_name = existing_options.get(name, {}).get('version_added_collection') existing_version = existing_options.get(name, {}).get('version_added') if existing_version: break + current_collection_name = details.get('version_added_collection') current_version = details.get('version_added') - if str(current_version) != str(existing_version): + if current_collection_name != existing_collection_name: + self.reporter.error( + path=self.object_path, + code='option-incorrect-version-added-collection', + msg=('version_added for existing option (%s) should ' + 'belong to collection %r. Currently belongs to %r' % + (option, current_collection_name, existing_collection_name)) + ) + elif str(current_version) != str(existing_version): self.reporter.error( path=self.object_path, code='option-incorrect-version-added', @@ -2013,12 +2061,22 @@ class ModuleValidator(Validator): continue try: + collection_name = details.get('version_added_collection') version_added = self._create_strict_version( - self._extract_version_from_tag_for_msg(str(details.get('version_added', '0.0')))) - except ValueError: - # already reported during schema validation + str(details.get('version_added', '0.0')), + collection_name=collection_name) + except ValueError as e: + self.reporter.error( + 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 + if collection_name != self.collection_name: + continue if (strict_ansible_version != mod_version_added and (version_added < strict_ansible_version or strict_ansible_version < version_added)): @@ -2085,24 +2143,28 @@ class ModuleValidator(Validator): if 'removed_in' in docs['deprecated']: removed_in = None - try: - collection_name, version = self._split_tagged_version(docs['deprecated']['removed_in']) - if collection_name != self.collection_name: + collection_name = docs['deprecated'].get('removed_from_collection') + version = docs['deprecated']['removed_in'] + if collection_name != self.collection_name: + self.reporter.error( + path=self.object_path, + code='invalid-module-deprecation-source', + msg=('The deprecation version for a module must be added in this collection') + ) + else: + try: + removed_in = self._create_strict_version(str(version), collection_name=collection_name) + except ValueError as e: self.reporter.error( path=self.object_path, - code='invalid-module-deprecation-source', - msg=('The deprecation version for a module must be added in this collection') + code='invalid-module-deprecation-version', + msg=('The deprecation version %r cannot be parsed: %s' % (version, e)) ) - else: - removed_in = self._create_strict_version(str(version)) - - except ValueError: - # ignore and hope we previouslly reported - pass if removed_in: if not self.collection: - strict_ansible_version = self._create_strict_version('.'.join(ansible_version.split('.')[:2])) + strict_ansible_version = self._create_strict_version( + '.'.join(ansible_version.split('.')[:2]), self.collection_name) end_of_deprecation_should_be_removed_only = strict_ansible_version >= removed_in elif self.collection_version: strict_ansible_version = self.collection_version @@ -2116,7 +2178,7 @@ class ModuleValidator(Validator): msg = "Module's deprecated.removed_at_date date '%s' is before today" % removed_at_date self.reporter.error(path=self.object_path, code='deprecated-date', msg=msg) except ValueError: - # ignore and hope we previouslly reported + # This happens if the date cannot be parsed. This is already checked by the schema. pass if self._python_module() and not self._just_docs() and not end_of_deprecation_should_be_removed_only: @@ -2140,8 +2202,7 @@ class ModuleValidator(Validator): if re.search(pattern, self.text) and self.object_name not in self.PS_ARG_VALIDATE_BLACKLIST: with ModuleValidator(docs_path, base_branch=self.base_branch, git_cache=self.git_cache) as docs_mv: docs = docs_mv._validate_docs()[1] - # Don't expect tagged dates! - self._validate_ansible_module_call(docs, dates_tagged=False) + self._validate_ansible_module_call(docs) self._check_gpl3_header() if not self._just_docs() and not end_of_deprecation_should_be_removed_only: 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 1dcb081bfd8..8735ae4695f 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 @@ -6,16 +6,14 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -import datetime import re -from functools import partial +from distutils.version import StrictVersion 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.common.collections import is_iterable from ansible.utils.version import SemanticVersion -from distutils.version import StrictVersion from .utils import parse_isodate @@ -37,31 +35,6 @@ def _add_ansible_error_code(exception, error_code): return exception -def semantic_version(v, error_code=None): - if not isinstance(v, string_types): - raise _add_ansible_error_code(Invalid('Semantic version must be a string'), error_code or 'collection-invalid-version') - if not v: - raise _add_ansible_error_code( - Invalid('Empty string is not a valid semantic version'), - error_code or 'collection-invalid-version') - try: - SemanticVersion(v) - except ValueError as e: - raise _add_ansible_error_code(Invalid(str(e)), error_code or 'collection-invalid-version') - return v - - -def ansible_version(v, error_code=None): - # Assumes argument is a string or float - if 'historical' == v: - return v - try: - StrictVersion(str(v)) - except ValueError as e: - raise _add_ansible_error_code(Invalid(str(e)), error_code or 'ansible-invalid-version') - return v - - def isodate(v, error_code=None): try: parse_isodate(v) @@ -70,50 +43,29 @@ def isodate(v, error_code=None): return v -TAGGED_VERSION_RE = re.compile('^([^.]+.[^.]+):(.*)$') +COLLECTION_NAME_RE = re.compile('^([^.]+.[^.]+)$') -def tagged_version(v, error_code=None): +def collection_name(v, error_code=None): if not isinstance(v, string_types): - # Should never happen to versions tagged by code - raise _add_ansible_error_code(Invalid('Tagged version must be a string'), 'invalid-tagged-version') - m = TAGGED_VERSION_RE.match(v) - if not m: - # Should never happen to versions tagged by code - raise _add_ansible_error_code(Invalid('Tagged version does not match format'), 'invalid-tagged-version') - collection = m.group(1) - version = m.group(2) - if collection != 'ansible.builtin': - semantic_version(version, error_code=error_code) - else: - ansible_version(version, error_code=error_code) - return v - - -def tagged_isodate(v, error_code=None): - if not isinstance(v, string_types): - # Should never happen to dates tagged by code - raise _add_ansible_error_code(Invalid('Tagged date must be a string'), 'invalid-tagged-date') - m = TAGGED_VERSION_RE.match(v) + raise _add_ansible_error_code( + Invalid('Collection name must be a string'), error_code or 'collection-invalid-name') + m = COLLECTION_NAME_RE.match(v) if not m: - # Should never happen to dates tagged by code - raise _add_ansible_error_code(Invalid('Tagged date does not match format'), 'invalid-tagged-date') - isodate(m.group(2), error_code=error_code) + raise _add_ansible_error_code( + Invalid('Collection name must be of format `.`'), error_code or 'collection-invalid-name') return v -def version(for_collection=False, tagged='never', error_code=None): - if tagged == 'always': - return Any(partial(tagged_version, error_code=error_code)) +def version(for_collection=False): if for_collection: - return Any(partial(semantic_version, error_code=error_code)) - return All(Any(float, *string_types), partial(ansible_version, error_code=error_code)) + # We do not accept floats for versions in collections + return Any(*string_types) + return Any(float, *string_types) -def date(tagged='never', error_code=None): - if tagged == 'always': - return Any(partial(tagged_isodate, error_code=error_code)) - return Any(isodate) +def date(error_code=None): + return Any(isodate, error_code=error_code) def is_callable(v): @@ -189,8 +141,24 @@ def options_with_apply_defaults(v): return v -def argument_spec_schema(for_collection, dates_tagged=True): - dates_tagged = 'always' if dates_tagged else 'never' +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'): + raise _add_ansible_error_code( + Invalid('Only one of removed_in_version and removed_at_date must be specified'), + error_code='deprecation-either-date-or-version') + if not v.get('removed_from_collection'): + raise _add_ansible_error_code( + Invalid('If removed_in_version or removed_at_date is specified, ' + 'removed_from_collection must be specified as well'), + error_code='deprecation-collection-missing') + return + if v.get('removed_from_collection'): + raise Invalid('removed_from_collection cannot be specified without either ' + 'removed_in_version or removed_at_date') + + +def argument_spec_schema(for_collection): any_string_types = Any(*string_types) schema = { any_string_types: { @@ -206,17 +174,20 @@ def argument_spec_schema(for_collection, dates_tagged=True): 'no_log': bool, 'aliases': Any(list_string_types, tuple(list_string_types)), 'apply_defaults': bool, - 'removed_in_version': version(for_collection, tagged='always'), - 'removed_at_date': date(tagged=dates_tagged), + 'removed_in_version': version(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(tagged=dates_tagged), + Required('date'): date(), + Required('collection'): collection_name, }, { Required('name'): Any(*string_types), - Required('version'): version(for_collection, tagged='always'), + Required('version'): version(for_collection), + Required('collection'): collection_name, }, )]), } @@ -227,13 +198,14 @@ def argument_spec_schema(for_collection, dates_tagged=True): Schema({any_string_types: no_required_with_default}), Schema({any_string_types: elements_with_list}), Schema({any_string_types: options_with_apply_defaults}), + Schema({any_string_types: option_deprecation}), ) return Schema(schemas) -def ansible_module_kwargs_schema(for_collection, dates_tagged=True): +def ansible_module_kwargs_schema(for_collection): schema = { - 'argument_spec': argument_spec_schema(for_collection, dates_tagged=dates_tagged), + 'argument_spec': argument_spec_schema(for_collection), 'bypass_checks': bool, 'no_log': bool, 'check_invalid_arguments': Any(None, bool), @@ -253,6 +225,39 @@ json_value = Schema(Any( )) +def version_added(v): + if 'version_added' in v: + version_added = v.get('version_added') + if isinstance(version_added, string_types): + # If it is not a string, schema validation will have already complained + # - or we have a float and we are in ansible/ansible, in which case we're + # also happy. + if v.get('version_added_collection') == 'ansible.builtin': + try: + version = StrictVersion() + version.parse(version_added) + except ValueError as exc: + raise _add_ansible_error_code( + Invalid('version_added (%r) is not a valid ansible-base version: ' + '%s' % (version_added, exc)), + error_code='deprecation-either-date-or-version') + else: + try: + version = SemanticVersion() + version.parse(version_added) + except ValueError as exc: + raise _add_ansible_error_code( + Invalid('version_added (%r) is not a valid collection version ' + '(see specification at https://semver.org/): ' + '%s' % (version_added, exc)), + error_code='deprecation-either-date-or-version') + elif 'version_added_collection' in v: + # Must have been manual intervention, since version_added_collection is only + # added automatically when version_added is present + raise Invalid('version_added_collection cannot be specified without version_added') + return v + + def list_dict_option_schema(for_collection): suboption_schema = Schema( { @@ -260,7 +265,8 @@ def list_dict_option_schema(for_collection): 'required': bool, 'choices': list, 'aliases': Any(list_string_types), - 'version_added': version(for_collection, tagged='always', error_code='option-invalid-version-added'), + 'version_added': version(for_collection), + 'version_added_collection': collection_name, 'default': json_value, # Note: Types are strings, not literal bools, such as True or False 'type': Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'sid', 'str'), @@ -282,7 +288,8 @@ def list_dict_option_schema(for_collection): 'required': bool, 'choices': list, 'aliases': Any(list_string_types), - 'version_added': version(for_collection, tagged='always', error_code='option-invalid-version-added'), + 'version_added': version(for_collection), + 'version_added_collection': collection_name, 'default': json_value, 'suboptions': Any(None, *list_dict_suboption_schema), # Note: Types are strings, not literal bools, such as True or False @@ -293,9 +300,16 @@ def list_dict_option_schema(for_collection): extra=PREVENT_EXTRA ) + option_version_added = Schema( + All({ + 'suboptions': Any(None, *[{str_type: Self} for str_type in string_types]), + }, version_added), + extra=ALLOW_EXTRA + ) + # This generates list of dicts with keys from string_types and option_schema value # for example in Python 3: {str: option_schema} - return [{str_type: option_schema} for str_type in string_types] + return [{str_type: All(option_schema, option_version_added)} for str_type in string_types] def return_contains(v): @@ -318,7 +332,8 @@ def return_schema(for_collection): Required('description'): Any(list_string_types, *string_types), 'returned': Any(*string_types), # only returned on top level Required('type'): Any('bool', 'complex', 'dict', 'float', 'int', 'list', 'str'), - 'version_added': version(for_collection, error_code='return-invalid-version-added'), + 'version_added': version(for_collection), + 'version_added_collection': collection_name, 'sample': json_value, 'example': json_value, 'contains': Any(None, *list({str_type: Self} for str_type in string_types)), @@ -326,7 +341,8 @@ def return_schema(for_collection): 'elements': Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'sid', 'str'), } ), - Schema(return_contains) + Schema(return_contains), + Schema(version_added), ), Schema(type(None)), ) @@ -343,7 +359,8 @@ def return_schema(for_collection): Required('description'): Any(list_string_types, *string_types), Required('returned'): Any(*string_types), Required('type'): Any('bool', 'complex', 'dict', 'float', 'int', 'list', 'str'), - 'version_added': version(for_collection, error_code='return-invalid-version-added'), + 'version_added': version(for_collection), + 'version_added_collection': collection_name, 'sample': json_value, 'example': json_value, 'contains': Any(None, *list_dict_return_contains_schema), @@ -352,7 +369,8 @@ def return_schema(for_collection): } } ), - Schema({any_string_types: return_contains}) + Schema({any_string_types: return_contains}), + Schema({any_string_types: version_added}), ), Schema(type(None)), ) @@ -362,17 +380,18 @@ def deprecation_schema(for_collection): main_fields = { Required('why'): Any(*string_types), Required('alternative'): Any(*string_types), + Required('removed_from_collection'): collection_name, 'removed': Any(True), } date_schema = { - Required('removed_at_date'): date(tagged='always'), + Required('removed_at_date'): date(), } date_schema.update(main_fields) if for_collection: version_schema = { - Required('removed_in'): version(for_collection, tagged='always'), + Required('removed_in'): version(for_collection), } else: version_schema = { @@ -381,9 +400,7 @@ def deprecation_schema(for_collection): # 2.3 -> removed_in: "2.5" + n for docs stub # 2.4 -> removed_in: "2.8" + n for docs stub Required('removed_in'): Any( - "ansible.builtin:2.2", "ansible.builtin:2.3", "ansible.builtin:2.4", "ansible.builtin:2.5", - "ansible.builtin:2.6", "ansible.builtin:2.8", "ansible.builtin:2.9", "ansible.builtin:2.10", - "ansible.builtin:2.11", "ansible.builtin:2.12", "ansible.builtin:2.13", "ansible.builtin:2.14"), + "2.2", "2.3", "2.4", "2.5", "2.6", "2.8", "2.9", "2.10", "2.11", "2.12", "2.13", "2.14"), } version_schema.update(main_fields) @@ -419,16 +436,15 @@ def doc_schema(module_name, for_collection=False, deprecated_module=False): 'requirements': list_string_types, 'todo': Any(None, list_string_types, *string_types), 'options': Any(None, *list_dict_option_schema(for_collection)), - 'extends_documentation_fragment': Any(list_string_types, *string_types) + 'extends_documentation_fragment': Any(list_string_types, *string_types), + 'version_added_collection': collection_name, } if for_collection: # Optional - doc_schema_dict['version_added'] = version( - for_collection=True, tagged='always', error_code='module-invalid-version-added') + doc_schema_dict['version_added'] = version(for_collection=True) else: - doc_schema_dict[Required('version_added')] = version( - for_collection=False, tagged='always', error_code='module-invalid-version-added') + doc_schema_dict[Required('version_added')] = version(for_collection=False) if deprecated_module: deprecation_required_scheme = { diff --git a/test/support/integration/plugins/module_utils/azure_rm_common.py b/test/support/integration/plugins/module_utils/azure_rm_common.py index e995daa02ec..a7b55e972e5 100644 --- a/test/support/integration/plugins/module_utils/azure_rm_common.py +++ b/test/support/integration/plugins/module_utils/azure_rm_common.py @@ -449,8 +449,8 @@ class AzureRMModuleBase(object): ''' self.module.fail_json(msg=msg, **kwargs) - def deprecate(self, msg, version=None): - self.module.deprecate(msg, version) + def deprecate(self, msg, version=None, collection_name=None): + self.module.deprecate(msg, version, collection_name=collection_name) def log(self, msg, pretty_print=False): if pretty_print: diff --git a/test/support/integration/plugins/modules/aws_az_info.py b/test/support/integration/plugins/modules/aws_az_info.py index 0685a0b5746..c1efed6f0b9 100644 --- a/test/support/integration/plugins/modules/aws_az_info.py +++ b/test/support/integration/plugins/modules/aws_az_info.py @@ -86,7 +86,8 @@ def main(): module = AnsibleAWSModule(argument_spec=argument_spec) if module._name == 'aws_az_facts': - module.deprecate("The 'aws_az_facts' module has been renamed to 'aws_az_info'", version='ansible.builtin:2.14') + module.deprecate("The 'aws_az_facts' module has been renamed to 'aws_az_info'", + version='2.14', collection_name='ansible.builtin') connection = module.client('ec2', retry_decorator=AWSRetry.jittered_backoff()) diff --git a/test/support/integration/plugins/modules/azure_rm_functionapp_info.py b/test/support/integration/plugins/modules/azure_rm_functionapp_info.py index 19652577dd5..40672f952d4 100644 --- a/test/support/integration/plugins/modules/azure_rm_functionapp_info.py +++ b/test/support/integration/plugins/modules/azure_rm_functionapp_info.py @@ -136,7 +136,8 @@ class AzureRMFunctionAppInfo(AzureRMModuleBase): is_old_facts = self.module._name == 'azure_rm_functionapp_facts' if is_old_facts: - self.module.deprecate("The 'azure_rm_functionapp_facts' module has been renamed to 'azure_rm_functionapp_info'", version='ansible.builtin:2.13') + self.module.deprecate("The 'azure_rm_functionapp_facts' module has been renamed to 'azure_rm_functionapp_info'", + version='2.13', collection_name='ansible.builtin') for key in self.module_arg_spec: setattr(self, key, kwargs[key]) diff --git a/test/support/integration/plugins/modules/azure_rm_mariadbconfiguration_info.py b/test/support/integration/plugins/modules/azure_rm_mariadbconfiguration_info.py index df1b41da13d..3faac5eb5ad 100644 --- a/test/support/integration/plugins/modules/azure_rm_mariadbconfiguration_info.py +++ b/test/support/integration/plugins/modules/azure_rm_mariadbconfiguration_info.py @@ -140,7 +140,7 @@ class AzureRMMariaDbConfigurationInfo(AzureRMModuleBase): is_old_facts = self.module._name == 'azure_rm_mariadbconfiguration_facts' if is_old_facts: self.module.deprecate("The 'azure_rm_mariadbconfiguration_facts' module has been renamed to 'azure_rm_mariadbconfiguration_info'", - version='ansible.builtin:2.13') + version='2.13', collection_name='ansible.builtin') for key in self.module_arg_spec: setattr(self, key, kwargs[key]) diff --git a/test/support/integration/plugins/modules/azure_rm_mariadbdatabase_info.py b/test/support/integration/plugins/modules/azure_rm_mariadbdatabase_info.py index dbbb2da970e..e9c99c1483c 100644 --- a/test/support/integration/plugins/modules/azure_rm_mariadbdatabase_info.py +++ b/test/support/integration/plugins/modules/azure_rm_mariadbdatabase_info.py @@ -146,7 +146,7 @@ class AzureRMMariaDbDatabaseInfo(AzureRMModuleBase): is_old_facts = self.module._name == 'azure_rm_mariadbdatabase_facts' if is_old_facts: self.module.deprecate("The 'azure_rm_mariadbdatabase_facts' module has been renamed to 'azure_rm_mariadbdatabase_info'", - version='ansible.builtin:2.13') + version='2.13', collection_name='ansible.builtin') for key in self.module_arg_spec: setattr(self, key, kwargs[key]) diff --git a/test/support/integration/plugins/modules/azure_rm_mariadbfirewallrule_info.py b/test/support/integration/plugins/modules/azure_rm_mariadbfirewallrule_info.py index c6857bea482..ef71be8dce5 100644 --- a/test/support/integration/plugins/modules/azure_rm_mariadbfirewallrule_info.py +++ b/test/support/integration/plugins/modules/azure_rm_mariadbfirewallrule_info.py @@ -142,7 +142,7 @@ class AzureRMMariaDbFirewallRuleInfo(AzureRMModuleBase): is_old_facts = self.module._name == 'azure_rm_mariadbfirewallrule_facts' if is_old_facts: self.module.deprecate("The 'azure_rm_mariadbfirewallrule_facts' module has been renamed to 'azure_rm_mariadbfirewallrule_info'", - version='ansible.builtin:2.13') + version='2.13', collection_name='ansible.builtin') for key in self.module_arg_spec: setattr(self, key, kwargs[key]) diff --git a/test/support/integration/plugins/modules/azure_rm_mariadbserver_info.py b/test/support/integration/plugins/modules/azure_rm_mariadbserver_info.py index 5e8e1a82da0..464aa4d8a5e 100644 --- a/test/support/integration/plugins/modules/azure_rm_mariadbserver_info.py +++ b/test/support/integration/plugins/modules/azure_rm_mariadbserver_info.py @@ -193,7 +193,8 @@ class AzureRMMariaDbServerInfo(AzureRMModuleBase): def exec_module(self, **kwargs): is_old_facts = self.module._name == 'azure_rm_mariadbserver_facts' if is_old_facts: - self.module.deprecate("The 'azure_rm_mariadbserver_facts' module has been renamed to 'azure_rm_mariadbserver_info'", version='ansible.builtin:2.13') + self.module.deprecate("The 'azure_rm_mariadbserver_facts' module has been renamed to 'azure_rm_mariadbserver_info'", + version='2.13', collection_name='ansible.builtin') for key in self.module_arg_spec: setattr(self, key, kwargs[key]) diff --git a/test/support/integration/plugins/modules/azure_rm_resource_info.py b/test/support/integration/plugins/modules/azure_rm_resource_info.py index 5d553a0d42c..f797f66219c 100644 --- a/test/support/integration/plugins/modules/azure_rm_resource_info.py +++ b/test/support/integration/plugins/modules/azure_rm_resource_info.py @@ -336,7 +336,8 @@ class AzureRMResourceInfo(AzureRMModuleBase): def exec_module(self, **kwargs): is_old_facts = self.module._name == 'azure_rm_resource_facts' if is_old_facts: - self.module.deprecate("The 'azure_rm_resource_facts' module has been renamed to 'azure_rm_resource_info'", version='ansible.builtin:2.13') + self.module.deprecate("The 'azure_rm_resource_facts' module has been renamed to 'azure_rm_resource_info'", + version='2.13', collection_name='ansible.builtin') for key in self.module_arg_spec: setattr(self, key, kwargs[key]) diff --git a/test/support/integration/plugins/modules/azure_rm_webapp_info.py b/test/support/integration/plugins/modules/azure_rm_webapp_info.py index e722ffdf165..22286803064 100644 --- a/test/support/integration/plugins/modules/azure_rm_webapp_info.py +++ b/test/support/integration/plugins/modules/azure_rm_webapp_info.py @@ -265,7 +265,8 @@ class AzureRMWebAppInfo(AzureRMModuleBase): def exec_module(self, **kwargs): is_old_facts = self.module._name == 'azure_rm_webapp_facts' if is_old_facts: - self.module.deprecate("The 'azure_rm_webapp_facts' module has been renamed to 'azure_rm_webapp_info'", version='ansible.builtin:2.13') + self.module.deprecate("The 'azure_rm_webapp_facts' module has been renamed to 'azure_rm_webapp_info'", + version='2.13', collection_name='ansible.builtin') for key in self.module_arg_spec: setattr(self, key, kwargs[key]) diff --git a/test/support/integration/plugins/modules/cloudformation_info.py b/test/support/integration/plugins/modules/cloudformation_info.py index d10429326d1..ee2e5c178b3 100644 --- a/test/support/integration/plugins/modules/cloudformation_info.py +++ b/test/support/integration/plugins/modules/cloudformation_info.py @@ -302,7 +302,8 @@ def main(): is_old_facts = module._name == 'cloudformation_facts' if is_old_facts: module.deprecate("The 'cloudformation_facts' module has been renamed to 'cloudformation_info', " - "and the renamed one no longer returns ansible_facts", version='ansible.builtin:2.13') + "and the renamed one no longer returns ansible_facts", + version='2.13', collection_name='ansible.builtin') service_mgr = CloudFormationServiceManager(module) diff --git a/test/support/integration/plugins/modules/docker_swarm.py b/test/support/integration/plugins/modules/docker_swarm.py index e802e202457..a2c076c5183 100644 --- a/test/support/integration/plugins/modules/docker_swarm.py +++ b/test/support/integration/plugins/modules/docker_swarm.py @@ -450,7 +450,7 @@ class SwarmManager(DockerBaseClass): if self.state == 'inspect': self.client.module.deprecate( "The 'inspect' state is deprecated, please use 'docker_swarm_info' to inspect swarm cluster", - version='ansible.builtin:2.12') + version='2.12', collection_name='ansible.builtin') choice_map.get(self.state)() diff --git a/test/support/integration/plugins/modules/ec2.py b/test/support/integration/plugins/modules/ec2.py index b01c5a66d5e..952aa5a1a6f 100644 --- a/test/support/integration/plugins/modules/ec2.py +++ b/test/support/integration/plugins/modules/ec2.py @@ -1695,7 +1695,7 @@ def main(): module.deprecate( msg='Support for passing both group and group_id has been deprecated. ' 'Currently group_id is ignored, in future passing both will result in an error', - version='ansible.builtin:2.14') + version='2.14', collection_name='ansible.builtin') if not HAS_BOTO: module.fail_json(msg='boto required for this module') diff --git a/test/support/integration/plugins/modules/ec2_ami_info.py b/test/support/integration/plugins/modules/ec2_ami_info.py index e7cbe0532c9..53c2374de8d 100644 --- a/test/support/integration/plugins/modules/ec2_ami_info.py +++ b/test/support/integration/plugins/modules/ec2_ami_info.py @@ -270,7 +270,8 @@ def main(): module = AnsibleAWSModule(argument_spec=argument_spec, supports_check_mode=True) if module._module._name == 'ec2_ami_facts': - module._module.deprecate("The 'ec2_ami_facts' module has been renamed to 'ec2_ami_info'", version='ansible.builtin:2.13') + module._module.deprecate("The 'ec2_ami_facts' module has been renamed to 'ec2_ami_info'", + version='2.13', collection_name='ansible.builtin') ec2_client = module.client('ec2') diff --git a/test/support/integration/plugins/modules/ec2_eni_info.py b/test/support/integration/plugins/modules/ec2_eni_info.py index 31efbfdd389..1e281a49381 100644 --- a/test/support/integration/plugins/modules/ec2_eni_info.py +++ b/test/support/integration/plugins/modules/ec2_eni_info.py @@ -259,7 +259,8 @@ def main(): module = AnsibleModule(argument_spec=argument_spec) if module._name == 'ec2_eni_facts': - module.deprecate("The 'ec2_eni_facts' module has been renamed to 'ec2_eni_info'", version='ansible.builtin:2.13') + module.deprecate("The 'ec2_eni_facts' module has been renamed to 'ec2_eni_info'", + version='2.13', collection_name='ansible.builtin') if not HAS_BOTO3: module.fail_json(msg='boto3 required for this module') diff --git a/test/support/integration/plugins/modules/ec2_instance_info.py b/test/support/integration/plugins/modules/ec2_instance_info.py index cd8e0cd7d54..0836ef3b80c 100644 --- a/test/support/integration/plugins/modules/ec2_instance_info.py +++ b/test/support/integration/plugins/modules/ec2_instance_info.py @@ -552,7 +552,8 @@ def main(): supports_check_mode=True ) if module._name == 'ec2_instance_facts': - module.deprecate("The 'ec2_instance_facts' module has been renamed to 'ec2_instance_info'", version='ansible.builtin:2.13') + module.deprecate("The 'ec2_instance_facts' module has been renamed to 'ec2_instance_info'", + version='2.13', collection_name='ansible.builtin') if not HAS_BOTO3: module.fail_json(msg='boto3 required for this module') diff --git a/test/support/integration/plugins/modules/iam_role.py b/test/support/integration/plugins/modules/iam_role.py index 1027232be03..bd666f249c9 100644 --- a/test/support/integration/plugins/modules/iam_role.py +++ b/test/support/integration/plugins/modules/iam_role.py @@ -637,7 +637,8 @@ def main(): if module.params.get('purge_policies') is None: module.deprecate('In Ansible 2.14 the default value of purge_policies will change from true to false.' - ' To maintain the existing behaviour explicity set purge_policies=true', version='ansible.builtin:2.14') + ' To maintain the existing behaviour explicity set purge_policies=true', + version='2.14', collection_name='ansible.builtin') if module.params.get('boundary'): if module.params.get('create_instance_profile'): diff --git a/test/support/integration/plugins/modules/k8s_info.py b/test/support/integration/plugins/modules/k8s_info.py index 37e80668959..f480bcedc3f 100644 --- a/test/support/integration/plugins/modules/k8s_info.py +++ b/test/support/integration/plugins/modules/k8s_info.py @@ -142,7 +142,8 @@ class KubernetesInfoModule(KubernetesAnsibleModule): supports_check_mode=True, **kwargs) if self._name == 'k8s_facts': - self.deprecate("The 'k8s_facts' module has been renamed to 'k8s_info'", version='ansible.builtin:2.13') + self.deprecate("The 'k8s_facts' module has been renamed to 'k8s_info'", + version='2.13', collection_name='ansible.builtin') def execute_module(self): self.client = self.get_api_client() diff --git a/test/support/integration/plugins/modules/openssl_certificate.py b/test/support/integration/plugins/modules/openssl_certificate.py index d8af142e1ba..28780bf22cd 100644 --- a/test/support/integration/plugins/modules/openssl_certificate.py +++ b/test/support/integration/plugins/modules/openssl_certificate.py @@ -2654,7 +2654,7 @@ def main(): if provider == 'assertonly': module.deprecate("The 'assertonly' provider is deprecated; please see the examples of " "the 'openssl_certificate' module on how to replace it with other modules", - version='ansible.builtin:2.13') + version='2.13', collection_name='ansible.builtin') elif provider == 'selfsigned': if module.params['privatekey_path'] is None and module.params['privatekey_content'] is None: module.fail_json(msg='One of privatekey_path and privatekey_content must be specified for the selfsigned provider.') @@ -2702,7 +2702,8 @@ def main(): except AttributeError: module.fail_json(msg='You need to have PyOpenSSL>=0.15') - module.deprecate('The module is using the PyOpenSSL backend. This backend has been deprecated', version='ansible.builtin:2.13') + module.deprecate('The module is using the PyOpenSSL backend. This backend has been deprecated', + version='2.13', collection_name='ansible.builtin') if provider == 'selfsigned': certificate = SelfSignedCertificate(module) elif provider == 'acme': diff --git a/test/support/integration/plugins/modules/openssl_certificate_info.py b/test/support/integration/plugins/modules/openssl_certificate_info.py index 2b947a87776..27e65153ead 100644 --- a/test/support/integration/plugins/modules/openssl_certificate_info.py +++ b/test/support/integration/plugins/modules/openssl_certificate_info.py @@ -845,7 +845,8 @@ def main(): except AttributeError: module.fail_json(msg='You need to have PyOpenSSL>=0.15') - module.deprecate('The module is using the PyOpenSSL backend. This backend has been deprecated', version='ansible.builtin:2.13') + module.deprecate('The module is using the PyOpenSSL backend. This backend has been deprecated', + version='2.13', collection_name='ansible.builtin') certificate = CertificateInfoPyOpenSSL(module) elif backend == 'cryptography': if not CRYPTOGRAPHY_FOUND: diff --git a/test/support/integration/plugins/modules/openssl_csr.py b/test/support/integration/plugins/modules/openssl_csr.py index b184cabe9d7..2d831f35bfc 100644 --- a/test/support/integration/plugins/modules/openssl_csr.py +++ b/test/support/integration/plugins/modules/openssl_csr.py @@ -1091,7 +1091,8 @@ def main(): if module.params['version'] != 1: module.deprecate('The version option will only support allowed values from Ansible 2.14 on. ' - 'Currently, only the value 1 is allowed by RFC 2986', version='ansible.builtin:2.14') + 'Currently, only the value 1 is allowed by RFC 2986', + version='2.14', collection_name='ansible.builtin') base_dir = os.path.dirname(module.params['path']) or '.' if not os.path.isdir(base_dir): @@ -1125,7 +1126,8 @@ def main(): except AttributeError: module.fail_json(msg='You need to have PyOpenSSL>=0.15 to generate CSRs') - module.deprecate('The module is using the PyOpenSSL backend. This backend has been deprecated', version='ansible.builtin:2.13') + module.deprecate('The module is using the PyOpenSSL backend. This backend has been deprecated', + version='2.13', collection_name='ansible.builtin') csr = CertificateSigningRequestPyOpenSSL(module) elif backend == 'cryptography': if not CRYPTOGRAPHY_FOUND: diff --git a/test/support/integration/plugins/modules/openssl_privatekey.py b/test/support/integration/plugins/modules/openssl_privatekey.py index d0ce6ae7b0f..9c247a39426 100644 --- a/test/support/integration/plugins/modules/openssl_privatekey.py +++ b/test/support/integration/plugins/modules/openssl_privatekey.py @@ -908,7 +908,8 @@ def main(): if not PYOPENSSL_FOUND: module.fail_json(msg=missing_required_lib('pyOpenSSL >= {0}'.format(MINIMAL_PYOPENSSL_VERSION)), exception=PYOPENSSL_IMP_ERR) - module.deprecate('The module is using the PyOpenSSL backend. This backend has been deprecated', version='ansible.builtin:2.13') + module.deprecate('The module is using the PyOpenSSL backend. This backend has been deprecated', + version='2.13', collection_name='ansible.builtin') private_key = PrivateKeyPyOpenSSL(module) elif backend == 'cryptography': if not CRYPTOGRAPHY_FOUND: diff --git a/test/support/integration/plugins/modules/python_requirements_info.py b/test/support/integration/plugins/modules/python_requirements_info.py index e89f5db26e3..5b5c3e50fa1 100644 --- a/test/support/integration/plugins/modules/python_requirements_info.py +++ b/test/support/integration/plugins/modules/python_requirements_info.py @@ -118,7 +118,8 @@ def main(): supports_check_mode=True, ) if module._name == 'python_requirements_facts': - module.deprecate("The 'python_requirements_facts' module has been renamed to 'python_requirements_info'", version='ansible.builtin:2.13') + module.deprecate("The 'python_requirements_facts' module has been renamed to 'python_requirements_info'", + version='2.13', collection_name='ansible.builtin') if not HAS_DISTUTILS: module.fail_json( msg='Could not import "distutils" and "pkg_resources" libraries to introspect python environment.', diff --git a/test/support/integration/plugins/modules/xml.py b/test/support/integration/plugins/modules/xml.py index 281e9f5558c..b5b35a384e3 100644 --- a/test/support/integration/plugins/modules/xml.py +++ b/test/support/integration/plugins/modules/xml.py @@ -886,7 +886,7 @@ def main(): # TODO: Remove this in Ansible v2.12 (and reinstate strict parameter test above) and remove the integration test example if content == 'attribute' and attribute is not None: module.deprecate("Parameter 'attribute=%s' is ignored when using 'content=attribute' only 'xpath' is used. Please remove entry." % attribute, - 'ansible.builtin:2.12') + '2.12', collection_name='ansible.builtin') # Check if the file exists if xml_string: diff --git a/test/units/module_utils/basic/test_deprecate_warn.py b/test/units/module_utils/basic/test_deprecate_warn.py index 2e686f966a4..38ecc5c4a13 100644 --- a/test/units/module_utils/basic/test_deprecate_warn.py +++ b/test/units/module_utils/basic/test_deprecate_warn.py @@ -22,23 +22,31 @@ def test_warn(am, capfd): @pytest.mark.parametrize('stdin', [{}], indirect=['stdin']) def test_deprecate(am, capfd): am.deprecate('deprecation1') - am.deprecate('deprecation2', 'ansible.builtin:2.3') - am.deprecate('deprecation3', version='ansible.builtin:2.4') - am.deprecate('deprecation4', date='ansible.builtin:2020-03-10') + am.deprecate('deprecation2', '2.3') # pylint: disable=ansible-deprecated-no-collection-name + am.deprecate('deprecation3', version='2.4') # pylint: disable=ansible-deprecated-no-collection-name + am.deprecate('deprecation4', date='2020-03-10') # pylint: disable=ansible-deprecated-no-collection-name + am.deprecate('deprecation5', collection_name='ansible.builtin') + am.deprecate('deprecation6', '2.3', collection_name='ansible.builtin') + am.deprecate('deprecation7', version='2.4', collection_name='ansible.builtin') + am.deprecate('deprecation8', date='2020-03-10', collection_name='ansible.builtin') with pytest.raises(SystemExit): - am.exit_json(deprecations=['deprecation5', ('deprecation6', 'ansible.builtin:2.4')]) + am.exit_json(deprecations=['deprecation9', ('deprecation10', '2.4')]) out, err = capfd.readouterr() output = json.loads(out) assert ('warnings' not in output or output['warnings'] == []) assert output['deprecations'] == [ - {u'msg': u'deprecation1', u'version': None}, - {u'msg': u'deprecation2', u'version': 'ansible.builtin:2.3'}, - {u'msg': u'deprecation3', u'version': 'ansible.builtin:2.4'}, - {u'msg': u'deprecation4', u'date': 'ansible.builtin:2020-03-10'}, - {u'msg': u'deprecation5', u'version': None}, - {u'msg': u'deprecation6', u'version': 'ansible.builtin:2.4'}, + {u'msg': u'deprecation1', u'version': None, u'collection_name': None}, + {u'msg': u'deprecation2', u'version': '2.3', u'collection_name': None}, + {u'msg': u'deprecation3', u'version': '2.4', u'collection_name': None}, + {u'msg': u'deprecation4', u'date': '2020-03-10', u'collection_name': None}, + {u'msg': u'deprecation5', u'version': None, u'collection_name': 'ansible.builtin'}, + {u'msg': u'deprecation6', u'version': '2.3', u'collection_name': 'ansible.builtin'}, + {u'msg': u'deprecation7', u'version': '2.4', u'collection_name': 'ansible.builtin'}, + {u'msg': u'deprecation8', u'date': '2020-03-10', u'collection_name': 'ansible.builtin'}, + {u'msg': u'deprecation9', u'version': None, u'collection_name': None}, + {u'msg': u'deprecation10', u'version': '2.4', u'collection_name': None}, ] @@ -51,7 +59,7 @@ def test_deprecate_without_list(am, capfd): output = json.loads(out) assert ('warnings' not in output or output['warnings'] == []) assert output['deprecations'] == [ - {u'msg': u'Simple deprecation warning', u'version': None}, + {u'msg': u'Simple deprecation warning', u'version': None, u'collection_name': None}, ] diff --git a/test/units/module_utils/common/warnings/test_deprecate.py b/test/units/module_utils/common/warnings/test_deprecate.py index b85264d0eba..42046bfebc7 100644 --- a/test/units/module_utils/common/warnings/test_deprecate.py +++ b/test/units/module_utils/common/warnings/test_deprecate.py @@ -16,20 +16,50 @@ from ansible.module_utils.six import PY3 @pytest.fixture def deprecation_messages(): return [ - {'msg': 'First deprecation', 'version': None}, - {'msg': 'Second deprecation', 'version': '2.14'}, - {'msg': 'Third deprecation', 'version': '2.9'}, + {'msg': 'First deprecation', 'version': None, 'collection_name': None}, + {'msg': 'Second deprecation', 'version': None, 'collection_name': 'ansible.builtin'}, + {'msg': 'Third deprecation', 'version': '2.14', 'collection_name': None}, + {'msg': 'Fourth deprecation', 'version': '2.9', 'collection_name': None}, + {'msg': 'Fifth deprecation', 'version': '2.9', 'collection_name': 'ansible.builtin'}, + {'msg': 'Sixth deprecation', 'date': '2199-12-31', 'collection_name': None}, + {'msg': 'Seventh deprecation', 'date': '2199-12-31', 'collection_name': 'ansible.builtin'}, ] def test_deprecate_message_only(): deprecate('Deprecation message') - assert warnings._global_deprecations == [{'msg': 'Deprecation message', 'version': None}] + assert warnings._global_deprecations == [ + {'msg': 'Deprecation message', 'version': None, 'collection_name': None}] + + +def test_deprecate_with_collection(): + deprecate(msg='Deprecation message', collection_name='ansible.builtin') + assert warnings._global_deprecations == [ + {'msg': 'Deprecation message', 'version': None, 'collection_name': 'ansible.builtin'}] def test_deprecate_with_version(): deprecate(msg='Deprecation message', version='2.14') - assert warnings._global_deprecations == [{'msg': 'Deprecation message', 'version': '2.14'}] + assert warnings._global_deprecations == [ + {'msg': 'Deprecation message', 'version': '2.14', 'collection_name': None}] + + +def test_deprecate_with_version_and_collection(): + deprecate(msg='Deprecation message', version='2.14', collection_name='ansible.builtin') + assert warnings._global_deprecations == [ + {'msg': 'Deprecation message', 'version': '2.14', 'collection_name': 'ansible.builtin'}] + + +def test_deprecate_with_date(): + deprecate(msg='Deprecation message', date='2199-12-31') + assert warnings._global_deprecations == [ + {'msg': 'Deprecation message', 'date': '2199-12-31', 'collection_name': None}] + + +def test_deprecate_with_date_and_collection(): + deprecate(msg='Deprecation message', date='2199-12-31', collection_name='ansible.builtin') + assert warnings._global_deprecations == [ + {'msg': 'Deprecation message', 'date': '2199-12-31', 'collection_name': 'ansible.builtin'}] def test_multiple_deprecations(deprecation_messages): @@ -45,7 +75,7 @@ def test_get_deprecation_messages(deprecation_messages): accessor_deprecations = get_deprecation_messages() assert isinstance(accessor_deprecations, tuple) - assert len(accessor_deprecations) == 3 + assert len(accessor_deprecations) == 7 @pytest.mark.parametrize(