actually show plugin config warnings/deprecations (#82593)

previouslly we recorded but did not show to avoid spam
since we could not dedup from forks, that was already
fixed in another PR so now we can show/display them.

Also:
  * funcitonalize deprecation msg construct from docs
  * reuse formatting func in cli
  * normalize alternatives: most of the code used intended plural
    but some and most data/tests used the singular
  * update schemas and tests

Co-authored-by: Matt Davis <6775756+nitzmahone@users.noreply.github.com>
Co-authored-by: Felix Fontein <felix@fontein.de>

changelogs/fragments/getoffmylawn.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - plugins, deprecations and warnings concerning configuration are now displayed to the user, technical issue that prevented 'de-duplication' have been resolved.

lib/ansible/cli/doc.py

@@ -1089,7 +1089,7 @@ class DocCLI(CLI, RoleMixin):
             text = DocCLI.get_man_text(doc, collection_name, plugin_type)
         except Exception as e:
             display.vvv(traceback.format_exc())
-            raise AnsibleError("Unable to retrieve documentation from '%s' due to: %s" % (plugin, to_native(e)), orig_exc=e)
+            raise AnsibleError("Unable to retrieve documentation from '%s'" % (plugin), orig_exc=e)
 
         return text
 
@@ -1387,16 +1387,15 @@ class DocCLI(CLI, RoleMixin):
         if doc.get('deprecated', False):
             text.append(_format("DEPRECATED: ", 'bold', 'DEP'))
             if isinstance(doc['deprecated'], dict):
-                if 'removed_at_date' in doc['deprecated']:
+                if 'removed_at_date' not in doc['deprecated'] and 'version' in doc['deprecated'] and 'removed_in' not in doc['deprecated']:
-                    text.append(
-                        "\tReason: %(why)s\n\tWill be removed in a release after %(removed_at_date)s\n\tAlternatives: %(alternative)s" % doc.pop('deprecated')
-                    )
-                else:
-                    if 'version' in doc['deprecated'] and 'removed_in' not in doc['deprecated']:
                     doc['deprecated']['removed_in'] = doc['deprecated']['version']
-                    text.append("\tReason: %(why)s\n\tWill be removed in: Ansible %(removed_in)s\n\tAlternatives: %(alternative)s" % doc.pop('deprecated'))
+                try:
+                    text.append('\t' + C.config.get_deprecated_msg_from_config(doc['deprecated'], True))
+                except KeyError as e:
+                    raise AnsibleError("Invalid deprecation documentation structure", orig_exc=e)
             else:
-                text.append("%s" % doc.pop('deprecated'))
+                text.append("%s" % doc['deprecated'])
+            del doc['deprecated']
 
         if doc.pop('has_action', False):
             text.append("")

lib/ansible/config/base.yml

@@ -2116,4 +2116,35 @@ VERBOSE_TO_STDERR:
     - section: defaults
       key: verbose_to_stderr
   type: bool
-...
+_Z_TEST_ENTRY:
+  name: testentry
+  description: for tests
+  env:
+    - name: ANSIBLE_TEST_ENTRY
+    - name: ANSIBLE_TEST_ENTRY_D
+      deprecated:
+        why: for testing
+        version: '3.30'
+        alternatives: nothing
+  ini:
+    - section: testing
+      key: valid
+    - section: testing
+      key: deprecated
+      deprecated:
+        why: for testing
+        version: '3.30'
+        alternatives: nothing
+_Z_TEST_ENTRY_2:
+  version_added: '2.18'
+  name: testentry
+  description: for tests
+  deprecated:
+    why: for testing
+    version: '3.30'
+    alternatives: nothing
+  env:
+    - name: ANSIBLE_TEST_ENTRY2
+  ini:
+    - section: testing
+      key: valid2

lib/ansible/config/manager.py

@@ -672,3 +672,17 @@ class ConfigManager(object):
             self._plugins[plugin_type] = {}
 
         self._plugins[plugin_type][name] = defs
+
+    @staticmethod
+    def get_deprecated_msg_from_config(dep_docs, include_removal=False):
+
+        removal = ''
+        if include_removal:
+            if 'removed_at_date' in dep_docs:
+                removal = f"Will be removed in a release after {dep_docs['removed_at_date']}\n\t"
+            else:
+                removal = f"Will be removed in: Ansible {dep_docs['removed_in']}\n\t"
+
+        # TODO: choose to deprecate either singular or plural
+        alt = dep_docs.get('alternatives', dep_docs.get('alternative', ''))
+        return f"Reason: {dep_docs['why']}\n\t{removal}Alternatives: {alt}"

lib/ansible/constants.py

@@ -15,6 +15,10 @@ from ansible.module_utils.parsing.convert_bool import BOOLEANS_TRUE
 from ansible.release import __version__
 from ansible.utils.fqcn import add_internal_fqcns
 
+# initialize config manager/config data to read/store global settings
+# and generate 'pseudo constants' for app consumption.
+config = ConfigManager()
+
 
 def _warning(msg):
     ''' display is not guaranteed here, nor it being the full class, but try anyways, fallback to sys.stderr.write '''
@@ -36,6 +40,26 @@ def _deprecated(msg, version):
         sys.stderr.write(' [DEPRECATED] %s, to be removed in %s\n' % (msg, version))
 
 
+def handle_config_noise(display=None):
+
+    if display is not None:
+        w = display.warning
+        d = display.deprecated
+    else:
+        w = _warning
+        d = _deprecated
+
+    while config.WARNINGS:
+        warn = config.WARNINGS.pop(0)
+        w(warn)
+
+    while config.DEPRECATED:
+        # tuple with name and options
+        dep = config.DEPRECATED.pop(0)
+        msg = config.get_deprecated_msg_from_config(dep[1])
+        d(msg, version=dep[1]['version'])
+
+
 def set_constant(name, value, export=vars()):
     ''' sets constants and returns resolved options dict '''
     export[name] = value
@@ -218,11 +242,8 @@ MAGIC_VARIABLE_MAPPING = dict(
 )
 
 # POPULATE SETTINGS FROM CONFIG ###
-config = ConfigManager()
-
-# Generate constants from config
 for setting in config.get_configuration_definitions():
     set_constant(setting, config.get_config_value(setting, variables=vars()))
 
-for warn in config.WARNINGS:
+# emit any warnings or deprecations
-    _warning(warn)
+handle_config_noise()

lib/ansible/plugins/__init__.py

@@ -92,6 +92,7 @@ class AnsiblePlugin(ABC):
 
     def set_option(self, option, value):
         self._options[option] = C.config.get_config_value(option, plugin_type=self.plugin_type, plugin_name=self._load_name, direct={option: value})
+        C.handle_config_noise(display)
 
     def set_options(self, task_keys=None, var_options=None, direct=None):
         '''
@@ -108,6 +109,7 @@ class AnsiblePlugin(ABC):
         if self.allow_extras and var_options and '_extras' in var_options:
             # these are largely unvalidated passthroughs, either plugin or underlying API will validate
             self._options['_extras'] = var_options['_extras']
+        C.handle_config_noise(display)
 
     def has_option(self, option):
         if not self._options:

test/integration/targets/ansible-doc/test.yml

@@ -18,7 +18,7 @@
         that:
           - result is failed
           - |
-            "ERROR! Unable to retrieve documentation from 'test_docs_missing_description' due to: All (sub-)options and return values must have a 'description' field"
+            "ERROR! Unable to retrieve documentation from 'test_docs_missing_description'. All (sub-)options and return values must have a 'description' field"
             in result.stderr
 
     - name: module with suboptions (avoid first line as it has full path)

test/integration/targets/deprecations/aliases

@@ -0,0 +1,2 @@
+shippable/posix/group3
+context/controller

test/integration/targets/deprecations/cache_plugins/notjsonfile.py

@@ -0,0 +1,82 @@
+# (c) 2020 Ansible Project
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import annotations
+
+DOCUMENTATION = '''
+    cache: notjsonfile
+    short_description: NotJSON cache plugin
+    description: This cache uses is NOT JSON
+    author: Ansible Core (@ansible-core)
+    version_added: 0.7.0
+    options:
+      _uri:
+        required: True
+        description:
+          - Path in which the cache plugin will save the JSON files
+        env:
+          - name: ANSIBLE_CACHE_PLUGIN_CONNECTION
+            version_added: 1.2.0
+        ini:
+          - key: fact_caching_connection
+            section: notjsonfile_cache
+          - key: fact_caching_connection
+            section: defaults
+      _prefix:
+        description: User defined prefix to use when creating the JSON files
+        env:
+          - name: ANSIBLE_CACHE_PLUGIN_PREFIX
+            version_added: 1.1.0
+        ini:
+          - key: fact_caching_prefix
+            section: defaults
+          - key: fact_caching_prefix
+            section: notjson_cache
+            deprecated:
+              alternative: section is notjsonfile_cache
+              why: Another test deprecation
+              removed_at_date: '2050-01-01'
+          - key: fact_caching_prefix
+            section: notjsonfile_cache
+      _timeout:
+        default: 86400
+        description: Expiration timeout for the cache plugin data
+        env:
+          - name: ANSIBLE_CACHE_PLUGIN_TIMEOUT
+          - name: ANSIBLE_NOTJSON_CACHE_PLUGIN_TIMEOUT
+            deprecated:
+              alternative: do not use a variable
+              why: Test deprecation
+              version: '3.0.0'
+        ini:
+          - key: fact_caching_timeout
+            section: defaults
+          - key: fact_caching_timeout
+            section: notjsonfile_cache
+        vars:
+          - name: notsjonfile_fact_caching_timeout
+            version_added: 1.5.0
+        type: integer
+      removeme:
+        default: 86400
+        description: Expiration timeout for the cache plugin data
+        deprecated:
+            alternative: cause i need to test it
+            why: Test deprecation
+            version: '2.0.0'
+        env:
+          - name: ANSIBLE_NOTJSON_CACHE_PLUGIN_REMOVEME
+'''
+
+from ansible.plugins.cache import BaseFileCacheModule
+
+
+class CacheModule(BaseFileCacheModule):
+    """
+    A caching module backed by json files.
+    """
+    def _dump(self):
+        pass
+
+    def _load(self):
+        pass

test/integration/targets/deprecations/entry_key_deprecated.cfg

@@ -0,0 +1,2 @@
+[testing]
+deprecated=false

test/integration/targets/deprecations/entry_key_deprecated2.cfg

@@ -0,0 +1,3 @@
+[testing]
+# ini key not deprecated, but parent setting is
+valid2=true

test/integration/targets/deprecations/entry_key_not_deprecated.cfg

@@ -0,0 +1,2 @@
+[testing]
+valid=false

test/integration/targets/deprecations/library/removeoption.py

@@ -0,0 +1,78 @@
+# -*- coding: utf-8 -*-
+# Copyright: Ansible Project
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import annotations
+
+
+DOCUMENTATION = r'''
+---
+module: removeoption
+short_description: noop
+description: does nothing, test for removal of option
+options:
+  one:
+    description:
+    - first option
+    type: bool
+    default: no
+  two:
+    description:
+    - second option
+    deprecated:
+        removed_in: '3.30'
+        why: cause i wanna test this!
+        alternatives: none needed
+notes:
+- Just noop to test module deprecation
+seealso:
+- module: willremove
+author:
+- Ansible Core Team
+attributes:
+    action:
+      support: full
+    async:
+      support: full
+    bypass_host_loop:
+      support: none
+    check_mode:
+      support: full
+    diff_mode:
+      support: none
+    platform:
+      platforms: all
+'''
+
+EXAMPLES = r'''
+- name: useless
+  remove_option:
+    one: true
+    two: /etc/file.conf
+'''
+
+RETURN = r'''
+'''
+
+from ansible.module_utils.basic import AnsibleModule
+
+
+def main():
+    module = AnsibleModule(
+        argument_spec=dict(
+            one=dict(type='bool', default='no'),
+            two=dict(type='str', removed_in_version='3.30'),
+        ),
+        supports_check_mode=True
+    )
+
+    one = module.params['one']
+    two = module.params['two']
+
+    result = {'yolo': 'lola'}
+
+    module.exit_json(**result)
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/deprecations/library/willremove.py

@@ -0,0 +1,79 @@
+# -*- coding: utf-8 -*-
+# Copyright: Ansible Project
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import annotations
+
+
+DOCUMENTATION = r'''
+---
+module: willremove
+version_added: histerical
+short_description: does nothing
+description: does nothing, this is deprecation test
+deprecated:
+    removed_in: '3.30'
+    why: cause i wanna!
+    alternatives: no soup for you!
+options:
+  one:
+    description:
+    - first option
+    type: bool
+    default: no
+  two:
+    description:
+    - second option
+notes:
+- Just noop to test module deprecation
+seealso:
+- module: removeoption
+author:
+- Ansible Core Team
+attributes:
+    action:
+      support: full
+    async:
+      support: full
+    bypass_host_loop:
+      support: none
+    check_mode:
+      support: full
+    diff_mode:
+      support: none
+    platform:
+      platforms: all
+'''
+
+EXAMPLES = r'''
+- name: useless
+  willremove:
+    one: true
+    two: /etc/file.conf
+'''
+
+RETURN = r'''
+'''
+
+from ansible.module_utils.basic import AnsibleModule
+
+
+def main():
+    module = AnsibleModule(
+        argument_spec=dict(
+            one=dict(type='bool', default='no'),
+            two=dict(type='str'),
+        ),
+        supports_check_mode=True
+    )
+
+    one = module.params['one']
+    two = module.params['two']
+
+    result = {'yolo': 'lola'}
+
+    module.exit_json(**result)
+
+
+if __name__ == '__main__':
+    main()

test/integration/targets/deprecations/runme.sh

@@ -0,0 +1,39 @@
+#!/usr/bin/env bash
+
+set -eux -o pipefail
+
+export ANSIBLE_DEPRECATION_WARNINGS=True
+
+### check general config
+
+# check for entry key valid, no deprecation
+[ "$(ANSIBLE_CONFIG='entry_key_not_deprecated.cfg' ansible -m meta -a 'noop'  localhost 2>&1 | grep -c 'DEPRECATION')" -eq "0" ]
+
+# check for entry key deprecation, must be defined to trigger
+[ "$(ANSIBLE_CONFIG='entry_key_deprecated.cfg' ansible -m meta -a 'noop'  localhost 2>&1 | grep -c 'DEPRECATION')" -eq "1" ]
+
+# check for deprecation of entry itself, must be consumed to trigger
+[ "$(ANSIBLE_TEST_ENTRY2=1 ansible -m debug -a 'msg={{q("config", "_Z_TEST_ENTRY_2")}}' localhost  2>&1 | grep -c 'DEPRECATION')" -eq "1" ]
+
+# check for entry deprecation, just need key defined to trigger
+[ "$(ANSIBLE_CONFIG='entry_key_deprecated2.cfg' ansible -m meta -a 'noop'  localhost 2>&1 | grep -c 'DEPRECATION')" -eq "1" ]
+
+
+### check plugin config
+
+# force use of the test plugin
+export ANSIBLE_CACHE_PLUGIN_CONNECTION=/var/tmp
+export ANSIBLE_CACHE_PLUGIN=notjsonfile
+
+# check for plugin(s) config option and setting non deprecation
+[ "$(ANSIBLE_CACHE_PLUGIN_TIMEOUT=1 ansible -m meta -a 'noop'  localhost --playbook-dir ./ 2>&1 | grep -c 'DEPRECATION')" -eq "0" ]
+
+# check for plugin(s) config option setting deprecation
+[ "$(ANSIBLE_NOTJSON_CACHE_PLUGIN_TIMEOUT=1 ansible -m meta -a 'noop'  localhost --playbook-dir ./ 2>&1 | grep -c 'DEPRECATION')" -eq "1" ]
+
+# check for plugin(s) config option deprecation
+[ "$(ANSIBLE_NOTJSON_CACHE_PLUGIN_REMOVEME=1 ansible -m meta -a 'noop'  localhost --playbook-dir ./ 2>&1 | grep -c 'DEPRECATION')" -eq "1" ]
+
+# TODO: check for module deprecation
+# TODO: check for module option deprecation
+# TODO: check for plugin deprecation

test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py

@@ -1235,7 +1235,7 @@ class ModuleValidator(Validator):
                         self._validate_semantic_markup(entry.get(key))
 
         if isinstance(docs.get('deprecated'), dict):
-            for key in ('why', 'alternative'):
+            for key in ('why', 'alternative', 'alternatives'):
                 self._validate_semantic_markup(docs.get('deprecated').get(key))
 
         self._validate_semantic_markup_options(docs.get('options'))

test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/schema.py

@@ -84,6 +84,22 @@ def date(error_code=None):
     return Any(isodate, error_code=error_code)
 
 
+def require_only_one(keys):
+    def f(obj):
+        found = None
+        for k in obj.keys():
+            if k in keys:
+                if k is None:
+                    found = k
+                else:
+                    raise Invalid('Found conflicting keys, must contain only one of {}'.format(keys))
+        if found is None:
+            raise Invalid('Must contain one of {}'.format(keys))
+
+        return obj
+    return f
+
+
 # Roles can also be referenced by semantic markup
 _VALID_PLUGIN_TYPES = set(DOCUMENTABLE_PLUGINS + ('role', ))
 
@@ -568,7 +584,9 @@ def list_dict_option_schema(for_collection, plugin_type):
                     {
                         # This definition makes sure everything has the correct types/values
                         'why': doc_string,
-                        'alternatives': doc_string,
+                        # TODO: phase out either plural or singular, 'alt' is exclusive group
+                        Exclusive('alternative', 'alt'): doc_string,
+                        Exclusive('alternatives', 'alt'): doc_string,
                         # vod stands for 'version or date'; this is the name of the exclusive group
                         Exclusive('removed_at_date', 'vod'): date(),
                         Exclusive('version', 'vod'): version(for_collection),
@@ -577,7 +595,7 @@ def list_dict_option_schema(for_collection, plugin_type):
                     {
                         # This definition makes sure that everything we require is there
                         Required('why'): Any(*string_types),
-                        'alternatives': Any(*string_types),
+                        Required(Any('alternatives', 'alternative')): Any(*string_types),
                         Required(Any('removed_at_date', 'version')): Any(*string_types),
                         Required('collection_name'): Any(*string_types),
                     },
@@ -761,13 +779,16 @@ def return_schema(for_collection, plugin_type='module'):
 
 
 def deprecation_schema(for_collection):
+
     main_fields = {
         Required('why'): doc_string,
-        Required('alternative'): doc_string,
+        'alternative': doc_string,
-        Required('removed_from_collection'): collection_name,
+        'alternatives': doc_string,
-        'removed': Any(True),
     }
 
+    if for_collection:
+        main_fields.update({Required('removed_from_collection'): collection_name, 'removed': Any(True)})
+
     date_schema = {
         Required('removed_at_date'): date(),
     }
@@ -791,6 +812,7 @@ def deprecation_schema(for_collection):
     if for_collection:
         result = All(
             result,
+            require_only_one(['alternative', 'alternatives']),
             partial(check_removal_version,
                     version_field='removed_in',
                     collection_name_field='removed_from_collection',